[7.6.0] Permit literal dicts as extra keyword args in MODULE.bazel (#25614)

Valid repository names are a strict superset of valid Starlark
identifiers, but need to be specified as keyword arguments for
`MODULE.bazel` functions such as `override_repo` and `use_repo`. This is
made possible by relaxing the syntax checker for `MODULE.bazel` files
only, which now allows
```
override_repo(ext2, **{"foo.2": "foo"})
```
but not
```
KWARGS = {"foo.2": "foo"}
override_repo(ext2, **KWARGS)
```

Fixes #19635
Fixes #25551

Closes #25553.

PiperOrigin-RevId: 737928093
Change-Id: Iff5d740ef2101973e6d3da95edf73befd21afab7
(cherry picked from commit 1f54144)

Fixes #25614

Author: fmeum

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/CompiledModuleFile.java

@@ -93,7 +93,8 @@ public static CompiledModuleFile parseAndCompile(
   static ImmutableList<IncludeStatement> checkModuleFileSyntax(StarlarkFile starlarkFile)
       throws SyntaxError.Exception {
     var includeStatements = ImmutableList.<IncludeStatement>builder();
-    new DotBazelFileSyntaxChecker("MODULE.bazel files", /* canLoadBzl= */ false) {
+    new DotBazelFileSyntaxChecker(
+        "MODULE.bazel files", /* canLoadBzl= */ false, /* allowLiteralStarStarArgs= */ true) {
       // Once `include` the identifier is assigned to, we no longer care about its appearance
       // anywhere. This allows `include` to be used as a module extension proxy (and technically
       // any other variable binding).

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java

@@ -627,10 +627,15 @@ public void export(EventHandler handler, Label bzlFileLabel, String name) {
           @Param(
               name = "kwargs",
               doc =
-                  "Specifies certain repos to import into the scope of the current module with"
-                      + " different names. The keys should be the name to use in the current scope,"
-                      + " whereas the values should be the original names exported by the module"
-                      + " extension."),
+                  """
+                  Specifies certain repos to import into the scope of the current module with
+                  different names. The keys should be the name to use in the current scope,
+                  whereas the values should be the original names exported by the module
+                  extension.
+                  <p>Keys that are not valid identifiers can be specified via a literal dict
+                  passed as extra keyword arguments, e.g.,
+                  <code>use_repo(extension_proxy, **{"foo.2": "foo"})</code>.
+                  """),
       useStarlarkThread = true)
   public void useRepo(
       ModuleExtensionProxy extensionProxy,
@@ -679,7 +684,11 @@ public void useRepo(
                   """
                   The overrides to apply to the repos generated by the extension, where the values
                   are the names of repos in the scope of the current module and the keys are the
-                  names of the repos they will override in the extension."""),
+                  names of the repos they will override in the extension.
+                  <p>Keys that are not valid identifiers can be specified via a literal dict
+                  passed as extra keyword arguments, e.g.,
+                  <code>override_repo(extension_proxy, **{"foo.2": "foo"})</code>.
+                  """),
       useStarlarkThread = true)
   public void overrideRepo(
       ModuleExtensionProxy extensionProxy,
@@ -732,7 +741,11 @@ public void overrideRepo(
                   """
                   The new repos to inject into the extension, where the values are the names of
                   repos in the scope of the current module and the keys are the name they will be
-                  visible under in the extension."""),
+                  visible under in the extension.
+                  <p>Keys that are not valid identifiers can be specified via a literal dict
+                  passed as extra keyword arguments, e.g.,
+                  <code>inject_repo(extension_proxy, **{"foo.2": "foo"})</code>.
+                  """),
       useStarlarkThread = true)
   public void injectRepo(
       ModuleExtensionProxy extensionProxy,

src/main/java/com/google/devtools/build/lib/packages/DotBazelFileSyntaxChecker.java

@@ -20,6 +20,7 @@
 import net.starlark.java.syntax.Argument;
 import net.starlark.java.syntax.CallExpression;
 import net.starlark.java.syntax.DefStatement;
+import net.starlark.java.syntax.DictExpression;
 import net.starlark.java.syntax.ForStatement;
 import net.starlark.java.syntax.IfStatement;
 import net.starlark.java.syntax.LambdaExpression;
@@ -46,16 +47,26 @@
 public class DotBazelFileSyntaxChecker extends NodeVisitor {
   private final String where;
   private final boolean canLoadBzl;
+  private final boolean allowLiteralStarStarArgs;
   private ImmutableList.Builder<SyntaxError> errors = ImmutableList.builder();
 
   /**
    * @param where describes the type of file being checked.
    * @param canLoadBzl whether the file type being check supports load statements. This is used to
    *     generate more informative error messages.
+   * @param allowLiteralStarStarArgs whether to allow **kwargs in function calls if the dict is a
+   *     literal. This is needed for some functions that take arbitrary keyword arguments whose keys
+   *     may have to contain non-identifier characters.
    */
-  public DotBazelFileSyntaxChecker(String where, boolean canLoadBzl) {
+  public DotBazelFileSyntaxChecker(
+      String where, boolean canLoadBzl, boolean allowLiteralStarStarArgs) {
     this.where = where;
     this.canLoadBzl = canLoadBzl;
+    this.allowLiteralStarStarArgs = allowLiteralStarStarArgs;
+  }
+
+  public DotBazelFileSyntaxChecker(String where, boolean canLoadBzl) {
+    this(where, canLoadBzl, /* allowLiteralStarStarArgs= */ false);
   }
 
   public final void check(StarlarkFile file) throws SyntaxError.Exception {
@@ -74,12 +85,19 @@ protected void error(Location loc, String message) {
   // Reject f(*args) and f(**kwargs) calls.
   private void rejectStarArgs(CallExpression call) {
     for (Argument arg : call.getArguments()) {
-      if (arg instanceof Argument.StarStar) {
-        error(
-            arg.getStartLocation(),
-            "**kwargs arguments are not allowed in "
-                + where
-                + ". Pass the arguments in explicitly.");
+      if (arg instanceof Argument.StarStar starStar) {
+        if (!allowLiteralStarStarArgs) {
+          error(
+              arg.getStartLocation(),
+              "**kwargs arguments are not allowed in "
+                  + where
+                  + ". Pass the arguments in explicitly.");
+        }
+        if (!(starStar.getValue() instanceof DictExpression)) {
+          error(
+              arg.getStartLocation(),
+              "**kwargs arguments must be a literal dict in " + where + ".");
+        }
       } else if (arg instanceof Argument.Star) {
         error(
             arg.getStartLocation(),

src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java

@@ -1947,6 +1947,56 @@ public void testOverrideRepo_cycle_multipleExtensions() throws Exception {
         which is not supported.""");
   }
 
+  @Test
+  public void testOverrideRepo_extraKeywordArguments_literal() throws Exception {
+    scratch.overwriteFile(
+        rootDirectory.getRelative("MODULE.bazel").getPathString(),
+        """
+        module(name='aaa')
+        ext1 = use_extension('//:defs.bzl', 'ext1')
+        use_repo(ext1, "foo")
+        ext2 = use_extension('//:defs.bzl', 'ext2')
+        use_repo(ext2, "bar.2")
+        override_repo(ext2, **{"foo.2": "foo"})
+        """);
+
+    EvaluationResult<RootModuleFileValue> result =
+        evaluator.evaluate(
+            ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext);
+    assertThat(result.hasError()).isFalse();
+
+    var ext2Usage =
+        result.get(ModuleFileValue.KEY_FOR_ROOT_MODULE).module().getExtensionUsages().get(1);
+    assertThat(ext2Usage.getRepoOverrides().keySet()).containsExactly("foo.2");
+  }
+
+  @Test
+  public void testOverrideRepo_extraKeywordArguments_nonLiteral() throws Exception {
+    scratch.overwriteFile(
+        rootDirectory.getRelative("MODULE.bazel").getPathString(),
+        """
+        module(name='aaa')
+        ext1 = use_extension('//:defs.bzl', 'ext1')
+        use_repo(ext1, "foo")
+        ext2 = use_extension('//:defs.bzl', 'ext2')
+        use_repo(ext2, "bar.2")
+        KWARGS = {"foo.2": "foo"}
+        override_repo(ext2, **KWARGS)
+        """);
+
+    reporter.removeHandler(failFastHandler);
+    EvaluationResult<RootModuleFileValue> result =
+        evaluator.evaluate(
+            ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext);
+    assertThat(result.hasError()).isTrue();
+
+    assertContainsEvent(
+        """
+        ERROR /workspace/MODULE.bazel:7:21: **kwargs arguments must be a literal dict in \
+        MODULE.bazel files.\
+        """);
+  }
+
   @Test
   public void testInjectRepo_imported() throws Exception {
     scratch.overwriteFile(