-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: add iotyper linter #6212
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
base: main
Are you sure you want to change the base?
feat: add iotyper linter #6212
Conversation
|
Hey, thank you for opening your first Pull Request ! |
|
In order for a pull request adding a linter to be reviewed, the linter and the PR must follow some requirements.
Pull Request Description
Base RequirementsThese requirements are not declarative; the team will verify them.
Linter
The Linter Tests Inside Golangci-lint
|
|
I have several remarks.
It's better if the repository and module names are the same as the base package. (avoid non-letter characters) I don't see the point with having an builtin standard type for This doesn't feel like a good practice: I can understand the usage of explicit custom type when defining enum, in some cases. // ❌ Bad
const (
StatusPending int = iota
StatusActive
StatusClosed
)// ✅ Good but only if this is useful to have a type.
type Status uint64
const (
StatusPending Status = iota
StatusActive
StatusClosed
)Based on my previous remarks, as a standalone linter, I think this is not a good idea, but I also think this can be an option (not enabled by default) of |
|
@ldez Sorry for the confusing example — to clarify: So the intention is not to allow something like: const (
Foo int = iota
)but rather to ensure that when type Status uint64
const (
StatusPending Status = iota
StatusActive
StatusClosed
) |
I already understood the objective, but your linter will result into encouraging builtin types usage.
This is not what your linter is doing: it reports all missing explicit types. |
|
@ldez You're absolutely right that, as currently designed, the linter ends up encouraging the use of standard types, because users may simply add And yes, you are also correct that the current implementation reports all missing explicit types, not just enum-like usages. The examples and tests indeed reflect this unintended behavior. Given your feedback, I'm thinking of revising the rule so that it only applies when Before I move in that direction, I want to confirm: Thanks again for pointing out the issue — it’s been very helpful. |
|
Inside golangci-lint we differentiate 2 types of analyzers:
The detectors are not allowed because of the number of false positives. IMO, your analyzer is currently a detector: it detects missing explicitly typed I think we agree on that. Maybe, if your analyzer was forcing the usage of dedicated types, so being more opinionated, it could be see as a linter, but even in this case I'm not sure, I need to think about. |
Adding iotyper linter.
This linter detects iota usage in const declarations without explicit type specification.
Explicit type annotation improves code clarity and type safety.
Example
This linter flags the following code: