Pull check logic out of check_ast.rs (#135)

src/ast.rs

@@ -0,0 +1,5 @@
+pub mod checks;
+pub mod operations;
+pub mod relocate;
+pub mod types;
+pub mod visitor;

src/ast/checks.rs

@@ -0,0 +1,397 @@
+use std::collections::BTreeSet;
+
+use itertools::izip;
+use rustpython_parser::ast::{
+    Arg, Arguments, Cmpop, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprKind, Keyword,
+    Location, Stmt, Unaryop,
+};
+
+use crate::ast::operations::SourceCodeLocator;
+use crate::ast::types::{Binding, BindingKind, Scope};
+use crate::autofix::{fixer, fixes};
+use crate::checks::{Check, CheckKind, Fix, RejectedCmpop};
+
+/// Check IfTuple compliance.
+pub fn check_if_tuple(test: &Expr, location: Location) -> Option<Check> {
+    if let ExprKind::Tuple { elts, .. } = &test.node {
+        if !elts.is_empty() {
+            return Some(Check::new(CheckKind::IfTuple, location));
+        }
+    }
+    None
+}
+
+/// Check AssertTuple compliance.
+pub fn check_assert_tuple(test: &Expr, location: Location) -> Option<Check> {
+    if let ExprKind::Tuple { elts, .. } = &test.node {
+        if !elts.is_empty() {
+            return Some(Check::new(CheckKind::AssertTuple, location));
+        }
+    }
+    None
+}
+
+/// Check NotInTest and NotIsTest compliance.
+pub fn check_not_tests(
+    op: &Unaryop,
+    operand: &Expr,
+    check_not_in: bool,
+    check_not_is: bool,
+) -> Vec<Check> {
+    let mut checks: Vec<Check> = vec![];
+
+    if matches!(op, Unaryop::Not) {
+        if let ExprKind::Compare { ops, .. } = &operand.node {
+            match ops[..] {
+                [Cmpop::In] => {
+                    if check_not_in {
+                        checks.push(Check::new(CheckKind::NotInTest, operand.location));
+                    }
+                }
+                [Cmpop::Is] => {
+                    if check_not_is {
+                        checks.push(Check::new(CheckKind::NotIsTest, operand.location));
+                    }
+                }
+                _ => {}
+            }
+        }
+    }
+
+    checks
+}
+
+/// Check UnusedVariable compliance.
+pub fn check_unused_variables(scope: &Scope) -> Vec<Check> {
+    let mut checks: Vec<Check> = vec![];
+
+    for (name, binding) in scope.values.iter() {
+        // TODO(charlie): Ignore if using `locals`.
+        if binding.used.is_none()
+            && name != "_"
+            && name != "__tracebackhide__"
+            && name != "__traceback_info__"
+            && name != "__traceback_supplement__"
+            && matches!(binding.kind, BindingKind::Assignment)
+        {
+            checks.push(Check::new(
+                CheckKind::UnusedVariable(name.to_string()),
+                binding.location,
+            ));
+        }
+    }
+
+    checks
+}
+
+/// Check DoNotAssignLambda compliance.
+pub fn check_do_not_assign_lambda(value: &Expr, location: Location) -> Option<Check> {
+    if let ExprKind::Lambda { .. } = &value.node {
+        Some(Check::new(CheckKind::DoNotAssignLambda, location))
+    } else {
+        None
+    }
+}
+
+/// Check UselessObjectInheritance compliance.
+pub fn check_useless_object_inheritance(
+    stmt: &Stmt,
+    name: &str,
+    bases: &[Expr],
+    keywords: &[Keyword],
+    scope: &Scope,
+    locator: &mut SourceCodeLocator,
+    autofix: &fixer::Mode,
+) -> Option<Check> {
+    for expr in bases {
+        if let ExprKind::Name { id, .. } = &expr.node {
+            if id == "object" {
+                match scope.values.get(id) {
+                    None
+                    | Some(Binding {
+                        kind: BindingKind::Builtin,
+                        ..
+                    }) => {
+                        let mut check = Check::new(
+                            CheckKind::UselessObjectInheritance(name.to_string()),
+                            expr.location,
+                        );
+                        if matches!(autofix, fixer::Mode::Generate)
+                            || matches!(autofix, fixer::Mode::Apply)
+                        {
+                            if let Some(fix) = fixes::remove_class_def_base(
+                                locator,
+                                &stmt.location,
+                                expr.location,
+                                bases,
+                                keywords,
+                            ) {
+                                check.amend(fix);
+                            }
+                        }
+                        return Some(check);
+                    }
+                    _ => {}
+                }
+            }
+        }
+    }
+
+    None
+}
+
+/// Check DefaultExceptNotLast compliance.
+pub fn check_default_except_not_last(handlers: &Vec<Excepthandler>) -> Option<Check> {
+    for (idx, handler) in handlers.iter().enumerate() {
+        let ExcepthandlerKind::ExceptHandler { type_, .. } = &handler.node;
+        if type_.is_none() && idx < handlers.len() - 1 {
+            return Some(Check::new(
+                CheckKind::DefaultExceptNotLast,
+                handler.location,
+            ));
+        }
+    }
+
+    None
+}
+
+/// Check RaiseNotImplemented compliance.
+pub fn check_raise_not_implemented(expr: &Expr) -> Option<Check> {
+    match &expr.node {
+        ExprKind::Call { func, .. } => {
+            if let ExprKind::Name { id, .. } = &func.node {
+                if id == "NotImplemented" {
+                    return Some(Check::new(CheckKind::RaiseNotImplemented, expr.location));
+                }
+            }
+        }
+        ExprKind::Name { id, .. } => {
+            if id == "NotImplemented" {
+                return Some(Check::new(CheckKind::RaiseNotImplemented, expr.location));
+            }
+        }
+        _ => {}
+    }
+
+    None
+}
+
+/// Check DuplicateArgumentName compliance.
+pub fn check_duplicate_arguments(arguments: &Arguments) -> Vec<Check> {
+    let mut checks: Vec<Check> = vec![];
+
+    // Collect all the arguments into a single vector.
+    let mut all_arguments: Vec<&Arg> = arguments
+        .args
+        .iter()
+        .chain(arguments.posonlyargs.iter())
+        .chain(arguments.kwonlyargs.iter())
+        .collect();
+    if let Some(arg) = &arguments.vararg {
+        all_arguments.push(arg);
+    }
+    if let Some(arg) = &arguments.kwarg {
+        all_arguments.push(arg);
+    }
+
+    // Search for duplicates.
+    let mut idents: BTreeSet<&str> = BTreeSet::new();
+    for arg in all_arguments {
+        let ident = &arg.node.arg;
+        if idents.contains(ident.as_str()) {
+            checks.push(Check::new(CheckKind::DuplicateArgumentName, arg.location));
+        }
+        idents.insert(ident);
+    }
+
+    checks
+}
+
+/// Check AssertEquals compliance.
+pub fn check_assert_equals(expr: &Expr, autofix: &fixer::Mode) -> Option<Check> {
+    if let ExprKind::Attribute { value, attr, .. } = &expr.node {
+        if attr == "assertEquals" {
+            if let ExprKind::Name { id, .. } = &value.node {
+                if id == "self" {
+                    let mut check = Check::new(CheckKind::NoAssertEquals, expr.location);
+                    if matches!(autofix, fixer::Mode::Generate)
+                        || matches!(autofix, fixer::Mode::Apply)
+                    {
+                        check.amend(Fix {
+                            content: "assertEqual".to_string(),
+                            start: Location::new(expr.location.row(), expr.location.column() + 1),
+                            end: Location::new(
+                                expr.location.row(),
+                                expr.location.column() + 1 + "assertEquals".len(),
+                            ),
+                            applied: false,
+                        });
+                    }
+                    return Some(check);
+                }
+            }
+        }
+    }
+    None
+}
+
+#[derive(Debug, PartialEq)]
+enum DictionaryKey<'a> {
+    Constant(&'a Constant),
+    Variable(&'a String),
+}
+
+fn convert_to_value(expr: &Expr) -> Option<DictionaryKey> {
+    match &expr.node {
+        ExprKind::Constant { value, .. } => Some(DictionaryKey::Constant(value)),
+        ExprKind::Name { id, .. } => Some(DictionaryKey::Variable(id)),
+        _ => None,
+    }
+}
+
+/// Check MultiValueRepeatedKeyLiteral and MultiValueRepeatedKeyVariable compliance.
+pub fn check_repeated_keys(
+    keys: &Vec<Expr>,
+    check_repeated_literals: bool,
+    check_repeated_variables: bool,
+) -> Vec<Check> {
+    let mut checks: Vec<Check> = vec![];
+
+    let num_keys = keys.len();
+    for i in 0..num_keys {
+        let k1 = &keys[i];
+        let v1 = convert_to_value(k1);
+        for k2 in keys.iter().take(num_keys).skip(i + 1) {
+            let v2 = convert_to_value(k2);
+            match (&v1, &v2) {
+                (Some(DictionaryKey::Constant(v1)), Some(DictionaryKey::Constant(v2))) => {
+                    if check_repeated_literals && v1 == v2 {
+                        checks.push(Check::new(
+                            CheckKind::MultiValueRepeatedKeyLiteral,
+                            k2.location,
+                        ))
+                    }
+                }
+                (Some(DictionaryKey::Variable(v1)), Some(DictionaryKey::Variable(v2))) => {
+                    if check_repeated_variables && v1 == v2 {
+                        checks.push(Check::new(
+                            CheckKind::MultiValueRepeatedKeyVariable(v2.to_string()),
+                            k2.location,
+                        ))
+                    }
+                }
+                _ => {}
+            }
+        }
+    }
+
+    checks
+}
+
+/// Check TrueFalseComparison and NoneComparison compliance.
+pub fn check_literal_comparisons(
+    left: &Expr,
+    ops: &Vec<Cmpop>,
+    comparators: &Vec<Expr>,
+    check_none_comparisons: bool,
+    check_true_false_comparisons: bool,
+) -> Vec<Check> {
+    let mut checks: Vec<Check> = 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!(op, Cmpop::Eq) {
+            checks.push(Check::new(
+                CheckKind::NoneComparison(RejectedCmpop::Eq),
+                comparator.location,
+            ));
+        }
+        if matches!(op, Cmpop::NotEq) {
+            checks.push(Check::new(
+                CheckKind::NoneComparison(RejectedCmpop::NotEq),
+                comparator.location,
+            ));
+        }
+    }
+
+    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),
+                    comparator.location,
+                ));
+            }
+            if matches!(op, Cmpop::NotEq) {
+                checks.push(Check::new(
+                    CheckKind::TrueFalseComparison(value, RejectedCmpop::NotEq),
+                    comparator.location,
+                ));
+            }
+        }
+    }
+
+    // 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),
+                    comparator.location,
+                ));
+            }
+            if matches!(op, Cmpop::NotEq) {
+                checks.push(Check::new(
+                    CheckKind::NoneComparison(RejectedCmpop::NotEq),
+                    comparator.location,
+                ));
+            }
+        }
+
+        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),
+                        comparator.location,
+                    ));
+                }
+                if matches!(op, Cmpop::NotEq) {
+                    checks.push(Check::new(
+                        CheckKind::TrueFalseComparison(value, RejectedCmpop::NotEq),
+                        comparator.location,
+                    ));
+                }
+            }
+        }
+    }
+
+    checks
+}

