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/go/analysis: optionally ignore generated files #43481

Closed
gordonklaus opened this issue Jan 3, 2021 · 7 comments
Closed

x/tools/go/analysis: optionally ignore generated files #43481

gordonklaus opened this issue Jan 3, 2021 · 7 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@gordonklaus
Copy link
Contributor

This is a proposal to add an option to analysis.Analyzer to declare its disinterest in generated files. Since such files' contents are not directly within the user's control, it may be better for an analyzer to ignore them.

For example, we have converted ineffassign, which checks for ineffectual assignments, into an Analyzer. Such ineffectual assignments are not always indicative of an error but only sloppiness, which sometimes a code generator ought to be allowed to get away with.

Previously, we were using dmitri.shuralyov.com/go/generated, which checks if a file has a comment like // Code generated _ DO NOT EDIT., but now we scan Pass.Files[i].Comments rather than reading each whole file again.

Anyway, this proposal is to add a field Analyzer.IgnoreGeneratedFiles which, if true, would result in those files not being present in Pass.Files.

@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jan 3, 2021
@gopherbot gopherbot added this to the Unreleased milestone Jan 3, 2021
@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 5, 2021
@toothrot
Copy link
Contributor

toothrot commented Jan 5, 2021

/cc @matloob

@mvdan
Copy link
Member

mvdan commented Jan 11, 2021

Why is it a problem to filter which files you're interested in, when looping over Pass.Files?

@gordonklaus
Copy link
Contributor Author

It's not a problem; it just seemed like it might be common enough that this package should support it.

It is quite trivial for each tool author to implement but they still need to know about the standard. Better to enshrine it in code than documentation.

@mvdan
Copy link
Member

mvdan commented Jan 11, 2021

Indeed. But the solution then seems to me to have an API or package to detect the standard, not to add it to the internals of go/analysis. Detecting whether a file is generated is something generally useful, not just as part of go/analysis.

@gordonklaus
Copy link
Contributor Author

I agree it should live in its own package, and in fact it does: dmitri.shuralyov.com/go/generated. Additionally, my thought was that if some majority (60%? 90%?) of users of go/analysis used this feature, it would make sense to make it even more discoverable by bundling it in with go/analysis. It really depends whether it is considered a core feature.

I'm imagining that many users of go/analysis will be writing linters, which should ignore generated code. But I don't have any idea just what proportion would use it. Maybe someone could do an analysis of importers of go/analysis to see how many of them check for generated code (whether via dmitri.shuralyov.com/go/generated or by directly checking for a comment matching // Code generated ... DO NOT EDIT)? I recall @campoy used to do some BigQuery analyses to aid in design decisions – is that still easy to do?

@mvdan
Copy link
Member

mvdan commented Jan 14, 2021

#28089 is about adding this API to x/tools, which is official and more discoverable.

I still think there's no strong argument in favor of adding it straight to go/analysis. At least, most of the analyzers I've written apply to generated code as well.

@gordonklaus
Copy link
Contributor Author

Thanks for pointing out #28089. I somehow didn't come across it in my search.

It looks like the resolution of that issue will make for an easily discoverable API. I'll close this issue in favor of that.

@golang golang locked and limited conversation to collaborators Jan 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. 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