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

feat: disable copyloopvar and intrange on Go < 1.22 #4397

Merged
merged 10 commits into from
Feb 19, 2024

Conversation

ldez
Copy link
Member

@ldez ldez commented Feb 17, 2024

I propose to disable copyloopvar and intrange for Go < 1.22 to avoid reporting "false-positives" on at least Go 1.21.

$ ./golangci-lint run                                
WARN [linters_context] copyloopvar: This linter is disabled because the Go version of your project is lower than Go 1.22. 
WARN [linters_context] intrange: This linter is disabled because the Go version of your project is lower than Go 1.22. 

The log level is warn because we don't display logs with a lower level (ex: info).
But maybe we can just be quiet and just use the level info 🤔

Note: I know that loopvar can be enabled on go1.21 but if you do that it's better to either use go1.22 or override the Go version inside the golangci-lint configuration and enable the right linters.

@ldez ldez added enhancement New feature or improvement linter: update Update the linter implementation inside golangci-lint labels Feb 17, 2024
@ldez ldez changed the title feat: disable copyloopvar and intrange on Go 1.22 feat: disable copyloopvar and intrange on Go < 1.22 Feb 17, 2024
@bombsimon
Copy link
Member

Nice, makes a lot of sense! I also don't know what's best here regarding log level but making the user aware must be a good thing I guess. I think the common case for this would be users using enable-all on older Go versions and they can easily just disable the linters to get rid of it just like with deprecated linters.

@ldez
Copy link
Member Author

ldez commented Feb 17, 2024

I forget to add directives on the test files 😄

@ldez ldez force-pushed the feat/disable-go122-linters branch 2 times, most recently from 6fcbb73 to c6a3ee4 Compare February 17, 2024 20:04
pkg/lint/linter/config.go Outdated Show resolved Hide resolved
pkg/config/config.go Show resolved Hide resolved
test/run_test.go Outdated Show resolved Hide resolved
pkg/lint/linter/config.go Outdated Show resolved Hide resolved
@ldez ldez marked this pull request as draft February 17, 2024 20:30
@ldez
Copy link
Member Author

ldez commented Feb 17, 2024

Sorry for all the commits, switching between 2 Go versions to run tests is a bit painful and confusing.

@ldez ldez marked this pull request as ready for review February 18, 2024 03:30
@ldez ldez requested a review from Antonboom February 18, 2024 03:30
// TODO(ldez) there is a major problem with the executor:
// the parsing of the configuration and the timing to load the configuration and linters are creating unmanageable situations.
// There is no simple solution because it's spaghetti code.
// I need to completely rewrite the command line system and the executor because it's extremely time consuming to debug,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

offtop: let's make an issue?
or how do you track to-do backlog?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm currently working on the topic.

pkg/config/config.go Show resolved Hide resolved
pkg/lint/linter/linter.go Outdated Show resolved Hide resolved
pkg/lint/linter/linter.go Outdated Show resolved Hide resolved
@ldez ldez enabled auto-merge (squash) February 19, 2024 13:52
@ldez ldez merged commit 64492b5 into golangci:master Feb 19, 2024
12 checks passed
@ldez ldez deleted the feat/disable-go122-linters branch February 19, 2024 13:58
@ldez ldez mentioned this pull request Feb 21, 2024
Antonboom pushed a commit to Antonboom/golangci-lint that referenced this pull request Mar 3, 2024
@ldez ldez added this to the next milestone Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement linter: update Update the linter implementation inside golangci-lint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants