improve diagnostics by adding the checker name to the diagnostic message #50
Conversation
This will help people to understand where the diagnostic is coming from. But also, to easily understand what checker could be deactivated if they want to. The change is fully compatible with the current tests. It explains why the tests are not updated in this commit.
|
@mmorel-35 @alexandear @catenacyber please review FYI: I introduced type aliases in order to virtually create a type, that can be easily understood. Please let me know it you think it's overkill or OK |
| @@ -54,6 +54,48 @@ func newPerfSprint() *perfSprint { | |||
| } | |||
| } | |||
|
|
|||
| // checker is a type of check that can be performed by the analyzer. | |||
| // A checker can activate or deactivate specific rules. | |||
| type checker = string | |||
There was a problem hiding this comment.
You mean versus:
- a string
- or versus
type checker string(without the =)
I wanted a type, but I wanted something that didn't require type juggling when passing constants as flags
As I said in my first comment, I can remove everything and use string directly.
There was a problem hiding this comment.
Let's use a string then.
(Or ok for type checker string and I do not think you need type juggling, just defining the constants with the right type and using them...)
| checkerOptionSprintf1 checkerOption = "sprintf1" | ||
| // checkerOptionStrConcat optimizes string concatenation | ||
| // This option is only available when [CheckerStringFormat] is activated. | ||
| checkerOptionStrConcat checkerOption = "strconcat" |
There was a problem hiding this comment.
This looks unnecessary to define a string to use it only once...
There was a problem hiding this comment.
I iterated, I was using them at some point. In the diagnostic, but then I left them like this.
But I agree, now the logic of type and even constants for the suboption seems overkill.
| checkerHexFormat checker = "hex-format" | ||
| ) | ||
|
|
||
| // checkerOption is a type a specific [checker] that can be activated or deactivated for a [checker]. |
There was a problem hiding this comment.
[] are redundant, because pkgsite ignores documentation for unexported types:
| // checkerOption is a type a specific [checker] that can be activated or deactivated for a [checker]. | |
| // checkerOption is a type a specific checker that can be activated or deactivated for a checker. |
| type checkerOption = checker | ||
|
|
||
| const ( | ||
| // checkerOptionIntConversion optimizes even if it requires an int or uint type cast |
There was a problem hiding this comment.
nit:
| // checkerOptionIntConversion optimizes even if it requires an int or uint type cast | |
| // checkerOptionIntConversion optimizes even if it requires an int or uint type cast. |
| // checkerOptionIntConversion optimizes even if it requires an int or uint type cast | ||
| // This option is only available when [CheckerIntegerFormat] is activated. | ||
| checkerOptionIntConversion checkerOption = "int-conversion" | ||
| // checkerOptionErrError optimizes into err.Error() even if it is only equivalent for non-nil errors |
There was a problem hiding this comment.
| // checkerOptionErrError optimizes into err.Error() even if it is only equivalent for non-nil errors | |
| // checkerOptionErrError optimizes into err.Error() even if it is only equivalent for non-nil errors. |
| // checkerOptionErrError optimizes into err.Error() even if it is only equivalent for non-nil errors | ||
| // This option is only available when [CheckerErrorFormat] is activated. | ||
| checkerOptionErrError checkerOption = "err-error" | ||
| // checkerOptionErrorf optimizes fmt.Errorf |
There was a problem hiding this comment.
| // checkerOptionErrorf optimizes fmt.Errorf | |
| // checkerOptionErrorf optimizes fmt.Errorf. |
| // checkerOptionErrorf optimizes fmt.Errorf | ||
| // This option is only available when [CheckerErrorFormat] is activated. | ||
| checkerOptionErrorf checkerOption = "errorf" | ||
| // checkerOptionSprintf1 optimizes fmt.Sprintf with only one argument |
There was a problem hiding this comment.
| // checkerOptionSprintf1 optimizes fmt.Sprintf with only one argument | |
| // checkerOptionSprintf1 optimizes fmt.Sprintf with only one argument. |
| // checkerOptionSprintf1 optimizes fmt.Sprintf with only one argument | ||
| // This option is only available when [CheckerStringFormat] is activated. | ||
| checkerOptionSprintf1 checkerOption = "sprintf1" | ||
| // checkerOptionStrConcat optimizes string concatenation |
There was a problem hiding this comment.
| // checkerOptionStrConcat optimizes string concatenation | |
| // checkerOptionStrConcat optimizes string concatenation. |
| // This option is only available when [CheckerStringFormat] is activated. | ||
| checkerOptionStrConcat checkerOption = "strconcat" | ||
|
|
||
| // checkerFixImports fix needed imports from other fixes |
There was a problem hiding this comment.
| // checkerFixImports fix needed imports from other fixes | |
| // checkerFixImports fix needed imports from other fixes. |
|
I'm |
This will help people to understand where the diagnostic is coming from.
But also, to easily understand what checker could be deactivated if they want to.
This is the implementation of what has been planned with #31
Fixes #43