src/ast/operations.rs

@@ -1,58 +1,6 @@
-use std::collections::BTreeMap;
-use std::sync::atomic::{AtomicUsize, Ordering};
-
 use rustpython_parser::ast::{Constant, Expr, ExprKind, Location, Stmt, StmtKind};
 
-fn id() -> usize {
+use crate::ast::types::{BindingKind, Scope};
-    static COUNTER: AtomicUsize = AtomicUsize::new(1);
-    COUNTER.fetch_add(1, Ordering::Relaxed)
-}
-
-#[derive(Clone, Debug)]
-pub enum ScopeKind {
-    Class,
-    Function,
-    Generator,
-    Module,
-}
-
-#[derive(Clone, Debug)]
-pub struct Scope {
-    pub id: usize,
-    pub kind: ScopeKind,
-    pub values: BTreeMap<String, Binding>,
-}
-
-impl Scope {
-    pub fn new(kind: ScopeKind) -> Self {
-        Scope {
-            id: id(),
-            kind,
-            values: BTreeMap::new(),
-        }
-    }
-}
-
-#[derive(Clone, Debug)]
-pub enum BindingKind {
-    Argument,
-    Assignment,
-    Builtin,
-    ClassDefinition,
-    Definition,
-    Export(Vec<String>),
-    FutureImportation,
-    Importation(String),
-    StarImportation,
-    SubmoduleImportation(String),
-}
-
-#[derive(Clone, Debug)]
-pub struct Binding {
-    pub kind: BindingKind,
-    pub location: Location,
-    pub used: Option<usize>,
-}
 
 /// Extract the names bound to a given __all__ assignment.
 pub fn extract_all_names(stmt: &Stmt, scope: &Scope) -> Vec<String> {

src/ast/types.rs

@@ -0,0 +1,55 @@
+use std::collections::BTreeMap;
+use std::sync::atomic::{AtomicUsize, Ordering};
+
+use rustpython_parser::ast::Location;
+
+fn id() -> usize {
+    static COUNTER: AtomicUsize = AtomicUsize::new(1);
+    COUNTER.fetch_add(1, Ordering::Relaxed)
+}
+
+#[derive(Clone, Debug)]
+pub enum ScopeKind {
+    Class,
+    Function,
+    Generator,
+    Module,
+}
+
+#[derive(Clone, Debug)]
+pub struct Scope {
+    pub id: usize,
+    pub kind: ScopeKind,
+    pub values: BTreeMap<String, Binding>,
+}
+
+impl Scope {
+    pub fn new(kind: ScopeKind) -> Self {
+        Scope {
+            id: id(),
+            kind,
+            values: BTreeMap::new(),
+        }
+    }
+}
+
+#[derive(Clone, Debug)]
+pub enum BindingKind {
+    Argument,
+    Assignment,
+    Builtin,
+    ClassDefinition,
+    Definition,
+    Export(Vec<String>),
+    FutureImportation,
+    Importation(String),
+    StarImportation,
+    SubmoduleImportation(String),
+}
+
+#[derive(Clone, Debug)]
+pub struct Binding {
+    pub kind: BindingKind,
+    pub location: Location,
+    pub used: Option<usize>,
+}

src/autofix.rs

@@ -1,216 +1,2 @@
-use std::fs;
+pub mod fixer;
-use std::path::Path;
+pub mod fixes;
-
-use anyhow::Result;
-use rustpython_parser::ast::Location;
-
-use crate::checks::{Check, Fix};
-
-#[derive(Hash)]
-pub enum Mode {
-    Generate,
-    Apply,
-    None,
-}
-
-impl From<bool> for Mode {
-    fn from(value: bool) -> Self {
-        match value {
-            true => Mode::Apply,
-            false => Mode::None,
-        }
-    }
-}
-
-/// Auto-fix errors in a file, and write the fixed source code to disk.
-pub fn fix_file(checks: &mut [Check], contents: &str, path: &Path) -> Result<()> {
-    if checks.iter().all(|check| check.fix.is_none()) {
-        return Ok(());
-    }
-
-    let output = apply_fixes(
-        checks.iter_mut().filter_map(|check| check.fix.as_mut()),
-        contents,
-    );
-
-    fs::write(path, output).map_err(|e| e.into())
-}
-
-/// Apply a series of fixes.
-fn apply_fixes<'a>(fixes: impl Iterator<Item = &'a mut Fix>, contents: &str) -> String {
-    let lines: Vec<&str> = contents.lines().collect();
-
-    let mut output = "".to_string();
-    let mut last_pos: Location = Location::new(0, 0);
-
-    for fix in fixes {
-        // Best-effort approach: if this fix overlaps with a fix we've already applied, skip it.
-        if last_pos > fix.start {
-            continue;
-        }
-
-        if fix.start.row() > last_pos.row() {
-            if last_pos.row() > 0 || last_pos.column() > 0 {
-                output.push_str(&lines[last_pos.row() - 1][last_pos.column() - 1..]);
-                output.push('\n');
-            }
-            for line in &lines[last_pos.row()..fix.start.row() - 1] {
-                output.push_str(line);
-                output.push('\n');
-            }
-            output.push_str(&lines[fix.start.row() - 1][..fix.start.column() - 1]);
-            output.push_str(&fix.content);
-        } else {
-            output.push_str(
-                &lines[last_pos.row() - 1][last_pos.column() - 1..fix.start.column() - 1],
-            );
-            output.push_str(&fix.content);
-        }
-
-        last_pos = fix.end;
-        fix.applied = true;
-    }
-
-    if last_pos.row() > 0 || last_pos.column() > 0 {
-        output.push_str(&lines[last_pos.row() - 1][last_pos.column() - 1..]);
-        output.push('\n');
-    }
-    for line in &lines[last_pos.row()..] {
-        output.push_str(line);
-        output.push('\n');
-    }
-
-    output
-}
-
-#[cfg(test)]
-mod tests {
-    use anyhow::Result;
-    use rustpython_parser::ast::Location;
-
-    use crate::autofix::apply_fixes;
-    use crate::checks::Fix;
-
-    #[test]
-    fn empty_file() -> Result<()> {
-        let mut fixes = vec![];
-        let actual = apply_fixes(fixes.iter_mut(), "");
-        let expected = "";
-
-        assert_eq!(actual, expected);
-
-        Ok(())
-    }
-
-    #[test]
-    fn apply_single_replacement() -> Result<()> {
-        let mut fixes = vec![Fix {
-            content: "Bar".to_string(),
-            start: Location::new(1, 9),
-            end: Location::new(1, 15),
-            applied: false,
-        }];
-        let actual = apply_fixes(
-            fixes.iter_mut(),
-            "class A(object):
-        ...
-",
-        );
-
-        let expected = "class A(Bar):
-        ...
-";
-
-        assert_eq!(actual, expected);
-
-        Ok(())
-    }
-
-    #[test]
-    fn apply_single_removal() -> Result<()> {
-        let mut fixes = vec![Fix {
-            content: "".to_string(),
-            start: Location::new(1, 8),
-            end: Location::new(1, 16),
-            applied: false,
-        }];
-        let actual = apply_fixes(
-            fixes.iter_mut(),
-            "class A(object):
-        ...
-",
-        );
-
-        let expected = "class A:
-        ...
-";
-
-        assert_eq!(actual, expected);
-
-        Ok(())
-    }
-
-    #[test]
-    fn apply_double_removal() -> Result<()> {
-        let mut fixes = vec![
-            Fix {
-                content: "".to_string(),
-                start: Location::new(1, 8),
-                end: Location::new(1, 17),
-                applied: false,
-            },
-            Fix {
-                content: "".to_string(),
-                start: Location::new(1, 17),
-                end: Location::new(1, 24),
-                applied: false,
-            },
-        ];
-        let actual = apply_fixes(
-            fixes.iter_mut(),
-            "class A(object, object):
-        ...
-",
-        );
-
-        let expected = "class A:
-        ...
-";
-
-        assert_eq!(actual, expected);
-
-        Ok(())
-    }
-
-    #[test]
-    fn ignore_overlapping_fixes() -> Result<()> {
-        let mut fixes = vec![
-            Fix {
-                content: "".to_string(),
-                start: Location::new(1, 8),
-                end: Location::new(1, 16),
-                applied: false,
-            },
-            Fix {
-                content: "ignored".to_string(),
-                start: Location::new(1, 10),
-                end: Location::new(1, 12),
-                applied: false,
-            },
-        ];
-        let actual = apply_fixes(
-            fixes.iter_mut(),
-            "class A(object):
-    ...
-",
-        );
-
-        let expected = "class A:
-    ...
-";
-
-        assert_eq!(actual, expected);
-
-        Ok(())
-    }
-}

