language/go: fix remaining naming convention issues

* Rename goNamingConventionExtern to goNamingConventionExternal,
  together with the flag and directive. No need to abbrev.
* Only migrate go_library if it matches the expected import path for
  the directory. This avoids migrating the wrong target when there are
  multiple go_libraries in the same directory.
* Only migrate go_test if it embeds the library.
* When fixing rules, don't add an embed attribute to go_test.

For bazel-contrib#5

language/go/config.go

@@ -80,9 +80,9 @@ type goConfig struct {
 	// goNamingConvention controls the name of generated targets
 	goNamingConvention namingConvention
 
-	// goNamingConventionExtern controls the default naming convention for
+	// goNamingConventionExternal controls the default naming convention for
 	// imports in external repositories with unknown naming conventions.
-	goNamingConventionExtern namingConvention
+	goNamingConventionExternal namingConvention
 
 	// goProtoCompilers is the protocol buffers compiler(s) to use for go code.
 	goProtoCompilers []string
@@ -330,7 +330,7 @@ func (*goLang) KnownDirectives() []string {
 		"go_generate_proto",
 		"go_grpc_compilers",
 		"go_naming_convention",
-		"go_naming_convention_extern",
+		"go_naming_convention_external",
 		"go_proto_compilers",
 		"go_visibility",
 		"importmap_prefix",
@@ -377,8 +377,8 @@ func (*goLang) RegisterFlags(fs *flag.FlagSet, cmd string, c *config.Config) {
 			"go_naming_convention",
 			"controls generated library names. One of (go_default_library, import, import_alias)")
 		fs.Var(
-			&namingConventionFlag{&gc.goNamingConventionExtern},
-			"go_naming_convention_extern",
+			&namingConventionFlag{&gc.goNamingConventionExternal},
+			"go_naming_convention_external",
 			"controls naming convention used when resolving libraries in external repositories with unknown conventions")
 
 	case "update-repos":
@@ -534,9 +534,9 @@ Update io_bazel_rules_go to a newer version in your WORKSPACE file.`
 					log.Print(err)
 				}
 
-			case "go_naming_convention_extern":
+			case "go_naming_convention_external":
 				if nc, err := namingConventionFromString(d.Value); err == nil {
-					gc.goNamingConventionExtern = nc
+					gc.goNamingConventionExternal = nc
 				} else {
 					log.Print(err)
 				}

language/go/fix.go

@@ -40,7 +40,7 @@ func (_ *goLang) Fix(c *config.Config, f *rule.File) {
 func migrateNamingConvention(c *config.Config, f *rule.File) {
 	// Determine old and new names for go_library and go_test.
 	nc := getGoConfig(c).goNamingConvention
-	importPath := findImportPath(f)
+	importPath := InferImportPath(c, f.Pkg)
 	if importPath == "" {
 		return
 	}
@@ -69,41 +69,41 @@ func migrateNamingConvention(c *config.Config, f *rule.File) {
 		switch {
 		case r.Name() == libName:
 			haveLib = true
-		case r.Kind() == "go_library" && r.Name() == migrateLibName:
+		case r.Kind() == "go_library" && r.Name() == migrateLibName && r.AttrString("importpath") == importPath:
 			haveMigrateLib = true
 		case r.Name() == testName:
 			haveTest = true
-		case r.Kind() == "go_test" && r.Name() == migrateTestName:
+		case r.Kind() == "go_test" && r.Name() == migrateTestName && strListAttrContains(r, "embed", ":"+migrateLibName):
 			haveMigrateTest = true
 		}
 	}
-	haveLibConflict := haveLib && haveMigrateLib
-	haveTestConflict := haveTest && haveMigrateTest
-	if haveLibConflict {
+	if haveLib && haveMigrateLib {
 		log.Printf("%[1]s: Tried to rename %[2]s to %[3]s, but %[3]s already exists.", f.Path, migrateLibName, libName)
 	}
-	if haveTestConflict {
+	if haveTest && haveMigrateTest {
 		log.Printf("%[1]s: Tried to rename %[2]s to %[3]s, but %[3]s already exists.", f.Path, migrateTestName, testName)
 	}
+	shouldMigrateLib := haveMigrateLib && !haveLib
+	shouldMigrateTest := haveMigrateTest && !haveTest
 
 	// Rename the targets and stuff in the same file that refers to them.
 	for _, r := range f.Rules {
 		// TODO(jayconrod): support map_kind directive.
 		// We'll need to move the metaresolver from resolve.RuleIndex to config.Config so we can access it from here.
 		switch r.Kind() {
 		case "go_binary":
-			if !haveLibConflict {
+			if haveMigrateLib && shouldMigrateLib {
 				replaceInStrListAttr(r, "embed", ":"+migrateLibName, ":"+libName)
 			}
 		case "go_library":
-			if r.Name() == migrateLibName && !haveLibConflict {
+			if r.Name() == migrateLibName && shouldMigrateLib {
 				r.SetName(libName)
 			}
 		case "go_test":
-			if r.Name() == migrateTestName && !haveTestConflict {
+			if r.Name() == migrateTestName && shouldMigrateTest {
 				r.SetName(testName)
 			}
-			if !haveLibConflict {
+			if shouldMigrateLib {
 				replaceInStrListAttr(r, "embed", ":"+migrateLibName, ":"+libName)
 			}
 		}
@@ -127,34 +127,28 @@ func fileContainsGoBinary(c *config.Config, f *rule.File) bool {
 	return false
 }
 
-// findImportPath returns the existing import path from the first encountered Go
-// rule with the attribute set.
-func findImportPath(f *rule.File) string {
-	for _, r := range f.Rules {
-		switch r.Kind() {
-		case "go_binary", "go_library", "go_test":
-			if ip, ok := r.Attr("importpath").(*bzl.StringExpr); ok && ip.Value != "" {
-				return ip.Value
-			}
+func replaceInStrListAttr(r *rule.Rule, attr, old, new string) {
+	items := r.AttrStrings(attr)
+	changed := false
+	for i := range items {
+		if items[i] == old {
+			changed = true
+			items[i] = new
 		}
 	}
-	return ""
+	if changed {
+		r.SetAttr(attr, items)
+	}
 }
 
-func replaceInStrListAttr(r *rule.Rule, attr, old, new string) {
-	l := r.AttrStrings(attr)
-	var shouldAdd = true
-	var items []string
-	for _, v := range l {
-		if v != old {
-			items = append(items, v)
+func strListAttrContains(r *rule.Rule, attr, s string) bool {
+	items := r.AttrStrings(attr)
+	for _, item := range items {
+		if item == s {
+			return true
 		}
-		shouldAdd = shouldAdd && v != new
 	}
-	if shouldAdd {
-		items = append(items, new)
-	}
-	r.SetAttr(attr, items)
+	return false
 }
 
 // migrateLibraryEmbed converts "library" attributes to "embed" attributes,