Ensure repository names are valid workspace names

RELNOTES: Repository rules must use names that are valid workspace names.

--
MOS_MIGRATED_REVID=113197588

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

@@ -31,7 +31,7 @@
  */
 public class ExternalPackageBuilder {
 
-  public ExternalPackageBuilder createAndAddRepositoryRule(
+  public Rule createAndAddRepositoryRule(
       Package.Builder pkg,
       RuleClass ruleClass,
       RuleClass bindRuleClass,
@@ -42,17 +42,17 @@ public ExternalPackageBuilder createAndAddRepositoryRule(
 
     StoredEventHandler eventHandler = new StoredEventHandler();
     BuildLangTypedAttributeValuesMap attributeValues = new BuildLangTypedAttributeValuesMap(kwargs);
-    Rule tempRule =
+    Rule rule =
         RuleFactory.createRule(
             pkg, ruleClass, attributeValues, eventHandler, ast, ast.getLocation(), /*env=*/ null);
     pkg.addEvents(eventHandler.getEvents());
-    overwriteRule(pkg, tempRule);
+    overwriteRule(pkg, rule);
     for (Map.Entry<String, Label> entry :
-        ruleClass.getExternalBindingsFunction().apply(tempRule).entrySet()) {
+        ruleClass.getExternalBindingsFunction().apply(rule).entrySet()) {
       Label nameLabel = Label.parseAbsolute("//external:" + entry.getKey());
-      addBindRule(pkg, bindRuleClass, nameLabel, entry.getValue(), tempRule.getLocation());
+      addBindRule(pkg, bindRuleClass, nameLabel, entry.getValue(), rule.getLocation());
     }
-    return this;
+    return rule;
   }
 
   public void addBindRule(

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

@@ -58,6 +58,7 @@
  */
 public class WorkspaceFactory {
   public static final String BIND = "bind";
+  private static final Pattern LEGAL_WORKSPACE_NAME = Pattern.compile("^\\p{Alpha}\\w*$");
 
   private final LegacyBuilder builder;
 
@@ -166,6 +167,13 @@ private void execute(BuildFileAST ast, @Nullable Map<String, Extension> imported
     localReporter.clear();
   }
 
+  private static void checkWorkspaceName(String name, FuncallExpression ast) throws EvalException {
+    Matcher matcher = LEGAL_WORKSPACE_NAME.matcher(name);
+    if (!matcher.matches()) {
+      throw new EvalException(ast.getLocation(), name + " is not a legal workspace name");
+    }
+  }
+
   @SkylarkSignature(name = "workspace", objectType = Object.class, returnType = SkylarkList.class,
       doc = "Sets the name for this workspace. Workspace names should be a Java-package-style "
           + "description of the project, using underscores as separators, e.g., "
@@ -182,12 +190,7 @@ public BuiltinFunction create() {
               "workspace", FunctionSignature.namedOnly("name"), BuiltinFunction.USE_AST_ENV) {
             public Object invoke(String name, FuncallExpression ast, Environment env)
                 throws EvalException {
-              Pattern legalWorkspaceName = Pattern.compile("^\\p{Alpha}\\w*$");
-              Matcher matcher = legalWorkspaceName.matcher(name);
-              if (!matcher.matches()) {
-                throw new EvalException(
-                    ast.getLocation(), name + " is not a legal workspace name");
-              }
+              checkWorkspaceName(name, ast);
               String errorMessage = LabelValidator.validateTargetName(name);
               if (errorMessage != null) {
                 throw new EvalException(ast.getLocation(), errorMessage);
@@ -247,9 +250,10 @@ public Object invoke(Map<String, Object> kwargs, FuncallExpression ast, Environm
           Builder builder = PackageFactory.getContext(env, ast).pkgBuilder;
           RuleClass ruleClass = ruleFactory.getRuleClass(ruleClassName);
           RuleClass bindRuleClass = ruleFactory.getRuleClass("bind");
-          builder
+          Rule rule = builder
               .externalPackageData()
               .createAndAddRepositoryRule(builder, ruleClass, bindRuleClass, kwargs, ast);
+          checkWorkspaceName(rule.getName(), ast);
         } catch (
             RuleFactory.InvalidRuleException | Package.NameConflictException | LabelSyntaxException
                 e) {

src/test/java/com/google/devtools/build/lib/packages/ExternalPackageTest.java

@@ -41,35 +41,35 @@ public final void setWorkspacePath() throws Exception {
   public void testMultipleRulesWithSameName() throws Exception {
     FileSystemUtils.writeIsoLatin1(workspacePath,
         "local_repository(",
-        "    name = 'my-rule',",
+        "    name = 'my_rule',",
         "    path = '/foo/bar',",
         ")",
         "new_local_repository(",
-        "    name = 'my-rule',",
+        "    name = 'my_rule',",
         "    path = '/foo/bar',",
         "    build_file = 'baz',",
         ")");
 
     invalidatePackages();
     // Make sure the second rule "wins."
-    assertEquals("new_local_repository rule", getTarget("//external:my-rule").getTargetKind());
+    assertEquals("new_local_repository rule", getTarget("//external:my_rule").getTargetKind());
   }
 
   @Test
   public void testOverridingBindRules() throws Exception {
     FileSystemUtils.writeIsoLatin1(workspacePath,
         "bind(",
-        "    name = 'my-rule',",
+        "    name = 'my_rule',",
         "    actual = '//foo:bar',",
         ")",
         "new_local_repository(",
-        "    name = 'my-rule',",
+        "    name = 'my_rule',",
         "    path = '/foo/bar',",
         "    build_file = 'baz',",
         ")");
 
     invalidatePackages();
     // Make sure the second rule "wins."
-    assertEquals("new_local_repository rule", getTarget("//external:my-rule").getTargetKind());
+    assertEquals("new_local_repository rule", getTarget("//external:my_rule").getTargetKind());
   }
 }

src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java

@@ -69,6 +69,15 @@ public void testWorkspaceWithIllegalCharacters() throws Exception {
     assertThat(helper.getParserError()).contains("a.b.c is not a legal workspace name");
   }
 
+  @Test
+  public void testIllegalRepoName() throws Exception {
+    WorkspaceFactoryHelper helper = parse("local_repository(",
+        "    name = 'foo/bar',",
+        "    path = '/foo/bar',",
+        ")");
+    assertThat(helper.getParserError()).contains("foo/bar is not a legal workspace name");
+  }
+
   private WorkspaceFactoryHelper parse(String... args) {
     return new WorkspaceFactoryHelper(args);
   }