Make implicit attribute label lookup consistent.

Previously, as of 0c8049f5fc866d785dd83769fa6c38ecf771ba96, these labels
were treated as relative to the repository of the rule in some places
but not others.

From the other commit's message:
They [implicit attribute defaults] now all point to @bazel_tools anyway,
so there is no need [to special-case them].

--
Change-Id: If337eb2579ae613ba09cab0e0c927691922c0a39
Reviewed-on: https://bazel-review.googlesource.com/#/c/2783/
MOS_MIGRATED_REVID=114313341

2 files changed, 37 insertions, 17 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
index 2d7edd2419..3021159ffd 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
@@ -319,6 +319,7 @@ public abstract class DependencyResolver {
             : BuildType.LABEL.cast(attribute.getDefaultValue(rule));
 
         if (label != null) {
+          label = ruleLabel.resolveRepositoryRelative(label);
           builder.put(attribute, LabelAndConfiguration.of(label, configuration));
         }
       } else if (attribute.getType() == BuildType.LABEL_LIST) {
@@ -326,23 +327,6 @@ public abstract class DependencyResolver {
         if (mappedAttributes.contains(attribute.getName())) {
           labelList = new ArrayList<>();
           for (Label label : attributeMap.get(attribute.getName(), BuildType.LABEL_LIST)) {
-            if (attribute.getName().equals("$config_dependencies")) {
-              // This is a hack necessary due to the confluence of the following circumstances:
-              //   - We need to call ruleLabel.resolveRepositoryRelative() on every label except
-              //     implicit ones so that the implicit labels specified in rule class definitions
-              //     work as expected
-              //   - The way dependencies for selectors is loaded is through the
-              //     $config_dependencies attribute, and thus the labels there need to be a verbatim
-              //     copy of those in the BUILD file (because
-              //     AggregatingAttributeMapper#visitLabels() calls
-              //     Label#resolveRepositoryRelative() on them, and calling it twice would be wrong
-              // Thus, we are stuck with the situation where the only implicit attribute on which
-              // Label#resolveRepositoryRelative needs to be called here is $config_dependencies.
-              //
-              // This is a bad state of affairs and the proper fix would be not to use labels in the
-              // default repository to signal configured targets in the main repository in SkyKeys.
-              label = ruleLabel.resolveRepositoryRelative(label);
-            }
             labelList.add(label);
           }
         } else {
@@ -350,6 +334,7 @@ public abstract class DependencyResolver {
         }
 
         for (Label label : labelList) {
+          label = ruleLabel.resolveRepositoryRelative(label);
           builder.put(attribute, LabelAndConfiguration.of(label, configuration));
         }
       }
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index e46c55fddf..a75c93ff57 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -659,6 +659,41 @@ public class SkylarkRuleContextTest extends SkylarkTestCase {
   }
 
   @Test
+  public void testLabelAttributeDefault() throws Exception {
+    scratch.file(
+        "my_rule.bzl",
+        "def _impl(ctx):",
+        "  return",
+        "my_rule = rule(",
+        "  implementation = _impl,",
+        "  attrs = {",
+        "    'explicit_dep': attr.label(default = Label('//:dep')),",
+        "    '_implicit_dep': attr.label(default = Label('//:dep')),",
+        "    'explicit_dep_list': attr.label_list(default = [Label('//:dep')]),",
+        "    '_implicit_dep_list': attr.label_list(default = [Label('//:dep')]),",
+        "  }",
+        ")");
+
+    scratch.file(
+        "BUILD", "filegroup(name='dep')", "load('/my_rule', 'my_rule')", "my_rule(name='r')");
+
+    invalidatePackages();
+    SkylarkRuleContext context = createRuleContext("@//:r");
+    Label explicitDepLabel =
+        (Label) evalRuleContextCode(context, "ruleContext.attr.explicit_dep.label");
+    assertThat(explicitDepLabel).isEqualTo(Label.parseAbsolute("@//:dep"));
+    Label implicitDepLabel =
+        (Label) evalRuleContextCode(context, "ruleContext.attr._implicit_dep.label");
+    assertThat(implicitDepLabel).isEqualTo(Label.parseAbsolute("@//:dep"));
+    Label explicitDepListLabel =
+        (Label) evalRuleContextCode(context, "ruleContext.attr.explicit_dep_list[0].label");
+    assertThat(explicitDepListLabel).isEqualTo(Label.parseAbsolute("@//:dep"));
+    Label implicitDepListLabel =
+        (Label) evalRuleContextCode(context, "ruleContext.attr._implicit_dep_list[0].label");
+    assertThat(implicitDepListLabel).isEqualTo(Label.parseAbsolute("@//:dep"));
+  }
+
+  @Test
   public void testRelativeLabelInExternalRepository() throws Exception {
     scratch.file("BUILD");
     scratch.file("external_rule.bzl",