Skip to content
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

[Ruff 0.6] Stabilise 9 pylint rules #12857

Merged
merged 2 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,12 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "E0237") => (RuleGroup::Stable, rules::pylint::rules::NonSlotAssignment),
(Pylint, "E0241") => (RuleGroup::Stable, rules::pylint::rules::DuplicateBases),
(Pylint, "E0302") => (RuleGroup::Stable, rules::pylint::rules::UnexpectedSpecialMethodSignature),
(Pylint, "E0303") => (RuleGroup::Preview, rules::pylint::rules::InvalidLengthReturnType),
(Pylint, "E0303") => (RuleGroup::Stable, rules::pylint::rules::InvalidLengthReturnType),
(Pylint, "E0304") => (RuleGroup::Preview, rules::pylint::rules::InvalidBoolReturnType),
(Pylint, "E0305") => (RuleGroup::Preview, rules::pylint::rules::InvalidIndexReturnType),
(Pylint, "E0305") => (RuleGroup::Stable, rules::pylint::rules::InvalidIndexReturnType),
(Pylint, "E0307") => (RuleGroup::Stable, rules::pylint::rules::InvalidStrReturnType),
(Pylint, "E0308") => (RuleGroup::Preview, rules::pylint::rules::InvalidBytesReturnType),
(Pylint, "E0309") => (RuleGroup::Preview, rules::pylint::rules::InvalidHashReturnType),
(Pylint, "E0308") => (RuleGroup::Stable, rules::pylint::rules::InvalidBytesReturnType),
(Pylint, "E0309") => (RuleGroup::Stable, rules::pylint::rules::InvalidHashReturnType),
(Pylint, "E0604") => (RuleGroup::Stable, rules::pylint::rules::InvalidAllObject),
(Pylint, "E0605") => (RuleGroup::Stable, rules::pylint::rules::InvalidAllFormat),
(Pylint, "E0643") => (RuleGroup::Stable, rules::pylint::rules::PotentialIndexError),
Expand All @@ -225,8 +225,8 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "E1307") => (RuleGroup::Stable, rules::pylint::rules::BadStringFormatType),
(Pylint, "E1310") => (RuleGroup::Stable, rules::pylint::rules::BadStrStripCall),
(Pylint, "E1507") => (RuleGroup::Stable, rules::pylint::rules::InvalidEnvvarValue),
(Pylint, "E1519") => (RuleGroup::Preview, rules::pylint::rules::SingledispatchMethod),
(Pylint, "E1520") => (RuleGroup::Preview, rules::pylint::rules::SingledispatchmethodFunction),
(Pylint, "E1519") => (RuleGroup::Stable, rules::pylint::rules::SingledispatchMethod),
(Pylint, "E1520") => (RuleGroup::Stable, rules::pylint::rules::SingledispatchmethodFunction),
(Pylint, "E1700") => (RuleGroup::Stable, rules::pylint::rules::YieldFromInAsyncFunction),
(Pylint, "E2502") => (RuleGroup::Stable, rules::pylint::rules::BidirectionalUnicode),
(Pylint, "E2510") => (RuleGroup::Stable, rules::pylint::rules::InvalidCharacterBackspace),
Expand Down Expand Up @@ -256,7 +256,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "R1711") => (RuleGroup::Stable, rules::pylint::rules::UselessReturn),
(Pylint, "R1714") => (RuleGroup::Stable, rules::pylint::rules::RepeatedEqualityComparison),
(Pylint, "R1722") => (RuleGroup::Stable, rules::pylint::rules::SysExitAlias),
(Pylint, "R1730") => (RuleGroup::Preview, rules::pylint::rules::IfStmtMinMax),
(Pylint, "R1730") => (RuleGroup::Stable, rules::pylint::rules::IfStmtMinMax),
(Pylint, "R1733") => (RuleGroup::Preview, rules::pylint::rules::UnnecessaryDictIndexLookup),
(Pylint, "R1736") => (RuleGroup::Stable, rules::pylint::rules::UnnecessaryListIndexLookup),
(Pylint, "R2004") => (RuleGroup::Stable, rules::pylint::rules::MagicValueComparison),
Expand All @@ -273,13 +273,13 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "W0129") => (RuleGroup::Stable, rules::pylint::rules::AssertOnStringLiteral),
(Pylint, "W0131") => (RuleGroup::Stable, rules::pylint::rules::NamedExprWithoutContext),
(Pylint, "W0133") => (RuleGroup::Stable, rules::pylint::rules::UselessExceptionStatement),
(Pylint, "W0211") => (RuleGroup::Preview, rules::pylint::rules::BadStaticmethodArgument),
(Pylint, "W0211") => (RuleGroup::Stable, rules::pylint::rules::BadStaticmethodArgument),
(Pylint, "W0245") => (RuleGroup::Stable, rules::pylint::rules::SuperWithoutBrackets),
(Pylint, "W0406") => (RuleGroup::Stable, rules::pylint::rules::ImportSelf),
(Pylint, "W0602") => (RuleGroup::Stable, rules::pylint::rules::GlobalVariableNotAssigned),
(Pylint, "W0603") => (RuleGroup::Stable, rules::pylint::rules::GlobalStatement),
(Pylint, "W0604") => (RuleGroup::Stable, rules::pylint::rules::GlobalAtModuleLevel),
(Pylint, "W0642") => (RuleGroup::Preview, rules::pylint::rules::SelfOrClsAssignment),
(Pylint, "W0642") => (RuleGroup::Stable, rules::pylint::rules::SelfOrClsAssignment),
(Pylint, "W0711") => (RuleGroup::Stable, rules::pylint::rules::BinaryOpException),
(Pylint, "W1501") => (RuleGroup::Stable, rules::pylint::rules::BadOpenMode),
(Pylint, "W1508") => (RuleGroup::Stable, rules::pylint::rules::InvalidEnvvarDefault),
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/rules/pylint/rules/if_stmt_min_max.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::fix::snippet::SourceCodeSnippet;
///
/// ## Why is this bad?
/// An `if` statement that selects the lesser or greater of two sub-expressions
/// can be replaced with a `min()` or `max()` call respectively. When possible,
/// can be replaced with a `min()` or `max()` call respectively. Where possible,
/// prefer `min()` and `max()`, as they're more concise and readable than the
/// equivalent `if` statements.
///
Expand Down Expand Up @@ -194,7 +194,7 @@ enum MinMax {
}

