mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-03 10:22:24 +00:00
Implement F632 (#190)
This commit is contained in:
parent
b03a8728b5
commit
2e1eb84cbf
10 changed files with 108 additions and 6 deletions
|
@ -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.)
|
||||
|
||||
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.)
|
||||
|
||||
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 |
|
||||
| F622 | TwoStarredExpressions | two starred expressions in assignment |
|
||||
| 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 |
|
||||
| F634 | IfTuple | If test is a tuple, which is always `True` |
|
||||
| F701 | BreakOutsideLoop | `break` outside loop |
|
||||
|
|
|
@ -21,6 +21,7 @@ fn main() {
|
|||
CheckKind::ImportStarNotPermitted("...".to_string()),
|
||||
CheckKind::ImportStarUsage("...".to_string()),
|
||||
CheckKind::InvalidPrintSyntax,
|
||||
CheckKind::IsLiteral,
|
||||
CheckKind::LateFutureImport,
|
||||
CheckKind::LineTooLong(89, 88),
|
||||
CheckKind::ModuleImportNotAtTopOfFile,
|
||||
|
|
5
resources/test/fixtures/F632.py
vendored
Normal file
5
resources/test/fixtures/F632.py
vendored
Normal file
|
@ -0,0 +1,5 @@
|
|||
if x is "abc":
|
||||
pass
|
||||
|
||||
if 123 is not y:
|
||||
pass
|
1
resources/test/fixtures/pyproject.toml
vendored
1
resources/test/fixtures/pyproject.toml
vendored
|
@ -25,6 +25,7 @@ select = [
|
|||
"F621",
|
||||
"F622",
|
||||
"F631",
|
||||
"F632",
|
||||
"F633",
|
||||
"F634",
|
||||
"F701",
|
||||
|
|
|
@ -432,6 +432,50 @@ pub fn check_literal_comparisons(
|
|||
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.
|
||||
pub fn check_starred_expressions(
|
||||
elts: &[Expr],
|
||||
|
|
|
@ -737,6 +737,15 @@ where
|
|||
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 {
|
||||
value: Constant::Str(value),
|
||||
|
|
|
@ -31,6 +31,7 @@ pub enum CheckCode {
|
|||
F621,
|
||||
F622,
|
||||
F631,
|
||||
F632,
|
||||
F633,
|
||||
F634,
|
||||
F701,
|
||||
|
@ -77,6 +78,7 @@ impl FromStr for CheckCode {
|
|||
"F621" => Ok(CheckCode::F621),
|
||||
"F622" => Ok(CheckCode::F622),
|
||||
"F631" => Ok(CheckCode::F631),
|
||||
"F632" => Ok(CheckCode::F632),
|
||||
"F633" => Ok(CheckCode::F633),
|
||||
"F634" => Ok(CheckCode::F634),
|
||||
"F701" => Ok(CheckCode::F701),
|
||||
|
@ -123,6 +125,7 @@ impl CheckCode {
|
|||
CheckCode::F621 => "F621",
|
||||
CheckCode::F622 => "F622",
|
||||
CheckCode::F631 => "F631",
|
||||
CheckCode::F632 => "F632",
|
||||
CheckCode::F633 => "F633",
|
||||
CheckCode::F634 => "F634",
|
||||
CheckCode::F701 => "F701",
|
||||
|
@ -182,9 +185,10 @@ pub enum CheckKind {
|
|||
FutureFeatureNotDefined(String),
|
||||
IOError(String),
|
||||
IfTuple,
|
||||
InvalidPrintSyntax,
|
||||
ImportStarNotPermitted(String),
|
||||
ImportStarUsage(String),
|
||||
InvalidPrintSyntax,
|
||||
IsLiteral,
|
||||
LateFutureImport,
|
||||
LineTooLong(usize, usize),
|
||||
ModuleImportNotAtTopOfFile,
|
||||
|
@ -222,14 +226,15 @@ impl CheckKind {
|
|||
CheckKind::DoNotAssignLambda => "DoNotAssignLambda",
|
||||
CheckKind::DoNotUseBareExcept => "DoNotUseBareExcept",
|
||||
CheckKind::DuplicateArgumentName => "DuplicateArgumentName",
|
||||
CheckKind::ForwardAnnotationSyntaxError(_) => "ForwardAnnotationSyntaxError",
|
||||
CheckKind::FStringMissingPlaceholders => "FStringMissingPlaceholders",
|
||||
CheckKind::ForwardAnnotationSyntaxError(_) => "ForwardAnnotationSyntaxError",
|
||||
CheckKind::FutureFeatureNotDefined(_) => "FutureFeatureNotDefined",
|
||||
CheckKind::IOError(_) => "IOError",
|
||||
CheckKind::IfTuple => "IfTuple",
|
||||
CheckKind::InvalidPrintSyntax => "InvalidPrintSyntax",
|
||||
CheckKind::ImportStarNotPermitted(_) => "ImportStarNotPermitted",
|
||||
CheckKind::ImportStarUsage(_) => "ImportStarUsage",
|
||||
CheckKind::InvalidPrintSyntax => "InvalidPrintSyntax",
|
||||
CheckKind::IsLiteral => "IsLiteral",
|
||||
CheckKind::LateFutureImport => "LateFutureImport",
|
||||
CheckKind::LineTooLong(_, _) => "LineTooLong",
|
||||
CheckKind::ModuleImportNotAtTopOfFile => "ModuleImportNotAtTopOfFile",
|
||||
|
@ -269,14 +274,15 @@ impl CheckKind {
|
|||
CheckKind::DoNotAssignLambda => &CheckCode::E731,
|
||||
CheckKind::DoNotUseBareExcept => &CheckCode::E722,
|
||||
CheckKind::DuplicateArgumentName => &CheckCode::F831,
|
||||
CheckKind::ForwardAnnotationSyntaxError(_) => &CheckCode::F722,
|
||||
CheckKind::FStringMissingPlaceholders => &CheckCode::F541,
|
||||
CheckKind::ForwardAnnotationSyntaxError(_) => &CheckCode::F722,
|
||||
CheckKind::FutureFeatureNotDefined(_) => &CheckCode::F407,
|
||||
CheckKind::IOError(_) => &CheckCode::E902,
|
||||
CheckKind::IfTuple => &CheckCode::F634,
|
||||
CheckKind::InvalidPrintSyntax => &CheckCode::F633,
|
||||
CheckKind::ImportStarNotPermitted(_) => &CheckCode::F406,
|
||||
CheckKind::ImportStarUsage(_) => &CheckCode::F403,
|
||||
CheckKind::InvalidPrintSyntax => &CheckCode::F633,
|
||||
CheckKind::IsLiteral => &CheckCode::F632,
|
||||
CheckKind::LateFutureImport => &CheckCode::F404,
|
||||
CheckKind::LineTooLong(_, _) => &CheckCode::E501,
|
||||
CheckKind::ModuleImportNotAtTopOfFile => &CheckCode::E402,
|
||||
|
@ -348,6 +354,7 @@ impl CheckKind {
|
|||
CheckKind::ImportStarUsage(name) => {
|
||||
format!("`from {name} import *` used; unable to detect undefined names")
|
||||
}
|
||||
CheckKind::IsLiteral => "use ==/!= to compare constant literals".to_string(),
|
||||
CheckKind::LateFutureImport => {
|
||||
"from __future__ imports must occur at the beginning of the file".to_string()
|
||||
}
|
||||
|
|
|
@ -851,6 +851,38 @@ mod tests {
|
|||
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]
|
||||
fn f633() -> Result<()> {
|
||||
let mut actual = check_path(
|
||||
|
|
|
@ -283,6 +283,7 @@ other-attribute = 1
|
|||
CheckCode::F621,
|
||||
CheckCode::F622,
|
||||
CheckCode::F631,
|
||||
CheckCode::F632,
|
||||
CheckCode::F633,
|
||||
CheckCode::F634,
|
||||
CheckCode::F701,
|
||||
|
|
|
@ -65,6 +65,7 @@ impl Settings {
|
|||
CheckCode::F621,
|
||||
CheckCode::F622,
|
||||
CheckCode::F631,
|
||||
CheckCode::F632,
|
||||
CheckCode::F633,
|
||||
CheckCode::F634,
|
||||
CheckCode::F701,
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue