Remove CheckLocator abstraction (#627)

This commit is contained in:
Charlie Marsh 2022-11-06 17:42:10 -05:00 committed by GitHub
parent 99d9aa61bf
commit 85b882fc54
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
23 changed files with 348 additions and 335 deletions

View file

@ -100,10 +100,6 @@ pub struct Binding {
pub used: Option<(usize, Range)>,
}
pub trait CheckLocator {
fn locate_check(&self, default: Range) -> Range;
}
#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)]
pub enum ImportKind {
Import,

File diff suppressed because it is too large Load diff

View file

@ -1,6 +1,6 @@
use rustpython_ast::{Constant, Expr, ExprContext, ExprKind, Stmt, StmtKind};
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::autofix::Fix;
use crate::check_ast::Checker;
use crate::checks::{Check, CheckKind};
@ -42,10 +42,7 @@ pub fn assert_false(checker: &mut Checker, stmt: &Stmt, test: &Expr, msg: &Optio
..
} = &test.node
{
let mut check = Check::new(
CheckKind::DoNotAssertFalse,
checker.locate_check(Range::from_located(test)),
);
let mut check = Check::new(CheckKind::DoNotAssertFalse, Range::from_located(test));
if checker.patch() {
let mut generator = SourceGenerator::new();
if let Ok(()) = generator.unparse_stmt(&assertion_error(msg)) {

View file

@ -1,7 +1,7 @@
use rustpython_ast::{ExprKind, Stmt, Withitem};
use crate::ast::helpers::match_name_or_attr;
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::check_ast::Checker;
use crate::checks::{Check, CheckKind};
@ -17,7 +17,7 @@ pub fn assert_raises_exception(checker: &mut Checker, stmt: &Stmt, items: &[With
{
checker.add_check(Check::new(
CheckKind::NoAssertRaisesException,
checker.locate_check(Range::from_located(stmt)),
Range::from_located(stmt),
));
}
}

View file

@ -1,6 +1,6 @@
use rustpython_ast::{Expr, ExprKind};
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::check_ast::Checker;
use crate::checks::{Check, CheckKind};
@ -14,7 +14,7 @@ pub fn assignment_to_os_environ(checker: &mut Checker, targets: &[Expr]) {
if id == "os" {
checker.add_check(Check::new(
CheckKind::AssignmentToOsEnviron,
checker.locate_check(Range::from_located(target)),
Range::from_located(target),
));
}
}

View file

@ -1,6 +1,6 @@
use rustpython_ast::{Expr, ExprKind};
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::check_ast::Checker;
use crate::checks::{Check, CheckKind};
@ -9,7 +9,7 @@ pub fn cannot_raise_literal(checker: &mut Checker, expr: &Expr) {
if let ExprKind::Constant { .. } = &expr.node {
checker.add_check(Check::new(
CheckKind::CannotRaiseLiteral,
checker.locate_check(Range::from_located(expr)),
Range::from_located(expr),
));
}
}

View file

@ -4,7 +4,7 @@ use itertools::Itertools;
use rustpython_ast::{Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind, Stmt};
use crate::ast::helpers;
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::autofix::Fix;
use crate::check_ast::Checker;
use crate::checks::{Check, CheckCode, CheckKind};
@ -47,7 +47,7 @@ pub fn duplicate_handler_exceptions(
CheckKind::DuplicateHandlerException(
duplicates.into_iter().sorted().collect::<Vec<String>>(),
),
checker.locate_check(Range::from_located(expr)),
Range::from_located(expr),
);
if checker.patch() {
// TODO(charlie): If we have a single element, remove the tuple.
@ -106,7 +106,7 @@ pub fn duplicate_exceptions(checker: &mut Checker, stmt: &Stmt, handlers: &[Exce
for duplicate in duplicates.into_iter().sorted() {
checker.add_check(Check::new(
CheckKind::DuplicateTryBlockException(duplicate),
checker.locate_check(Range::from_located(stmt)),
Range::from_located(stmt),
));
}
}

View file

@ -1,7 +1,7 @@
use rustpython_ast::{Arguments, Constant, Expr, ExprKind};
use crate::ast::helpers::compose_call_path;
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::ast::visitor;
use crate::ast::visitor::Visitor;
use crate::check_ast::Checker;
@ -91,6 +91,6 @@ pub fn function_call_argument_default(checker: &mut Checker, arguments: &Argumen
visitor.visit_expr(expr);
}
for (check, range) in visitor.checks {
checker.add_check(Check::new(check, checker.locate_check(range)));
checker.add_check(Check::new(check, range));
}
}

View file

@ -1,6 +1,6 @@
use rustpython_ast::{Arguments, Expr, ExprKind};
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::check_ast::Checker;
use crate::checks::{Check, CheckKind};
@ -45,14 +45,14 @@ pub fn mutable_argument_default(checker: &mut Checker, arguments: &Arguments) {
| ExprKind::SetComp { .. } => {
checker.add_check(Check::new(
CheckKind::MutableArgumentDefault,
checker.locate_check(Range::from_located(expr)),
Range::from_located(expr),
));
}
ExprKind::Call { func, .. } => {
if is_mutable_func(func) {
checker.add_check(Check::new(
CheckKind::MutableArgumentDefault,
checker.locate_check(Range::from_located(expr)),
Range::from_located(expr),
));
}
}

View file

@ -1,6 +1,6 @@
use rustpython_ast::{Excepthandler, ExcepthandlerKind, ExprKind};
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::check_ast::Checker;
use crate::checks::{Check, CheckKind};
@ -13,7 +13,7 @@ pub fn redundant_tuple_in_exception_handler(checker: &mut Checker, handlers: &[E
if elts.len() == 1 {
checker.add_check(Check::new(
CheckKind::RedundantTupleInExceptionHandler(elts[0].to_string()),
checker.locate_check(Range::from_located(type_)),
Range::from_located(type_),
));
}
}

View file

@ -1,6 +1,6 @@
use rustpython_ast::{Expr, ExprKind, Unaryop};
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::check_ast::Checker;
use crate::checks::{Check, CheckKind};
@ -11,7 +11,7 @@ pub fn unary_prefix_increment(checker: &mut Checker, expr: &Expr, op: &Unaryop,
if matches!(op, Unaryop::UAdd) {
checker.add_check(Check::new(
CheckKind::UnaryPrefixIncrement,
checker.locate_check(Range::from_located(expr)),
Range::from_located(expr),
))
}
}

View file

@ -2,7 +2,7 @@ use std::collections::BTreeMap;
use rustpython_ast::{Expr, ExprKind, Stmt};
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::ast::visitor;
use crate::ast::visitor::Visitor;
use crate::autofix::Fix;
@ -64,7 +64,7 @@ pub fn unused_loop_control_variable(checker: &mut Checker, target: &Expr, body:
let mut check = Check::new(
CheckKind::UnusedLoopControlVariable(name.to_string()),
checker.locate_check(Range::from_located(expr)),
Range::from_located(expr),
);
if checker.patch() {
// Prefix the variable name with an underscore.

View file

@ -1,6 +1,6 @@
use rustpython_ast::{Expr, ExprKind};
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::check_ast::Checker;
use crate::checks::{Check, CheckKind};
@ -8,7 +8,7 @@ pub fn useless_comparison(checker: &mut Checker, expr: &Expr) {
if let ExprKind::Compare { left, .. } = &expr.node {
checker.add_check(Check::new(
CheckKind::UselessComparison,
checker.locate_check(Range::from_located(left)),
Range::from_located(left),
));
}
}

View file

@ -1,6 +1,6 @@
use rustpython_ast::{Constant, ExprKind, Stmt, StmtKind};
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::check_ast::Checker;
use crate::checks::{Check, CheckKind};
@ -12,7 +12,7 @@ pub fn useless_expression(checker: &mut Checker, body: &[Stmt]) {
ExprKind::List { .. } | ExprKind::Dict { .. } | ExprKind::Set { .. } => {
checker.add_check(Check::new(
CheckKind::UselessExpression,
checker.locate_check(Range::from_located(value)),
Range::from_located(value),
));
}
ExprKind::Constant { value: val, .. } => match &val {
@ -20,7 +20,7 @@ pub fn useless_expression(checker: &mut Checker, body: &[Stmt]) {
_ => {
checker.add_check(Check::new(
CheckKind::UselessExpression,
checker.locate_check(Range::from_located(value)),
Range::from_located(value),
));
}
},

View file

@ -1,7 +1,7 @@
use log::error;
use rustpython_ast::{Expr, Stmt, StmtKind};
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::autofix::helpers;
use crate::check_ast::Checker;
use crate::checks::CheckCode;
@ -13,7 +13,7 @@ pub fn print_call(checker: &mut Checker, expr: &Expr, func: &Expr) {
func,
checker.settings.enabled.contains(&CheckCode::T201),
checker.settings.enabled.contains(&CheckCode::T203),
checker.locate_check(Range::from_located(expr)),
Range::from_located(expr),
) {
if checker.patch() {
let context = checker.binding_context();

View file

@ -2,7 +2,7 @@ use itertools::izip;
use rustpython_ast::Location;
use rustpython_parser::ast::{Cmpop, Constant, Expr, ExprKind, Unaryop};
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::checks::{Check, CheckKind, RejectedCmpop};
use crate::source_code_locator::SourceCodeLocator;
@ -61,7 +61,6 @@ pub fn not_tests(
operand: &Expr,
check_not_in: bool,
check_not_is: bool,
locator: &dyn CheckLocator,
) -> Vec<Check> {
let mut checks: Vec<Check> = vec![];
@ -73,7 +72,7 @@ pub fn not_tests(
if check_not_in {
checks.push(Check::new(
CheckKind::NotInTest,
locator.locate_check(Range::from_located(operand)),
Range::from_located(operand),
));
}
}
@ -81,7 +80,7 @@ pub fn not_tests(
if check_not_is {
checks.push(Check::new(
CheckKind::NotIsTest,
locator.locate_check(Range::from_located(operand)),
Range::from_located(operand),
));
}
}
@ -101,7 +100,6 @@ pub fn literal_comparisons(
comparators: &[Expr],
check_none_comparisons: bool,
check_true_false_comparisons: bool,
locator: &dyn CheckLocator,
) -> Vec<Check> {
let mut checks: Vec<Check> = vec![];
@ -121,13 +119,13 @@ pub fn literal_comparisons(
if matches!(op, Cmpop::Eq) {
checks.push(Check::new(
CheckKind::NoneComparison(RejectedCmpop::Eq),
locator.locate_check(Range::from_located(comparator)),
Range::from_located(comparator),
));
}
if matches!(op, Cmpop::NotEq) {
checks.push(Check::new(
CheckKind::NoneComparison(RejectedCmpop::NotEq),
locator.locate_check(Range::from_located(comparator)),
Range::from_located(comparator),
));
}
}
@ -141,13 +139,13 @@ pub fn literal_comparisons(
if matches!(op, Cmpop::Eq) {
checks.push(Check::new(
CheckKind::TrueFalseComparison(value, RejectedCmpop::Eq),
locator.locate_check(Range::from_located(comparator)),
Range::from_located(comparator),
));
}
if matches!(op, Cmpop::NotEq) {
checks.push(Check::new(
CheckKind::TrueFalseComparison(value, RejectedCmpop::NotEq),
locator.locate_check(Range::from_located(comparator)),
Range::from_located(comparator),
));
}
}
@ -167,13 +165,13 @@ pub fn literal_comparisons(
if matches!(op, Cmpop::Eq) {
checks.push(Check::new(
CheckKind::NoneComparison(RejectedCmpop::Eq),
locator.locate_check(Range::from_located(comparator)),
Range::from_located(comparator),
));
}
if matches!(op, Cmpop::NotEq) {
checks.push(Check::new(
CheckKind::NoneComparison(RejectedCmpop::NotEq),
locator.locate_check(Range::from_located(comparator)),
Range::from_located(comparator),
));
}
}
@ -187,13 +185,13 @@ pub fn literal_comparisons(
if matches!(op, Cmpop::Eq) {
checks.push(Check::new(
CheckKind::TrueFalseComparison(value, RejectedCmpop::Eq),
locator.locate_check(Range::from_located(comparator)),
Range::from_located(comparator),
));
}
if matches!(op, Cmpop::NotEq) {
checks.push(Check::new(
CheckKind::TrueFalseComparison(value, RejectedCmpop::NotEq),
locator.locate_check(Range::from_located(comparator)),
Range::from_located(comparator),
));
}
}

View file

@ -5,7 +5,7 @@ use rustpython_parser::ast::{
Arg, Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprKind, Stmt, StmtKind,
};
use crate::ast::types::{BindingKind, CheckLocator, FunctionScope, Range, Scope, ScopeKind};
use crate::ast::types::{BindingKind, FunctionScope, Range, Scope, ScopeKind};
use crate::checks::{Check, CheckKind};
/// F631
@ -28,12 +28,30 @@ pub fn if_tuple(test: &Expr, location: Range) -> Option<Check> {
None
}
/// F821
pub fn undefined_local(scopes: &[&Scope], name: &str) -> Option<Check> {
let current = &scopes.last().expect("No current scope found.");
if matches!(current.kind, ScopeKind::Function(_)) && !current.values.contains_key(name) {
for scope in scopes.iter().rev().skip(1) {
if matches!(scope.kind, ScopeKind::Function(_) | ScopeKind::Module) {
if let Some(binding) = scope.values.get(name) {
if let Some((scope_id, location)) = binding.used {
if scope_id == current.id {
return Some(Check::new(
CheckKind::UndefinedLocal(name.to_string()),
location,
));
}
}
}
}
}
}
None
}
/// F841
pub fn unused_variables(
scope: &Scope,
locator: &dyn CheckLocator,
dummy_variable_rgx: &Regex,
) -> Vec<Check> {
pub fn unused_variables(scope: &Scope, dummy_variable_rgx: &Regex) -> Vec<Check> {
let mut checks: Vec<Check> = vec![];
if matches!(
@ -53,7 +71,7 @@ pub fn unused_variables(
{
checks.push(Check::new(
CheckKind::UnusedVariable(name.to_string()),
locator.locate_check(binding.range),
binding.range,
));
}
}
@ -129,7 +147,6 @@ pub fn repeated_keys(
keys: &[Expr],
check_repeated_literals: bool,
check_repeated_variables: bool,
locator: &dyn CheckLocator,
) -> Vec<Check> {
let mut checks: Vec<Check> = vec![];
@ -144,7 +161,7 @@ pub fn repeated_keys(
if check_repeated_literals && v1 == v2 {
checks.push(Check::new(
CheckKind::MultiValueRepeatedKeyLiteral,
locator.locate_check(Range::from_located(k2)),
Range::from_located(k2),
))
}
}
@ -152,7 +169,7 @@ pub fn repeated_keys(
if check_repeated_variables && v1 == v2 {
checks.push(Check::new(
CheckKind::MultiValueRepeatedKeyVariable((*v2).to_string()),
locator.locate_check(Range::from_located(k2)),
Range::from_located(k2),
))
}
}
@ -195,12 +212,7 @@ pub fn starred_expressions(
}
/// F701
pub fn break_outside_loop(
stmt: &Stmt,
parents: &[&Stmt],
parent_stack: &[usize],
locator: &dyn CheckLocator,
) -> Option<Check> {
pub fn break_outside_loop(stmt: &Stmt, parents: &[&Stmt], parent_stack: &[usize]) -> Option<Check> {
let mut allowed: bool = false;
let mut parent = stmt;
for index in parent_stack.iter().rev() {
@ -228,7 +240,7 @@ pub fn break_outside_loop(
if !allowed {
Some(Check::new(
CheckKind::BreakOutsideLoop,
locator.locate_check(Range::from_located(stmt)),
Range::from_located(stmt),
))
} else {
None
@ -240,7 +252,6 @@ pub fn continue_outside_loop(
stmt: &Stmt,
parents: &[&Stmt],
parent_stack: &[usize],
locator: &dyn CheckLocator,
) -> Option<Check> {
let mut allowed: bool = false;
let mut parent = stmt;
@ -269,7 +280,7 @@ pub fn continue_outside_loop(
if !allowed {
Some(Check::new(
CheckKind::ContinueOutsideLoop,
locator.locate_check(Range::from_located(stmt)),
Range::from_located(stmt),
))
} else {
None

View file

@ -1,13 +1,12 @@
use rustpython_ast::{Expr, Stmt};
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::check_ast::Checker;
use crate::pyflakes::checks;
/// F631
pub fn assert_tuple(checker: &mut Checker, stmt: &Stmt, test: &Expr) {
if let Some(check) = checks::assert_tuple(test, checker.locate_check(Range::from_located(stmt)))
{
if let Some(check) = checks::assert_tuple(test, Range::from_located(stmt)) {
checker.add_check(check);
}
}

View file

@ -1,12 +1,12 @@
use rustpython_ast::{Expr, Stmt};
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::check_ast::Checker;
use crate::pyflakes::checks;
/// F634
pub fn if_tuple(checker: &mut Checker, stmt: &Stmt, test: &Expr) {
if let Some(check) = checks::if_tuple(test, checker.locate_check(Range::from_located(stmt))) {
if let Some(check) = checks::if_tuple(test, Range::from_located(stmt)) {
checker.add_check(check);
}
}

View file

@ -1,6 +1,6 @@
use rustpython_ast::{Expr, ExprKind};
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::autofix::Fix;
use crate::check_ast::Checker;
use crate::checks::{Check, CheckKind};
@ -27,10 +27,7 @@ fn match_not_implemented(expr: &Expr) -> Option<&Expr> {
/// F901
pub fn raise_not_implemented(checker: &mut Checker, expr: &Expr) {
if let Some(expr) = match_not_implemented(expr) {
let mut check = Check::new(
CheckKind::RaiseNotImplemented,
checker.locate_check(Range::from_located(expr)),
);
let mut check = Check::new(CheckKind::RaiseNotImplemented, Range::from_located(expr));
if checker.patch() {
check.amend(Fix::replacement(
"NotImplementedError".to_string(),

View file

@ -1,15 +1,13 @@
use rustpython_ast::Expr;
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::autofix::Fix;
use crate::check_ast::Checker;
use crate::checks::CheckKind;
use crate::pyupgrade::checks;
pub fn type_of_primitive(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) {
if let Some(mut check) =
checks::type_of_primitive(func, args, checker.locate_check(Range::from_located(expr)))
{
if let Some(mut check) = checks::type_of_primitive(func, args, Range::from_located(expr)) {
if checker.patch() {
if let CheckKind::TypeOfPrimitive(primitive) = &check.kind {
check.amend(Fix::replacement(

View file

@ -1,14 +1,12 @@
use rustpython_ast::Expr;
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::autofix::Fix;
use crate::check_ast::Checker;
use crate::pyupgrade::checks;
pub fn unnecessary_abspath(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) {
if let Some(mut check) =
checks::unnecessary_abspath(func, args, checker.locate_check(Range::from_located(expr)))
{
if let Some(mut check) = checks::unnecessary_abspath(func, args, Range::from_located(expr)) {
if checker.patch() {
check.amend(Fix::replacement(
"__file__".to_string(),

View file

@ -1,17 +1,15 @@
use log::error;
use rustpython_ast::{Expr, Stmt};
use crate::ast::types::{CheckLocator, Range};
use crate::ast::types::Range;
use crate::autofix::helpers;
use crate::check_ast::Checker;
use crate::pyupgrade::checks;
pub fn useless_metaclass_type(checker: &mut Checker, stmt: &Stmt, value: &Expr, targets: &[Expr]) {
if let Some(mut check) = checks::useless_metaclass_type(
targets,
value,
checker.locate_check(Range::from_located(stmt)),
) {
if let Some(mut check) =
checks::useless_metaclass_type(targets, value, Range::from_located(stmt))
{
if checker.patch() {
let context = checker.binding_context();
let deleted: Vec<&Stmt> = checker