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
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@muirdm
Copy link

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 gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Jan 31, 2020
@gopherbot
Copy link

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

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 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 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 as completed Jan 31, 2020
@dominikh
Copy link
Member

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

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

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 subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants