From 69fa6cb169e23c1b985bfa08ea8bc0b1f1199a63 Mon Sep 17 00:00:00 2001
From: Fabian Meumertzheim <fabian@meumertzhe.im>
Date: Wed, 3 Jan 2024 10:11:52 -0800
Subject: [PATCH] Improve `use_repo_rule` error when not referencing a
 `repository_rule`

Makes the error message less confusing when referencing an existing symbol that happens to be a macro, not a repository rule.

Closes #20657.

PiperOrigin-RevId: 595434847
Change-Id: Ifc37a65685c0196301d79a439f3245037cf39e21
---
 .../bzlmod/SingleExtensionEvalFunction.java   | 13 +++++++-
 .../bzlmod/ModuleExtensionResolutionTest.java | 30 ++++++++++++++++++-
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java
index 533cf65a7b6136..a077a6f3021030 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java
@@ -620,7 +620,7 @@ public RunModuleExtensionResult run(
       // Instiantiate the repos one by one.
       for (InnateExtensionRepo repo : repos) {
         Object exported = repo.loadedBzl().getModule().getGlobal(repo.ruleName());
-        if (!(exported instanceof RepositoryRuleFunction)) {
+        if (exported == null) {
           ImmutableSet<String> exportedRepoRules =
               repo.loadedBzl().getModule().getGlobals().entrySet().stream()
                   .filter(e -> e.getValue() instanceof RepositoryRuleFunction)
@@ -636,6 +636,17 @@ public RunModuleExtensionResult run(
                   repo.tag().getLocation(),
                   SpellChecker.didYouMean(repo.ruleName(), exportedRepoRules)),
               Transience.PERSISTENT);
+        } else if (!(exported instanceof RepositoryRuleFunction)) {
+          throw new SingleExtensionEvalFunctionException(
+              ExternalDepsException.withMessage(
+                  Code.BAD_MODULE,
+                  "%s exports a value called %s of type %s, yet a repository_rule is requested"
+                      + " at %s",
+                  repo.bzlLabel(),
+                  repo.ruleName(),
+                  Starlark.type(exported),
+                  repo.tag().getLocation()),
+              Transience.PERSISTENT);
         }
         RepositoryRuleFunction repoRule = (RepositoryRuleFunction) exported;
         Dict<String, Object> kwargs = repo.tag().getAttributeValues().attributes();
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java
index fd571658c43687..c6dfee8ae0d3ac 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java
@@ -2453,7 +2453,35 @@ public void innate_noSuchRepoRule() throws Exception {
         "load('@data//:data.bzl', self_data='data')",
         "data=self_data");
     scratch.file(
-        workspaceRoot.getRelative("repo.bzl").getPathString(), "data_repo = 3  # not a repo rule");
+        workspaceRoot.getRelative("repo.bzl").getPathString(),
+        "# not a repo rule",
+        "def data_repo(name):",
+        "    pass");
+
+    SkyKey skyKey = BzlLoadValue.keyForBuild(Label.parseCanonical("//:data.bzl"));
+    reporter.removeHandler(failFastHandler);
+    EvaluationResult<BzlLoadValue> result =
+        evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext);
+    assertThat(result.hasError()).isTrue();
+    assertThat(result.getError().getException())
+        .hasMessageThat()
+        .contains(
+            "//:repo.bzl exports a value called data_repo of type function, yet a repository_rule"
+                + " is requested at /ws/MODULE.bazel");
+  }
+
+  @Test
+  public void innate_noSuchValue() throws Exception {
+    scratch.file(
+        workspaceRoot.getRelative("MODULE.bazel").getPathString(),
+        "data_repo = use_repo_rule('//:repo.bzl', 'data_repo')",
+        "data_repo(name='data', data='get up at 6am.')");
+    scratch.file(workspaceRoot.getRelative("BUILD").getPathString());
+    scratch.file(
+        workspaceRoot.getRelative("data.bzl").getPathString(),
+        "load('@data//:data.bzl', self_data='data')",
+        "data=self_data");
+    scratch.file(workspaceRoot.getRelative("repo.bzl").getPathString(), "");
 
     SkyKey skyKey = BzlLoadValue.keyForBuild(Label.parseCanonical("//:data.bzl"));
     reporter.removeHandler(failFastHandler);