-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add rule deprecation infrastructure #9689
Changes from all commits
0905672
a6bca14
0585649
1746a10
b0db050
fc82f68
0892737
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,6 +52,9 @@ pub enum RuleGroup { | |
Stable, | ||
/// The rule is unstable, and preview mode must be enabled for usage. | ||
Preview, | ||
/// The rule has been deprecated, warnings will be displayed during selection in stable | ||
/// and errors will be raised if used with preview mode enabled. | ||
Deprecated, | ||
/// Legacy category for unstable rules, supports backwards compatible selection. | ||
#[deprecated(note = "Use `RuleGroup::Preview` for new rules instead")] | ||
Nursery, | ||
|
@@ -424,8 +427,8 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { | |
(Flake8Annotations, "001") => (RuleGroup::Stable, rules::flake8_annotations::rules::MissingTypeFunctionArgument), | ||
(Flake8Annotations, "002") => (RuleGroup::Stable, rules::flake8_annotations::rules::MissingTypeArgs), | ||
(Flake8Annotations, "003") => (RuleGroup::Stable, rules::flake8_annotations::rules::MissingTypeKwargs), | ||
(Flake8Annotations, "101") => (RuleGroup::Stable, rules::flake8_annotations::rules::MissingTypeSelf), | ||
(Flake8Annotations, "102") => (RuleGroup::Stable, rules::flake8_annotations::rules::MissingTypeCls), | ||
(Flake8Annotations, "101") => (RuleGroup::Deprecated, rules::flake8_annotations::rules::MissingTypeSelf), | ||
(Flake8Annotations, "102") => (RuleGroup::Deprecated, rules::flake8_annotations::rules::MissingTypeCls), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm less certain on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't see why. It think the annotations here are either There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PEP-673 has a really annoying restriction that says you can't use You also might need to use a custom type variable for But I agree that this seems like a pretty pointless rule, honestly. The vast majority of the time, you can leave these unannotated, and no type checker is going to care if you leave them unannotated. If you need to annotate |
||
(Flake8Annotations, "201") => (RuleGroup::Stable, rules::flake8_annotations::rules::MissingReturnTypeUndocumentedPublicFunction), | ||
(Flake8Annotations, "202") => (RuleGroup::Stable, rules::flake8_annotations::rules::MissingReturnTypePrivateFunction), | ||
(Flake8Annotations, "204") => (RuleGroup::Stable, rules::flake8_annotations::rules::MissingReturnTypeSpecialMethod), | ||
|
@@ -842,7 +845,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { | |
(Tryceratops, "002") => (RuleGroup::Stable, rules::tryceratops::rules::RaiseVanillaClass), | ||
(Tryceratops, "003") => (RuleGroup::Stable, rules::tryceratops::rules::RaiseVanillaArgs), | ||
(Tryceratops, "004") => (RuleGroup::Stable, rules::tryceratops::rules::TypeCheckWithoutTypeError), | ||
(Tryceratops, "200") => (RuleGroup::Stable, rules::tryceratops::rules::ReraiseNoCause), | ||
(Tryceratops, "200") => (RuleGroup::Deprecated, rules::tryceratops::rules::ReraiseNoCause), | ||
(Tryceratops, "201") => (RuleGroup::Stable, rules::tryceratops::rules::VerboseRaise), | ||
(Tryceratops, "300") => (RuleGroup::Stable, rules::tryceratops::rules::TryConsiderElse), | ||
(Tryceratops, "301") => (RuleGroup::Stable, rules::tryceratops::rules::RaiseWithinTry), | ||
|
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.
I kind of preferred what we had before over a separate section, but I assume you tried to just extend the previous layout and this looked better?
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.
For the legend specifically? I didn't like how much blank space there was so I tried removing that and it felt awkward. I find having all of sentences weirdly verbose for documenting the meanings of the icons.
One thing we could do here is link to the legend from the icons in the rule table. I've tried clicking them multiple times :D
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.
That's interesting. We could even put it at the bottom I guess.
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.
Let's focus on this separately #9711