Fix the translation of names of implicit or latebound attributes to their Skylark equivalent (from '$attr' and ':attr' to '_attr').

This CL eliminates duplicate code and makes sure that attribute names in SkylarkClassObjects are properly translated.

--
PiperOrigin-RevId: 145986380
MOS_MIGRATED_REVID=145986380

3 files changed, 24 insertions, 23 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
index e41f4b32ce..28f9324179 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
@@ -26,6 +26,7 @@ import com.google.devtools.build.lib.bazel.repository.downloader.HttpUtils;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.SkylarkClassObject;
 import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor;
@@ -83,19 +84,6 @@ public class SkylarkRepositoryContext {
   private final HttpDownloader httpDownloader;
 
   /**
-   * Convert attribute name from native naming convention to Skylark naming convention.
-   *
-   * <p>In native code, private values start with $ or :. In Skylark, private values start
-   * with _, because of the grammar.
-   */
-  private String attributeToSkylark(String oldName) {
-    if (!oldName.isEmpty() && (oldName.charAt(0) == '$' || oldName.charAt(0) == ':')) {
-      return "_" + oldName.substring(1);
-    }
-    return oldName;
-  }
-
-  /**
    * Create a new context (repository_ctx) object for a skylark repository rule ({@code rule}
    * argument).
    */
@@ -114,7 +102,7 @@ public class SkylarkRepositoryContext {
       if (!name.equals("$local")) {
         Object val = attrs.getObject(name);
         attrBuilder.put(
-            attributeToSkylark(name),
+            Attribute.getSkylarkName(name),
             val == null
                 ? Runtime.NONE
                 // Attribute values should be type safe
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
index 420e6b81d1..389c2d082b 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
@@ -1084,7 +1084,7 @@ public final class Attribute implements Comparable<Attribute> {
       // do not modify this.allowedFileTypesForLabels, instead create a copy.
       FileTypeSet allowedFileTypesForLabels = this.allowedFileTypesForLabels;
       if ((type == BuildType.LABEL) || (type == BuildType.LABEL_LIST)) {
-        if ((name.startsWith("$") || name.startsWith(":")) && allowedFileTypesForLabels == null) {
+        if (isPrivateAttribute(name) && allowedFileTypesForLabels == null) {
           allowedFileTypesForLabels = FileTypeSet.ANY_FILE;
         }
         Preconditions.checkNotNull(
@@ -1391,7 +1391,7 @@ public final class Attribute implements Comparable<Attribute> {
       final String msg =
           String.format(
               "Cannot compute default value of attribute '%s' in rule '%s': ",
-              attr.getName().replace('$', '_'), rule.getLabel());
+              attr.getPublicName(), rule.getLabel());
       final AtomicReference<EvalException> caughtEvalExceptionIfAny = new AtomicReference<>();
       ComputationStrategy<InterruptedException> strategy =
           new ComputationStrategy<InterruptedException>() {
@@ -1813,12 +1813,7 @@ public final class Attribute implements Comparable<Attribute> {
    * Implicit and late-bound attributes start with '_' (instead of '$' or ':').
    */
   public String getPublicName() {
-    String name = getName();
-    // latebound and implicit attributes have a one-character prefix we want to drop
-    if (isLateBound() || isImplicit()) {
-      return "_" + name.substring(1);
-    }
-    return name;
+    return getSkylarkName(getName());
   }
 
   /**
@@ -2130,6 +2125,23 @@ public final class Attribute implements Comparable<Attribute> {
     return name.startsWith(":");
   }
 
+  /** Returns whether this attribute is considered private in Skylark. */
+  private static boolean isPrivateAttribute(String nativeAttrName) {
+    return isLateBound(nativeAttrName) || isImplicit(nativeAttrName);
+  }
+
+  /**
+   * Returns the Skylark-usable name of this attribute.
+   *
+   * Implicit and late-bound attributes start with '_' (instead of '$' or ':').
+   */
+  public static String getSkylarkName(String nativeAttrName) {
+    if (isPrivateAttribute(nativeAttrName)) {
+      return "_" + nativeAttrName.substring(1);
+    }
+    return nativeAttrName;
+  }
+
   @Override
   public String toString() {
     return "Attribute(" + name + ", " + type + ")";
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObject.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObject.java
index 65fff09f52..6946ea30f9 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObject.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObject.java
@@ -92,7 +92,8 @@ public class SkylarkClassObject implements ClassObject, SkylarkValue, Concatable
   private ImmutableMap<String, Object> copyValues(Map<String, Object> values) {
     ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder();
     for (Map.Entry<String, Object> e : values.entrySet()) {
-      builder.put(e.getKey(), SkylarkType.convertToSkylark(e.getValue(), null));
+      builder.put(
+          Attribute.getSkylarkName(e.getKey()), SkylarkType.convertToSkylark(e.getValue(), null));
     }
     return builder.build();
   }