From b53615954193618fde37e193f67ebcc4039e148f Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 8 Sep 2022 22:46:42 -0400 Subject: [PATCH] Pull check logic out of check_ast.rs (#135) --- src/ast.rs | 5 + src/ast/checks.rs | 397 ++++++++++++++++++++++ src/{ast_ops.rs => ast/operations.rs} | 54 +-- src/{relocator.rs => ast/relocate.rs} | 0 src/ast/types.rs | 55 +++ src/{ => ast}/visitor.rs | 0 src/autofix.rs | 218 +----------- src/autofix/fixer.rs | 216 ++++++++++++ src/{fixer.rs => autofix/fixes.rs} | 2 +- src/cache.rs | 8 +- src/check_ast.rs | 467 +++++--------------------- src/lib.rs | 5 +- src/linter.rs | 72 ++-- 13 files changed, 808 insertions(+), 691 deletions(-) create mode 100644 src/ast.rs create mode 100644 src/ast/checks.rs rename src/{ast_ops.rs => ast/operations.rs} (76%) rename src/{relocator.rs => ast/relocate.rs} (100%) create mode 100644 src/ast/types.rs rename src/{ => ast}/visitor.rs (100%) create mode 100644 src/autofix/fixer.rs rename src/{fixer.rs => autofix/fixes.rs} (98%) diff --git a/src/ast.rs b/src/ast.rs new file mode 100644 index 0000000000..bd3f072545 --- /dev/null +++ b/src/ast.rs @@ -0,0 +1,5 @@ +pub mod checks; +pub mod operations; +pub mod relocate; +pub mod types; +pub mod visitor; diff --git a/src/ast/checks.rs b/src/ast/checks.rs new file mode 100644 index 0000000000..cdf6094168 --- /dev/null +++ b/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 { + 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 { + 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 { + let mut checks: Vec = 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 { + let mut checks: Vec = 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 { + 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 { + 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) -> Option { + 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 { + 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 { + let mut checks: Vec = 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 { + 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 { + 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, + check_repeated_literals: bool, + check_repeated_variables: bool, +) -> Vec { + let mut checks: Vec = 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, + comparators: &Vec, + check_none_comparisons: bool, + check_true_false_comparisons: bool, +) -> Vec { + let mut checks: Vec = vec![]; + + let op = ops.first().unwrap(); + let comparator = left; + + // Check `left`. + if check_none_comparisons + && matches!( + comparator.node, + ExprKind::Constant { + value: Constant::None, + kind: None + } + ) + { + if matches!(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 +} diff --git a/src/ast_ops.rs b/src/ast/operations.rs similarity index 76% rename from src/ast_ops.rs rename to src/ast/operations.rs index b2c160142b..2c12779adb 100644 --- a/src/ast_ops.rs +++ b/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 { - 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, -} - -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), - FutureImportation, - Importation(String), - StarImportation, - SubmoduleImportation(String), -} - -#[derive(Clone, Debug)] -pub struct Binding { - pub kind: BindingKind, - pub location: Location, - pub used: Option, -} +use crate::ast::types::{BindingKind, Scope}; /// Extract the names bound to a given __all__ assignment. pub fn extract_all_names(stmt: &Stmt, scope: &Scope) -> Vec { diff --git a/src/relocator.rs b/src/ast/relocate.rs similarity index 100% rename from src/relocator.rs rename to src/ast/relocate.rs diff --git a/src/ast/types.rs b/src/ast/types.rs new file mode 100644 index 0000000000..953ea878ba --- /dev/null +++ b/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, +} + +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), + FutureImportation, + Importation(String), + StarImportation, + SubmoduleImportation(String), +} + +#[derive(Clone, Debug)] +pub struct Binding { + pub kind: BindingKind, + pub location: Location, + pub used: Option, +} diff --git a/src/visitor.rs b/src/ast/visitor.rs similarity index 100% rename from src/visitor.rs rename to src/ast/visitor.rs diff --git a/src/autofix.rs b/src/autofix.rs index 29371d40c7..f52e096c63 100644 --- a/src/autofix.rs +++ b/src/autofix.rs @@ -1,216 +1,2 @@ -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 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, 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(()) - } -} +pub mod fixer; +pub mod fixes; diff --git a/src/autofix/fixer.rs b/src/autofix/fixer.rs new file mode 100644 index 0000000000..756ca0394a --- /dev/null +++ b/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 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, 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(()) + } +} diff --git a/src/fixer.rs b/src/autofix/fixes.rs similarity index 98% rename from src/fixer.rs rename to src/autofix/fixes.rs index bf0074c915..1c864c7a6e 100644 --- a/src/fixer.rs +++ b/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. diff --git a/src/cache.rs b/src/cache.rs index c686f129cf..b5f5df82af 100644 --- a/src/cache.rs +++ b/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> { 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, ) { diff --git a/src/check_ast.rs b/src/check_ast.rs index 0e08a2f0ec..025174ed71 100644 --- a/src/check_ast.rs +++ b/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, - KeywordData, Location, Stmt, StmtKind, Suite, Unaryop, + Arg, Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind, + KeywordData, Location, Stmt, StmtKind, Suite, }; use rustpython_parser::parser; -use crate::ast_ops::{ - extract_all_names, Binding, BindingKind, Scope, ScopeKind, SourceCodeLocator, -}; +use crate::ast::operations::{extract_all_names, 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::relocator::relocate_expr; +use crate::checks::{Check, CheckCode, CheckKind}; 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, @@ -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 { - 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,41 +167,18 @@ where .. } => { if self.settings.select.contains(&CheckCode::R001) { - for expr in bases { - if let ExprKind::Name { id, .. } = &expr.node { - if id == "object" { - let scope = &self.scopes - [*(self.scope_stack.last().expect("No current scope found."))]; - 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, - ) { - check.amend(fix); - } - } else { - } - self.checks.push(check); - } - _ => {} - } - } - } + let scope = + &self.scopes[*(self.scope_stack.last().expect("No current scope found."))]; + if let Some(check) = checks::check_useless_object_inheritance( + stmt, + name, + bases, + keywords, + scope, + &mut self.locator, + self.autofix, + ) { + self.checks.push(check); } } @@ -327,11 +288,7 @@ where }, ); - if self - .settings - .select - .contains(CheckKind::ImportStarUsage.code()) - { + if self.settings.select.contains(&CheckCode::F403) { 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 - .settings - .select - .contains(CheckKind::RaiseNotImplemented.code()) - { + if self.settings.select.contains(&CheckCode::F901) { if let Some(expr) = exc { - match &expr.node { - ExprKind::Call { func, .. } => { - 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, - )); - } - } - _ => {} + if let Some(check) = checks::check_raise_not_implemented(expr) { + self.checks.push(check); } } } @@ -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 !elts.is_empty() { - self.checks - .push(Check::new(CheckKind::AssertTuple, stmt.location)); - } + if let Some(check) = checks::check_assert_tuple(test, stmt.location) { + self.checks.push(check); } } } StmtKind::Try { handlers, .. } => { - if self - .settings - .select - .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, - )); - } + if self.settings.select.contains(&CheckCode::F707) { + if let Some(check) = checks::check_default_except_not_last(handlers) { + self.checks.push(check); } } } @@ -436,28 +355,20 @@ where } StmtKind::Assign { value, .. } => { self.seen_non_import = true; - if self - .settings - .select - .contains(CheckKind::DoNotAssignLambda.code()) - { - if let ExprKind::Lambda { .. } = &value.node { - self.checks - .push(Check::new(CheckKind::DoNotAssignLambda, stmt.location)); + if self.settings.select.contains(&CheckCode::E731) { + if let Some(check) = checks::check_do_not_assign_lambda(value, stmt.location) { + self.checks.push(check); } } } StmtKind::AnnAssign { value, .. } => { self.seen_non_import = true; - if self - .settings - .select - .contains(CheckKind::DoNotAssignLambda.code()) - { - if let Some(v) = value { - if let ExprKind::Lambda { .. } = v.node { - self.checks - .push(Check::new(CheckKind::DoNotAssignLambda, stmt.location)); + if self.settings.select.contains(&CheckCode::E731) { + if let Some(value) = value { + if let Some(check) = + checks::check_do_not_assign_lambda(value, stmt.location) + { + self.checks.push(check); } } } @@ -530,80 +441,22 @@ where }, ExprKind::Call { func, .. } => { if self.settings.select.contains(&CheckCode::R002) { - if let ExprKind::Attribute { value, attr, .. } = &func.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!(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); - } - } - } + if let Some(check) = checks::check_assert_equals(func, self.autofix) { + self.checks.push(check) } } } ExprKind::Dict { keys, .. } => { - if self.settings.select.contains(&CheckCode::F601) - || self.settings.select.contains(&CheckCode::F602) - { - 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 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, - )) - } - } - _ => {} - } - } - } + let check_repeated_literals = self.settings.select.contains(&CheckCode::F601); + let check_repeated_variables = self.settings.select.contains(&CheckCode::F602); + if check_repeated_literals || check_repeated_variables { + self.checks.extend(checks::check_repeated_keys( + keys, + check_repeated_literals, + check_repeated_variables, + )); } } - 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) { - if let ExprKind::Compare { ops, .. } = &operand.node { - match ops[..] { - [Cmpop::In] => { - if self.settings.select.contains(CheckKind::NotInTest.code()) { - self.checks - .push(Check::new(CheckKind::NotInTest, operand.location)); - } - } - [Cmpop::Is] => { - if self.settings.select.contains(CheckKind::NotIsTest.code()) { - self.checks - .push(Check::new(CheckKind::NotIsTest, operand.location)); - } - } - _ => {} - } - } + let check_not_in = self.settings.select.contains(&CheckCode::E713); + let check_not_is = self.settings.select.contains(&CheckCode::E714); + if check_not_in || check_not_is { + self.checks.extend(checks::check_not_tests( + op, + operand, + check_not_in, + check_not_is, + )); } } ExprKind::Compare { @@ -661,99 +505,16 @@ where ops, comparators, } => { - let op = ops.first().unwrap(); - let comparator = left; - - // Check `left`. - 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, - )); - } - } - } - - // 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, - )); - } - } - } + let check_none_comparisons = self.settings.select.contains(&CheckCode::E711); + let check_true_false_comparisons = self.settings.select.contains(&CheckCode::E712); + if check_none_comparisons || check_true_false_comparisons { + self.checks.extend(checks::check_literal_comparisons( + left, + ops, + comparators, + check_none_comparisons, + check_true_false_comparisons, + )); } } ExprKind::Constant { @@ -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 - .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)); - break; - } - idents.insert(ident); - } + if self.settings.select.contains(&CheckCode::F831) { + self.checks + .extend(checks::check_duplicate_arguments(arguments)); } - 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() { - // TODO(charlie): Ignore if using `locals`. - 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, - )); - } + if self.settings.select.contains(&CheckCode::F841) { + self.checks.extend(checks::check_unused_variables(scope)); } 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() { - // TODO(charlie): Ignore if using `locals`. - 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, - )); - } + if self.settings.select.contains(&CheckCode::F841) { + self.checks.extend(checks::check_unused_variables(scope)); } 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 { let mut checker = Checker::new(settings, autofix, path, content); diff --git a/src/lib.rs b/src/lib.rs index 6d9ef229c0..bdd726bc63 100644 --- a/src/lib.rs +++ b/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; diff --git a/src/linter.rs b/src/linter.rs index c14f325d77..7ff4a9eaac 100644 --- a/src/linter.rs +++ b/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> { +fn check_path(path: &Path, settings: &Settings, autofix: &fixer::Mode) -> Result> { // 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> { 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),