Rules_go: nogo: include and exclude targets in configuration

[nikunjy]

What version of rules_go are you using?

 http_archive(
        name = "io_bazel_rules_go",
        sha256 = "87f0fb9747854cb76a0a82430adccb6269f7d394237104a4523b51061c469171",
        urls = [
            "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.23.1/rules_go-v0.23.1.tar.gz",
            "https://github.com/bazelbuild/rules_go/releases/download/v0.23.1/rules_go-v0.23.1.tar.gz",
        ],
    )

What version of gazelle are you using?

    http_archive(
        name = "bazel_gazelle",
        sha256 = "bfd86b3cbe855d6c16c6fce60d76bd51f5c8dbc9cfcaef7a2bb5c1aafd0710e8",
        urls = [
            "https://mirror.bazel.build/github.com/bazelbuild/bazel-gazelle/releases/download/v0.21.0/bazel-gazelle-v0.21.0.tar.gz",
            "https://github.com/bazelbuild/bazel-gazelle/releases/download/v0.21.0/bazel-gazelle-v0.21.0.tar.gz",
        ],
    )

What version of Bazel are you using?

bazel 3.1.0-homebrew

Does this issue reproduce with the latest releases of all the above?

Yes

What operating system and processor architecture are you using?

Macos

Any other potentially useful information about your toolchain?

I am using go modules.

What did you do?

go_register_toolchains(nogo = "@io_bazel_rules_go//:tools_nogo")
I want to use nogo. I followed the instructions ( I know it mentions this) and it lines external packages which I have no control over. Is there a way I can limit it my code ?

What did you expect to see?

Lint only my code

What did you see instead?

Lints everything in the external.

external/org_golang_x_tools/go/vcs/vcs.go:532:2: declaration of "metaImport" shadows declaration at line 599

[jayconrod]

This is kind of hard to set up. The Configuring analyzers section in the nogo documentation has some information on this. However, the configuration language leaves a lot to be desired, and there's no simple way to exclude external packages. I'll retitle the issue to reflect that.

Nogo definitely needs to be able to analyze packages in external repos though, and that should remain the default.

[blico]

Our repo ran into an issue where we were unintentionally linting an external library.

As an attempt to prevent linting external libraries within our nogo config all of our linters have the line:

    "only_files": {
      "src/": ""
    }

However, if an external repo contains src/ within its path it will still get linted because the regexp matches (e.g. @github_foo//src/bar.go)

At first, I tried to fix by adding an anchor (e.g. ^src/) to only_files, but I discovered the entire bazel outputRoot is included in the file path matched against the regex. What I ended up doing is adding the following to every linter:

    "exclude_files": {
      "__main__/external": ""
    }

[sluongng]

Its a bit confusing how nogo works... even with provided documentation.

I am running into this

analyzer "cgocall" failed: can't parse raw cgo file: open /tmp/rules_go_work-906102496/backup.go: no such file or directory

As we have an indirect dependency on github.com/mattn/go-sqlite3 which is cgo, I have added

     "cgocall": {
       "exclude_files": {
         "external/": "external doesn't pass vet"
       }
     },
     "unsafeptr": {
       "exclude_files": {
         "external/": "external doesn't pass vet"
       }
     },

to our config... but it still failing as its scanning from rules_go_work-* for whatever reason

So adding another exclude on rules_go_work-.*/ works for unsafeptr lint error, but the cgocall just straight up failing...

[jayconrod]

@sluongng That's #2396.

[tommyknows]

I'm hitting the same issue; tried to setup nogo but I get a ton of build-time code analysis errors for external projects (maybe also for internal code, I'm not getting that far).

Is it not possible to have some kind of option in the register-toolchain and disable nogo at runtime / analysis phase even?

Edit: I just gave this a shot and "solved" this here: https://github.com/bazelbuild/rules_go/compare/master...tommyknows:nogo-exclude

It's not really pretty, but maybe a good starting point for a proper PR?
I'd love to work on this, but have just started using Bazel and don't really know about any conventions or anything.
My commit would probably need to be cleaned up, variables named clearer etc...

[tian000]

@tommyknows That solution looks reasonable, and would definitely increase the usability of nogo for people running into the same issue. I think you should open a PR!, I am happy to open one based on your commit too.

[brianwolfe]

Would it make sense to just add an extra _all field to nogo configuration that has some merge semantics with the individual analysis fields?

[jayconrod]

Copying a comment from #2675:

I think it would be reasonable to add fields like only_targets and exclude_targets, like the only_files and exclude_files fields but the keys would be Bazel label patterns (like @my_repo//...) instead file regular expressions matching file names.

[jayconrod]

+1 to @brianwolfe's suggestion: we could reserve the analyzer name all to match all analyzers with some reasonable merge semantics. I prefer the name all over _all though.

[brianwolfe]

As long as we are confident there won't be an analyzer called all, I'm fine without the _ prefix 😛.