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

Validate gocritic settings. Return error if settings includes a unsupported gocritic checker #1563

Merged
merged 2 commits into from Jan 4, 2021

Conversation

sebastien-rosset
Copy link
Contributor

@sebastien-rosset sebastien-rosset commented Dec 17, 2020

This PR performs additional gocritic settings validation. The settings attribute is supposed to be a map where each key is the name of a supported gocritic checker.

The existing validation already warns if the config data contains a settings for a disabled checker. But it does not validate the checker is supported. There is also an existing validation that the specific parameter is supported or not.

For example, as shown in the yaml config data below, ruleguard is a supported gocritic checker, but rangeValueCopy is not (the actual checker is rangeValCopy).

  gocritic:
    settings:
      rangeValueCopy:
        sizeThreshold: 32
      ruleguard:
        rules: myrules.go

@CLAassistant
Copy link

CLAassistant commented Dec 17, 2020

CLA assistant check
All committers have signed the CLA.

@sebastien-rosset
Copy link
Contributor Author

sebastien-rosset commented Dec 17, 2020

Note: I've noticed a separate but related problem: if the input ruleguard rules.go has a syntax error, the error is silently ignored and the entire ruleguard file is ignored. This is unlike the gocritic and ruleguard behavior which print an error message and exit with error status. I'm trying to find why this is happening.

@sebastien-rosset
Copy link
Contributor Author

@quasilyte, would you be able to review this PR? thank you.

Copy link
Contributor

@quasilyte quasilyte left a comment

Choose a reason for hiding this comment

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

Looks good.

@sebastien-rosset
Copy link
Contributor Author

@ldez , I see you have removed two reviewers. I had added them because they seemed to be the most recent committers for this file. Are you the reviewer?

Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

From my point of view, it's better to let people add themselves as reviewers if they want to review.

@ldez ldez added the enhancement New feature or improvement label Jan 4, 2021
@ldez ldez merged commit 62710a8 into golangci:master Jan 4, 2021
ashanbrown pushed a commit to ashanbrown/golangci-lint that referenced this pull request Feb 20, 2021
@ldez ldez added this to the v1.35 milestone Mar 6, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants