diff --git a/Cargo.lock b/Cargo.lock index 07a799ca95..5cb90877cd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1727,7 +1727,7 @@ dependencies = [ [[package]] name = "rustpython-ast" version = "0.1.0" -source = "git+https://github.com/RustPython/RustPython.git?rev=e9970edd775a617226007d3d3cb0fbbc39ed3948#e9970edd775a617226007d3d3cb0fbbc39ed3948" +source = "git+https://github.com/RustPython/RustPython.git?rev=7a688e0b6c2904f286acac1e4af3f1628dd38589#7a688e0b6c2904f286acac1e4af3f1628dd38589" dependencies = [ "num-bigint", "rustpython-compiler-core", @@ -1736,7 +1736,7 @@ dependencies = [ [[package]] name = "rustpython-compiler-core" version = "0.1.2" -source = "git+https://github.com/RustPython/RustPython.git?rev=e9970edd775a617226007d3d3cb0fbbc39ed3948#e9970edd775a617226007d3d3cb0fbbc39ed3948" +source = "git+https://github.com/RustPython/RustPython.git?rev=7a688e0b6c2904f286acac1e4af3f1628dd38589#7a688e0b6c2904f286acac1e4af3f1628dd38589" dependencies = [ "bincode", "bitflags", @@ -1753,7 +1753,7 @@ dependencies = [ [[package]] name = "rustpython-parser" version = "0.1.2" -source = "git+https://github.com/RustPython/RustPython.git?rev=e9970edd775a617226007d3d3cb0fbbc39ed3948#e9970edd775a617226007d3d3cb0fbbc39ed3948" +source = "git+https://github.com/RustPython/RustPython.git?rev=7a688e0b6c2904f286acac1e4af3f1628dd38589#7a688e0b6c2904f286acac1e4af3f1628dd38589" dependencies = [ "ahash", "anyhow", diff --git a/Cargo.toml b/Cargo.toml index 7ec6ced537..a603a793bd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,7 +23,7 @@ notify = { version = "4.0.17" } pyo3 = { version = "0.16.5", features = ["extension-module", "abi3-py37"] } rayon = { version = "1.5.3" } regex = { version = "1.6.0" } -rustpython-parser = { features = ["lalrpop"], git = "https://github.com/RustPython/RustPython.git", rev = "e9970edd775a617226007d3d3cb0fbbc39ed3948" } +rustpython-parser = { features = ["lalrpop"], git = "https://github.com/RustPython/RustPython.git", rev = "7a688e0b6c2904f286acac1e4af3f1628dd38589" } serde = { version = "1.0.143", features = ["derive"] } serde_json = { version = "1.0.83" } toml = { version = "0.5.9" } diff --git a/resources/test/src/line_too_long.py b/resources/test/src/E501.py similarity index 100% rename from resources/test/src/line_too_long.py rename to resources/test/src/E501.py diff --git a/resources/test/src/F401.py b/resources/test/src/F401.py new file mode 100644 index 0000000000..905547e64e --- /dev/null +++ b/resources/test/src/F401.py @@ -0,0 +1,13 @@ +import os +import functools +from collections import ( + Counter, + OrderedDict, +) + + +class X: + def a(self): + x = os.environ["1"] + y = Counter() + return X diff --git a/resources/test/src/F403.py b/resources/test/src/F403.py new file mode 100644 index 0000000000..56b0ad778d --- /dev/null +++ b/resources/test/src/F403.py @@ -0,0 +1,6 @@ +from F634 import * +from F634 import * # noqa: E501 + +from F634 import * # noqa +from F634 import * # noqa: F403 +from F634 import * # noqa: F403, E501 diff --git a/resources/test/src/f_string_missing_placeholders.py b/resources/test/src/F541.py similarity index 80% rename from resources/test/src/f_string_missing_placeholders.py rename to resources/test/src/F541.py index c9b6c43d41..083211aa12 100644 --- a/resources/test/src/f_string_missing_placeholders.py +++ b/resources/test/src/F541.py @@ -7,3 +7,5 @@ e = ( f"def" + "ghi" ) + +g = f"ghi{123:{45}}" diff --git a/resources/test/src/if_tuple.py b/resources/test/src/F634.py similarity index 100% rename from resources/test/src/if_tuple.py rename to resources/test/src/F634.py diff --git a/resources/test/src/return_outside_function.py b/resources/test/src/F706.py similarity index 100% rename from resources/test/src/return_outside_function.py rename to resources/test/src/F706.py diff --git a/resources/test/src/duplicate_argument_name.py b/resources/test/src/F831.py similarity index 100% rename from resources/test/src/duplicate_argument_name.py rename to resources/test/src/F831.py diff --git a/resources/test/src/raise_not_implemented.py b/resources/test/src/F901.py similarity index 100% rename from resources/test/src/raise_not_implemented.py rename to resources/test/src/F901.py diff --git a/resources/test/src/import_star_usage.py b/resources/test/src/import_star_usage.py deleted file mode 100644 index 5936ac786d..0000000000 --- a/resources/test/src/import_star_usage.py +++ /dev/null @@ -1,6 +0,0 @@ -from if_tuple import * -from if_tuple import * # noqa: E501 - -from if_tuple import * # noqa -from if_tuple import * # noqa: F403 -from if_tuple import * # noqa: F403, E501 diff --git a/src/check_ast.rs b/src/check_ast.rs index 185c11f85e..e7a24fd2e6 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -1,8 +1,10 @@ -use std::collections::BTreeSet; +use std::collections::{BTreeMap, BTreeSet}; + +use rustpython_parser::ast::{ + Arg, Arguments, Expr, ExprContext, ExprKind, Location, Stmt, StmtKind, Suite, +}; use crate::check_ast::ScopeKind::{Class, Function, Generator, Module}; -use rustpython_parser::ast::{Arg, Arguments, Expr, ExprKind, Stmt, StmtKind, Suite}; - use crate::checks::{Check, CheckKind}; use crate::settings::Settings; use crate::visitor; @@ -17,12 +19,33 @@ enum ScopeKind { struct Scope { kind: ScopeKind, + values: BTreeMap, +} + +enum BindingKind { + Argument, + Assignment, + ClassDefinition, + Definition, + FutureImportation, + Importation, + StarImportation, + SubmoduleImportation, +} + +struct Binding { + kind: BindingKind, + name: String, + location: Location, + used: bool, } struct Checker<'a> { settings: &'a Settings, checks: Vec, scopes: Vec, + dead_scopes: Vec, + in_f_string: bool, } impl Checker<'_> { @@ -30,7 +53,9 @@ impl Checker<'_> { Checker { settings, checks: vec![], - scopes: vec![Scope { kind: Module }], + scopes: vec![], + dead_scopes: vec![], + in_f_string: false, } } } @@ -38,8 +63,30 @@ impl Checker<'_> { impl Visitor for Checker<'_> { fn visit_stmt(&mut self, stmt: &Stmt) { match &stmt.node { - StmtKind::FunctionDef { .. } => self.scopes.push(Scope { kind: Function }), - StmtKind::AsyncFunctionDef { .. } => self.scopes.push(Scope { kind: Function }), + StmtKind::FunctionDef { name, .. } => { + self.push_scope(Scope { + kind: Function, + values: BTreeMap::new(), + }); + self.add_binding(Binding { + kind: BindingKind::ClassDefinition, + name: name.clone(), + used: false, + location: stmt.location, + }) + } + StmtKind::AsyncFunctionDef { name, .. } => { + self.push_scope(Scope { + kind: Function, + values: BTreeMap::new(), + }); + self.add_binding(Binding { + kind: BindingKind::ClassDefinition, + name: name.clone(), + used: false, + location: stmt.location, + }) + } StmtKind::Return { .. } => { if self .settings @@ -59,20 +106,76 @@ impl Visitor for Checker<'_> { } } } - StmtKind::ClassDef { .. } => self.scopes.push(Scope { kind: Class }), - StmtKind::ImportFrom { names, .. } => { - if self - .settings - .select - .contains(CheckKind::ImportStarUsage.code()) - { - for alias in names { - if alias.node.name == "*" { + StmtKind::ClassDef { .. } => self.push_scope(Scope { + kind: Class, + values: BTreeMap::new(), + }), + StmtKind::Import { names } => { + for alias in names { + if alias.node.name.contains('.') && alias.node.asname.is_none() { + self.add_binding(Binding { + kind: BindingKind::SubmoduleImportation, + name: alias.node.name.clone(), + used: false, + location: stmt.location, + }) + } else { + self.add_binding(Binding { + kind: BindingKind::Importation, + name: alias + .node + .asname + .clone() + .unwrap_or_else(|| alias.node.name.clone()), + used: false, + location: stmt.location, + }) + } + } + } + StmtKind::ImportFrom { names, module, .. } => { + for alias in names { + let name = alias + .node + .asname + .clone() + .unwrap_or_else(|| alias.node.name.clone()); + if module + .clone() + .map(|name| name == "future") + .unwrap_or_default() + { + self.add_binding(Binding { + kind: BindingKind::FutureImportation, + name, + used: true, + location: stmt.location, + }); + } else if alias.node.name == "*" { + self.add_binding(Binding { + kind: BindingKind::StarImportation, + name, + used: false, + location: stmt.location, + }); + + if self + .settings + .select + .contains(CheckKind::ImportStarUsage.code()) + { self.checks.push(Check { kind: CheckKind::ImportStarUsage, location: stmt.location, }); } + } else { + self.add_binding(Binding { + kind: BindingKind::Importation, + name, + used: false, + location: stmt.location, + }); } } } @@ -117,6 +220,9 @@ impl Visitor for Checker<'_> { } } } + StmtKind::AugAssign { target, .. } => { + self.handle_node_load(target); + } _ => {} } @@ -126,21 +232,43 @@ impl Visitor for Checker<'_> { StmtKind::ClassDef { .. } | StmtKind::FunctionDef { .. } | StmtKind::AsyncFunctionDef { .. } => { - self.scopes.pop(); + self.pop_scope(); } _ => {} }; + + if let StmtKind::ClassDef { name, .. } = &stmt.node { + self.add_binding(Binding { + kind: BindingKind::Definition, + name: name.clone(), + used: false, + location: stmt.location, + }); + } } fn visit_expr(&mut self, expr: &Expr) { + let initial = self.in_f_string; match &expr.node { - ExprKind::GeneratorExp { .. } => self.scopes.push(Scope { kind: Generator }), - ExprKind::Lambda { .. } => self.scopes.push(Scope { kind: Function }), + ExprKind::Name { ctx, .. } => match ctx { + ExprContext::Load => self.handle_node_load(expr), + ExprContext::Store => self.handle_node_store(expr), + ExprContext::Del => {} + }, + ExprKind::GeneratorExp { .. } => self.push_scope(Scope { + kind: Generator, + values: BTreeMap::new(), + }), + ExprKind::Lambda { .. } => self.push_scope(Scope { + kind: Function, + values: BTreeMap::new(), + }), ExprKind::JoinedStr { values } => { - if self - .settings - .select - .contains(CheckKind::FStringMissingPlaceholders.code()) + if !self.in_f_string + && self + .settings + .select + .contains(CheckKind::FStringMissingPlaceholders.code()) && !values .iter() .any(|value| matches!(value.node, ExprKind::FormattedValue { .. })) @@ -150,6 +278,7 @@ impl Visitor for Checker<'_> { location: expr.location, }); } + self.in_f_string = true; } _ => {} }; @@ -158,7 +287,12 @@ impl Visitor for Checker<'_> { match &expr.node { ExprKind::GeneratorExp { .. } | ExprKind::Lambda { .. } => { - self.scopes.pop(); + if let Some(scope) = self.scopes.pop() { + self.dead_scopes.push(scope); + } + } + ExprKind::JoinedStr { .. } => { + self.in_f_string = initial; } _ => {} }; @@ -201,63 +335,99 @@ impl Visitor for Checker<'_> { visitor::walk_arguments(self, arguments); } -} -pub fn check_ast(python_ast: &Suite, settings: &Settings) -> Vec { - python_ast - .iter() - .flat_map(|stmt| { - let mut checker = Checker::new(settings); - checker.visit_stmt(stmt); - checker.checks - }) - .collect() -} - -#[cfg(test)] -mod tests { - use std::collections::BTreeSet; - - use rustpython_parser::ast::{Alias, AliasData, Location, Stmt, StmtKind}; - - use crate::check_ast::Checker; - use crate::checks::CheckKind::ImportStarUsage; - use crate::checks::{Check, CheckCode}; - use crate::settings::Settings; - use crate::visitor::Visitor; - - #[test] - fn import_star_usage() { - let settings = Settings { - line_length: 88, - exclude: vec![], - select: BTreeSet::from([CheckCode::F403]), - }; - let mut checker = Checker::new(&settings); - checker.visit_stmt(&Stmt { - location: Location::new(1, 1), - custom: (), - node: StmtKind::ImportFrom { - module: Some("bar".to_string()), - names: vec![Alias::new( - Default::default(), - AliasData { - name: "*".to_string(), - asname: None, - }, - )], - level: None, - }, + fn visit_arg(&mut self, arg: &Arg) { + self.add_binding(Binding { + kind: BindingKind::Argument, + name: arg.node.arg.clone(), + used: false, + location: arg.location, }); + visitor::walk_arg(self, arg); + } +} - let actual = checker.checks; - let expected = vec![Check { - kind: ImportStarUsage, - location: Location::new(1, 1), - }]; - assert_eq!(actual.len(), expected.len()); - for i in 0..actual.len() { - assert_eq!(actual[i], expected[i]); +impl Checker<'_> { + fn push_scope(&mut self, scope: Scope) { + self.scopes.push(scope); + } + + fn pop_scope(&mut self) { + self.dead_scopes + .push(self.scopes.pop().expect("Attempted to pop without scope.")); + } + + fn add_binding(&mut self, binding: Binding) { + // TODO(charlie): Don't treat annotations as assignments if there is an existing value. + let scope = self.scopes.last_mut().expect("No current scope found."); + scope.values.insert( + binding.name.clone(), + match scope.values.get(&binding.name) { + None => binding, + Some(existing) => Binding { + kind: binding.kind, + name: binding.name, + location: binding.location, + used: existing.used, + }, + }, + ); + } + + fn handle_node_load(&mut self, expr: &Expr) { + if let ExprKind::Name { id, .. } = &expr.node { + for scope in self.scopes.iter_mut().rev() { + if matches!(scope.kind, Class) { + if id == "__class__" { + return; + } else { + continue; + } + } + if let Some(binding) = scope.values.get_mut(id) { + binding.used = true; + } + } + } + } + + fn handle_node_store(&mut self, expr: &Expr) { + if let ExprKind::Name { id, .. } = &expr.node { + // TODO(charlie): Handle alternate binding types (like `Annotation`). + self.add_binding(Binding { + kind: BindingKind::Assignment, + name: id.to_string(), + used: false, + location: expr.location, + }); + } + } + + fn check_dead_scopes(&mut self) { + // TODO(charlie): Handle `__all__`. + for scope in &self.dead_scopes { + for (name, binding) in scope.values.iter().rev() { + if !binding.used && matches!(binding.kind, BindingKind::Importation) { + self.checks.push(Check { + kind: CheckKind::UnusedImport(name.clone()), + location: binding.location, + }); + } + } } } } + +pub fn check_ast(python_ast: &Suite, settings: &Settings) -> Vec { + let mut checker = Checker::new(settings); + checker.push_scope(Scope { + kind: Module, + values: BTreeMap::new(), + }); + for stmt in python_ast { + checker.visit_stmt(stmt); + } + checker.pop_scope(); + checker.check_dead_scopes(); + checker.checks +} diff --git a/src/checks.rs b/src/checks.rs index 5de28dc654..7c6a90ffa3 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -3,25 +3,41 @@ use serde::{Deserialize, Serialize}; #[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Hash, PartialOrd, Ord)] pub enum CheckCode { - F831, + E501, + F401, + F403, F541, F634, - F403, F706, + F831, F901, - E501, } impl CheckCode { pub fn as_str(&self) -> &str { match self { - CheckCode::F831 => "F831", + CheckCode::E501 => "E501", + CheckCode::F401 => "F401", + CheckCode::F403 => "F403", CheckCode::F541 => "F541", CheckCode::F634 => "F634", - CheckCode::F403 => "F403", CheckCode::F706 => "F706", + CheckCode::F831 => "F831", CheckCode::F901 => "F901", - CheckCode::E501 => "E501", + } + } + + /// The source for the check (either the AST, or the physical lines). + pub fn lint_source(&self) -> &'static LintSource { + match self { + CheckCode::E501 => &LintSource::Lines, + CheckCode::F401 => &LintSource::AST, + CheckCode::F403 => &LintSource::AST, + CheckCode::F541 => &LintSource::AST, + CheckCode::F634 => &LintSource::AST, + CheckCode::F706 => &LintSource::AST, + CheckCode::F831 => &LintSource::AST, + CheckCode::F901 => &LintSource::AST, } } } @@ -38,25 +54,13 @@ pub enum CheckKind { FStringMissingPlaceholders, IfTuple, ImportStarUsage, - ReturnOutsideFunction, - RaiseNotImplemented, LineTooLong, + RaiseNotImplemented, + ReturnOutsideFunction, + UnusedImport(String), } impl CheckKind { - /// Get the CheckKind for a corresponding code. - pub fn new(code: &CheckCode) -> Self { - match code { - CheckCode::F831 => CheckKind::DuplicateArgumentName, - CheckCode::F541 => CheckKind::FStringMissingPlaceholders, - CheckCode::F634 => CheckKind::IfTuple, - CheckCode::F403 => CheckKind::ImportStarUsage, - CheckCode::F706 => CheckKind::ReturnOutsideFunction, - CheckCode::F901 => CheckKind::RaiseNotImplemented, - CheckCode::E501 => CheckKind::LineTooLong, - } - } - /// A four-letter shorthand code for the check. pub fn code(&self) -> &'static CheckCode { match self { @@ -64,37 +68,34 @@ impl CheckKind { CheckKind::FStringMissingPlaceholders => &CheckCode::F541, CheckKind::IfTuple => &CheckCode::F634, CheckKind::ImportStarUsage => &CheckCode::F403, - CheckKind::ReturnOutsideFunction => &CheckCode::F706, - CheckKind::RaiseNotImplemented => &CheckCode::F901, CheckKind::LineTooLong => &CheckCode::E501, + CheckKind::RaiseNotImplemented => &CheckCode::F901, + CheckKind::ReturnOutsideFunction => &CheckCode::F706, + CheckKind::UnusedImport(_) => &CheckCode::F401, } } /// The body text for the check. - pub fn body(&self) -> &'static str { + pub fn body(&self) -> String { match self { - CheckKind::DuplicateArgumentName => "Duplicate argument name in function definition", - CheckKind::FStringMissingPlaceholders => "f-string without any placeholders", - CheckKind::IfTuple => "If test is a tuple, which is always `True`", - CheckKind::ImportStarUsage => "Unable to detect undefined names", - CheckKind::ReturnOutsideFunction => "a `return` statement outside of a function/method", - CheckKind::RaiseNotImplemented => { - "'raise NotImplemented' should be 'raise NotImplementedError" + CheckKind::DuplicateArgumentName => { + "Duplicate argument name in function definition".to_string() } - CheckKind::LineTooLong => "Line too long", - } - } - - /// The source for the checks (either the AST, or the physical lines). - pub fn lint_source(&self) -> &'static LintSource { - match self { - CheckKind::DuplicateArgumentName => &LintSource::AST, - CheckKind::FStringMissingPlaceholders => &LintSource::AST, - CheckKind::IfTuple => &LintSource::AST, - CheckKind::ImportStarUsage => &LintSource::AST, - CheckKind::ReturnOutsideFunction => &LintSource::AST, - CheckKind::RaiseNotImplemented => &LintSource::AST, - CheckKind::LineTooLong => &LintSource::Lines, + CheckKind::FStringMissingPlaceholders => { + "f-string without any placeholders".to_string() + } + CheckKind::IfTuple => { + "If test is a tuple.to_string(), which is always `True`".to_string() + } + CheckKind::ImportStarUsage => "Unable to detect undefined names".to_string(), + CheckKind::LineTooLong => "Line too long".to_string(), + CheckKind::RaiseNotImplemented => { + "'raise NotImplemented' should be 'raise NotImplementedError".to_string() + } + CheckKind::ReturnOutsideFunction => { + "a `return` statement outside of a function/method".to_string() + } + CheckKind::UnusedImport(name) => format!("`{name}` imported but unused"), } } } diff --git a/src/linter.rs b/src/linter.rs index 034b03a28b..5b404d6149 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -6,7 +6,7 @@ use rustpython_parser::parser; use crate::check_ast::check_ast; use crate::check_lines::check_lines; -use crate::checks::{Check, CheckKind, LintSource}; +use crate::checks::{Check, LintSource}; use crate::message::Message; use crate::settings::Settings; use crate::{cache, fs}; @@ -28,7 +28,7 @@ pub fn check_path(path: &Path, settings: &Settings, mode: &cache::Mode) -> Resul if settings .select .iter() - .any(|check_code| matches!(CheckKind::new(check_code).lint_source(), LintSource::AST)) + .any(|check_code| matches!(check_code.lint_source(), LintSource::AST)) { let python_ast = parser::parse_program(&contents, &path.to_string_lossy())?; checks.extend(check_ast(&python_ast, settings)); @@ -38,7 +38,7 @@ pub fn check_path(path: &Path, settings: &Settings, mode: &cache::Mode) -> Resul if settings .select .iter() - .any(|check_code| matches!(CheckKind::new(check_code).lint_source(), LintSource::Lines)) + .any(|check_code| matches!(check_code.lint_source(), LintSource::Lines)) { checks.extend(check_lines(&contents, settings)); } @@ -72,31 +72,50 @@ mod tests { use crate::{cache, settings}; #[test] - fn duplicate_argument_name() -> Result<()> { + fn e501() -> Result<()> { let actual = check_path( - &Path::new("./resources/test/src/duplicate_argument_name.py"), + &Path::new("./resources/test/src/E501.py"), &settings::Settings { line_length: 88, exclude: vec![], - select: BTreeSet::from([CheckCode::F831]), + select: BTreeSet::from([CheckCode::E501]), + }, + &cache::Mode::None, + )?; + let expected = vec![Message { + kind: CheckKind::LineTooLong, + location: Location::new(5, 89), + filename: "./resources/test/src/E501.py".to_string(), + }]; + assert_eq!(actual.len(), expected.len()); + for i in 0..actual.len() { + assert_eq!(actual[i], expected[i]); + } + + Ok(()) + } + + #[test] + fn f401() -> Result<()> { + let actual = check_path( + &Path::new("./resources/test/src/F401.py"), + &settings::Settings { + line_length: 88, + exclude: vec![], + select: BTreeSet::from([CheckCode::F401]), }, &cache::Mode::None, )?; let expected = vec![ Message { - kind: CheckKind::DuplicateArgumentName, - location: Location::new(1, 25), - filename: "./resources/test/src/duplicate_argument_name.py".to_string(), + kind: CheckKind::UnusedImport("functools".to_string()), + location: Location::new(2, 1), + filename: "./resources/test/src/F401.py".to_string(), }, Message { - kind: CheckKind::DuplicateArgumentName, - location: Location::new(5, 28), - filename: "./resources/test/src/duplicate_argument_name.py".to_string(), - }, - Message { - kind: CheckKind::DuplicateArgumentName, - location: Location::new(9, 27), - filename: "./resources/test/src/duplicate_argument_name.py".to_string(), + kind: CheckKind::UnusedImport("OrderedDict".to_string()), + location: Location::new(3, 1), + filename: "./resources/test/src/F401.py".to_string(), }, ]; assert_eq!(actual.len(), expected.len()); @@ -108,76 +127,9 @@ mod tests { } #[test] - fn f_string_missing_placeholders() -> Result<()> { + fn f403() -> Result<()> { let actual = check_path( - &Path::new("./resources/test/src/f_string_missing_placeholders.py"), - &settings::Settings { - line_length: 88, - exclude: vec![], - select: BTreeSet::from([CheckCode::F541]), - }, - &cache::Mode::None, - )?; - let expected = vec![ - Message { - kind: CheckKind::FStringMissingPlaceholders, - location: Location::new(4, 7), - filename: "./resources/test/src/f_string_missing_placeholders.py".to_string(), - }, - Message { - kind: CheckKind::FStringMissingPlaceholders, - location: Location::new(5, 7), - filename: "./resources/test/src/f_string_missing_placeholders.py".to_string(), - }, - Message { - kind: CheckKind::FStringMissingPlaceholders, - location: Location::new(7, 7), - filename: "./resources/test/src/f_string_missing_placeholders.py".to_string(), - }, - ]; - assert_eq!(actual.len(), expected.len()); - for i in 0..actual.len() { - assert_eq!(actual[i], expected[i]); - } - - Ok(()) - } - - #[test] - fn if_tuple() -> Result<()> { - let actual = check_path( - &Path::new("./resources/test/src/if_tuple.py"), - &settings::Settings { - line_length: 88, - exclude: vec![], - select: BTreeSet::from([CheckCode::F634]), - }, - &cache::Mode::None, - )?; - let expected = vec![ - Message { - kind: CheckKind::IfTuple, - location: Location::new(1, 1), - filename: "./resources/test/src/if_tuple.py".to_string(), - }, - Message { - kind: CheckKind::IfTuple, - location: Location::new(7, 5), - filename: "./resources/test/src/if_tuple.py".to_string(), - }, - ]; - assert_eq!(actual.len(), expected.len()); - for i in 0..actual.len() { - assert_eq!(actual[i], expected[i]); - } - - Ok(()) - } - - #[test] - fn import_star_usage() -> Result<()> { - let actual = check_path( - &Path::new("./resources/test/src/import_star_usage.py"), + &Path::new("./resources/test/src/F403.py"), &settings::Settings { line_length: 88, exclude: vec![], @@ -189,12 +141,47 @@ mod tests { Message { kind: CheckKind::ImportStarUsage, location: Location::new(1, 1), - filename: "./resources/test/src/import_star_usage.py".to_string(), + filename: "./resources/test/src/F403.py".to_string(), }, Message { kind: CheckKind::ImportStarUsage, location: Location::new(2, 1), - filename: "./resources/test/src/import_star_usage.py".to_string(), + filename: "./resources/test/src/F403.py".to_string(), + }, + ]; + assert_eq!(actual.len(), expected.len()); + for i in 0..actual.len() { + assert_eq!(actual[i], expected[i]); + } + + Ok(()) + } + #[test] + fn f541() -> Result<()> { + let actual = check_path( + &Path::new("./resources/test/src/F541.py"), + &settings::Settings { + line_length: 88, + exclude: vec![], + select: BTreeSet::from([CheckCode::F541]), + }, + &cache::Mode::None, + )?; + let expected = vec![ + Message { + kind: CheckKind::FStringMissingPlaceholders, + location: Location::new(4, 7), + filename: "./resources/test/src/F541.py".to_string(), + }, + Message { + kind: CheckKind::FStringMissingPlaceholders, + location: Location::new(5, 7), + filename: "./resources/test/src/F541.py".to_string(), + }, + Message { + kind: CheckKind::FStringMissingPlaceholders, + location: Location::new(7, 7), + filename: "./resources/test/src/F541.py".to_string(), }, ]; assert_eq!(actual.len(), expected.len()); @@ -206,9 +193,40 @@ mod tests { } #[test] - fn return_outside_function() -> Result<()> { + fn f634() -> Result<()> { let actual = check_path( - &Path::new("./resources/test/src/return_outside_function.py"), + &Path::new("./resources/test/src/F634.py"), + &settings::Settings { + line_length: 88, + exclude: vec![], + select: BTreeSet::from([CheckCode::F634]), + }, + &cache::Mode::None, + )?; + let expected = vec![ + Message { + kind: CheckKind::IfTuple, + location: Location::new(1, 1), + filename: "./resources/test/src/F634.py".to_string(), + }, + Message { + kind: CheckKind::IfTuple, + location: Location::new(7, 5), + filename: "./resources/test/src/F634.py".to_string(), + }, + ]; + assert_eq!(actual.len(), expected.len()); + for i in 0..actual.len() { + assert_eq!(actual[i], expected[i]); + } + + Ok(()) + } + + #[test] + fn f706() -> Result<()> { + let actual = check_path( + &Path::new("./resources/test/src/F706.py"), &settings::Settings { line_length: 88, exclude: vec![], @@ -220,12 +238,12 @@ mod tests { Message { kind: CheckKind::ReturnOutsideFunction, location: Location::new(6, 5), - filename: "./resources/test/src/return_outside_function.py".to_string(), + filename: "./resources/test/src/F706.py".to_string(), }, Message { kind: CheckKind::ReturnOutsideFunction, location: Location::new(9, 1), - filename: "./resources/test/src/return_outside_function.py".to_string(), + filename: "./resources/test/src/F706.py".to_string(), }, ]; assert_eq!(actual.len(), expected.len()); @@ -237,9 +255,45 @@ mod tests { } #[test] - fn raise_not_implemented() -> Result<()> { + fn f831() -> Result<()> { let actual = check_path( - &Path::new("./resources/test/src/raise_not_implemented.py"), + &Path::new("./resources/test/src/F831.py"), + &settings::Settings { + line_length: 88, + exclude: vec![], + select: BTreeSet::from([CheckCode::F831]), + }, + &cache::Mode::None, + )?; + let expected = vec![ + Message { + kind: CheckKind::DuplicateArgumentName, + location: Location::new(1, 25), + filename: "./resources/test/src/F831.py".to_string(), + }, + Message { + kind: CheckKind::DuplicateArgumentName, + location: Location::new(5, 28), + filename: "./resources/test/src/F831.py".to_string(), + }, + Message { + kind: CheckKind::DuplicateArgumentName, + location: Location::new(9, 27), + filename: "./resources/test/src/F831.py".to_string(), + }, + ]; + assert_eq!(actual.len(), expected.len()); + for i in 0..actual.len() { + assert_eq!(actual[i], expected[i]); + } + + Ok(()) + } + + #[test] + fn f901() -> Result<()> { + let actual = check_path( + &Path::new("./resources/test/src/F901.py"), &settings::Settings { line_length: 88, exclude: vec![], @@ -251,12 +305,12 @@ mod tests { Message { kind: CheckKind::RaiseNotImplemented, location: Location::new(2, 5), - filename: "./resources/test/src/raise_not_implemented.py".to_string(), + filename: "./resources/test/src/F901.py".to_string(), }, Message { kind: CheckKind::RaiseNotImplemented, location: Location::new(6, 5), - filename: "./resources/test/src/raise_not_implemented.py".to_string(), + filename: "./resources/test/src/F901.py".to_string(), }, ]; assert_eq!(actual.len(), expected.len()); @@ -266,28 +320,4 @@ mod tests { Ok(()) } - - #[test] - fn line_too_long() -> Result<()> { - let actual = check_path( - &Path::new("./resources/test/src/line_too_long.py"), - &settings::Settings { - line_length: 88, - exclude: vec![], - select: BTreeSet::from([CheckCode::E501]), - }, - &cache::Mode::None, - )?; - let expected = vec![Message { - kind: CheckKind::LineTooLong, - location: Location::new(5, 89), - filename: "./resources/test/src/line_too_long.py".to_string(), - }]; - assert_eq!(actual.len(), expected.len()); - for i in 0..actual.len() { - assert_eq!(actual[i], expected[i]); - } - - Ok(()) - } } diff --git a/src/settings.rs b/src/settings.rs index 3c5a9175c9..6fe6f56145 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -7,6 +7,7 @@ use anyhow::Result; use crate::checks::CheckCode; use crate::pyproject::load_config; +#[derive(Debug)] pub struct Settings { pub line_length: usize, pub exclude: Vec, @@ -41,13 +42,14 @@ impl Settings { .collect(), select: config.select.unwrap_or_else(|| { BTreeSet::from([ - CheckCode::F831, + CheckCode::E501, + CheckCode::F401, + CheckCode::F403, CheckCode::F541, CheckCode::F634, - CheckCode::F403, CheckCode::F706, + CheckCode::F831, CheckCode::F901, - CheckCode::E501, ]) }), })