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

var-naming rule to check that plural vars with ID are capitalised #947

Closed
VincentBaron opened this issue Nov 29, 2023 · 7 comments · Fixed by #964
Closed

var-naming rule to check that plural vars with ID are capitalised #947

VincentBaron opened this issue Nov 29, 2023 · 7 comments · Fixed by #964

Comments

@VincentBaron
Copy link
Contributor

VincentBaron commented Nov 29, 2023

Is your feature request related to a problem? Please describe.
the var-naming rule only checks that vars ending in ID are capitalised (e.g. userId => userID). I think it would be nice that it also check that vars ending in IDs are checked. (e.g. userIds => userIDs).

Describe the solution you'd like
Add a condition to the check that plurals vars such as userIDs are well capitalised.
Happy to do the PR 🤗

@denisvmedia
Copy link
Collaborator

That's an interesting proposal. But we should be careful in the scope definition. E.g. userIdx is not something that should be capitalized. Do you have a definition of what should be capitalized and what not?

@VincentBaron
Copy link
Contributor Author

I would say all vars ending in Ids should be IDs. That's all. What do you think?

@denisvmedia
Copy link
Collaborator

Probably ti is fine. However, I'm wondering how hard it affects the existing projects? Can you check any existing project like Kubernetes how it will affect it?

@mfederowicz
Copy link
Contributor

mfederowicz commented Dec 30, 2023

@denisvmedia I run on kubernetes master branch (https://github.com/kubernetes/kubernetes) :

revive -config ~/revive-var-naming.toml -formatter stylish ./...

and there are 10863 problems (0 errors) (10863 warnings) - only related with var-naming rule (pkg/staging/vendor/plugin dirs) maybe they are not using linters :P

@denisvmedia
Copy link
Collaborator

I see. Please feel free to submit a PR for this issue.

@mfederowicz
Copy link
Contributor

hehe of course with not using linters in kubernetes projcect was a joke:

git:master ~/git-repos/kubernetes 
=$ make lint
go version go1.21.5 linux/amd64
installing golangci-lint and logcheck plugin from hack/tools into /home/mfederowicz/git-repos/kubernetes/_output/local/bin
running /home/mfederowicz/git-repos/kubernetes/_output/local/bin/golangci-lint run --color=always --config=/home/mfederowicz/git-repos/kubernetes/hack/golangci.yaml ./...

but my local test was with only one rule: var-naming in config:

=$ cat revive-var-naming.toml
ignoreGeneratedHeader = false
severity = "warning"
confidence = 0.8
errorCode = 0
warningCode = 0

[rule.var-naming]

so @VincentBaron if you like issue is yours :P

@VincentBaron
Copy link
Contributor Author

Okay, thanks guys! Working on it 😊

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

Successfully merging a pull request may close this issue.

4 participants