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

Skip analysis of generated files #148

Merged

Conversation

kseniadumpling
Copy link
Contributor

Hi! I have a suggestion to make, hope you don't mind :)
Since golangci-lint doesn't support SuggestedFixes (golangci/golangci-lint#1779), in my project I call wsl --fix separately. And it turns out that the linter fixes everything, including generated files (for example, files that are generated by protoc-gen-go).
This PR suggest to skip such files. It uses ast.IsGenerated(file *ast.File) as recommended in golang/go#43481 and golang/go#28089

@bombsimon
Copy link
Owner

bombsimon commented May 24, 2024

Thanks! I think this makes sense! Either you're not in control of the generator and can't do anything about it or you are and should fix the generator. Since generated files usually shouldn't be edited I can't think of a reason to not do this or make it an opt-in and just always ignore them.

I'll merge this and if someone really want's to fix generated files we can add a flag in the future.

@kseniadumpling There were some flakes in CI that got fixed, can you rebase on master please?

@coveralls
Copy link

coveralls commented May 24, 2024

Coverage Status

coverage: 93.651% (+0.01%) from 93.637%
when pulling 99d9859 on kseniadumpling:feature/skip-generated-files
into 6e3ba97 on bombsimon:master.

@kseniadumpling kseniadumpling force-pushed the feature/skip-generated-files branch from 52dcdb2 to 99d9859 Compare May 24, 2024 20:40
@kseniadumpling
Copy link
Contributor Author

@bombsimon sure!
rebased 👌🏻

@bombsimon bombsimon merged commit cf0fb6d into bombsimon:master May 24, 2024
4 checks passed
@kseniadumpling
Copy link
Contributor Author

Hi @bombsimon
Sorry to bother you, but is there any chance of seeing this change in the tagged version? :)

@bombsimon
Copy link
Owner

Hi @bombsimon Sorry to bother you, but is there any chance of seeing this change in the tagged version? :)

Not at all, thanks for the ping and patiently waiting! This was a miss by me, I've tagged v4.3.0 with this change included. Hope that helps!

@kseniadumpling
Copy link
Contributor Author

Yep, thanks a lot!

@ldez
Copy link
Contributor

ldez commented Jun 8, 2024

First, I agree with skipping generated files, but there is a missing case here: when you work on a generator.

When you work on a generator, you try to generate "right" files, so you need linters.

I think it should be an option.

@bombsimon
Copy link
Owner

bombsimon commented Jun 9, 2024

First, I agree with skipping generated files, but there is a missing case here: when you work on a generator.

When you work on a generator, you try to generate "right" files, so you need linters.

I think it should be an option.

Thanks for your input!

To me this feels like a rare edge case. I think this is reasonable for linters that check for bugs or security issues but for styling it makes less sense since you're likely not going to read (and never change) generated code.

Although not an optimal flow, a workaround if you're the author of the generator and want the code to be linted could be to simply avoid adding the Code generated comment and it will be picked up.

I can add a flag for it. If so, do you think it makes sense to ignore generated code by default?

I created PR #152 to add a flag, PTAL and let me know if this is what you had in mind @ldez

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

Successfully merging this pull request may close these issues.

4 participants