RFC: nogo ignore generated source code

[sluongng]

Implements a map that track usages of:

• Generated source code by rules_go
• Rewritten source code by rules_go

In case of generated source code, nogo will ignore those files by
default. In case of rewritten source code (i.e. coverage tests), nogo
should only be run over the original source file.

What type of PR is this?

Uncomment one line below and remove others.

Bug fix

What does this PR do? Why is it needed?

Enabled staticcheck to run in 2 cases:

• During 'bazel coverage' where test files are rewritten with a snake_case_variable
• When the package has _empty.go source file generated by rules_go

Which issues(s) does this PR fix?

Fixes #2984

Other notes for review

This is a more complex, experimental fix to replace #2995 as discussed in #2984.

It's worth noting how these code and logic are also duplicated in file go/tools/builders/compile.go but I could not find how compile.go is used. Through out my testing, only changes to compilepkg.go get picked up, so I did not bother updating the other file.

Similarly, coverage test rewrite logic is also shown in go/private/actions/cover.bzl but I don't see how that is used through out my testing.

Although I have done some rudimentary testing to ensure that this does indeed work on a real repo, this PR should be considered as an RFC and reviewed carefully as the current code health of this file is not the best there is.

[sluongng]

I wrote this RFC at a late hour on a Sunday and I think I might have jumped some words, making the PR description a bit gibberish to read through. I do apologies and have since tried to make some edit to fix some of the obvious mistakes. But if you see any incomplete sentences and things don't make sense, please do ask questions!

---

It's worth noting how these code and logic are also duplicated in file go/tools/builders/compile.go but I could not find how compile.go is used. Through out my testing, only changes to compilepkg.go get picked up, so I did not bother updating the other file.

Digging a bit more into commit history and this seems to explained it.

commit 528f6faf83f85c23da367d61f784893d1b3bd72b
Author: Jay Conrod <[email protected]>
Date:   Sat Apr 20 14:45:13 2019 -0400

    GoCompilePkg: unified action for building a Go package (#2027)

    GoCompilePkg will be a replacement for GoCompile and several other
    actions. It will consolidate all the work of building a Go archive
    into a single Bazel action: it will run the Go compiler, the C/C++
    compilers, the assembler, cgo, coverage, nogo, and pack all as one
    action. This should reduce sandboxing overhead for local
    configurations and file transfer overhead for remote configurations.

    This change introduces a very limited version of GoCompilePkg with
    most of the above functionality marked "TODO". It can only run the Go
    compiler. emit_archive uses this action for archives that don't
    require any other features.

    Updates #1957

So compile.go is most likely unused, though it was still receiving some updates since April 20th 2019.

While Jay is inactive, I see @steeve was active in #1957 so a small ping for comment request 🤗

[linzhp]

With regards to compile.go, here are more information: #2461 (comment)

[linzhp]

Minor comments. Otherwise looks good to me

[linzhp]

Also, can you add some tests to tests/core/nogo/coverage to verify the behavior?

[sluongng]

Minor comments. Otherwise looks good to me

Thanks for the review. All suggestions applied 🤝

Also, can you add some tests to tests/core/nogo/coverage to verify the behavior?

Let me look into this in the next few days.
I am currently testing this with an analyzer extracted from staticcheck,
so I don't know how much work would it be to set that up in the test. 🤔

[sluongng]

@linzhp I have a hard time writing test for this PR. So I raised #3129 in order to get the standard staticcheck analyzers into rules_go first.

Once thats resolved, we can simply have a test that would enable all staticcheck analyzers in a build. Some analyzer would then fail and this PR should help fixing that.

[sluongng]

I will close this RFC for now.

I have a series of PRs prepared with up-to-date code + new tests that should make this a bit nicer to review.