-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: add modernize analyzer suite #6126
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
Conversation
6067bf6
to
b9b3d11
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dang, that's a lot of test cases, nice! ⭐
I noticed there's 100 comments saying nope
and 24 saying noop
(x2 for the fixed version). Is this intentional? I guess noop
would make more sense since there's no operation from any analyzer in those scenarios.
Also super nit, but there's a mix between Nope.
, nope:
and nope,
prefixed lines where we don't expect any diagnostic.
Of course, none of this is something I would block on and the clarification to why they don't have any diagnostic is what matters!
The test files are from |
- '^math\.' | ||
- '^http\.StatusText$' | ||
|
||
modernize: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great to finally see this in here! 🚀 What are the default values for this linter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default values are all modernize analyzers.
Related to #6123
Related to #686