diff --git a/src/check_ast.rs b/src/check_ast.rs index 5b3f48142d..fdb570a712 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -3,33 +3,51 @@ use std::collections::HashSet; use rustpython_parser::ast::{Arg, Arguments, Expr, ExprKind, Stmt, StmtKind, Suite}; use crate::checks::{Check, CheckKind}; +use crate::settings::Settings; use crate::visitor; use crate::visitor::Visitor; -#[derive(Default)] -struct Checker { +struct Checker<'a> { + settings: &'a Settings, checks: Vec, } -impl Visitor for Checker { +impl Checker<'_> { + pub fn new(settings: &Settings) -> Checker { + Checker { + settings, + checks: vec![], + } + } +} + +impl Visitor for Checker<'_> { fn visit_stmt(&mut self, stmt: &Stmt) { match &stmt.node { StmtKind::ImportFrom { names, .. } => { - for alias in names { - if alias.name == "*" { - self.checks.push(Check { - kind: CheckKind::ImportStarUsage, - location: stmt.location, - }); + if self + .settings + .select + .contains(CheckKind::ImportStarUsage.code()) + { + for alias in names { + if alias.name == "*" { + self.checks.push(Check { + kind: CheckKind::ImportStarUsage, + location: stmt.location, + }); + } } } } StmtKind::If { test, .. } => { - if let ExprKind::Tuple { .. } = test.node { - self.checks.push(Check { - kind: CheckKind::IfTuple, - location: stmt.location, - }); + if self.settings.select.contains(CheckKind::IfTuple.code()) { + if let ExprKind::Tuple { .. } = test.node { + self.checks.push(Check { + kind: CheckKind::IfTuple, + location: stmt.location, + }); + } } } _ => {} @@ -39,15 +57,21 @@ impl Visitor for Checker { } fn visit_expr(&mut self, expr: &Expr) { - if let ExprKind::JoinedStr { values } = &expr.node { - if !values - .iter() - .any(|value| matches!(value.node, ExprKind::FormattedValue { .. })) - { - self.checks.push(Check { - kind: CheckKind::FStringMissingPlaceholders, - location: expr.location, - }); + if self + .settings + .select + .contains(CheckKind::FStringMissingPlaceholders.code()) + { + if let ExprKind::JoinedStr { values } = &expr.node { + if !values + .iter() + .any(|value| matches!(value.node, ExprKind::FormattedValue { .. })) + { + self.checks.push(Check { + kind: CheckKind::FStringMissingPlaceholders, + location: expr.location, + }); + } } } @@ -55,43 +79,49 @@ impl Visitor for Checker { } fn visit_arguments(&mut self, arguments: &Arguments) { - // 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: HashSet = HashSet::new(); - for arg in all_arguments { - let ident = &arg.node.arg; - if idents.contains(ident) { - self.checks.push(Check { - kind: CheckKind::DuplicateArgumentName, - location: arg.location, - }); - break; + 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: HashSet = HashSet::new(); + for arg in all_arguments { + let ident = &arg.node.arg; + if idents.contains(ident) { + self.checks.push(Check { + kind: CheckKind::DuplicateArgumentName, + location: arg.location, + }); + break; + } + idents.insert(ident.clone()); } - idents.insert(ident.clone()); } visitor::walk_arguments(self, arguments); } } -pub fn check_ast(python_ast: &Suite) -> Vec { +pub fn check_ast(python_ast: &Suite, settings: &Settings) -> Vec { python_ast .iter() .flat_map(|stmt| { - let mut checker: Checker = Default::default(); + let mut checker = Checker::new(settings); checker.visit_stmt(stmt); checker.checks }) @@ -101,15 +131,22 @@ pub fn check_ast(python_ast: &Suite) -> Vec { #[cfg(test)] mod tests { use rustpython_parser::ast::{Alias, Location, Stmt, StmtKind}; + use std::collections::HashSet; use crate::check_ast::Checker; - use crate::checks::Check; use crate::checks::CheckKind::ImportStarUsage; + use crate::checks::{Check, CheckCode}; + use crate::settings::Settings; use crate::visitor::Visitor; #[test] fn import_star_usage() { - let mut checker: Checker = Default::default(); + let settings = Settings { + line_length: 88, + exclude: vec![], + select: HashSet::from([CheckCode::F403]), + }; + let mut checker = Checker::new(&settings); checker.visit_stmt(&Stmt { location: Location::new(1, 1), custom: (), diff --git a/src/check_lines.rs b/src/check_lines.rs index c67210e982..2f566d0568 100644 --- a/src/check_lines.rs +++ b/src/check_lines.rs @@ -1,7 +1,6 @@ use rustpython_parser::ast::Location; -use crate::checks::Check; -use crate::checks::CheckKind::LineTooLong; +use crate::checks::{Check, CheckKind}; use crate::settings::Settings; pub fn check_lines(contents: &str, settings: &Settings) -> Vec { @@ -9,19 +8,19 @@ pub fn check_lines(contents: &str, settings: &Settings) -> Vec { .lines() .enumerate() .filter_map(|(row, line)| { - if line.len() > settings.line_length { + if settings.select.contains(CheckKind::LineTooLong.code()) + && line.len() > settings.line_length + { let chunks: Vec<&str> = line.split_whitespace().collect(); - if chunks.len() == 1 || (chunks.len() == 2 && chunks[0] == "#") { - None - } else { - Some(Check { - kind: LineTooLong, + if !(chunks.len() == 1 || (chunks.len() == 2 && chunks[0] == "#")) { + return Some(Check { + kind: CheckKind::LineTooLong, location: Location::new(row + 1, settings.line_length + 1), - }) + }); } - } else { - None } + + None }) .collect() } diff --git a/src/checks.rs b/src/checks.rs index 9b024fae68..59b6ad92e7 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -1,6 +1,33 @@ use rustpython_parser::ast::Location; use serde::{Deserialize, Serialize}; +#[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Hash)] +pub enum CheckCode { + F831, + F541, + F634, + F403, + E501, +} + +impl CheckCode { + pub fn as_str(&self) -> &str { + match self { + CheckCode::F831 => "F831", + CheckCode::F541 => "F541", + CheckCode::F634 => "F634", + CheckCode::F403 => "F403", + CheckCode::E501 => "E501", + } + } +} + +#[allow(clippy::upper_case_acronyms)] +pub enum LintSource { + AST, + Lines, +} + #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] pub enum CheckKind { DuplicateArgumentName, @@ -11,14 +38,25 @@ pub enum CheckKind { } 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::E501 => CheckKind::LineTooLong, + } + } + /// A four-letter shorthand code for the check. - pub fn code(&self) -> &'static str { + pub fn code(&self) -> &'static CheckCode { match self { - CheckKind::DuplicateArgumentName => "F831", - CheckKind::FStringMissingPlaceholders => "F541", - CheckKind::IfTuple => "F634", - CheckKind::ImportStarUsage => "F403", - CheckKind::LineTooLong => "E501", + CheckKind::DuplicateArgumentName => &CheckCode::F831, + CheckKind::FStringMissingPlaceholders => &CheckCode::F541, + CheckKind::IfTuple => &CheckCode::F634, + CheckKind::ImportStarUsage => &CheckCode::F403, + CheckKind::LineTooLong => &CheckCode::E501, } } @@ -32,6 +70,17 @@ impl CheckKind { 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::LineTooLong => &LintSource::Lines, + } + } } #[derive(Debug, PartialEq, Eq)] diff --git a/src/linter.rs b/src/linter.rs index e0cd23b62b..8cf773fc49 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -6,6 +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::message::Message; use crate::settings::Settings; use crate::{cache, fs}; @@ -20,11 +21,31 @@ pub fn check_path(path: &Path, settings: &Settings, mode: &cache::Mode) -> Resul // Read the file from disk. let contents = fs::read_file(path)?; - // Run the linter. - let python_ast = parser::parse_program(&contents)?; - let messages: Vec = check_ast(&python_ast) + // Aggregate all checks. + let mut checks: Vec = vec![]; + + // Run the AST-based checks. + if settings + .select + .iter() + .any(|check_code| matches!(CheckKind::new(check_code).lint_source(), LintSource::AST)) + { + let python_ast = parser::parse_program(&contents)?; + checks.extend(check_ast(&python_ast, settings)); + } + + // Run the lines-based checks. + if settings + .select + .iter() + .any(|check_code| matches!(CheckKind::new(check_code).lint_source(), LintSource::Lines)) + { + checks.extend(check_lines(&contents, settings)); + } + + // Convert to messages. + let messages: Vec = checks .into_iter() - .chain(check_lines(&contents, settings)) .map(|check| Message { kind: check.kind, location: check.location, @@ -39,11 +60,13 @@ pub fn check_path(path: &Path, settings: &Settings, mode: &cache::Mode) -> Resul #[cfg(test)] mod tests { + use std::collections::HashSet; use std::path::Path; use anyhow::Result; use rustpython_parser::ast::Location; + use crate::checks::CheckCode; use crate::checks::CheckKind::{ DuplicateArgumentName, FStringMissingPlaceholders, IfTuple, ImportStarUsage, LineTooLong, }; @@ -58,6 +81,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + select: HashSet::from([CheckCode::F831]), }, &cache::Mode::None, )?; @@ -93,6 +117,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + select: HashSet::from([CheckCode::F541]), }, &cache::Mode::None, )?; @@ -128,6 +153,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + select: HashSet::from([CheckCode::F634]), }, &cache::Mode::None, )?; @@ -158,6 +184,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + select: HashSet::from([CheckCode::F403]), }, &cache::Mode::None, )?; @@ -188,6 +215,7 @@ mod tests { &settings::Settings { line_length: 88, exclude: vec![], + select: HashSet::from([CheckCode::E501]), }, &cache::Mode::None, )?; diff --git a/src/message.rs b/src/message.rs index bce133c705..0d3d0f451b 100644 --- a/src/message.rs +++ b/src/message.rs @@ -48,7 +48,7 @@ impl Message { .map(|code| code.trim()) .filter(|code| !code.is_empty()) { - if code == self.kind.code() { + if code == self.kind.code().as_str() { return true; } } @@ -75,7 +75,7 @@ impl fmt::Display for Message { ":".cyan(), self.location.column(), ":".cyan(), - self.kind.code().red().bold(), + self.kind.code().as_str().red().bold(), self.kind.body() ) } diff --git a/src/pyproject.rs b/src/pyproject.rs index 22e6bf9d08..8e7dbf94f1 100644 --- a/src/pyproject.rs +++ b/src/pyproject.rs @@ -1,5 +1,7 @@ +use std::collections::HashSet; use std::path::{Path, PathBuf}; +use crate::checks::CheckCode; use anyhow::Result; use common_path::common_path_all; use log::debug; @@ -30,6 +32,7 @@ pub fn load_config<'a>(paths: impl IntoIterator) -> Result<(Pat pub struct Config { pub line_length: Option, pub exclude: Option>, + pub select: Option>, } #[derive(Debug, PartialEq, Eq, Deserialize)] @@ -82,8 +85,10 @@ fn find_project_root<'a>(sources: impl IntoIterator) -> Option< #[cfg(test)] mod tests { + use std::collections::HashSet; use std::path::Path; + use crate::checks::CheckCode; use anyhow::Result; use crate::pyproject::{ @@ -113,7 +118,8 @@ mod tests { Some(Tools { linter: Some(Config { line_length: None, - exclude: None + exclude: None, + select: None, }) }) ); @@ -130,7 +136,8 @@ line-length = 79 Some(Tools { linter: Some(Config { line_length: Some(79), - exclude: None + exclude: None, + select: None, }) }) ); @@ -147,7 +154,26 @@ exclude = ["foo.py"] Some(Tools { linter: Some(Config { line_length: None, - exclude: Some(vec![Path::new("foo.py").to_path_buf()]) + exclude: Some(vec![Path::new("foo.py").to_path_buf()]), + select: None, + }) + }) + ); + + let pyproject: PyProject = toml::from_str( + r#" +[tool.black] +[tool.linter] +select = ["E501"] +"#, + )?; + assert_eq!( + pyproject.tool, + Some(Tools { + linter: Some(Config { + line_length: None, + exclude: None, + select: Some(HashSet::from([CheckCode::E501])), }) }) ); @@ -165,6 +191,15 @@ line_length = 79 r#" [tool.black] [tool.linter] +select = ["E123"] +"#, + ) + .is_err()); + + assert!(toml::from_str::( + r#" +[tool.black] +[tool.linter] line-length = 79 other-attribute = 1 "#, @@ -193,7 +228,8 @@ other-attribute = 1 config, Config { line_length: Some(88), - exclude: Some(vec![Path::new("excluded.py").to_path_buf()]) + exclude: Some(vec![Path::new("excluded.py").to_path_buf()]), + select: None, } ); diff --git a/src/settings.rs b/src/settings.rs index f85b50e323..0ebf251627 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -1,5 +1,7 @@ +use std::collections::HashSet; use std::path::{Path, PathBuf}; +use crate::checks::CheckCode; use anyhow::Result; use crate::pyproject::load_config; @@ -7,16 +9,14 @@ use crate::pyproject::load_config; pub struct Settings { pub line_length: usize, pub exclude: Vec, + pub select: HashSet, } -static DEFAULT_MAX_LINE_LENGTH: usize = 88; - impl Settings { pub fn from_paths<'a>(paths: impl IntoIterator) -> Result { let (project_root, config) = load_config(paths)?; - Ok(Settings { - line_length: config.line_length.unwrap_or(DEFAULT_MAX_LINE_LENGTH), + line_length: config.line_length.unwrap_or(88), exclude: config .exclude .unwrap_or_default() @@ -29,6 +29,15 @@ impl Settings { } }) .collect(), + select: config.select.unwrap_or_else(|| { + HashSet::from([ + CheckCode::F831, + CheckCode::F541, + CheckCode::F634, + CheckCode::F403, + CheckCode::E501, + ]) + }), }) } }