src/autofix/fixer.rs

@@ -0,0 +1,216 @@
+use std::fs;
+use std::path::Path;
+
+use anyhow::Result;
+use rustpython_parser::ast::Location;
+
+use crate::checks::{Check, Fix};
+
+#[derive(Hash)]
+pub enum Mode {
+    Generate,
+    Apply,
+    None,
+}
+
+impl From<bool> for Mode {
+    fn from(value: bool) -> Self {
+        match value {
+            true => Mode::Apply,
+            false => Mode::None,
+        }
+    }
+}
+
+/// Auto-fix errors in a file, and write the fixed source code to disk.
+pub fn fix_file(checks: &mut [Check], contents: &str, path: &Path) -> Result<()> {
+    if checks.iter().all(|check| check.fix.is_none()) {
+        return Ok(());
+    }
+
+    let output = apply_fixes(
+        checks.iter_mut().filter_map(|check| check.fix.as_mut()),
+        contents,
+    );
+
+    fs::write(path, output).map_err(|e| e.into())
+}
+
+/// Apply a series of fixes.
+fn apply_fixes<'a>(fixes: impl Iterator<Item = &'a mut Fix>, contents: &str) -> String {
+    let lines: Vec<&str> = contents.lines().collect();
+
+    let mut output = "".to_string();
+    let mut last_pos: Location = Location::new(0, 0);
+
+    for fix in fixes {
+        // Best-effort approach: if this fix overlaps with a fix we've already applied, skip it.
+        if last_pos > fix.start {
+            continue;
+        }
+
+        if fix.start.row() > last_pos.row() {
+            if last_pos.row() > 0 || last_pos.column() > 0 {
+                output.push_str(&lines[last_pos.row() - 1][last_pos.column() - 1..]);
+                output.push('\n');
+            }
+            for line in &lines[last_pos.row()..fix.start.row() - 1] {
+                output.push_str(line);
+                output.push('\n');
+            }
+            output.push_str(&lines[fix.start.row() - 1][..fix.start.column() - 1]);
+            output.push_str(&fix.content);
+        } else {
+            output.push_str(
+                &lines[last_pos.row() - 1][last_pos.column() - 1..fix.start.column() - 1],
+            );
+            output.push_str(&fix.content);
+        }
+
+        last_pos = fix.end;
+        fix.applied = true;
+    }
+
+    if last_pos.row() > 0 || last_pos.column() > 0 {
+        output.push_str(&lines[last_pos.row() - 1][last_pos.column() - 1..]);
+        output.push('\n');
+    }
+    for line in &lines[last_pos.row()..] {
+        output.push_str(line);
+        output.push('\n');
+    }
+
+    output
+}
+
+#[cfg(test)]
+mod tests {
+    use anyhow::Result;
+    use rustpython_parser::ast::Location;
+
+    use crate::autofix::fixer::apply_fixes;
+    use crate::checks::Fix;
+
+    #[test]
+    fn empty_file() -> Result<()> {
+        let mut fixes = vec![];
+        let actual = apply_fixes(fixes.iter_mut(), "");
+        let expected = "";
+
+        assert_eq!(actual, expected);
+
+        Ok(())
+    }
+
+    #[test]
+    fn apply_single_replacement() -> Result<()> {
+        let mut fixes = vec![Fix {
+            content: "Bar".to_string(),
+            start: Location::new(1, 9),
+            end: Location::new(1, 15),
+            applied: false,
+        }];
+        let actual = apply_fixes(
+            fixes.iter_mut(),
+            "class A(object):
+        ...
+",
+        );
+
+        let expected = "class A(Bar):
+        ...
+";
+
+        assert_eq!(actual, expected);
+
+        Ok(())
+    }
+
+    #[test]
+    fn apply_single_removal() -> Result<()> {
+        let mut fixes = vec![Fix {
+            content: "".to_string(),
+            start: Location::new(1, 8),
+            end: Location::new(1, 16),
+            applied: false,
+        }];
+        let actual = apply_fixes(
+            fixes.iter_mut(),
+            "class A(object):
+        ...
+",
+        );
+
+        let expected = "class A:
+        ...
+";
+
+        assert_eq!(actual, expected);
+
+        Ok(())
+    }
+
+    #[test]
+    fn apply_double_removal() -> Result<()> {
+        let mut fixes = vec![
+            Fix {
+                content: "".to_string(),
+                start: Location::new(1, 8),
+                end: Location::new(1, 17),
+                applied: false,
+            },
+            Fix {
+                content: "".to_string(),
+                start: Location::new(1, 17),
+                end: Location::new(1, 24),
+                applied: false,
+            },
+        ];
+        let actual = apply_fixes(
+            fixes.iter_mut(),
+            "class A(object, object):
+        ...
+",
+        );
+
+        let expected = "class A:
+        ...
+";
+
+        assert_eq!(actual, expected);
+
+        Ok(())
+    }
+
+    #[test]
+    fn ignore_overlapping_fixes() -> Result<()> {
+        let mut fixes = vec![
+            Fix {
+                content: "".to_string(),
+                start: Location::new(1, 8),
+                end: Location::new(1, 16),
+                applied: false,
+            },
+            Fix {
+                content: "ignored".to_string(),
+                start: Location::new(1, 10),
+                end: Location::new(1, 12),
+                applied: false,
+            },
+        ];
+        let actual = apply_fixes(
+            fixes.iter_mut(),
+            "class A(object):
+    ...
+",
+        );
+
+        let expected = "class A:
+    ...
+";
+
+        assert_eq!(actual, expected);
+
+        Ok(())
+    }
+}

