Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls: stylistic diagnostics emitted for generated code #36914

Closed
muirdm opened this issue Jan 31, 2020 · 7 comments
Closed

x/tools/gopls: stylistic diagnostics emitted for generated code #36914

muirdm opened this issue Jan 31, 2020 · 7 comments
Labels
Milestone

Comments

@muirdm
Copy link

@muirdm muirdm commented Jan 31, 2020

In particular the staticcheck "ST*" stylistic checks seem to be reporting diagnostics for generated code. I think the staticcheck standalone driver suppresses stylistic errors for generated files.

To reproduce, ensure gopls staticcheck integration is enabled and create a file foo.gen.go:

// Code generated by robot.
// DO NOT EDIT!

package foo

var REALLY_Good_Name = 1

I expect to see no diagnostic for "REALLY_Good_Name" but I get ST1003.

@gopherbot gopherbot added this to the Unreleased milestone Jan 31, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Jan 31, 2020

Thank you for filing a gopls issue! Please take a look at the Troubleshooting guide, and make sure that you have provided all of the relevant information here.

@stamblerre stamblerre modified the milestones: Unreleased, gopls/v1.0.0 Jan 31, 2020
@dominikh
Copy link
Member

@dominikh dominikh commented Jan 31, 2020

The issue here seems to be that your "code generated by" comment doesn't match the agreed upon format, nor any of the explicit exceptions that staticcheck handles. See #13560 and #13560 (comment) in particular.

@muirdm
Copy link
Author

@muirdm muirdm commented Jan 31, 2020

I based my comment off of tools/internal/lsp/source/util.go:

// Matches cgo generated comment as well as the proposed standard:
//	https://golang.org/s/generatedcode
var generatedRx = regexp.MustCompile(`// .*DO NOT EDIT\.?`)

This updated repro still reports the stylistic warning:

// Code generated by robot. DO NOT EDIT
package foo
var FOO_Bar = 1

The generated file suppression is in the staticcheck driver, no in the checks themselves, right? I didn't see any code in gopls trying to skip analyses based on generatedness.

@muirdm
Copy link
Author

@muirdm muirdm commented Jan 31, 2020

Ah, nevermind. I see the fact now in ReportfFG.

If I add the period at the end // Code generated by robot. DO NOT EDIT. it seems to work. There is some funny caching going on that made transitioning from not-generated to generated not work.

@muirdm muirdm closed this Jan 31, 2020
@dominikh
Copy link
Member

@dominikh dominikh commented Jan 31, 2020

For people watching at home: AFAIK gopls has no notion of hiding diagnostics in generated files, but the checks in staticcheck use a special analysis (honnef.co/go/tools/facts.Generated) to detect generated files. As such, using staticcheck's analyses in gopls (or any other runner) still benefits from the detection of generated files. Each analysis itself gets to decide whether a diagnostic should be hidden from generated files or not, without the runner's involvement.

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Jan 31, 2020

FWIW, gopls does detect generated files in some cases (for example, if you start to edit the file). We don't hide diagnostics because you would want to know if it doesn't compile, but I guess what you're saying is that we can also rely on staticcheck to make a reasonable decision about if a diagnostic is worth showing in a generated file?

@dominikh
Copy link
Member

@dominikh dominikh commented Jan 31, 2020

but I guess what you're saying is that we can also rely on staticcheck to make a reasonable decision about if a diagnostic is worth showing in a generated file?

That is correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.