Skip to content

Commit

Permalink
Remove CheckLocator abstraction (#627)
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh authored Nov 6, 2022
1 parent 99d9aa6 commit 85b882f
Show file tree
Hide file tree
Showing 23 changed files with 348 additions and 335 deletions.
4 changes: 0 additions & 4 deletions src/ast/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
507 changes: 264 additions & 243 deletions src/check_ast.rs

Large diffs are not rendered by default.

7 changes: 2 additions & 5 deletions src/flake8_bugbear/plugins/assert_false.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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)) {
Expand Down
4 changes: 2 additions & 2 deletions src/flake8_bugbear/plugins/assert_raises_exception.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand All @@ -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),
));
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/flake8_bugbear/plugins/assignment_to_os_environ.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand All @@ -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),
));
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/flake8_bugbear/plugins/cannot_raise_literal.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand All @@ -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),
));
}
}
6 changes: 3 additions & 3 deletions src/flake8_bugbear/plugins/duplicate_exceptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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),
));
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/flake8_bugbear/plugins/function_call_argument_default.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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));
}
}
6 changes: 3 additions & 3 deletions src/flake8_bugbear/plugins/mutable_argument_default.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -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),
));
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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};

Expand All @@ -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_),
));
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/flake8_bugbear/plugins/unary_prefix_increment.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand All @@ -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),
))
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/flake8_bugbear/plugins/unused_loop_control_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions src/flake8_bugbear/plugins/useless_comparison.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
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};

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),
));
}
}
6 changes: 3 additions & 3 deletions src/flake8_bugbear/plugins/useless_expression.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand All @@ -12,15 +12,15 @@ 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 {
Constant::Str { .. } | Constant::Ellipsis => {}
_ => {
checker.add_check(Check::new(
CheckKind::UselessExpression,
checker.locate_check(Range::from_located(value)),
Range::from_located(value),
));
}
},
Expand Down
4 changes: 2 additions & 2 deletions src/flake8_print/plugins/print_call.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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();
Expand Down
24 changes: 11 additions & 13 deletions src/pycodestyle/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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![];

Expand All @@ -73,15 +72,15 @@ 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),
));
}
}
Cmpop::Is => {
if check_not_is {
checks.push(Check::new(
CheckKind::NotIsTest,
locator.locate_check(Range::from_located(operand)),
Range::from_located(operand),
));
}
}
Expand All @@ -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![];

Expand All @@ -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),
));
}
}
Expand All @@ -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),
));
}
}
Expand All @@ -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),
));
}
}
Expand All @@ -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),
));
}
}
Expand Down
Loading

0 comments on commit 85b882f

Please sign in to comment.