Show display form labels in java_* buildozer fixups

src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java

@@ -34,6 +34,7 @@
 import com.google.devtools.build.lib.actions.PathMapper;
 import com.google.devtools.build.lib.actions.SingleStringArgFormatter;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.concurrent.BlazeInterners;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
@@ -580,9 +581,16 @@ static class PrefixArg implements ArgvFragment {
 
     private static final UUID PREFIX_UUID = UUID.fromString("a95eccdf-4f54-46fc-b925-c8c7e1f50c95");
 
-    private static void push(List<Object> arguments, String before, Object arg) {
+    private static void push(
+        List<Object> arguments,
+        String before,
+        Object arg,
+        @Nullable RepositoryMapping mainRepoMapping) {
       arguments.add(INSTANCE);
       arguments.add(before);
+      if (mainRepoMapping != null) {
+        arguments.add(mainRepoMapping);
+      }
       arguments.add(arg);
     }
 
@@ -594,6 +602,9 @@ public int eval(
         PathMapper pathMapper) {
       String before = (String) arguments.get(argi++);
       Object arg = arguments.get(argi++);
+      if (arg instanceof RepositoryMapping mainRepoMapping) {
+        arg = ((Label) arguments.get(argi++)).getDisplayForm(mainRepoMapping);
+      }
       builder.add(before + CommandLineItem.expandToCommandLine(arg));
       return argi;
     }
@@ -606,7 +617,11 @@ public int addToFingerprint(
         Fingerprint fingerprint) {
       fingerprint.addUUID(PREFIX_UUID);
       fingerprint.addString((String) arguments.get(argi++));
-      fingerprint.addString(CommandLineItem.expandToCommandLine(arguments.get(argi++)));
+      Object arg = arguments.get(argi++);
+      if (arg instanceof RepositoryMapping mainRepoMapping) {
+        arg = ((Label) arguments.get(argi++)).getDisplayForm(mainRepoMapping);
+      }
+      fingerprint.addString(CommandLineItem.expandToCommandLine(arg));
       return argi;
     }
   }
@@ -888,22 +903,28 @@ public Builder addFormatted(@FormatString String formatStr, Object... args) {
 
     /** Concatenates the passed prefix string and the string. */
     public Builder addPrefixed(@CompileTimeConstant String prefix, @Nullable String arg) {
-      return addPrefixedInternal(prefix, arg);
+      return addPrefixedInternal(prefix, arg, /* mainRepoMapping= */ null);
     }
 
-    /** Concatenates the passed prefix string and the label using {@link Label#getCanonicalForm}. */
-    public Builder addPrefixedLabel(@CompileTimeConstant String prefix, @Nullable Label arg) {
-      return addPrefixedInternal(prefix, arg);
+    /**
+     * Concatenates the passed prefix string and the label using {@link Label#getDisplayForm}, which
+     * is identical to {@link Label#getCanonicalForm()} for main repo labels.
+     */
+    public Builder addPrefixedLabel(
+        @CompileTimeConstant String prefix,
+        @Nullable Label arg,
+        RepositoryMapping mainRepoMapping) {
+      return addPrefixedInternal(prefix, arg, mainRepoMapping);
     }
 
     /** Concatenates the passed prefix string and the path. */
     public Builder addPrefixedPath(@CompileTimeConstant String prefix, @Nullable PathFragment arg) {
-      return addPrefixedInternal(prefix, arg);
+      return addPrefixedInternal(prefix, arg, /* mainRepoMapping= */ null);
     }
 
     /** Concatenates the passed prefix string and the artifact's exec path. */
     public Builder addPrefixedExecPath(@CompileTimeConstant String prefix, @Nullable Artifact arg) {
-      return addPrefixedInternal(prefix, arg);
+      return addPrefixedInternal(prefix, arg, /* mainRepoMapping= */ null);
     }
 
     /**
@@ -1129,10 +1150,11 @@ private Builder addObjectInternal(@CompileTimeConstant String arg, @Nullable Obj
     }
 
     @CanIgnoreReturnValue
-    private Builder addPrefixedInternal(String prefix, @Nullable Object arg) {
+    private Builder addPrefixedInternal(
+        String prefix, @Nullable Object arg, @Nullable RepositoryMapping mainRepoMapping) {
       Preconditions.checkNotNull(prefix);
       if (arg != null) {
-        PrefixArg.push(arguments, prefix, arg);
+        PrefixArg.push(arguments, prefix, arg, mainRepoMapping);
       }
       return this;
     }

src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java

@@ -208,7 +208,8 @@ public JavaCompileOutputs<Artifact> createOutputs(JavaOutput output) {
     return builder.build();
   }
 
-  public void createCompileAction(JavaCompileOutputs<Artifact> outputs) throws RuleErrorException {
+  public void createCompileAction(JavaCompileOutputs<Artifact> outputs)
+      throws RuleErrorException, InterruptedException {
     if (outputs.genClass() != null) {
       createGenJarAction(
           outputs.output(),
@@ -542,7 +543,7 @@ private Artifact turbineOutput(Artifact classJar, String newExtension) {
    * @param headerDeps the .jdeps output of this java compilation
    */
   public void createHeaderCompilationAction(Artifact headerJar, Artifact headerDeps)
-      throws RuleErrorException {
+      throws RuleErrorException, InterruptedException {
 
     JavaTargetAttributes attributes = getAttributes();
 
@@ -687,7 +688,8 @@ public void createSourceJarAction(Artifact outputJar, @Nullable Artifact gensrcJ
    * @return the header jar (if requested), or ijar (if requested), or else the class jar
    */
   public Artifact createCompileTimeJarAction(
-      Artifact runtimeJar, JavaCompilationArtifacts.Builder builder) throws RuleErrorException {
+      Artifact runtimeJar, JavaCompilationArtifacts.Builder builder)
+      throws RuleErrorException, InterruptedException {
     Artifact jar;
     boolean isFullJar = false;
     if (shouldUseHeaderCompilation()) {

src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileActionBuilder.java

@@ -171,7 +171,7 @@ public JavaCompileActionBuilder(
     this.execGroup = execGroup;
   }
 
-  public JavaCompileAction build() throws RuleErrorException {
+  public JavaCompileAction build() throws RuleErrorException, InterruptedException {
     // TODO(bazel-team): all the params should be calculated before getting here, and the various
     // aggregation code below should go away.
 
@@ -271,7 +271,7 @@ private ImmutableSet<Artifact> allOutputs() {
   }
 
   private CustomCommandLine buildParamFileContents(ImmutableList<String> javacOpts)
-      throws RuleErrorException {
+      throws RuleErrorException, InterruptedException {
 
     CustomCommandLine.Builder result = CustomCommandLine.builder();
 
@@ -304,7 +304,8 @@ private CustomCommandLine buildParamFileContents(ImmutableList<String> javacOpts
       } else {
         // @-prefixed strings will be assumed to be filenames and expanded by
         // {@link JavaLibraryBuildRequest}, so add an extra &at; to escape it.
-        result.addPrefixedLabel("@", targetLabel);
+        result.addPrefixedLabel(
+            "@", targetLabel, ruleContext.getAnalysisEnvironment().getMainRepoMapping());
       }
     }
     result.add("--injecting_rule_kind", injectingRuleKind);

src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileAction.java

@@ -368,7 +368,8 @@ public Builder enableDirectClasspath(boolean enableDirectClasspath) {
     }
 
     /** Builds and registers the action for a header compilation. */
-    public void build(JavaToolchainProvider javaToolchain) throws RuleErrorException {
+    public void build(JavaToolchainProvider javaToolchain)
+        throws RuleErrorException, InterruptedException {
       checkNotNull(outputDepsProto, "outputDepsProto must not be null");
       checkNotNull(sourceFiles, "sourceFiles must not be null");
       checkNotNull(sourceJars, "sourceJars must not be null");
@@ -478,7 +479,8 @@ public void build(JavaToolchainProvider javaToolchain) throws RuleErrorException
         } else {
           // @-prefixed strings will be assumed to be params filenames and expanded,
           // so add an extra @ to escape it.
-          commandLine.addPrefixedLabel("@", targetLabel);
+          commandLine.addPrefixedLabel(
+              "@", targetLabel, ruleContext.getAnalysisEnvironment().getMainRepoMapping());
         }
       }
 

src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java

@@ -134,7 +134,11 @@ public void createHeaderCompilationAction(
       Object injectingRuleKind,
       boolean enableDirectClasspath,
       Sequence<?> additionalInputs)
-      throws EvalException, TypeException, RuleErrorException, LabelSyntaxException {
+      throws EvalException,
+          TypeException,
+          RuleErrorException,
+          LabelSyntaxException,
+          InterruptedException {
     checkJavaToolchainIsDeclaredOnRule(ctx.getRuleContext());
     JavaTargetAttributes.Builder attributesBuilder =
         new JavaTargetAttributes.Builder(javaSemantics)
@@ -199,7 +203,11 @@ public void createCompilationAction(
       boolean enableDirectClasspath,
       Sequence<?> additionalInputs,
       Sequence<?> additionalOutputs)
-      throws EvalException, TypeException, RuleErrorException, LabelSyntaxException {
+      throws EvalException,
+          TypeException,
+          RuleErrorException,
+          LabelSyntaxException,
+          InterruptedException {
     checkJavaToolchainIsDeclaredOnRule(ctx.getRuleContext());
     JavaCompileOutputs<Artifact> outputs =
         JavaCompileOutputs.builder()

src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCommonApi.java

@@ -523,7 +523,11 @@ void createHeaderCompilationAction(
       Object injectingRuleKind,
       boolean enableDirectClasspath,
       Sequence<?> additionalInputs)
-      throws EvalException, TypeException, RuleErrorException, LabelSyntaxException;
+      throws EvalException,
+          TypeException,
+          RuleErrorException,
+          LabelSyntaxException,
+          InterruptedException;
 
   @StarlarkMethod(
       name = "create_compilation_action",
@@ -587,7 +591,11 @@ void createCompilationAction(
       boolean enableDirectClasspath,
       Sequence<?> additionalInputs,
       Sequence<?> additionalOutputs)
-      throws EvalException, TypeException, RuleErrorException, LabelSyntaxException;
+      throws EvalException,
+          TypeException,
+          RuleErrorException,
+          LabelSyntaxException,
+          InterruptedException;
 
   @StarlarkMethod(
       name = "default_javac_opts",

src/test/java/com/google/devtools/build/lib/actions/CustomCommandLineTest.java

@@ -20,6 +20,7 @@
 
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
 import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
 import com.google.devtools.build.lib.actions.ArtifactRoot.RootType;
@@ -28,6 +29,8 @@
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg;
 import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg.SimpleVectorArg;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
@@ -126,7 +129,8 @@ public void addPrefixed_addsPrefixForArguments() throws Exception {
         .containsExactly("prefix-foo");
     assertThat(
             builder()
-                .addPrefixedLabel("prefix-", Label.parseCanonical("//a:b"))
+                .addPrefixedLabel(
+                    "prefix-", Label.parseCanonical("//a:b"), /* mainRepoMapping= */ null)
                 .build()
                 .arguments())
         .containsExactly("prefix-//a:b");
@@ -137,6 +141,22 @@ public void addPrefixed_addsPrefixForArguments() throws Exception {
         .containsExactly("prefix-dir/file1.txt");
   }
 
+  @Test
+  public void addPrefixedLabel_emitsExternalLabelInDisplayForm() throws Exception {
+    assertThat(
+            builder()
+                .addPrefixedLabel(
+                    "prefix-",
+                    Label.parseCanonical("@@canonical_name//a:b"),
+                    RepositoryMapping.create(
+                        ImmutableMap.of(
+                            "apparent_name", RepositoryName.createUnvalidated("canonical_name")),
+                        RepositoryName.MAIN))
+                .build()
+                .arguments())
+        .containsExactly("prefix-@apparent_name//a:b");
+  }
+
   @Test
   public void addAll_addsAllArguments() throws Exception {
     assertThat(builder().addAll(list("val1", "val2")).build().arguments())
@@ -527,7 +547,7 @@ public void addNulls_addsNothing() throws Exception {
             .addExecPath("foo", null)
             .addLazyString("foo", null)
             .addPrefixed("prefix", null)
-            .addPrefixedLabel("prefix", null)
+            .addPrefixedLabel("prefix", null, /* mainRepoMapping= */ null)
             .addPrefixedPath("prefix", null)
             .addPrefixedExecPath("prefix", null)
             .addAll((ImmutableList<String>) null)

src/test/shell/bazel/bazel_java_test.sh

@@ -1998,4 +1998,90 @@ EOF
   expect_log "buildozer 'add deps @c//:c' //pkg:a"
 }
 
+function test_strict_deps_error_external_repo_header_compile_action() {
+  cat << 'EOF' > MODULE.bazel
+bazel_dep(
+    name = "lib_c",
+    repo_name = "c",
+)
+local_path_override(
+    module_name = "lib_c",
+    path = "lib_c",
+)
+EOF
+
+  mkdir -p pkg
+  cat << 'EOF' > pkg/BUILD
+java_binary(name = "Main", srcs = ["Main.java"], deps = [":a"])
+java_library(name = "a", srcs = ["A.java"], deps = [":b"])
+java_library(name = "b", srcs = ["B.java"], deps = ["@c"])
+EOF
+  cat << 'EOF' > pkg/Main.java
+public class Main extends A {}
+EOF
+  cat << 'EOF' > pkg/A.java
+public class A extends B implements C {}
+EOF
+  cat << 'EOF' > pkg/B.java
+public class B implements C {}
+EOF
+
+  mkdir -p lib_c
+  cat << 'EOF' > lib_c/MODULE.bazel
+module(name = "lib_c")
+EOF
+  cat << 'EOF' > lib_c/BUILD
+java_library(name = "c", srcs = ["C.java"], visibility = ["//visibility:public"])
+EOF
+  cat << 'EOF' > lib_c/C.java
+public interface C {}
+EOF
+
+  bazel build //pkg:a >& $TEST_log && fail "build should fail"
+  expect_log "buildozer 'add deps @c//:c' //pkg:a"
+}
+
+function test_strict_deps_error_external_repo_compile_action() {
+  cat << 'EOF' > MODULE.bazel
+bazel_dep(
+    name = "lib_c",
+    repo_name = "c",
+)
+local_path_override(
+    module_name = "lib_c",
+    path = "lib_c",
+)
+EOF
+
+  mkdir -p pkg
+  cat << 'EOF' > pkg/BUILD
+java_library(name = "a", srcs = ["A.java"], deps = [":b"])
+java_library(name = "b", srcs = ["B.java"], deps = ["@c"])
+EOF
+  cat << 'EOF' > pkg/A.java
+public class A extends B {
+  boolean foo() {
+    return this instanceof C;
+  }
+}
+EOF
+  cat << 'EOF' > pkg/B.java
+public class B implements C {}
+EOF
+
+  mkdir -p lib_c
+  cat << 'EOF' > lib_c/MODULE.bazel
+module(name = "lib_c")
+EOF
+  cat << 'EOF' > lib_c/BUILD
+java_library(name = "c", srcs = ["C.java"], visibility = ["//visibility:public"])
+EOF
+  cat << 'EOF' > lib_c/C.java
+public interface C {}
+EOF
+
+  bazel build //pkg:a >& $TEST_log && fail "build should fail"
+  expect_log "buildozer 'add deps @c//:c' //pkg:a"
+}
+
 run_suite "Java integration tests"

src/test/shell/bazel/local_repository_test.sh

@@ -1411,7 +1411,7 @@ EOF
 
   bazel build @x_repo//a >& $TEST_log && fail "Building @x_repo//a should error out"
   expect_log "** Please add the following dependencies:"
-  expect_log "@@x_repo//x to @@x_repo//a"
+  expect_log " @x_repo//x to @x_repo//a"
 }
 
 # This test verifies that the `public` pattern includes external dependencies.