language/go: merge new Go naming convention to master

[jayconrod]

What type of PR is this?

Feature

What package or component does this PR mostly affect?

//language/go

What does this PR do? Why is it needed?

• Adds a configurable naming convention for the the //language/go extension.
  • In import mode, libraries and tests will be named based on the last component of their import paths.
  • In go_default_library mode, libraries and tests will be named go_default_library and go_default_test.
  • In import_alias mode, targets are generated with the import mode, but aliases are generated with the go_default_library mode. This should be used in repositories that other projects may depend on. go_repository uses this mode by default.
• If no naming convention is specified, Gazelle will attempt to detect one by reading build files in the root directory and subdirectories (non-recursively). If no naming convention is detected (as in a new project), Gazelle defaults to import.
• To simplify migration, Gazelle can rename targets previously generated in a different naming convention. This happens for both fix and update.
• Gazelle itself now uses the import_alias naming convention.

Credit to @tomlu for implementing most of this.

Which issues(s) does this PR fix?

Fixes #5

Other notes for review

cc @linzhp @blico: Please take a look and see if this will cause any issues for you. My expectation is that it should auto-detect the go_default_library mode, and then very little should change until you explicitly specify another mode.

cc @tomlu: I think you've been using this branch for a while, but please confirm this final version works for you.

[linzhp]

Checking

[tomlu]

I ran this on our depot and ran into the issue where local_repository repos are assumed to have go_default_library naming convention. As previously discussed we can go and add directives to the WORKSPACE file (about 60-70 or so at present), but I'd also have to do something to make sure they are added for new go repos. This would be slightly hard because I can't tell locally from a local_repository rule whether it's a Go repo or not.

I don't suppose there could be yet another directive to set the default if there is no info?

[blico]

My expectation is that we should be able to pull in this change without having to update our naming convention from go_default_library to import mode. Because of the structure of our repo, Gazelle does not auto-detect any naming conventions, meaning it defaults to import mode. To fix, I added -go_naming_convention=go_default_library and -go_naming_convetion_extern=go_default_library to our Gazelle invocation. I also added these directives to all of our go_repository rules to avoid having to update any dep attributes within our repo.

After adding these flags I am still running into some issues:

no such target '@com_github_pmezard_go_difflib//difflib:difflib': target 'difflib' not declared in package 'difflib' (did you mean 'difflib.go'?) defined by /private/var/tmp/_bazel_blico/c2029ca547e9eb006cd11184438b8215/external/com_github_pmezard_go_difflib/difflib/BUILD.bazel and referenced by '@bazel_gazelle//cmd/gazelle:gazelle_lib

The problem here appears to be that Gazelle and our repo share a dependency on com_github_pmezard_go_difflib, and Gazelle depends on the imports version whereas our repo depends on the go_default_library version. I see a similar error for all shared dependencies between Gazelle and our repo.

Gazelle is also adding an unwanted :go_default_library to the embed attr of go_test rules where we have a BUILD.bazel with two go_test rules defined. For example:

go_library(
    name = "go_default_library",
    srcs = [
        "constants.go",
        "handler.go",
    ],
    importpath = "github.com/foo",
    visibility = ["//visibility:public"],
)

EXTERNAL_TEST_SRCS = [
    "open_diff_test.go",
]

go_test(
    name = "go_default_test",
    # Remove once https://github.com/bazelbuild/rules_go/issues/1877 is resolved.
    # keep
    srcs = glob(
        ["*_test.go"],
        exclude = EXTERNAL_TEST_SRCS,
    ),
    embed = [":go_default_library"],
)

go_test(
    name = "go_external_test",
    srcs = ["open_diff_test.go"],
+   embed = [":go_default_library"],
    deps = [
        ":go_default_library",
    ],
)

Finally, Gazelle is giving a warning for the following case with multiple go_library rules defined in a single BUILD.bazel:

genrule(
   name = "gen",
   out = "bar.go",
)

go_library(
    name = "gen",
    srcs = ["bar.go"],
    importpath = "github.com/foo/gen",
    deps = [
        ":go_default_library",
    ],
)

go_library(
    name = "go_default_library",
    srcs = ["foo.go"],
    importpath = "github.com/foo",
)

Warning: Tried to rename gen to go_default_library, but go_default_library already exists.

[jayconrod]

Thanks @blico, that feedback is really helpful.

I'm curious why it was necessary to set build_naming_convention = "go_default_library" on go_repository rules. By default, go_repository should use import_alias, which will generate targets with the new names and alias with the old names, so either should work. As of the latest commit in this PR (yesterday afternoon), Gazelle should resolve external dependencies using -go_naming_convention_extern if that's set, or the naming convention in the current directory if not. Either way, I'd expect your deps to follow the go_default_library convention.

I'll put together a PR soon with the following changes:

• If needed, rewrite deps in the Gazelle repo with -go_naming_convention_external=go_default_library for now.
• Don't add embed of :go_default_library when multiple go_test targets are present.
• Narrow the cases where renaming is performed to avoid that warning.
• Probably change go_naming_convention_extern to go_naming_convention_external. I realized there's no good reason to abbreviate "external".

[blico]

I was able to confirm we do not need to set go_naming_convention=go_default_library on all go_repository rules (only the ones with patches to BUILD.bazel files), meaning it is no longer a problem for us to have the Gazelle repo deps with the new convention.

[jayconrod]

Ok, the issues above are fixed, so I think this is basically ready to go. Let me know if you see any other problems. If not, I'll merge this tomorrow.

[achew22]

Congrats, @jayconrod and @tomlu! This is a huge step foward for rules_go