From a3cd3dd8028cb16781258b7b3cd5773896d7ab8f Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 6 Nov 2022 17:52:48 -0500 Subject: [PATCH] Move last-mile check filtering to linter.rs --- src/check_ast.rs | 61 ++++----- src/flake8_annotations/plugins.rs | 165 ++++++++++--------------- src/flake8_print/checks.rs | 14 +-- src/flake8_print/plugins/print_call.rs | 9 +- src/linter.rs | 6 + src/pycodestyle/checks.rs | 141 +++++++++------------ src/pyflakes/checks.rs | 27 ++-- 7 files changed, 165 insertions(+), 258 deletions(-) diff --git a/src/check_ast.rs b/src/check_ast.rs index 157afade95..38b797ab14 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -970,17 +970,14 @@ where } ExprKind::Tuple { elts, ctx } | ExprKind::List { elts, ctx } => { if matches!(ctx, ExprContext::Store) { - let check_too_many_expressions = - self.settings.enabled.contains(&CheckCode::F621); - let check_two_starred_expressions = - self.settings.enabled.contains(&CheckCode::F622); - if let Some(check) = pyflakes::checks::starred_expressions( - elts, - check_too_many_expressions, - check_two_starred_expressions, - Range::from_located(expr), - ) { - self.add_check(check); + if self.settings.enabled.contains(&CheckCode::F621) + || self.settings.enabled.contains(&CheckCode::F622) + { + if let Some(check) = + pyflakes::checks::starred_expressions(elts, Range::from_located(expr)) + { + self.add_check(check); + } } } } @@ -1279,17 +1276,10 @@ where } } ExprKind::Dict { keys, .. } => { - let check_repeated_literals = self.settings.enabled.contains(&CheckCode::F601); - let check_repeated_variables = self.settings.enabled.contains(&CheckCode::F602); - if check_repeated_literals || check_repeated_variables { - self.add_checks( - pyflakes::checks::repeated_keys( - keys, - check_repeated_literals, - check_repeated_variables, - ) - .into_iter(), - ); + if self.settings.enabled.contains(&CheckCode::F601) + || self.settings.enabled.contains(&CheckCode::F602) + { + self.add_checks(pyflakes::checks::repeated_keys(keys).into_iter()); } } ExprKind::Yield { .. } | ExprKind::YieldFrom { .. } | ExprKind::Await { .. } => { @@ -1328,13 +1318,10 @@ where } } ExprKind::UnaryOp { op, operand } => { - let check_not_in = self.settings.enabled.contains(&CheckCode::E713); - let check_not_is = self.settings.enabled.contains(&CheckCode::E714); - if check_not_in || check_not_is { - self.add_checks( - pycodestyle::checks::not_tests(op, operand, check_not_in, check_not_is) - .into_iter(), - ); + if self.settings.enabled.contains(&CheckCode::E713) + || self.settings.enabled.contains(&CheckCode::E714) + { + self.add_checks(pycodestyle::checks::not_tests(op, operand).into_iter()); } if self.settings.enabled.contains(&CheckCode::B002) { @@ -1346,18 +1333,12 @@ where ops, comparators, } => { - let check_none_comparisons = self.settings.enabled.contains(&CheckCode::E711); - let check_true_false_comparisons = self.settings.enabled.contains(&CheckCode::E712); - if check_none_comparisons || check_true_false_comparisons { + if self.settings.enabled.contains(&CheckCode::E711) + || self.settings.enabled.contains(&CheckCode::E712) + { self.add_checks( - pycodestyle::checks::literal_comparisons( - left, - ops, - comparators, - check_none_comparisons, - check_true_false_comparisons, - ) - .into_iter(), + pycodestyle::checks::literal_comparisons(left, ops, comparators) + .into_iter(), ); } diff --git a/src/flake8_annotations/plugins.rs b/src/flake8_annotations/plugins.rs index d6c35e0e71..da58ae7c8f 100644 --- a/src/flake8_annotations/plugins.rs +++ b/src/flake8_annotations/plugins.rs @@ -4,7 +4,7 @@ use crate::ast::types::Range; use crate::ast::visitor; use crate::ast::visitor::Visitor; use crate::check_ast::Checker; -use crate::checks::{CheckCode, CheckKind}; +use crate::checks::CheckKind; use crate::docstrings::definition::{Definition, DefinitionKind}; use crate::visibility::Visibility; use crate::{visibility, Check}; @@ -92,12 +92,10 @@ pub fn definition(checker: &mut Checker, definition: &Definition, visibility: &V if !(checker.settings.flake8_annotations.suppress_dummy_args && checker.settings.dummy_variable_rgx.is_match(&arg.node.arg)) { - if checker.settings.enabled.contains(&CheckCode::ANN001) { - checker.add_check(Check::new( - CheckKind::MissingTypeFunctionArgument(arg.node.arg.to_string()), - Range::from_located(arg), - )); - } + checker.add_check(Check::new( + CheckKind::MissingTypeFunctionArgument(arg.node.arg.to_string()), + Range::from_located(arg), + )); } } } @@ -108,12 +106,10 @@ pub fn definition(checker: &mut Checker, definition: &Definition, visibility: &V if !(checker.settings.flake8_annotations.suppress_dummy_args && checker.settings.dummy_variable_rgx.is_match(&arg.node.arg)) { - if checker.settings.enabled.contains(&CheckCode::ANN002) { - checker.add_check(Check::new( - CheckKind::MissingTypeArgs(arg.node.arg.to_string()), - Range::from_located(arg), - )); - } + checker.add_check(Check::new( + CheckKind::MissingTypeArgs(arg.node.arg.to_string()), + Range::from_located(arg), + )); } } } @@ -124,12 +120,10 @@ pub fn definition(checker: &mut Checker, definition: &Definition, visibility: &V if !(checker.settings.flake8_annotations.suppress_dummy_args && checker.settings.dummy_variable_rgx.is_match(&arg.node.arg)) { - if checker.settings.enabled.contains(&CheckCode::ANN003) { - checker.add_check(Check::new( - CheckKind::MissingTypeKwargs(arg.node.arg.to_string()), - Range::from_located(arg), - )); - } + checker.add_check(Check::new( + CheckKind::MissingTypeKwargs(arg.node.arg.to_string()), + Range::from_located(arg), + )); } } } @@ -146,20 +140,16 @@ pub fn definition(checker: &mut Checker, definition: &Definition, visibility: &V match visibility { Visibility::Public => { - if checker.settings.enabled.contains(&CheckCode::ANN201) { - checker.add_check(Check::new( - CheckKind::MissingReturnTypePublicFunction(name.to_string()), - Range::from_located(stmt), - )); - } + checker.add_check(Check::new( + CheckKind::MissingReturnTypePublicFunction(name.to_string()), + Range::from_located(stmt), + )); } Visibility::Private => { - if checker.settings.enabled.contains(&CheckCode::ANN202) { - checker.add_check(Check::new( - CheckKind::MissingReturnTypePrivateFunction(name.to_string()), - Range::from_located(stmt), - )); - } + checker.add_check(Check::new( + CheckKind::MissingReturnTypePrivateFunction(name.to_string()), + Range::from_located(stmt), + )); } } } @@ -183,12 +173,10 @@ pub fn definition(checker: &mut Checker, definition: &Definition, visibility: &V if !(checker.settings.flake8_annotations.suppress_dummy_args && checker.settings.dummy_variable_rgx.is_match(&arg.node.arg)) { - if checker.settings.enabled.contains(&CheckCode::ANN001) { - checker.add_check(Check::new( - CheckKind::MissingTypeFunctionArgument(arg.node.arg.to_string()), - Range::from_located(arg), - )); - } + checker.add_check(Check::new( + CheckKind::MissingTypeFunctionArgument(arg.node.arg.to_string()), + Range::from_located(arg), + )); } } else { has_any_typed_arg = true; @@ -201,12 +189,10 @@ pub fn definition(checker: &mut Checker, definition: &Definition, visibility: &V if !(checker.settings.flake8_annotations.suppress_dummy_args && checker.settings.dummy_variable_rgx.is_match(&arg.node.arg)) { - if checker.settings.enabled.contains(&CheckCode::ANN002) { - checker.add_check(Check::new( - CheckKind::MissingTypeArgs(arg.node.arg.to_string()), - Range::from_located(arg), - )); - } + checker.add_check(Check::new( + CheckKind::MissingTypeArgs(arg.node.arg.to_string()), + Range::from_located(arg), + )); } } else { has_any_typed_arg = true; @@ -219,12 +205,10 @@ pub fn definition(checker: &mut Checker, definition: &Definition, visibility: &V if !(checker.settings.flake8_annotations.suppress_dummy_args && checker.settings.dummy_variable_rgx.is_match(&arg.node.arg)) { - if checker.settings.enabled.contains(&CheckCode::ANN003) { - checker.add_check(Check::new( - CheckKind::MissingTypeKwargs(arg.node.arg.to_string()), - Range::from_located(arg), - )); - } + checker.add_check(Check::new( + CheckKind::MissingTypeKwargs(arg.node.arg.to_string()), + Range::from_located(arg), + )); } } else { has_any_typed_arg = true; @@ -236,19 +220,15 @@ pub fn definition(checker: &mut Checker, definition: &Definition, visibility: &V if let Some(arg) = args.args.first() { if arg.node.annotation.is_none() { if visibility::is_classmethod(stmt) { - if checker.settings.enabled.contains(&CheckCode::ANN102) { - checker.add_check(Check::new( - CheckKind::MissingTypeCls(arg.node.arg.to_string()), - Range::from_located(arg), - )); - } + checker.add_check(Check::new( + CheckKind::MissingTypeCls(arg.node.arg.to_string()), + Range::from_located(arg), + )); } else { - if checker.settings.enabled.contains(&CheckCode::ANN101) { - checker.add_check(Check::new( - CheckKind::MissingTypeSelf(arg.node.arg.to_string()), - Range::from_located(arg), - )); - } + checker.add_check(Check::new( + CheckKind::MissingTypeSelf(arg.node.arg.to_string()), + Range::from_located(arg), + )); } } } @@ -265,56 +245,43 @@ pub fn definition(checker: &mut Checker, definition: &Definition, visibility: &V } if visibility::is_classmethod(stmt) { - if checker.settings.enabled.contains(&CheckCode::ANN206) { - checker.add_check(Check::new( - CheckKind::MissingReturnTypeClassMethod(name.to_string()), - Range::from_located(stmt), - )); - } + checker.add_check(Check::new( + CheckKind::MissingReturnTypeClassMethod(name.to_string()), + Range::from_located(stmt), + )); } else if visibility::is_staticmethod(stmt) { - if checker.settings.enabled.contains(&CheckCode::ANN205) { - checker.add_check(Check::new( - CheckKind::MissingReturnTypeStaticMethod(name.to_string()), - Range::from_located(stmt), - )); - } + checker.add_check(Check::new( + CheckKind::MissingReturnTypeStaticMethod(name.to_string()), + Range::from_located(stmt), + )); } else if visibility::is_magic(stmt) { - if checker.settings.enabled.contains(&CheckCode::ANN204) { + checker.add_check(Check::new( + CheckKind::MissingReturnTypeMagicMethod(name.to_string()), + Range::from_located(stmt), + )); + } else if visibility::is_init(stmt) { + // Allow omission of return annotation in `__init__` functions, as long as at + // least one argument is typed. + if !(checker.settings.flake8_annotations.mypy_init_return && has_any_typed_arg) + { checker.add_check(Check::new( CheckKind::MissingReturnTypeMagicMethod(name.to_string()), Range::from_located(stmt), )); } - } else if visibility::is_init(stmt) { - // Allow omission of return annotation in `__init__` functions, as long as at - // least one argument is typed. - if checker.settings.enabled.contains(&CheckCode::ANN204) { - if !(checker.settings.flake8_annotations.mypy_init_return - && has_any_typed_arg) - { - checker.add_check(Check::new( - CheckKind::MissingReturnTypeMagicMethod(name.to_string()), - Range::from_located(stmt), - )); - } - } } else { match visibility { Visibility::Public => { - if checker.settings.enabled.contains(&CheckCode::ANN201) { - checker.add_check(Check::new( - CheckKind::MissingReturnTypePublicFunction(name.to_string()), - Range::from_located(stmt), - )); - } + checker.add_check(Check::new( + CheckKind::MissingReturnTypePublicFunction(name.to_string()), + Range::from_located(stmt), + )); } Visibility::Private => { - if checker.settings.enabled.contains(&CheckCode::ANN202) { - checker.add_check(Check::new( - CheckKind::MissingReturnTypePrivateFunction(name.to_string()), - Range::from_located(stmt), - )); - } + checker.add_check(Check::new( + CheckKind::MissingReturnTypePrivateFunction(name.to_string()), + Range::from_located(stmt), + )); } } } diff --git a/src/flake8_print/checks.rs b/src/flake8_print/checks.rs index 1fd6b98b1a..3329ec82eb 100644 --- a/src/flake8_print/checks.rs +++ b/src/flake8_print/checks.rs @@ -4,24 +4,18 @@ use crate::ast::types::Range; use crate::checks::{Check, CheckKind}; /// Check whether a function call is a `print` or `pprint` invocation -pub fn print_call( - expr: &Expr, - func: &Expr, - check_print: bool, - check_pprint: bool, - location: Range, -) -> Option { +pub fn print_call(expr: &Expr, func: &Expr, location: Range) -> Option { if let ExprKind::Name { id, .. } = &func.node { - if check_print && id == "print" { + if id == "print" { return Some(Check::new(CheckKind::PrintFound, Range::from_located(expr))); - } else if check_pprint && id == "pprint" { + } else if id == "pprint" { return Some(Check::new(CheckKind::PPrintFound, location)); } } if let ExprKind::Attribute { value, attr, .. } = &func.node { if let ExprKind::Name { id, .. } = &value.node { - if check_pprint && id == "pprint" && attr == "pprint" { + if id == "pprint" && attr == "pprint" { return Some(Check::new(CheckKind::PPrintFound, location)); } } diff --git a/src/flake8_print/plugins/print_call.rs b/src/flake8_print/plugins/print_call.rs index 435621c942..bf8ed1ca19 100644 --- a/src/flake8_print/plugins/print_call.rs +++ b/src/flake8_print/plugins/print_call.rs @@ -4,17 +4,10 @@ use rustpython_ast::{Expr, Stmt, StmtKind}; use crate::ast::types::Range; use crate::autofix::helpers; use crate::check_ast::Checker; -use crate::checks::CheckCode; use crate::flake8_print::checks; pub fn print_call(checker: &mut Checker, expr: &Expr, func: &Expr) { - if let Some(mut check) = checks::print_call( - expr, - func, - checker.settings.enabled.contains(&CheckCode::T201), - checker.settings.enabled.contains(&CheckCode::T203), - Range::from_located(expr), - ) { + if let Some(mut check) = checks::print_call(expr, func, Range::from_located(expr)) { if checker.patch() { let context = checker.binding_context(); if matches!( diff --git a/src/linter.rs b/src/linter.rs index 4c1568e463..fe3da366b3 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -98,6 +98,12 @@ pub(crate) fn check_path( // Run the lines-based checks. check_lines(&mut checks, contents, noqa_line_for, settings, autofix); + // Filter out any disabled checks. Throughout the codebase, we skip work based + // on the set of enabled checks, but we don't _guarantee_ that disabled + // checks aren't added. (Sometimes, it's not worth repeatedly checking the + // code list for similar checks.) + checks.retain(|check| settings.enabled.contains(check.kind.code())); + // Create path ignores. if !checks.is_empty() && !settings.per_file_ignores.is_empty() { let ignores = fs::ignores_from_path(path, &settings.per_file_ignores)?; diff --git a/src/pycodestyle/checks.rs b/src/pycodestyle/checks.rs index 864ae2d5be..d595551f4f 100644 --- a/src/pycodestyle/checks.rs +++ b/src/pycodestyle/checks.rs @@ -56,12 +56,7 @@ pub fn do_not_assign_lambda(value: &Expr, location: Range) -> Option { } /// E713, E714 -pub fn not_tests( - op: &Unaryop, - operand: &Expr, - check_not_in: bool, - check_not_is: bool, -) -> Vec { +pub fn not_tests(op: &Unaryop, operand: &Expr) -> Vec { let mut checks: Vec = vec![]; if matches!(op, Unaryop::Not) { @@ -69,20 +64,16 @@ pub fn not_tests( for op in ops { match op { Cmpop::In => { - if check_not_in { - checks.push(Check::new( - CheckKind::NotInTest, - Range::from_located(operand), - )); - } + checks.push(Check::new( + CheckKind::NotInTest, + Range::from_located(operand), + )); } Cmpop::Is => { - if check_not_is { - checks.push(Check::new( - CheckKind::NotIsTest, - Range::from_located(operand), - )); - } + checks.push(Check::new( + CheckKind::NotIsTest, + Range::from_located(operand), + )); } _ => {} } @@ -94,28 +85,20 @@ pub fn not_tests( } /// E711, E712 -pub fn literal_comparisons( - left: &Expr, - ops: &[Cmpop], - comparators: &[Expr], - check_none_comparisons: bool, - check_true_false_comparisons: bool, -) -> Vec { +pub fn literal_comparisons(left: &Expr, ops: &[Cmpop], comparators: &[Expr]) -> Vec { let mut checks: Vec = vec![]; let op = ops.first().unwrap(); let comparator = left; // Check `left`. - if check_none_comparisons - && matches!( - comparator.node, - ExprKind::Constant { - value: Constant::None, - kind: None - } - ) - { + if matches!( + comparator.node, + ExprKind::Constant { + value: Constant::None, + kind: None + } + ) { if matches!(op, Cmpop::Eq) { checks.push(Check::new( CheckKind::NoneComparison(RejectedCmpop::Eq), @@ -130,7 +113,48 @@ pub fn literal_comparisons( } } - if check_true_false_comparisons { + if let ExprKind::Constant { + value: Constant::Bool(value), + kind: None, + } = comparator.node + { + if matches!(op, Cmpop::Eq) { + checks.push(Check::new( + CheckKind::TrueFalseComparison(value, RejectedCmpop::Eq), + Range::from_located(comparator), + )); + } + if matches!(op, Cmpop::NotEq) { + checks.push(Check::new( + CheckKind::TrueFalseComparison(value, RejectedCmpop::NotEq), + Range::from_located(comparator), + )); + } + } + + // Check each comparator in order. + for (op, comparator) in izip!(ops, comparators) { + if matches!( + comparator.node, + ExprKind::Constant { + value: Constant::None, + kind: None + } + ) { + if matches!(op, Cmpop::Eq) { + checks.push(Check::new( + CheckKind::NoneComparison(RejectedCmpop::Eq), + Range::from_located(comparator), + )); + } + if matches!(op, Cmpop::NotEq) { + checks.push(Check::new( + CheckKind::NoneComparison(RejectedCmpop::NotEq), + Range::from_located(comparator), + )); + } + } + if let ExprKind::Constant { value: Constant::Bool(value), kind: None, @@ -151,53 +175,6 @@ pub fn literal_comparisons( } } - // Check each comparator in order. - for (op, comparator) in izip!(ops, comparators) { - if check_none_comparisons - && matches!( - comparator.node, - ExprKind::Constant { - value: Constant::None, - kind: None - } - ) - { - if matches!(op, Cmpop::Eq) { - checks.push(Check::new( - CheckKind::NoneComparison(RejectedCmpop::Eq), - Range::from_located(comparator), - )); - } - if matches!(op, Cmpop::NotEq) { - checks.push(Check::new( - CheckKind::NoneComparison(RejectedCmpop::NotEq), - Range::from_located(comparator), - )); - } - } - - if check_true_false_comparisons { - if let ExprKind::Constant { - value: Constant::Bool(value), - kind: None, - } = comparator.node - { - if matches!(op, Cmpop::Eq) { - checks.push(Check::new( - CheckKind::TrueFalseComparison(value, RejectedCmpop::Eq), - Range::from_located(comparator), - )); - } - if matches!(op, Cmpop::NotEq) { - checks.push(Check::new( - CheckKind::TrueFalseComparison(value, RejectedCmpop::NotEq), - Range::from_located(comparator), - )); - } - } - } - } - checks } diff --git a/src/pyflakes/checks.rs b/src/pyflakes/checks.rs index 29bc7e8ff3..0483c4249d 100644 --- a/src/pyflakes/checks.rs +++ b/src/pyflakes/checks.rs @@ -143,11 +143,7 @@ fn convert_to_value(expr: &Expr) -> Option { } /// F601, F602 -pub fn repeated_keys( - keys: &[Expr], - check_repeated_literals: bool, - check_repeated_variables: bool, -) -> Vec { +pub fn repeated_keys(keys: &[Expr]) -> Vec { let mut checks: Vec = vec![]; let num_keys = keys.len(); @@ -158,7 +154,7 @@ pub fn repeated_keys( let v2 = convert_to_value(k2); match (&v1, &v2) { (Some(DictionaryKey::Constant(v1)), Some(DictionaryKey::Constant(v2))) => { - if check_repeated_literals && v1 == v2 { + if v1 == v2 { checks.push(Check::new( CheckKind::MultiValueRepeatedKeyLiteral, Range::from_located(k2), @@ -166,7 +162,7 @@ pub fn repeated_keys( } } (Some(DictionaryKey::Variable(v1)), Some(DictionaryKey::Variable(v2))) => { - if check_repeated_variables && v1 == v2 { + if v1 == v2 { checks.push(Check::new( CheckKind::MultiValueRepeatedKeyVariable((*v2).to_string()), Range::from_located(k2), @@ -182,17 +178,12 @@ pub fn repeated_keys( } /// F621, F622 -pub fn starred_expressions( - elts: &[Expr], - check_too_many_expressions: bool, - check_two_starred_expressions: bool, - location: Range, -) -> Option { +pub fn starred_expressions(elts: &[Expr], location: Range) -> Option { let mut has_starred: bool = false; let mut starred_index: Option = None; for (index, elt) in elts.iter().enumerate() { if matches!(elt.node, ExprKind::Starred { .. }) { - if has_starred && check_two_starred_expressions { + if has_starred { return Some(Check::new(CheckKind::TwoStarredExpressions, location)); } has_starred = true; @@ -200,11 +191,9 @@ pub fn starred_expressions( } } - 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::ExpressionsInStarAssignment, location)); - } + if let Some(starred_index) = starred_index { + if starred_index >= 1 << 8 || elts.len() - starred_index > 1 << 24 { + return Some(Check::new(CheckKind::ExpressionsInStarAssignment, location)); } }