src/autofix/fixes.rs

@@ -2,7 +2,7 @@ use rustpython_parser::ast::{Expr, Keyword, Location};
 use rustpython_parser::lexer;
 use rustpython_parser::token::Tok;
 
-use crate::ast_ops::SourceCodeLocator;
+use crate::ast::operations::SourceCodeLocator;
 use crate::checks::Fix;
 
 /// Convert a location within a file (relative to `base`) to an absolute position.

src/cache.rs

@@ -8,7 +8,7 @@ use filetime::FileTime;
 use log::error;
 use serde::{Deserialize, Serialize};
 
-use crate::autofix;
+use crate::autofix::fixer;
 use crate::message::Message;
 use crate::settings::Settings;
 
@@ -71,7 +71,7 @@ fn cache_dir() -> &'static str {
     "./.ruff_cache"
 }
 
-fn cache_key(path: &Path, settings: &Settings, autofix: &autofix::Mode) -> String {
+fn cache_key(path: &Path, settings: &Settings, autofix: &fixer::Mode) -> String {
     let mut hasher = DefaultHasher::new();
     settings.hash(&mut hasher);
     autofix.hash(&mut hasher);
@@ -87,7 +87,7 @@ pub fn get(
     path: &Path,
     metadata: &Metadata,
     settings: &Settings,
-    autofix: &autofix::Mode,
+    autofix: &fixer::Mode,
     mode: &Mode,
 ) -> Option<Vec<Message>> {
     if !mode.allow_read() {
@@ -116,7 +116,7 @@ pub fn set(
     path: &Path,
     metadata: &Metadata,
     settings: &Settings,
-    autofix: &autofix::Mode,
+    autofix: &fixer::Mode,
     messages: &[Message],
     mode: &Mode,
 ) {

src/check_ast.rs

@@ -1,22 +1,20 @@
-use std::collections::BTreeSet;
 use std::path::Path;
 
-use itertools::izip;
 use rustpython_parser::ast::{
-    Arg, Arguments, Cmpop, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind,
+    Arg, Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind,
-    KeywordData, Location, Stmt, StmtKind, Suite, Unaryop,
+    KeywordData, Location, Stmt, StmtKind, Suite,
 };
 use rustpython_parser::parser;
 
-use crate::ast_ops::{
+use crate::ast::operations::{extract_all_names, SourceCodeLocator};
-    extract_all_names, Binding, BindingKind, Scope, ScopeKind, SourceCodeLocator,
+use crate::ast::relocate::relocate_expr;
-};
+use crate::ast::types::{Binding, BindingKind, Scope, ScopeKind};
+use crate::ast::visitor::{walk_excepthandler, Visitor};
+use crate::ast::{checks, visitor};
+use crate::autofix::fixer;
 use crate::builtins::{BUILTINS, MAGIC_GLOBALS};
-use crate::checks::{Check, CheckCode, CheckKind, Fix, RejectedCmpop};
+use crate::checks::{Check, CheckCode, CheckKind};
-use crate::relocator::relocate_expr;
 use crate::settings::Settings;
-use crate::visitor::{walk_excepthandler, Visitor};
-use crate::{autofix, fixer, visitor};
 
 pub const GLOBAL_SCOPE_INDEX: usize = 0;
 
@@ -24,7 +22,7 @@ struct Checker<'a> {
     // Input data.
     locator: SourceCodeLocator<'a>,
     settings: &'a Settings,
-    autofix: &'a autofix::Mode,
+    autofix: &'a fixer::Mode,
     path: &'a str,
     // Computed checks.
     checks: Vec<Check>,
@@ -49,7 +47,7 @@ struct Checker<'a> {
 impl<'a> Checker<'a> {
     pub fn new(
         settings: &'a Settings,
-        autofix: &'a autofix::Mode,
+        autofix: &'a fixer::Mode,
         path: &'a str,
         content: &'a str,
     ) -> Checker<'a> {
@@ -76,20 +74,6 @@ impl<'a> Checker<'a> {
     }
 }
 
-#[derive(Debug, PartialEq)]
-enum DictionaryKey<'a> {
-    Constant(&'a Constant),
-    Variable(&'a String),
-}
-
-fn convert_to_value(expr: &Expr) -> Option<DictionaryKey> {
-    match &expr.node {
-        ExprKind::Constant { value, .. } => Some(DictionaryKey::Constant(value)),
-        ExprKind::Name { id, .. } => Some(DictionaryKey::Variable(id)),
-        _ => None,
-    }
-}
-
 fn match_name_or_attr(expr: &Expr, target: &str) -> bool {
     match &expr.node {
         ExprKind::Attribute { attr, .. } => target == attr,
@@ -183,42 +167,19 @@ where
                 ..
             } => {
                 if self.settings.select.contains(&CheckCode::R001) {
-                    for expr in bases {
+                    let scope =
-                        if let ExprKind::Name { id, .. } = &expr.node {
+                        &self.scopes[*(self.scope_stack.last().expect("No current scope found."))];
-                            if id == "object" {
+                    if let Some(check) = checks::check_useless_object_inheritance(
-                                let scope = &self.scopes
+                        stmt,
-                                    [*(self.scope_stack.last().expect("No current scope found."))];
+                        name,
-                                match scope.values.get(id) {
-                                    None
-                                    | Some(Binding {
-                                        kind: BindingKind::Builtin,
-                                        ..
-                                    }) => {
-                                        let mut check = Check::new(
-                                            CheckKind::UselessObjectInheritance(name.to_string()),
-                                            expr.location,
-                                        );
-                                        if matches!(self.autofix, autofix::Mode::Generate)
-                                            || matches!(self.autofix, autofix::Mode::Apply)
-                                        {
-                                            if let Some(fix) = fixer::remove_class_def_base(
-                                                &mut self.locator,
-                                                &stmt.location,
-                                                expr.location,
                         bases,
                         keywords,
+                        scope,
+                        &mut self.locator,
+                        self.autofix,
                     ) {
-                                                check.amend(fix);
-                                            }
-                                        } else {
-                                        }
                         self.checks.push(check);
                     }
-                                    _ => {}
-                                }
-                            }
-                        }
-                    }
                 }
 
                 for expr in bases {
@@ -327,11 +288,7 @@ where
                             },
                         );
 
-                        if self
+                        if self.settings.select.contains(&CheckCode::F403) {
-                            .settings
-                            .select
-                            .contains(CheckKind::ImportStarUsage.code())
-                        {
                             self.checks
                                 .push(Check::new(CheckKind::ImportStarUsage, stmt.location));
                         }
@@ -348,43 +305,11 @@ where
                     }
                 }
             }
-            StmtKind::If { test, .. } => {
-                if self.settings.select.contains(CheckKind::IfTuple.code()) {
-                    if let ExprKind::Tuple { elts, .. } = &test.node {
-                        if !elts.is_empty() {
-                            self.checks
-                                .push(Check::new(CheckKind::IfTuple, stmt.location));
-                        }
-                    }
-                }
-            }
             StmtKind::Raise { exc, .. } => {
-                if self
+                if self.settings.select.contains(&CheckCode::F901) {
-                    .settings
-                    .select
-                    .contains(CheckKind::RaiseNotImplemented.code())
-                {
                     if let Some(expr) = exc {
-                        match &expr.node {
+                        if let Some(check) = checks::check_raise_not_implemented(expr) {
-                            ExprKind::Call { func, .. } => {
+                            self.checks.push(check);
-                                if let ExprKind::Name { id, .. } = &func.node {
-                                    if id == "NotImplemented" {
-                                        self.checks.push(Check::new(
-                                            CheckKind::RaiseNotImplemented,
-                                            stmt.location,
-                                        ));
-                                    }
-                                }
-                            }
-                            ExprKind::Name { id, .. } => {
-                                if id == "NotImplemented" {
-                                    self.checks.push(Check::new(
-                                        CheckKind::RaiseNotImplemented,
-                                        stmt.location,
-                                    ));
-                                }
-                            }
-                            _ => {}
                         }
                     }
                 }
@@ -393,31 +318,25 @@ where
                 self.seen_non_import = true;
                 self.handle_node_load(target);
             }
+            StmtKind::If { test, .. } => {
+                if self.settings.select.contains(&CheckCode::F634) {
+                    if let Some(check) = checks::check_if_tuple(test, stmt.location) {
+                        self.checks.push(check);
+                    }
+                }
+            }
             StmtKind::Assert { test, .. } => {
                 self.seen_non_import = true;
                 if self.settings.select.contains(CheckKind::AssertTuple.code()) {
-                    if let ExprKind::Tuple { elts, .. } = &test.node {
+                    if let Some(check) = checks::check_assert_tuple(test, stmt.location) {
-                        if !elts.is_empty() {
+                        self.checks.push(check);
-                            self.checks
-                                .push(Check::new(CheckKind::AssertTuple, stmt.location));
-                        }
                     }
                 }
             }
             StmtKind::Try { handlers, .. } => {
-                if self
+                if self.settings.select.contains(&CheckCode::F707) {
-                    .settings
+                    if let Some(check) = checks::check_default_except_not_last(handlers) {
-                    .select
+                        self.checks.push(check);
-                    .contains(CheckKind::DefaultExceptNotLast.code())
-                {
-                    for (idx, handler) in handlers.iter().enumerate() {
-                        let ExcepthandlerKind::ExceptHandler { type_, .. } = &handler.node;
-                        if type_.is_none() && idx < handlers.len() - 1 {
-                            self.checks.push(Check::new(
-                                CheckKind::DefaultExceptNotLast,
-                                handler.location,
-                            ));
-                        }
                     }
                 }
             }
@@ -436,28 +355,20 @@ where
             }
             StmtKind::Assign { value, .. } => {
                 self.seen_non_import = true;
-                if self
+                if self.settings.select.contains(&CheckCode::E731) {
-                    .settings
+                    if let Some(check) = checks::check_do_not_assign_lambda(value, stmt.location) {
-                    .select
+                        self.checks.push(check);
-                    .contains(CheckKind::DoNotAssignLambda.code())
-                {
-                    if let ExprKind::Lambda { .. } = &value.node {
-                        self.checks
-                            .push(Check::new(CheckKind::DoNotAssignLambda, stmt.location));
                     }
                 }
             }
             StmtKind::AnnAssign { value, .. } => {
                 self.seen_non_import = true;
-                if self
+                if self.settings.select.contains(&CheckCode::E731) {
-                    .settings
+                    if let Some(value) = value {
-                    .select
+                        if let Some(check) =
-                    .contains(CheckKind::DoNotAssignLambda.code())
+                            checks::check_do_not_assign_lambda(value, stmt.location)
                         {
-                    if let Some(v) = value {
+                            self.checks.push(check);
-                        if let ExprKind::Lambda { .. } = v.node {
-                            self.checks
-                                .push(Check::new(CheckKind::DoNotAssignLambda, stmt.location));
                         }
                     }
                 }
@@ -530,80 +441,22 @@ where
             },
             ExprKind::Call { func, .. } => {
                 if self.settings.select.contains(&CheckCode::R002) {
-                    if let ExprKind::Attribute { value, attr, .. } = &func.node {
+                    if let Some(check) = checks::check_assert_equals(func, self.autofix) {
-                        if attr == "assertEquals" {
+                        self.checks.push(check)
-                            if let ExprKind::Name { id, .. } = &value.node {
-                                if id == "self" {
-                                    let mut check =
-                                        Check::new(CheckKind::NoAssertEquals, expr.location);
-                                    if matches!(self.autofix, autofix::Mode::Generate)
-                                        || matches!(self.autofix, autofix::Mode::Apply)
-                                    {
-                                        check.amend(Fix {
-                                            content: "assertEqual".to_string(),
-                                            start: Location::new(
-                                                func.location.row(),
-                                                func.location.column() + 1,
-                                            ),
-                                            end: Location::new(
-                                                func.location.row(),
-                                                func.location.column() + 1 + "assertEquals".len(),
-                                            ),
-                                            applied: false,
-                                        });
-                                    }
-                                    self.checks.push(check);
-                                }
-                            }
-                        }
                     }
                 }
             }
             ExprKind::Dict { keys, .. } => {
-                if self.settings.select.contains(&CheckCode::F601)
+                let check_repeated_literals = self.settings.select.contains(&CheckCode::F601);
-                    || self.settings.select.contains(&CheckCode::F602)
+                let check_repeated_variables = self.settings.select.contains(&CheckCode::F602);
-                {
+                if check_repeated_literals || check_repeated_variables {
-                    let num_keys = keys.len();
+                    self.checks.extend(checks::check_repeated_keys(
-                    for i in 0..num_keys {
+                        keys,
-                        let k1 = &keys[i];
+                        check_repeated_literals,
-                        let v1 = convert_to_value(k1);
+                        check_repeated_variables,
-                        for k2 in keys.iter().take(num_keys).skip(i + 1) {
+                    ));
-                            let v2 = convert_to_value(k2);
-                            match (&v1, &v2) {
-                                (
-                                    Some(DictionaryKey::Constant(v1)),
-                                    Some(DictionaryKey::Constant(v2)),
-                                ) => {
-                                    if self.settings.select.contains(&CheckCode::F601) && v1 == v2 {
-                                        self.checks.push(Check::new(
-                                            CheckKind::MultiValueRepeatedKeyLiteral,
-                                            k2.location,
-                                        ))
                 }
             }
-                                (
-                                    Some(DictionaryKey::Variable(v1)),
-                                    Some(DictionaryKey::Variable(v2)),
-                                ) => {
-                                    if self.settings.select.contains(&CheckCode::F602) && v1 == v2 {
-                                        self.checks.push(Check::new(
-                                            CheckKind::MultiValueRepeatedKeyVariable(
-                                                v2.to_string(),
-                                            ),
-                                            k2.location,
-                                        ))
-                                    }
-                                }
-                                _ => {}
-                            }
-                        }
-                    }
-                }
-            }
-            ExprKind::GeneratorExp { .. }
-            | ExprKind::ListComp { .. }
-            | ExprKind::DictComp { .. }
-            | ExprKind::SetComp { .. } => self.push_scope(Scope::new(ScopeKind::Generator)),
             ExprKind::Yield { .. } | ExprKind::YieldFrom { .. } => {
                 let scope =
                     &self.scopes[*(self.scope_stack.last().expect("No current scope found."))];
@@ -636,24 +489,15 @@ where
                 self.in_f_string = true;
             }
             ExprKind::UnaryOp { op, operand } => {
-                if matches!(op, Unaryop::Not) {
+                let check_not_in = self.settings.select.contains(&CheckCode::E713);
-                    if let ExprKind::Compare { ops, .. } = &operand.node {
+                let check_not_is = self.settings.select.contains(&CheckCode::E714);
-                        match ops[..] {
+                if check_not_in || check_not_is {
-                            [Cmpop::In] => {
+                    self.checks.extend(checks::check_not_tests(
-                                if self.settings.select.contains(CheckKind::NotInTest.code()) {
+                        op,
-                                    self.checks
+                        operand,
-                                        .push(Check::new(CheckKind::NotInTest, operand.location));
+                        check_not_in,
-                                }
+                        check_not_is,
-                            }
+                    ));
-                            [Cmpop::Is] => {
-                                if self.settings.select.contains(CheckKind::NotIsTest.code()) {
-                                    self.checks
-                                        .push(Check::new(CheckKind::NotIsTest, operand.location));
-                                }
-                            }
-                            _ => {}
-                        }
-                    }
                 }
             }
             ExprKind::Compare {
@@ -661,100 +505,17 @@ where
                 ops,
                 comparators,
             } => {
-                let op = ops.first().unwrap();
+                let check_none_comparisons = self.settings.select.contains(&CheckCode::E711);
-                let comparator = left;
+                let check_true_false_comparisons = self.settings.select.contains(&CheckCode::E712);
-
+                if check_none_comparisons || check_true_false_comparisons {
-                // Check `left`.
+                    self.checks.extend(checks::check_literal_comparisons(
-                if self.settings.select.contains(&CheckCode::E711)
+                        left,
-                    && matches!(
+                        ops,
-                        comparator.node,
+                        comparators,
-                        ExprKind::Constant {
+                        check_none_comparisons,
-                            value: Constant::None,
+                        check_true_false_comparisons,
-                            kind: None
-                        }
-                    )
-                {
-                    if matches!(op, Cmpop::Eq) {
-                        self.checks.push(Check::new(
-                            CheckKind::NoneComparison(RejectedCmpop::Eq),
-                            comparator.location,
                     ));
                 }
-                    if matches!(op, Cmpop::NotEq) {
-                        self.checks.push(Check::new(
-                            CheckKind::NoneComparison(RejectedCmpop::NotEq),
-                            comparator.location,
-                        ));
-                    }
-                }
-
-                if self.settings.select.contains(&CheckCode::E712) {
-                    if let ExprKind::Constant {
-                        value: Constant::Bool(value),
-                        kind: None,
-                    } = comparator.node
-                    {
-                        if matches!(op, Cmpop::Eq) {
-                            self.checks.push(Check::new(
-                                CheckKind::TrueFalseComparison(value, RejectedCmpop::Eq),
-                                comparator.location,
-                            ));
-                        }
-                        if matches!(op, Cmpop::NotEq) {
-                            self.checks.push(Check::new(
-                                CheckKind::TrueFalseComparison(value, RejectedCmpop::NotEq),
-                                comparator.location,
-                            ));
-                        }
-                    }
-                }
-
-                // Check each comparator in order.
-                for (op, comparator) in izip!(ops, comparators) {
-                    if self.settings.select.contains(&CheckCode::E711)
-                        && matches!(
-                            comparator.node,
-                            ExprKind::Constant {
-                                value: Constant::None,
-                                kind: None
-                            }
-                        )
-                    {
-                        if matches!(op, Cmpop::Eq) {
-                            self.checks.push(Check::new(
-                                CheckKind::NoneComparison(RejectedCmpop::Eq),
-                                comparator.location,
-                            ));
-                        }
-                        if matches!(op, Cmpop::NotEq) {
-                            self.checks.push(Check::new(
-                                CheckKind::NoneComparison(RejectedCmpop::NotEq),
-                                comparator.location,
-                            ));
-                        }
-                    }
-
-                    if self.settings.select.contains(&CheckCode::E712) {
-                        if let ExprKind::Constant {
-                            value: Constant::Bool(value),
-                            kind: None,
-                        } = comparator.node
-                        {
-                            if matches!(op, Cmpop::Eq) {
-                                self.checks.push(Check::new(
-                                    CheckKind::TrueFalseComparison(value, RejectedCmpop::Eq),
-                                    comparator.location,
-                                ));
-                            }
-                            if matches!(op, Cmpop::NotEq) {
-                                self.checks.push(Check::new(
-                                    CheckKind::TrueFalseComparison(value, RejectedCmpop::NotEq),
-                                    comparator.location,
-                                ));
-                            }
-                        }
-                    }
-                }
             }
             ExprKind::Constant {
                 value: Constant::Str(value),
@@ -762,6 +523,10 @@ where
             } if self.in_annotation && !self.in_literal => {
                 self.deferred_annotations.push((expr.location, value));
             }
+            ExprKind::GeneratorExp { .. }
+            | ExprKind::ListComp { .. }
+            | ExprKind::DictComp { .. }
+            | ExprKind::SetComp { .. } => self.push_scope(Scope::new(ScopeKind::Generator)),
             _ => {}
         };
 
@@ -887,38 +652,10 @@ where
     }
 
     fn visit_arguments(&mut self, arguments: &'b Arguments) {
-        if self
+        if self.settings.select.contains(&CheckCode::F831) {
-            .settings
-            .select
-            .contains(CheckKind::DuplicateArgumentName.code())
-        {
-            // Collect all the arguments into a single vector.
-            let mut all_arguments: Vec<&Arg> = arguments
-                .args
-                .iter()
-                .chain(arguments.posonlyargs.iter())
-                .chain(arguments.kwonlyargs.iter())
-                .collect();
-            if let Some(arg) = &arguments.vararg {
-                all_arguments.push(arg);
-            }
-            if let Some(arg) = &arguments.kwarg {
-                all_arguments.push(arg);
-            }
-
-            // Search for duplicates.
-            let mut idents: BTreeSet<&str> = BTreeSet::new();
-            for arg in all_arguments {
-                let ident = &arg.node.arg;
-                if idents.contains(ident.as_str()) {
             self.checks
-                        .push(Check::new(CheckKind::DuplicateArgumentName, arg.location));
+                .extend(checks::check_duplicate_arguments(arguments));
-                    break;
         }
-                idents.insert(ident);
-            }
-        }
-
         visitor::walk_arguments(self, arguments);
     }
 
@@ -1146,21 +883,8 @@ impl<'a> Checker<'a> {
             }
 
             let scope = &self.scopes[*(self.scope_stack.last().expect("No current scope found."))];
-            for (name, binding) in scope.values.iter() {
+            if self.settings.select.contains(&CheckCode::F841) {
-                // TODO(charlie): Ignore if using `locals`.
+                self.checks.extend(checks::check_unused_variables(scope));
-                if self.settings.select.contains(&CheckCode::F841)
-                    && binding.used.is_none()
-                    && name != "_"
-                    && name != "__tracebackhide__"
-                    && name != "__traceback_info__"
-                    && name != "__traceback_supplement__"
-                    && matches!(binding.kind, BindingKind::Assignment)
-                {
-                    self.checks.push(Check::new(
-                        CheckKind::UnusedVariable(name.to_string()),
-                        binding.location,
-                    ));
-                }
             }
 
             self.pop_scope();
@@ -1181,21 +905,8 @@ impl<'a> Checker<'a> {
             }
 
             let scope = &self.scopes[*(self.scope_stack.last().expect("No current scope found."))];
-            for (name, binding) in scope.values.iter() {
+            if self.settings.select.contains(&CheckCode::F841) {
-                // TODO(charlie): Ignore if using `locals`.
+                self.checks.extend(checks::check_unused_variables(scope));
-                if self.settings.select.contains(&CheckCode::F841)
-                    && binding.used.is_none()
-                    && name != "_"
-                    && name != "__tracebackhide__"
-                    && name != "__traceback_info__"
-                    && name != "__traceback_supplement__"
-                    && matches!(binding.kind, BindingKind::Assignment)
-                {
-                    self.checks.push(Check::new(
-                        CheckKind::UnusedVariable(name.to_string()),
-                        binding.location,
-                    ));
-                }
             }
 
             self.pop_scope();
@@ -1264,7 +975,7 @@ pub fn check_ast(
     python_ast: &Suite,
     content: &str,
     settings: &Settings,
-    autofix: &autofix::Mode,
+    autofix: &fixer::Mode,
     path: &str,
 ) -> Vec<Check> {
     let mut checker = Checker::new(settings, autofix, path, content);

src/lib.rs

@@ -1,18 +1,15 @@
 extern crate core;
 
-mod ast_ops;
+mod ast;
 mod autofix;
 mod builtins;
 mod cache;
 pub mod check_ast;
 mod check_lines;
 pub mod checks;
-mod fixer;
 pub mod fs;
 pub mod linter;
 pub mod logging;
 pub mod message;
 mod pyproject;
-mod relocator;
 pub mod settings;
-mod visitor;

src/linter.rs

@@ -4,15 +4,16 @@ use anyhow::Result;
 use log::debug;
 use rustpython_parser::parser;
 
-use crate::autofix::fix_file;
+use crate::autofix::fixer;
+use crate::autofix::fixer::fix_file;
 use crate::check_ast::check_ast;
 use crate::check_lines::check_lines;
 use crate::checks::{Check, LintSource};
 use crate::message::Message;
 use crate::settings::Settings;
-use crate::{autofix, cache, fs};
+use crate::{cache, fs};
 
-fn check_path(path: &Path, settings: &Settings, autofix: &autofix::Mode) -> Result<Vec<Check>> {
+fn check_path(path: &Path, settings: &Settings, autofix: &fixer::Mode) -> Result<Vec<Check>> {
     // Read the file from disk.
     let contents = fs::read_file(path)?;
 
@@ -40,7 +41,7 @@ pub fn lint_path(
     path: &Path,
     settings: &Settings,
     mode: &cache::Mode,
-    autofix: &autofix::Mode,
+    autofix: &fixer::Mode,
 ) -> Result<Vec<Message>> {
     let metadata = path.metadata()?;
 
@@ -57,7 +58,7 @@ pub fn lint_path(
     let mut checks = check_path(path, settings, autofix)?;
 
     // Apply autofix.
-    if matches!(autofix, autofix::Mode::Apply) {
+    if matches!(autofix, fixer::Mode::Apply) {
         fix_file(&mut checks, &contents, path)?;
     };
 
@@ -84,9 +85,10 @@ mod tests {
     use anyhow::Result;
     use rustpython_parser::ast::Location;
 
+    use crate::autofix::fixer;
     use crate::checks::{Check, CheckCode, CheckKind, Fix, RejectedCmpop};
     use crate::linter::check_path;
-    use crate::{autofix, settings};
+    use crate::settings;
 
     #[test]
     fn e402() -> Result<()> {
@@ -97,7 +99,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::E402]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![Check {
@@ -122,7 +124,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::E501]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![Check {
@@ -147,7 +149,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::E711]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![
@@ -179,7 +181,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::E712]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![
@@ -222,7 +224,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::E713]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![Check {
@@ -247,7 +249,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::E714]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![Check {
@@ -272,7 +274,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::E731]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![
@@ -305,7 +307,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::F401]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![
@@ -342,7 +344,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::F403]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![
@@ -373,7 +375,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::F541]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![
@@ -410,7 +412,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::F601]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         let expected = vec![
             Check {
@@ -446,7 +448,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::F602]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         let expected = vec![Check {
             kind: CheckKind::MultiValueRepeatedKeyVariable("a".to_string()),
@@ -470,7 +472,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::F631]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![
@@ -502,7 +504,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::F634]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![
@@ -534,7 +536,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::F704]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![
@@ -571,7 +573,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::F706]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![
@@ -603,7 +605,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::F707]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![
@@ -640,7 +642,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::F821]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![
@@ -687,7 +689,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::F822]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![Check {
@@ -712,7 +714,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::F823]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![Check {
@@ -737,7 +739,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::F831]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![
@@ -774,7 +776,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::F841]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![
@@ -806,18 +808,18 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::F901]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![
             Check {
                 kind: CheckKind::RaiseNotImplemented,
-                location: Location::new(2, 5),
+                location: Location::new(2, 25),
                 fix: None,
             },
             Check {
                 kind: CheckKind::RaiseNotImplemented,
-                location: Location::new(6, 5),
+                location: Location::new(6, 11),
                 fix: None,
             },
         ];
@@ -838,7 +840,7 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::R001]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![
@@ -1060,13 +1062,13 @@ mod tests {
                 exclude: vec![],
                 select: BTreeSet::from([CheckCode::R002]),
             },
-            &autofix::Mode::Generate,
+            &fixer::Mode::Generate,
         )?;
         actual.sort_by_key(|check| check.location);
         let expected = vec![
             Check {
                 kind: CheckKind::NoAssertEquals,
-                location: Location::new(1, 19),
+                location: Location::new(1, 5),
                 fix: Some(Fix {
                     content: "assertEqual".to_string(),
                     start: Location::new(1, 6),
@@ -1076,7 +1078,7 @@ mod tests {
             },
             Check {
                 kind: CheckKind::NoAssertEquals,
-                location: Location::new(2, 18),
+                location: Location::new(2, 5),
                 fix: Some(Fix {
                     content: "assertEqual".to_string(),
                     start: Location::new(2, 6),