impl MinMax {
fn as_str(self) -> &'static str {
const fn as_str(self) -> &'static str {
match self {
Self::Min => "min",
Self::Max => "max",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for `__bytes__` implementations that return a type other than `bytes`.
/// Checks for `__bytes__` implementations that return types other than `bytes`.
///
/// ## Why is this bad?
/// The `__bytes__` method should return a `bytes` object. Returning a different
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for `__hash__` implementations that return a value other than an integer.
/// Checks for `__hash__` implementations that return non-integer values.
///
/// ## Why is this bad?
/// The `__hash__` method should return an integer. Returning a different
/// type may cause unexpected behavior.
///
/// Note: `bool` is a subclass of `int`, so it's technically valid for `__hash__` to
/// return `True` or `False`. However, for consistency with other rules, Ruff will
/// still raise when `__hash__` returns a `bool`.
/// still emit a diagnostic when `__hash__` returns a `bool`.
///
/// ## Example
/// ```python
Expand All @@ -36,7 +36,6 @@ use crate::checkers::ast::Checker;
/// return 2
/// ```
///
///
/// ## References
/// - [Python documentation: The `__hash__` method](https://docs.python.org/3/reference/datamodel.html#object.__hash__)
#[violation]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for `__index__` implementations that return a value other than an integer.
/// Checks for `__index__` implementations that return non-integer values.
///
/// ## Why is this bad?
/// The `__index__` method should return an integer. Returning a different
Expand All @@ -38,7 +38,6 @@ use crate::checkers::ast::Checker;
/// return 2
/// ```
///
///
/// ## References
/// - [Python documentation: The `__index__` method](https://docs.python.org/3/reference/datamodel.html#object.__index__)
#[violation]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@ use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for `__len__` implementations that return values other than a non-negative
/// integer.
/// Checks for `__len__` implementations that return values that are not non-negative
/// integers.
///
/// ## Why is this bad?
/// The `__len__` method should return a non-negative integer. Returning a different
/// value may cause unexpected behavior.
///
/// Note: `bool` is a subclass of `int`, so it's technically valid for `__len__` to
/// return `True` or `False`. However, for consistency with other rules, Ruff will
/// still raise when `__len__` returns a `bool`.
/// still emit a diagnostic when `__len__` returns a `bool`.
///
/// ## Example
/// ```python
Expand All @@ -37,7 +37,6 @@ use crate::checkers::ast::Checker;
/// return 2
/// ```
///
///
/// ## References
/// - [Python documentation: The `__len__` method](https://docs.python.org/3/reference/datamodel.html#object.__len__)
#[violation]
Expand Down
38 changes: 24 additions & 14 deletions crates/ruff_linter/src/rules/pylint/rules/self_or_cls_assignment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,30 +11,36 @@ use crate::checkers::ast::Checker;
/// Checks for assignment of `self` and `cls` in instance and class methods respectively.
///
/// ## Why is this bad?
/// The identifiers `self` and `cls` are conventional in Python for the first argument of instance
/// methods and class methods, respectively.
/// The identifiers `self` and `cls` are conventional in Python for the first parameter of instance
/// methods and class methods, respectively. Assigning new values to these variables can be
/// confusing for others reading your code; using a different variable name can lead to clearer
/// code.
///
/// ## Example
///
/// ```python
/// class Versions:
/// def add(self, version):
/// self = version
/// class Version:
/// def add(self, other):
/// self = self + other
/// return self
///
/// @classmethod
/// def from_list(cls, versions):
/// cls = versions
/// def superclass(cls):
/// cls = cls.__mro__[-1]
/// return cls
/// ```
///
/// Use instead:
/// ```python
/// class Versions:
/// def add(self, version):
/// self.versions.append(version)
/// class Version:
/// def add(self, other):
/// new_version = self + other
/// return new_version
///
/// @classmethod
/// def from_list(cls, versions):
/// return cls(versions)
/// def superclass(cls):
/// supercls = cls.__mro__[-1]
/// return supercls
/// ```
#[violation]
pub struct SelfOrClsAssignment {
Expand All @@ -47,10 +53,14 @@ impl Violation for SelfOrClsAssignment {
let SelfOrClsAssignment { method_type } = self;

format!(
"Invalid assignment to `{}` argument in {method_type} method",
"Confusing assignment to `{}` argument in {method_type} method",
method_type.arg_name(),
Comment on lines 55 to 57
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option could be "Re-assigned {} argument in {method_type} method"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that's better. Much more descriptive. I'll make another PR 😄

)
}

fn fix_title(&self) -> Option<String> {
Some("Consider using a different variable name".to_string())
}
Comment on lines +60 to +63
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is clever ;)

}

/// PLW0127
Expand Down Expand Up @@ -130,7 +140,7 @@ enum MethodType {
}

impl MethodType {
fn arg_name(self) -> &'static str {
const fn arg_name(self) -> &'static str {
match self {
MethodType::Instance => "self",
MethodType::Class => "cls",
Expand Down
Loading
Loading