mirror of https://github.com/astral-sh/ruff
Implement F632
This commit is contained in:
parent
b03a8728b5
commit
6aaee06d17
|
|
@ -124,7 +124,7 @@ ruff's goal is to achieve feature-parity with Flake8 when used (1) without any p
|
||||||
stylistic checks; limiting to Python 3 obviates the need for certain compatibility checks.)
|
stylistic checks; limiting to Python 3 obviates the need for certain compatibility checks.)
|
||||||
|
|
||||||
Under those conditions, Flake8 implements about 60 rules, give or take. At time of writing, ruff
|
Under those conditions, Flake8 implements about 60 rules, give or take. At time of writing, ruff
|
||||||
implements 39 rules. (Note that these 39 rules likely cover a disproportionate share of errors:
|
implements 40 rules. (Note that these 40 rules likely cover a disproportionate share of errors:
|
||||||
unused imports, undefined variables, etc.)
|
unused imports, undefined variables, etc.)
|
||||||
|
|
||||||
The unimplemented rules are tracked in #170, and include:
|
The unimplemented rules are tracked in #170, and include:
|
||||||
|
|
@ -166,6 +166,7 @@ Beyond rule-set parity, ruff suffers from the following limitations vis-à-vis F
|
||||||
| F621 | TooManyExpressionsInStarredAssignment | too many expressions in star-unpacking assignment |
|
| F621 | TooManyExpressionsInStarredAssignment | too many expressions in star-unpacking assignment |
|
||||||
| F622 | TwoStarredExpressions | two starred expressions in assignment |
|
| F622 | TwoStarredExpressions | two starred expressions in assignment |
|
||||||
| F631 | AssertTuple | Assert test is a non-empty tuple, which is always `True` |
|
| F631 | AssertTuple | Assert test is a non-empty tuple, which is always `True` |
|
||||||
|
| F632 | IsLiteral | use ==/!= to compare constant literals |
|
||||||
| F633 | InvalidPrintSyntax | use of >> is invalid with print function |
|
| F633 | InvalidPrintSyntax | use of >> is invalid with print function |
|
||||||
| F634 | IfTuple | If test is a tuple, which is always `True` |
|
| F634 | IfTuple | If test is a tuple, which is always `True` |
|
||||||
| F701 | BreakOutsideLoop | `break` outside loop |
|
| F701 | BreakOutsideLoop | `break` outside loop |
|
||||||
|
|
|
||||||
|
|
@ -21,6 +21,7 @@ fn main() {
|
||||||
CheckKind::ImportStarNotPermitted("...".to_string()),
|
CheckKind::ImportStarNotPermitted("...".to_string()),
|
||||||
CheckKind::ImportStarUsage("...".to_string()),
|
CheckKind::ImportStarUsage("...".to_string()),
|
||||||
CheckKind::InvalidPrintSyntax,
|
CheckKind::InvalidPrintSyntax,
|
||||||
|
CheckKind::IsLiteral,
|
||||||
CheckKind::LateFutureImport,
|
CheckKind::LateFutureImport,
|
||||||
CheckKind::LineTooLong(89, 88),
|
CheckKind::LineTooLong(89, 88),
|
||||||
CheckKind::ModuleImportNotAtTopOfFile,
|
CheckKind::ModuleImportNotAtTopOfFile,
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,5 @@
|
||||||
|
if x is "abc":
|
||||||
|
pass
|
||||||
|
|
||||||
|
if 123 is not y:
|
||||||
|
pass
|
||||||
|
|
@ -25,6 +25,7 @@ select = [
|
||||||
"F621",
|
"F621",
|
||||||
"F622",
|
"F622",
|
||||||
"F631",
|
"F631",
|
||||||
|
"F632",
|
||||||
"F633",
|
"F633",
|
||||||
"F634",
|
"F634",
|
||||||
"F701",
|
"F701",
|
||||||
|
|
|
||||||
|
|
@ -432,6 +432,50 @@ pub fn check_literal_comparisons(
|
||||||
checks
|
checks
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn is_constant(expr: &Expr) -> bool {
|
||||||
|
match &expr.node {
|
||||||
|
ExprKind::Constant { .. } => true,
|
||||||
|
ExprKind::Tuple { elts, .. } => elts.iter().all(is_constant),
|
||||||
|
_ => false,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn is_singleton(expr: &Expr) -> bool {
|
||||||
|
matches!(
|
||||||
|
expr.node,
|
||||||
|
ExprKind::Constant {
|
||||||
|
value: Constant::None | Constant::Bool(_) | Constant::Ellipsis,
|
||||||
|
..
|
||||||
|
}
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn is_constant_non_singleton(expr: &Expr) -> bool {
|
||||||
|
is_constant(expr) && !is_singleton(expr)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Check IsLiteral compliance.
|
||||||
|
pub fn check_is_literal(
|
||||||
|
left: &Expr,
|
||||||
|
ops: &Vec<Cmpop>,
|
||||||
|
comparators: &Vec<Expr>,
|
||||||
|
location: Location,
|
||||||
|
) -> Vec<Check> {
|
||||||
|
let mut checks: Vec<Check> = vec![];
|
||||||
|
|
||||||
|
let mut left = left;
|
||||||
|
for (op, right) in izip!(ops, comparators) {
|
||||||
|
if matches!(op, Cmpop::Is | Cmpop::IsNot)
|
||||||
|
&& (is_constant_non_singleton(left) || is_constant_non_singleton(right))
|
||||||
|
{
|
||||||
|
checks.push(Check::new(CheckKind::IsLiteral, location));
|
||||||
|
}
|
||||||
|
left = right;
|
||||||
|
}
|
||||||
|
|
||||||
|
checks
|
||||||
|
}
|
||||||
|
|
||||||
/// Check TwoStarredExpressions and TooManyExpressionsInStarredAssignment compliance.
|
/// Check TwoStarredExpressions and TooManyExpressionsInStarredAssignment compliance.
|
||||||
pub fn check_starred_expressions(
|
pub fn check_starred_expressions(
|
||||||
elts: &[Expr],
|
elts: &[Expr],
|
||||||
|
|
|
||||||
|
|
@ -737,6 +737,15 @@ where
|
||||||
check_true_false_comparisons,
|
check_true_false_comparisons,
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if self.settings.select.contains(&CheckCode::F632) {
|
||||||
|
self.checks.extend(checks::check_is_literal(
|
||||||
|
left,
|
||||||
|
ops,
|
||||||
|
comparators,
|
||||||
|
expr.location,
|
||||||
|
));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
ExprKind::Constant {
|
ExprKind::Constant {
|
||||||
value: Constant::Str(value),
|
value: Constant::Str(value),
|
||||||
|
|
|
||||||
|
|
@ -31,6 +31,7 @@ pub enum CheckCode {
|
||||||
F621,
|
F621,
|
||||||
F622,
|
F622,
|
||||||
F631,
|
F631,
|
||||||
|
F632,
|
||||||
F633,
|
F633,
|
||||||
F634,
|
F634,
|
||||||
F701,
|
F701,
|
||||||
|
|
@ -77,6 +78,7 @@ impl FromStr for CheckCode {
|
||||||
"F621" => Ok(CheckCode::F621),
|
"F621" => Ok(CheckCode::F621),
|
||||||
"F622" => Ok(CheckCode::F622),
|
"F622" => Ok(CheckCode::F622),
|
||||||
"F631" => Ok(CheckCode::F631),
|
"F631" => Ok(CheckCode::F631),
|
||||||
|
"F632" => Ok(CheckCode::F632),
|
||||||
"F633" => Ok(CheckCode::F633),
|
"F633" => Ok(CheckCode::F633),
|
||||||
"F634" => Ok(CheckCode::F634),
|
"F634" => Ok(CheckCode::F634),
|
||||||
"F701" => Ok(CheckCode::F701),
|
"F701" => Ok(CheckCode::F701),
|
||||||
|
|
@ -123,6 +125,7 @@ impl CheckCode {
|
||||||
CheckCode::F621 => "F621",
|
CheckCode::F621 => "F621",
|
||||||
CheckCode::F622 => "F622",
|
CheckCode::F622 => "F622",
|
||||||
CheckCode::F631 => "F631",
|
CheckCode::F631 => "F631",
|
||||||
|
CheckCode::F632 => "F632",
|
||||||
CheckCode::F633 => "F633",
|
CheckCode::F633 => "F633",
|
||||||
CheckCode::F634 => "F634",
|
CheckCode::F634 => "F634",
|
||||||
CheckCode::F701 => "F701",
|
CheckCode::F701 => "F701",
|
||||||
|
|
@ -182,9 +185,10 @@ pub enum CheckKind {
|
||||||
FutureFeatureNotDefined(String),
|
FutureFeatureNotDefined(String),
|
||||||
IOError(String),
|
IOError(String),
|
||||||
IfTuple,
|
IfTuple,
|
||||||
InvalidPrintSyntax,
|
|
||||||
ImportStarNotPermitted(String),
|
ImportStarNotPermitted(String),
|
||||||
ImportStarUsage(String),
|
ImportStarUsage(String),
|
||||||
|
InvalidPrintSyntax,
|
||||||
|
IsLiteral,
|
||||||
LateFutureImport,
|
LateFutureImport,
|
||||||
LineTooLong(usize, usize),
|
LineTooLong(usize, usize),
|
||||||
ModuleImportNotAtTopOfFile,
|
ModuleImportNotAtTopOfFile,
|
||||||
|
|
@ -222,14 +226,15 @@ impl CheckKind {
|
||||||
CheckKind::DoNotAssignLambda => "DoNotAssignLambda",
|
CheckKind::DoNotAssignLambda => "DoNotAssignLambda",
|
||||||
CheckKind::DoNotUseBareExcept => "DoNotUseBareExcept",
|
CheckKind::DoNotUseBareExcept => "DoNotUseBareExcept",
|
||||||
CheckKind::DuplicateArgumentName => "DuplicateArgumentName",
|
CheckKind::DuplicateArgumentName => "DuplicateArgumentName",
|
||||||
CheckKind::ForwardAnnotationSyntaxError(_) => "ForwardAnnotationSyntaxError",
|
|
||||||
CheckKind::FStringMissingPlaceholders => "FStringMissingPlaceholders",
|
CheckKind::FStringMissingPlaceholders => "FStringMissingPlaceholders",
|
||||||
|
CheckKind::ForwardAnnotationSyntaxError(_) => "ForwardAnnotationSyntaxError",
|
||||||
CheckKind::FutureFeatureNotDefined(_) => "FutureFeatureNotDefined",
|
CheckKind::FutureFeatureNotDefined(_) => "FutureFeatureNotDefined",
|
||||||
CheckKind::IOError(_) => "IOError",
|
CheckKind::IOError(_) => "IOError",
|
||||||
CheckKind::IfTuple => "IfTuple",
|
CheckKind::IfTuple => "IfTuple",
|
||||||
CheckKind::InvalidPrintSyntax => "InvalidPrintSyntax",
|
|
||||||
CheckKind::ImportStarNotPermitted(_) => "ImportStarNotPermitted",
|
CheckKind::ImportStarNotPermitted(_) => "ImportStarNotPermitted",
|
||||||
CheckKind::ImportStarUsage(_) => "ImportStarUsage",
|
CheckKind::ImportStarUsage(_) => "ImportStarUsage",
|
||||||
|
CheckKind::InvalidPrintSyntax => "InvalidPrintSyntax",
|
||||||
|
CheckKind::IsLiteral => "IsLiteral",
|
||||||
CheckKind::LateFutureImport => "LateFutureImport",
|
CheckKind::LateFutureImport => "LateFutureImport",
|
||||||
CheckKind::LineTooLong(_, _) => "LineTooLong",
|
CheckKind::LineTooLong(_, _) => "LineTooLong",
|
||||||
CheckKind::ModuleImportNotAtTopOfFile => "ModuleImportNotAtTopOfFile",
|
CheckKind::ModuleImportNotAtTopOfFile => "ModuleImportNotAtTopOfFile",
|
||||||
|
|
@ -269,14 +274,15 @@ impl CheckKind {
|
||||||
CheckKind::DoNotAssignLambda => &CheckCode::E731,
|
CheckKind::DoNotAssignLambda => &CheckCode::E731,
|
||||||
CheckKind::DoNotUseBareExcept => &CheckCode::E722,
|
CheckKind::DoNotUseBareExcept => &CheckCode::E722,
|
||||||
CheckKind::DuplicateArgumentName => &CheckCode::F831,
|
CheckKind::DuplicateArgumentName => &CheckCode::F831,
|
||||||
CheckKind::ForwardAnnotationSyntaxError(_) => &CheckCode::F722,
|
|
||||||
CheckKind::FStringMissingPlaceholders => &CheckCode::F541,
|
CheckKind::FStringMissingPlaceholders => &CheckCode::F541,
|
||||||
|
CheckKind::ForwardAnnotationSyntaxError(_) => &CheckCode::F722,
|
||||||
CheckKind::FutureFeatureNotDefined(_) => &CheckCode::F407,
|
CheckKind::FutureFeatureNotDefined(_) => &CheckCode::F407,
|
||||||
CheckKind::IOError(_) => &CheckCode::E902,
|
CheckKind::IOError(_) => &CheckCode::E902,
|
||||||
CheckKind::IfTuple => &CheckCode::F634,
|
CheckKind::IfTuple => &CheckCode::F634,
|
||||||
CheckKind::InvalidPrintSyntax => &CheckCode::F633,
|
|
||||||
CheckKind::ImportStarNotPermitted(_) => &CheckCode::F406,
|
CheckKind::ImportStarNotPermitted(_) => &CheckCode::F406,
|
||||||
CheckKind::ImportStarUsage(_) => &CheckCode::F403,
|
CheckKind::ImportStarUsage(_) => &CheckCode::F403,
|
||||||
|
CheckKind::InvalidPrintSyntax => &CheckCode::F633,
|
||||||
|
CheckKind::IsLiteral => &CheckCode::F632,
|
||||||
CheckKind::LateFutureImport => &CheckCode::F404,
|
CheckKind::LateFutureImport => &CheckCode::F404,
|
||||||
CheckKind::LineTooLong(_, _) => &CheckCode::E501,
|
CheckKind::LineTooLong(_, _) => &CheckCode::E501,
|
||||||
CheckKind::ModuleImportNotAtTopOfFile => &CheckCode::E402,
|
CheckKind::ModuleImportNotAtTopOfFile => &CheckCode::E402,
|
||||||
|
|
@ -348,6 +354,7 @@ impl CheckKind {
|
||||||
CheckKind::ImportStarUsage(name) => {
|
CheckKind::ImportStarUsage(name) => {
|
||||||
format!("`from {name} import *` used; unable to detect undefined names")
|
format!("`from {name} import *` used; unable to detect undefined names")
|
||||||
}
|
}
|
||||||
|
CheckKind::IsLiteral => "use ==/!= to compare constant literals".to_string(),
|
||||||
CheckKind::LateFutureImport => {
|
CheckKind::LateFutureImport => {
|
||||||
"from __future__ imports must occur at the beginning of the file".to_string()
|
"from __future__ imports must occur at the beginning of the file".to_string()
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -851,6 +851,38 @@ mod tests {
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn f632() -> Result<()> {
|
||||||
|
let mut actual = check_path(
|
||||||
|
Path::new("./resources/test/fixtures/F632.py"),
|
||||||
|
&settings::Settings {
|
||||||
|
line_length: 88,
|
||||||
|
exclude: vec![],
|
||||||
|
select: BTreeSet::from([CheckCode::F632]),
|
||||||
|
},
|
||||||
|
&fixer::Mode::Generate,
|
||||||
|
)?;
|
||||||
|
actual.sort_by_key(|check| check.location);
|
||||||
|
let expected = vec![
|
||||||
|
Check {
|
||||||
|
kind: CheckKind::IsLiteral,
|
||||||
|
location: Location::new(1, 6),
|
||||||
|
fix: None,
|
||||||
|
},
|
||||||
|
Check {
|
||||||
|
kind: CheckKind::IsLiteral,
|
||||||
|
location: Location::new(4, 8),
|
||||||
|
fix: None,
|
||||||
|
},
|
||||||
|
];
|
||||||
|
assert_eq!(actual.len(), expected.len());
|
||||||
|
for i in 0..actual.len() {
|
||||||
|
assert_eq!(actual[i], expected[i]);
|
||||||
|
}
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn f633() -> Result<()> {
|
fn f633() -> Result<()> {
|
||||||
let mut actual = check_path(
|
let mut actual = check_path(
|
||||||
|
|
|
||||||
|
|
@ -283,6 +283,7 @@ other-attribute = 1
|
||||||
CheckCode::F621,
|
CheckCode::F621,
|
||||||
CheckCode::F622,
|
CheckCode::F622,
|
||||||
CheckCode::F631,
|
CheckCode::F631,
|
||||||
|
CheckCode::F632,
|
||||||
CheckCode::F633,
|
CheckCode::F633,
|
||||||
CheckCode::F634,
|
CheckCode::F634,
|
||||||
CheckCode::F701,
|
CheckCode::F701,
|
||||||
|
|
|
||||||
|
|
@ -65,6 +65,7 @@ impl Settings {
|
||||||
CheckCode::F621,
|
CheckCode::F621,
|
||||||
CheckCode::F622,
|
CheckCode::F622,
|
||||||
CheckCode::F631,
|
CheckCode::F631,
|
||||||
|
CheckCode::F632,
|
||||||
CheckCode::F633,
|
CheckCode::F633,
|
||||||
CheckCode::F634,
|
CheckCode::F634,
|
||||||
CheckCode::F701,
|
CheckCode::F701,
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue