chore: refactor Diagnostic declaration #31
Conversation
mmorel-35
force-pushed
the
diagnostics
branch
from
099867e
to
d3a5621
Compare
mmorel-35
changed the title
|
Why do you want this change ? It is only cosmetic style, right ? |
|
It might seems yet but this will help bringing other changes like display the name of the rule that is causing an error For the moment I'm not able to identify clearly a "checker" for each diagnostic. This will help users identify which rule is causing errors. |
mmorel-35
force-pushed
the
diagnostics
branch
2 times, most recently
from
103df5d
to
244e106
Compare
mmorel-35
force-pushed
the
diagnostics
branch
2 times, most recently
from
d2d83bb
to
fbbb189
Compare
| Message: fn + " can be replaced with faster strconv.FormatBool", | ||
| SuggestedFixes: []analysis.SuggestedFix{ | ||
| d = newAnalysisDiagnostic( | ||
| "", // TODO: precise checker |
There was a problem hiding this comment.
Thanks for working on this
You help me to think further about my code suggestion here
Yes, these linters not only needs a name, reported as a prefix in the error message, but also they need an option to deactivate each option
There was a problem hiding this comment.
I guess a way to link the checker name with a CLI option to enable/disable it would be nice indeed.
mmorel-35
force-pushed
the
diagnostics
branch
2 times, most recently
from
221b96d
to
5d90147
Compare
analyzer/analyzer.go
Outdated
| intConversionChecker string = "int-conversion" | ||
| errErrorChecker string = "err-error" | ||
| errorfChecker string = "errorf" | ||
| sprintf1Checker string = "sprintf1" | ||
| fiximportsChecker string = "fiximports" | ||
| strconcatChecker string = "strconcat" |
There was a problem hiding this comment.
Please give them a common prefix, not suffix
For code completion plus clarity
There was a problem hiding this comment.
Thanks @mmorel-35 for doing the changes.
@alexandear do you know any linters about this?
Maybe it could be part of revive. I find this pattern so frequently in code.
Of course, this linter would be opinionated and might cause problem for exported constants
What do you think ?
We could move the discussion to revive repository maybe, but I would like to get feedbacks from you guys, before going further.
mmorel-35
force-pushed
the
diagnostics
branch
from
5d90147
to
06ac2e8
Compare
Ok, so can you add this change in a new commit ? |
|
Based on the changes made on this PR, I'll wait for this one to be merged before working on |
mmorel-35
force-pushed
the
diagnostics
branch
2 times, most recently
from
07192e5
to
ac50141
Compare
Signed-off-by: Matthieu MOREL <[email protected]>
mmorel-35
force-pushed
the
diagnostics
branch
from
ac50141
to
2bc4af1
Compare
|
Let's narrow this PR with new method only used with empty checker name. You first need to list and distinguish checkers options to proceed with the next changes. |
|
Thanks, waiting for the next step then |
Description
This provide a function to mutualise Diagnostic declaration