Clean up string representations for configured targets If --incompatible_descriptive_string_representations is passed, configured targets are converted to strings using `str`, `repr` and `print` functions differently (more descriptive, without leaking information that shouldn't be accessible). PiperOrigin-RevId: 161212989

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AbstractConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/AbstractConfiguredTarget.java
index 8b19729..0096575 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AbstractConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AbstractConfiguredTarget.java

@@ -26,6 +26,7 @@
 import com.google.devtools.build.lib.packages.PackageSpecification;
 import com.google.devtools.build.lib.packages.SkylarkClassObject;
 import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
 import com.google.devtools.build.lib.syntax.ClassObject;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.EvalUtils;
@@ -210,4 +211,15 @@
   /** Implement in subclasses to get a skylark provider for a given {@code providerKey}. */
   protected abstract Object rawGetSkylarkProvider(String providerKey);
 
+  @Override
+  public boolean isImmutable() {
+    return false;
+  }
+
+  // All main target classes must override this method to provide more descriptive strings.
+  // Exceptions are currently EnvironmentGroupConfiguredTarget and PackageGroupConfiguredTarget.
+  @Override
+  public void repr(SkylarkPrinter printer) {
+    printer.append("<unknown target " + getTarget().getLabel() + ">");
+  }
 }

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java
index cee9b85..62082cb 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java

@@ -16,6 +16,7 @@
 
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
 import com.google.devtools.build.lib.syntax.ClassObject;
 import javax.annotation.Nullable;
 
@@ -29,7 +30,7 @@
  * {@link TransitiveInfoCollection}s. Also, {@link ConfiguredTarget} objects should not be
  * accessible from the action graph.
  */
-public interface ConfiguredTarget extends TransitiveInfoCollection, ClassObject {
+public interface ConfiguredTarget extends TransitiveInfoCollection, ClassObject, SkylarkValue {
 
   /**
    *  All <code>ConfiguredTarget</code>s have a "label" field.

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/InputFileConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/InputFileConfiguredTarget.java
index 2374981..a6da763 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/InputFileConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/InputFileConfiguredTarget.java

@@ -20,6 +20,8 @@
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.packages.InputFile;
 import com.google.devtools.build.lib.packages.License;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
 import com.google.devtools.build.lib.util.Preconditions;
 
 /**
@@ -28,7 +30,7 @@
  * All InputFiles for the same target are equivalent, so configuration does not
  * play any role here and is always set to <b>null</b>.
  */
-public final class InputFileConfiguredTarget extends FileConfiguredTarget {
+public final class InputFileConfiguredTarget extends FileConfiguredTarget implements SkylarkValue {
   private final Artifact artifact;
   private final NestedSet<TargetLicense> licenses;
 
@@ -75,4 +77,9 @@
   public boolean hasOutputLicenses() {
     return false;
   }
+
+  @Override
+  public void repr(SkylarkPrinter printer) {
+    printer.append("<input file target " + getTarget().getLabel() + ">");
+  }
 }

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java
index f1b0b0d..2673fab 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java

@@ -18,6 +18,7 @@
 import com.google.devtools.build.lib.packages.ClassObjectConstructor;
 import com.google.devtools.build.lib.packages.ClassObjectConstructor.Key;
 import com.google.devtools.build.lib.packages.SkylarkClassObject;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.function.Consumer;
@@ -188,4 +189,9 @@
     }
     return providers;
   }
+
+  @Override
+  public void repr(SkylarkPrinter printer) {
+    printer.append("<merged target " + getLabel() + ">");
+  }
 }

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTarget.java
index e193a79..b9959e1 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTarget.java

@@ -19,6 +19,7 @@
 import com.google.devtools.build.lib.packages.OutputFile;
 import com.google.devtools.build.lib.rules.test.InstrumentedFilesProvider;
 import com.google.devtools.build.lib.rules.test.InstrumentedFilesProviderImpl;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.Preconditions;
 
@@ -115,4 +116,9 @@
     }
     return defaultValue;
   }
+
+  @Override
+  public void repr(SkylarkPrinter printer) {
+    printer.append("<output file target " + getTarget().getLabel() + ">");
+  }
 }

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java
index 62f0015..d7af3d4 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java

@@ -22,6 +22,7 @@
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.SkylarkClassObject;
 import com.google.devtools.build.lib.rules.SkylarkApiProvider;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
 import com.google.devtools.build.lib.util.Preconditions;
 import java.util.function.Consumer;
 import javax.annotation.Nullable;
@@ -135,4 +136,9 @@
   protected Object rawGetSkylarkProvider(String providerKey) {
     return providers.getProvider(providerKey);
   }
+
+  @Override
+  public void repr(SkylarkPrinter printer) {
+    printer.append("<target " + getLabel() + ">");
+  }
 }

diff --git a/src/main/java/com/google/devtools/build/lib/rules/Alias.java b/src/main/java/com/google/devtools/build/lib/rules/Alias.java
index 3a4570c..e1bc205 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/Alias.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/Alias.java

