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

Misspell corrected an imported constant's name. #10

Closed
bnkamalesh opened this issue Dec 18, 2023 · 7 comments
Closed

Misspell corrected an imported constant's name. #10

bnkamalesh opened this issue Dec 18, 2023 · 7 comments
Labels
question Further information is requested

Comments

@bnkamalesh
Copy link

bnkamalesh commented Dec 18, 2023

Misspell linter corrected a constant's name which was imported from a protobuff generated package.
The problematic constant name Status_STATUS_CANCELLED, was corrected to a single L. This broke our CI as well, because autofix: true is the default setting.

Ref:
microsoft/vscode#200638
golang/vscode-go#3087

Below I have provided the configuration of misspell.

  misspell:
    locale: US
    ignore-words:
      - cancelled // added this now as a work around

p.s: I added the respective word as a config under ignore-words as a work around.

@ldez
Copy link
Member

ldez commented Dec 18, 2023

Hello,

your issue is not clear: do you mean that ignore-words doesn't work?

@bnkamalesh
Copy link
Author

bnkamalesh commented Dec 19, 2023

Hello @ldez , thank you for the quick response and sorry for the confusion.

ignore-words is working as expected. The expected behaviour of misspell should only be correcting comments, but in this case it corrected a constant's name, which was imported from an external package.

@ldez
Copy link
Member

ldez commented Dec 19, 2023

The expected behaviour of misspell should only be correcting comments

I understand that can be surprising because it's a constant from another module but misspell checks all the code, neither just comments nor just code "own" by the current module, so the current behavior is expected.

The ignore-words is a solution but you can use different solutions based on exclusions like nolint.

func main() {
	//nolint:misspell
	fmt.Println(lib.Status_STATUS_CANCELLED)
}

https://golangci-lint.run/usage/false-positives/

@ldez ldez added the question Further information is requested label Dec 19, 2023
@bnkamalesh
Copy link
Author

Hello @ldez , I think there's some miscommunication if the expected behaviour of misspell is to correct any tokens. Because in the Readme it is mentioned explicitly

Are there special rules for golang source files?
Yes! If the file ends in .go, then misspell will only check spelling in comments.

And it does make sense to have this behaviour as auto-correcting spellings of tokens would easily break our applications.

@ldez
Copy link
Member

ldez commented Dec 20, 2023

The misspell readme says something wrong: the scope of the analysis is not related to file extension but to a specific mode, and the default mode is neither related to Go nor file extension.

This mode is related to a CLI flag (-source) and the doc of this flag is also wrong and misleading.
The doc of this flag says that there are 3 modes but, in the reality of the code, there are only 2: text (default) and go.

I also think that the go mode is misleading because it's not expected that a mode related to go only checks comments.

The current behavior of misspell inside golangci-lint is related to user's feedback (from 2019) golangci/golangci-lint#522

EDIT: I fixed the documentation (readme and CLI flags) of misspell.

@bnkamalesh
Copy link
Author

@ldez thanks for the clarification, and the quick responses are much appreciated. I'm closing the thread/issue as I believe it is solved.

@ldez
Copy link
Member

ldez commented Dec 20, 2023

@bnkamalesh I will add an option inside golangci-lint to handle the mode.

EDIT: golangci/golangci-lint#4275

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

No branches or pull requests

2 participants