Implement F621 and F622

This commit is contained in:
Charlie Marsh 2022-09-10 14:49:29 -04:00
parent 4fc68e0310
commit 55cd0887c2
10 changed files with 109 additions and 1 deletions

View File

@ -123,7 +123,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 58 rules, give or take. At time of writing, ruff Under those conditions, Flake8 implements about 58 rules, give or take. At time of writing, ruff
implements 24 rules. (Note that these 24 rules likely cover a disproportionate share of errors: implements 28 rules. (Note that these 28 rules likely cover a disproportionate share of errors:
unused imports, undefined variables, etc.) unused imports, undefined variables, etc.)
Of the unimplemented rules, ruff is missing: Of the unimplemented rules, ruff is missing:
@ -158,6 +158,8 @@ Beyond rule-set parity, ruff suffers from the following limitations vis-à-vis F
| F541 | FStringMissingPlaceholders | f-string without any placeholders | | F541 | FStringMissingPlaceholders | f-string without any placeholders |
| F601 | MultiValueRepeatedKeyLiteral | Dictionary key literal repeated | | F601 | MultiValueRepeatedKeyLiteral | Dictionary key literal repeated |
| F602 | MultiValueRepeatedKeyVariable | Dictionary key `...` repeated | | F602 | MultiValueRepeatedKeyVariable | Dictionary key `...` repeated |
| 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` | | F631 | AssertTuple | Assert test is a non-empty tuple, which is always `True` |
| F634 | IfTuple | If test is a tuple, which is always `True` | | F634 | IfTuple | If test is a tuple, which is always `True` |
| F704 | YieldOutsideFunction | a `yield` or `yield from` statement outside of a function/method | | F704 | YieldOutsideFunction | a `yield` or `yield from` statement outside of a function/method |

View File

@ -21,7 +21,9 @@ fn main() {
CheckKind::NotIsTest, CheckKind::NotIsTest,
CheckKind::RaiseNotImplemented, CheckKind::RaiseNotImplemented,
CheckKind::ReturnOutsideFunction, CheckKind::ReturnOutsideFunction,
CheckKind::TooManyExpressionsInStarredAssignment,
CheckKind::TrueFalseComparison(true, RejectedCmpop::Eq), CheckKind::TrueFalseComparison(true, RejectedCmpop::Eq),
CheckKind::TwoStarredExpressions,
CheckKind::UndefinedExport("...".to_string()), CheckKind::UndefinedExport("...".to_string()),
CheckKind::UndefinedLocal("...".to_string()), CheckKind::UndefinedLocal("...".to_string()),
CheckKind::UndefinedName("...".to_string()), CheckKind::UndefinedName("...".to_string()),

3
resources/test/fixtures/F622.py vendored Normal file
View File

@ -0,0 +1,3 @@
*a, *b, c = (1, 2, 3)
*a, b, c = (1, 2, 3)
a, b, *c = (1, 2, 3)

View File

@ -15,6 +15,8 @@ select = [
"F541", "F541",
"F601", "F601",
"F602", "F602",
"F621",
"F622",
"F631", "F631",
"F634", "F634",
"F704", "F704",

View File

@ -395,3 +395,36 @@ pub fn check_literal_comparisons(
checks checks
} }
/// Check TwoStarredExpressions and TooManyExpressionsInStarredAssignment compliance.
pub fn check_starred_expressions(
elts: &[Expr],
location: Location,
check_too_many_expressions: bool,
check_two_starred_expressions: bool,
) -> Option<Check> {
let mut has_starred: bool = false;
let mut starred_index: Option<usize> = None;
for (index, elt) in elts.iter().enumerate() {
if matches!(elt.node, ExprKind::Starred { .. }) {
if has_starred && check_two_starred_expressions {
return Some(Check::new(CheckKind::TwoStarredExpressions, location));
}
has_starred = true;
starred_index = Some(index);
}
}
if check_too_many_expressions {
if let Some(starred_index) = starred_index {
if starred_index >= 1 << 8 || elts.len() - starred_index > 1 << 24 {
return Some(Check::new(
CheckKind::TooManyExpressionsInStarredAssignment,
location,
));
}
}
}
None
}

View File

@ -430,6 +430,22 @@ where
self.in_literal = true; self.in_literal = true;
} }
} }
ExprKind::Tuple { elts, ctx } => {
if matches!(ctx, ExprContext::Store) {
let check_too_many_expressions =
self.settings.select.contains(&CheckCode::F621);
let check_two_starred_expressions =
self.settings.select.contains(&CheckCode::F622);
if let Some(check) = checks::check_starred_expressions(
elts,
expr.location,
check_too_many_expressions,
check_two_starred_expressions,
) {
self.checks.push(check);
}
}
}
ExprKind::Name { ctx, .. } => match ctx { ExprKind::Name { ctx, .. } => match ctx {
ExprContext::Load => self.handle_node_load(expr), ExprContext::Load => self.handle_node_load(expr),
ExprContext::Store => { ExprContext::Store => {

View File

@ -21,6 +21,8 @@ pub enum CheckCode {
F541, F541,
F601, F601,
F602, F602,
F621,
F622,
F631, F631,
F634, F634,
F704, F704,
@ -54,6 +56,8 @@ impl FromStr for CheckCode {
"F541" => Ok(CheckCode::F541), "F541" => Ok(CheckCode::F541),
"F601" => Ok(CheckCode::F601), "F601" => Ok(CheckCode::F601),
"F602" => Ok(CheckCode::F602), "F602" => Ok(CheckCode::F602),
"F621" => Ok(CheckCode::F621),
"F622" => Ok(CheckCode::F622),
"F631" => Ok(CheckCode::F631), "F631" => Ok(CheckCode::F631),
"F634" => Ok(CheckCode::F634), "F634" => Ok(CheckCode::F634),
"F704" => Ok(CheckCode::F704), "F704" => Ok(CheckCode::F704),
@ -88,6 +92,8 @@ impl CheckCode {
CheckCode::F541 => "F541", CheckCode::F541 => "F541",
CheckCode::F601 => "F601", CheckCode::F601 => "F601",
CheckCode::F602 => "F602", CheckCode::F602 => "F602",
CheckCode::F621 => "F621",
CheckCode::F622 => "F622",
CheckCode::F631 => "F631", CheckCode::F631 => "F631",
CheckCode::F634 => "F634", CheckCode::F634 => "F634",
CheckCode::F704 => "F704", CheckCode::F704 => "F704",
@ -120,6 +126,8 @@ impl CheckCode {
CheckCode::F541 => &LintSource::AST, CheckCode::F541 => &LintSource::AST,
CheckCode::F601 => &LintSource::AST, CheckCode::F601 => &LintSource::AST,
CheckCode::F602 => &LintSource::AST, CheckCode::F602 => &LintSource::AST,
CheckCode::F621 => &LintSource::AST,
CheckCode::F622 => &LintSource::AST,
CheckCode::F631 => &LintSource::AST, CheckCode::F631 => &LintSource::AST,
CheckCode::F634 => &LintSource::AST, CheckCode::F634 => &LintSource::AST,
CheckCode::F704 => &LintSource::AST, CheckCode::F704 => &LintSource::AST,
@ -170,7 +178,9 @@ pub enum CheckKind {
NotIsTest, NotIsTest,
RaiseNotImplemented, RaiseNotImplemented,
ReturnOutsideFunction, ReturnOutsideFunction,
TooManyExpressionsInStarredAssignment,
TrueFalseComparison(bool, RejectedCmpop), TrueFalseComparison(bool, RejectedCmpop),
TwoStarredExpressions,
UndefinedExport(String), UndefinedExport(String),
UndefinedLocal(String), UndefinedLocal(String),
UndefinedName(String), UndefinedName(String),
@ -202,7 +212,11 @@ impl CheckKind {
CheckKind::NotIsTest => "NotIsTest", CheckKind::NotIsTest => "NotIsTest",
CheckKind::RaiseNotImplemented => "RaiseNotImplemented", CheckKind::RaiseNotImplemented => "RaiseNotImplemented",
CheckKind::ReturnOutsideFunction => "ReturnOutsideFunction", CheckKind::ReturnOutsideFunction => "ReturnOutsideFunction",
CheckKind::TooManyExpressionsInStarredAssignment => {
"TooManyExpressionsInStarredAssignment"
}
CheckKind::TrueFalseComparison(_, _) => "TrueFalseComparison", CheckKind::TrueFalseComparison(_, _) => "TrueFalseComparison",
CheckKind::TwoStarredExpressions => "TwoStarredExpressions",
CheckKind::UndefinedExport(_) => "UndefinedExport", CheckKind::UndefinedExport(_) => "UndefinedExport",
CheckKind::UndefinedLocal(_) => "UndefinedLocal", CheckKind::UndefinedLocal(_) => "UndefinedLocal",
CheckKind::UndefinedName(_) => "UndefinedName", CheckKind::UndefinedName(_) => "UndefinedName",
@ -234,7 +248,9 @@ impl CheckKind {
CheckKind::NotIsTest => &CheckCode::E714, CheckKind::NotIsTest => &CheckCode::E714,
CheckKind::RaiseNotImplemented => &CheckCode::F901, CheckKind::RaiseNotImplemented => &CheckCode::F901,
CheckKind::ReturnOutsideFunction => &CheckCode::F706, CheckKind::ReturnOutsideFunction => &CheckCode::F706,
CheckKind::TooManyExpressionsInStarredAssignment => &CheckCode::F621,
CheckKind::TrueFalseComparison(_, _) => &CheckCode::E712, CheckKind::TrueFalseComparison(_, _) => &CheckCode::E712,
CheckKind::TwoStarredExpressions => &CheckCode::F622,
CheckKind::UndefinedExport(_) => &CheckCode::F822, CheckKind::UndefinedExport(_) => &CheckCode::F822,
CheckKind::UndefinedLocal(_) => &CheckCode::F823, CheckKind::UndefinedLocal(_) => &CheckCode::F823,
CheckKind::UndefinedName(_) => &CheckCode::F821, CheckKind::UndefinedName(_) => &CheckCode::F821,
@ -295,6 +311,9 @@ impl CheckKind {
CheckKind::ReturnOutsideFunction => { CheckKind::ReturnOutsideFunction => {
"a `return` statement outside of a function/method".to_string() "a `return` statement outside of a function/method".to_string()
} }
CheckKind::TooManyExpressionsInStarredAssignment => {
"too many expressions in star-unpacking assignment".to_string()
}
CheckKind::TrueFalseComparison(value, op) => match *value { CheckKind::TrueFalseComparison(value, op) => match *value {
true => match op { true => match op {
RejectedCmpop::Eq => { RejectedCmpop::Eq => {
@ -313,6 +332,7 @@ impl CheckKind {
} }
}, },
}, },
CheckKind::TwoStarredExpressions => "two starred expressions in assignment".to_string(),
CheckKind::UndefinedExport(name) => { CheckKind::UndefinedExport(name) => {
format!("Undefined name `{name}` in `__all__`") format!("Undefined name `{name}` in `__all__`")
} }
@ -356,7 +376,9 @@ impl CheckKind {
CheckKind::NoneComparison(_) => false, CheckKind::NoneComparison(_) => false,
CheckKind::RaiseNotImplemented => false, CheckKind::RaiseNotImplemented => false,
CheckKind::ReturnOutsideFunction => false, CheckKind::ReturnOutsideFunction => false,
CheckKind::TooManyExpressionsInStarredAssignment => false,
CheckKind::TrueFalseComparison(_, _) => false, CheckKind::TrueFalseComparison(_, _) => false,
CheckKind::TwoStarredExpressions => false,
CheckKind::UndefinedExport(_) => false, CheckKind::UndefinedExport(_) => false,
CheckKind::UndefinedLocal(_) => false, CheckKind::UndefinedLocal(_) => false,
CheckKind::UndefinedName(_) => false, CheckKind::UndefinedName(_) => false,

View File

@ -463,6 +463,30 @@ mod tests {
Ok(()) Ok(())
} }
#[test]
fn f622() -> Result<()> {
let actual = check_path(
Path::new("./resources/test/fixtures/F622.py"),
&settings::Settings {
line_length: 88,
exclude: vec![],
select: BTreeSet::from([CheckCode::F622]),
},
&fixer::Mode::Generate,
)?;
let expected = vec![Check {
kind: CheckKind::TwoStarredExpressions,
location: Location::new(1, 1),
fix: None,
}];
assert_eq!(actual.len(), expected.len());
for i in 0..actual.len() {
assert_eq!(actual[i], expected[i]);
}
Ok(())
}
#[test] #[test]
fn f631() -> Result<()> { fn f631() -> Result<()> {
let mut actual = check_path( let mut actual = check_path(

View File

@ -272,6 +272,8 @@ other-attribute = 1
CheckCode::F541, CheckCode::F541,
CheckCode::F601, CheckCode::F601,
CheckCode::F602, CheckCode::F602,
CheckCode::F621,
CheckCode::F622,
CheckCode::F631, CheckCode::F631,
CheckCode::F634, CheckCode::F634,
CheckCode::F704, CheckCode::F704,

View File

@ -57,6 +57,8 @@ impl Settings {
CheckCode::F541, CheckCode::F541,
CheckCode::F601, CheckCode::F601,
CheckCode::F602, CheckCode::F602,
CheckCode::F621,
CheckCode::F622,
CheckCode::F631, CheckCode::F631,
CheckCode::F634, CheckCode::F634,
CheckCode::F704, CheckCode::F704,