@@ -39,11 +39,13 @@
       throws InterruptedException, RuleErrorException {
     ConfiguredTarget actual = (ConfiguredTarget) ruleContext.getPrerequisite("actual", Mode.TARGET);
     return new AliasConfiguredTarget(
-        ruleContext.getConfiguration(),
+        ruleContext,
         actual,
         ImmutableMap.of(
-            AliasProvider.class, AliasProvider.fromAliasRule(ruleContext.getLabel(), actual),
-            VisibilityProvider.class, new VisibilityProviderImpl(ruleContext.getVisibility())));
+            AliasProvider.class,
+            AliasProvider.fromAliasRule(ruleContext.getLabel(), actual),
+            VisibilityProvider.class,
+            new VisibilityProviderImpl(ruleContext.getVisibility())));
   }
 
   /**

diff --git a/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java
index 6ef753c..369976a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java

@@ -19,6 +19,7 @@
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.FileProvider;
+import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -29,6 +30,7 @@
 import com.google.devtools.build.lib.packages.ClassObjectConstructor;
 import com.google.devtools.build.lib.packages.SkylarkClassObject;
 import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
 import com.google.devtools.build.lib.syntax.ClassObject;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
@@ -43,16 +45,18 @@
  */
 @Immutable
 public final class AliasConfiguredTarget implements ConfiguredTarget, ClassObject {
+  private final Label label;
   private final BuildConfiguration configuration;
   private final ConfiguredTarget actual;
   private final ImmutableMap<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider>
       overrides;
 
   public AliasConfiguredTarget(
-      BuildConfiguration configuration,
+      RuleContext ruleContext,
       @Nullable ConfiguredTarget actual,
       ImmutableMap<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider> overrides) {
-    this.configuration = Preconditions.checkNotNull(configuration);
+    this.label = ruleContext.getLabel();
+    this.configuration = Preconditions.checkNotNull(ruleContext.getConfiguration());
     this.actual = actual;
     this.overrides = Preconditions.checkNotNull(overrides);
   }
@@ -143,4 +147,18 @@
   public ConfiguredTarget getActual() {
     return actual;
   }
+
+  @Override
+  public boolean isImmutable() {
+    return false;
+  }
+
+  @Override
+  public void repr(SkylarkPrinter printer) {
+    printer.append("<alias target " + label);
+    if (actual != null) {
+      printer.append(" of " + actual.getLabel());
+    }
+    printer.append(">");
+  }
 }

diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/Bind.java b/src/main/java/com/google/devtools/build/lib/rules/repository/Bind.java
index 9f199a1..d6dfe7f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/Bind.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/Bind.java

@@ -44,12 +44,13 @@
 
     ConfiguredTarget actual = (ConfiguredTarget) ruleContext.getPrerequisite("actual", Mode.TARGET);
     return new AliasConfiguredTarget(
-        ruleContext.getConfiguration(),
+        ruleContext,
         actual,
         ImmutableMap.<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider>of(
-            AliasProvider.class, AliasProvider.fromAliasRule(ruleContext.getLabel(), actual),
-            VisibilityProvider.class, new VisibilityProviderImpl(
+            AliasProvider.class,
+            AliasProvider.fromAliasRule(ruleContext.getLabel(), actual),
+            VisibilityProvider.class,
+            new VisibilityProviderImpl(
                 NestedSetBuilder.create(Order.STABLE_ORDER, PackageSpecification.everything()))));
-
   }
 }

diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkStringRepresentationsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkStringRepresentationsTest.java
index fd218e2..f1146a1 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkStringRepresentationsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkStringRepresentationsTest.java

@@ -289,6 +289,27 @@
   }
 
   @Test
+  public void testStringRepresentations_Targets() throws Exception {
+    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
+
+    generateFilesToTestStrings();
+    ConfiguredTarget target = getConfiguredTarget("//test/skylark:check");
+
+    for (String suffix : SUFFIXES) {
+      assertThat(target.get("target" + suffix))
+          .isEqualTo("<target //test/skylark:foo>");
+      assertThat(target.get("input_target" + suffix))
+          .isEqualTo("<input file target //test/skylark:input.txt>");
+      assertThat(target.get("output_target" + suffix))
+          .isEqualTo("<output file target //test/skylark:output.txt>");
+      assertThat(target.get("alias_target" + suffix))
+          .isEqualTo("<alias target //test/skylark:foobar of //test/skylark:foo>");
+      assertThat(target.get("aspect_target" + suffix))
+          .isEqualTo("<merged target //test/skylark:bar>");
+    }
+  }
+
+  @Test
   public void testLegacyStringRepresentations_Labels() throws Exception {
     setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=false");
 
@@ -360,32 +381,4 @@
           .isEqualTo("rule_collection://test/skylark:bar");
     }
   }
-
-  @Test
-  public void testLegacyStringRepresentations_Targets() throws Exception {
-    // alias targets in skylark used to leak their memory addresses in string representations,
-    // we don't try to preserve this behaviour as it's harmful.
-    // An example of their legacy representation:
-    // "<com.google.devtools.build.lib.rules.AliasConfiguredTarget@12da9140>"
-
-    setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=false");
-
-    generateFilesToTestStrings();
-    ConfiguredTarget target = getConfiguredTarget("//test/skylark:check");
-
-    for (String suffix : SUFFIXES) {
-      assertThat(target.get("input_target" + suffix))
-          .isEqualTo("InputFileConfiguredTarget(//test/skylark:input.txt)");
-    }
-
-    // Legacy representation of several types of objects may contain nondeterministic chunks
-    for (String suffix : SUFFIXES) {
-      assertThat((String) target.get("target" + suffix))
-          .matches("ConfiguredTarget\\(//test/skylark:foo, [0-9a-f]+\\)");
-      assertThat((String) target.get("aspect_target" + suffix))
-          .matches("ConfiguredTarget\\(//test/skylark:bar, [0-9a-f]+\\)");
-      assertThat((String) target.get("output_target" + suffix))
-          .matches("ConfiguredTarget\\(//test/skylark:output.txt, [0-9a-f]+\\)");
-    }
-  }
 }