From f6a30b46a7fef82e02e5ef0e4c009f54305bbce3 Mon Sep 17 00:00:00 2001
From: "bazel.build machine account"
 <15028808+bazel-io@users.noreply.github.com>
Date: Mon, 7 Aug 2023 15:08:41 -0400
Subject: [PATCH] Friendlier error message for `bazel_dep`s without `version`
 (#19196)

Ideally we would also include the location of the offending bazel_dep, but that requires a (slightly?) bigger change to record the locations of all bazel_deps.

Fixes https://github.com/bazelbuild/bazel/issues/17126

PiperOrigin-RevId: 554406088
Change-Id: I46865c31dc983de18ac64a37e8ee15ebec624937

Co-authored-by: Googler <wyv@google.com>
---
 .../build/lib/bazel/bzlmod/ModuleFileFunction.java  | 10 ++++++++++
 .../lib/bazel/bzlmod/ModuleFileFunctionTest.java    | 13 +++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java
index 3d622633e53c57..2fec84fd7088cd 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java
@@ -351,6 +351,15 @@ private GetModuleFileResult getModuleFile(
     }
 
     // Otherwise, we should get the module file from a registry.
+    if (key.getVersion().isEmpty()) {
+      // Print a friendlier error message if the user forgets to specify a version *and* doesn't
+      // have a non-registry override.
+      throw errorf(
+          Code.MODULE_NOT_FOUND,
+          "bad bazel_dep on module '%s' with no version. Did you forget to specify a version, or a"
+              + " non-registry override?",
+          key.getName());
+    }
     // TODO(wyv): Move registry object creation to BazelRepositoryModule so we don't repeatedly
     //   create them, and we can better report the error (is it a flag error or override error?).
     List<String> registries = Objects.requireNonNull(REGISTRIES.get(env));
@@ -361,6 +370,7 @@ private GetModuleFileResult getModuleFile(
       }
     } else if (override != null) {
       // This should never happen.
+      // TODO(wyv): make ModuleOverride a sealed interface so this is checked at compile time.
       throw new IllegalStateException(
           String.format(
               "unrecognized override type %s for module %s",
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java
index 35b09dcdf214fd..761f9142ed9022 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java
@@ -278,6 +278,19 @@ public void testRootModule_badSelfOverride() throws Exception {
     assertThat(result.getError().toString()).contains("invalid override for the root module");
   }
 
+  @Test
+  public void forgotVersion() throws Exception {
+    FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
+    ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));
+
+    SkyKey skyKey = ModuleFileValue.key(createModuleKey("bbb", ""), null);
+    EvaluationResult<ModuleFileValue> result =
+        evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext);
+    assertThat(result.hasError()).isTrue();
+    assertThat(result.getError().toString())
+        .contains("bad bazel_dep on module 'bbb' with no version");
+  }
+
   @Test
   public void testRegistriesCascade() throws Exception {
     // Registry1 has no module B@1.0; registry2 and registry3 both have it. We should be using the