Add ability to selectively enable errors (#19)

This commit is contained in:
Charlie Marsh 2022-08-20 13:12:17 -04:00 committed by GitHub
parent b11a7eefa3
commit 4c62e1e22e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 241 additions and 83 deletions

View file

@ -3,18 +3,33 @@ use std::collections::HashSet;
use rustpython_parser::ast::{Arg, Arguments, Expr, ExprKind, Stmt, StmtKind, Suite}; use rustpython_parser::ast::{Arg, Arguments, Expr, ExprKind, Stmt, StmtKind, Suite};
use crate::checks::{Check, CheckKind}; use crate::checks::{Check, CheckKind};
use crate::settings::Settings;
use crate::visitor; use crate::visitor;
use crate::visitor::Visitor; use crate::visitor::Visitor;
#[derive(Default)] struct Checker<'a> {
struct Checker { settings: &'a Settings,
checks: Vec<Check>, checks: Vec<Check>,
} }
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) { fn visit_stmt(&mut self, stmt: &Stmt) {
match &stmt.node { match &stmt.node {
StmtKind::ImportFrom { names, .. } => { StmtKind::ImportFrom { names, .. } => {
if self
.settings
.select
.contains(CheckKind::ImportStarUsage.code())
{
for alias in names { for alias in names {
if alias.name == "*" { if alias.name == "*" {
self.checks.push(Check { self.checks.push(Check {
@ -24,7 +39,9 @@ impl Visitor for Checker {
} }
} }
} }
}
StmtKind::If { test, .. } => { StmtKind::If { test, .. } => {
if self.settings.select.contains(CheckKind::IfTuple.code()) {
if let ExprKind::Tuple { .. } = test.node { if let ExprKind::Tuple { .. } = test.node {
self.checks.push(Check { self.checks.push(Check {
kind: CheckKind::IfTuple, kind: CheckKind::IfTuple,
@ -32,6 +49,7 @@ impl Visitor for Checker {
}); });
} }
} }
}
_ => {} _ => {}
} }
@ -39,6 +57,11 @@ impl Visitor for Checker {
} }
fn visit_expr(&mut self, expr: &Expr) { fn visit_expr(&mut self, expr: &Expr) {
if self
.settings
.select
.contains(CheckKind::FStringMissingPlaceholders.code())
{
if let ExprKind::JoinedStr { values } = &expr.node { if let ExprKind::JoinedStr { values } = &expr.node {
if !values if !values
.iter() .iter()
@ -50,11 +73,17 @@ impl Visitor for Checker {
}); });
} }
} }
}
visitor::walk_expr(self, expr); visitor::walk_expr(self, expr);
} }
fn visit_arguments(&mut self, arguments: &Arguments) { fn visit_arguments(&mut self, arguments: &Arguments) {
if self
.settings
.select
.contains(CheckKind::DuplicateArgumentName.code())
{
// Collect all the arguments into a single vector. // Collect all the arguments into a single vector.
let mut all_arguments: Vec<&Arg> = arguments let mut all_arguments: Vec<&Arg> = arguments
.args .args
@ -82,16 +111,17 @@ impl Visitor for Checker {
} }
idents.insert(ident.clone()); idents.insert(ident.clone());
} }
}
visitor::walk_arguments(self, arguments); visitor::walk_arguments(self, arguments);
} }
} }
pub fn check_ast(python_ast: &Suite) -> Vec<Check> { pub fn check_ast(python_ast: &Suite, settings: &Settings) -> Vec<Check> {
python_ast python_ast
.iter() .iter()
.flat_map(|stmt| { .flat_map(|stmt| {
let mut checker: Checker = Default::default(); let mut checker = Checker::new(settings);
checker.visit_stmt(stmt); checker.visit_stmt(stmt);
checker.checks checker.checks
}) })
@ -101,15 +131,22 @@ pub fn check_ast(python_ast: &Suite) -> Vec<Check> {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use rustpython_parser::ast::{Alias, Location, Stmt, StmtKind}; use rustpython_parser::ast::{Alias, Location, Stmt, StmtKind};
use std::collections::HashSet;
use crate::check_ast::Checker; use crate::check_ast::Checker;
use crate::checks::Check;
use crate::checks::CheckKind::ImportStarUsage; use crate::checks::CheckKind::ImportStarUsage;
use crate::checks::{Check, CheckCode};
use crate::settings::Settings;
use crate::visitor::Visitor; use crate::visitor::Visitor;
#[test] #[test]
fn import_star_usage() { 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 { checker.visit_stmt(&Stmt {
location: Location::new(1, 1), location: Location::new(1, 1),
custom: (), custom: (),

View file

@ -1,7 +1,6 @@
use rustpython_parser::ast::Location; use rustpython_parser::ast::Location;
use crate::checks::Check; use crate::checks::{Check, CheckKind};
use crate::checks::CheckKind::LineTooLong;
use crate::settings::Settings; use crate::settings::Settings;
pub fn check_lines(contents: &str, settings: &Settings) -> Vec<Check> { pub fn check_lines(contents: &str, settings: &Settings) -> Vec<Check> {
@ -9,19 +8,19 @@ pub fn check_lines(contents: &str, settings: &Settings) -> Vec<Check> {
.lines() .lines()
.enumerate() .enumerate()
.filter_map(|(row, line)| { .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(); let chunks: Vec<&str> = line.split_whitespace().collect();
if chunks.len() == 1 || (chunks.len() == 2 && chunks[0] == "#") { if !(chunks.len() == 1 || (chunks.len() == 2 && chunks[0] == "#")) {
None return Some(Check {
} else { kind: CheckKind::LineTooLong,
Some(Check {
kind: LineTooLong,
location: Location::new(row + 1, settings.line_length + 1), location: Location::new(row + 1, settings.line_length + 1),
}) });
} }
} else { }
None None
}
}) })
.collect() .collect()
} }

View file

@ -1,6 +1,33 @@
use rustpython_parser::ast::Location; use rustpython_parser::ast::Location;
use serde::{Deserialize, Serialize}; 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)] #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)]
pub enum CheckKind { pub enum CheckKind {
DuplicateArgumentName, DuplicateArgumentName,
@ -11,14 +38,25 @@ pub enum CheckKind {
} }
impl 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. /// A four-letter shorthand code for the check.
pub fn code(&self) -> &'static str { pub fn code(&self) -> &'static CheckCode {
match self { match self {
CheckKind::DuplicateArgumentName => "F831", CheckKind::DuplicateArgumentName => &CheckCode::F831,
CheckKind::FStringMissingPlaceholders => "F541", CheckKind::FStringMissingPlaceholders => &CheckCode::F541,
CheckKind::IfTuple => "F634", CheckKind::IfTuple => &CheckCode::F634,
CheckKind::ImportStarUsage => "F403", CheckKind::ImportStarUsage => &CheckCode::F403,
CheckKind::LineTooLong => "E501", CheckKind::LineTooLong => &CheckCode::E501,
} }
} }
@ -32,6 +70,17 @@ impl CheckKind {
CheckKind::LineTooLong => "Line too long", 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)] #[derive(Debug, PartialEq, Eq)]

View file

@ -6,6 +6,7 @@ use rustpython_parser::parser;
use crate::check_ast::check_ast; use crate::check_ast::check_ast;
use crate::check_lines::check_lines; use crate::check_lines::check_lines;
use crate::checks::{Check, CheckKind, LintSource};
use crate::message::Message; use crate::message::Message;
use crate::settings::Settings; use crate::settings::Settings;
use crate::{cache, fs}; 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. // Read the file from disk.
let contents = fs::read_file(path)?; let contents = fs::read_file(path)?;
// Run the linter. // Aggregate all checks.
let mut checks: Vec<Check> = 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)?; let python_ast = parser::parse_program(&contents)?;
let messages: Vec<Message> = check_ast(&python_ast) 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<Message> = checks
.into_iter() .into_iter()
.chain(check_lines(&contents, settings))
.map(|check| Message { .map(|check| Message {
kind: check.kind, kind: check.kind,
location: check.location, location: check.location,
@ -39,11 +60,13 @@ pub fn check_path(path: &Path, settings: &Settings, mode: &cache::Mode) -> Resul
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use std::collections::HashSet;
use std::path::Path; use std::path::Path;
use anyhow::Result; use anyhow::Result;
use rustpython_parser::ast::Location; use rustpython_parser::ast::Location;
use crate::checks::CheckCode;
use crate::checks::CheckKind::{ use crate::checks::CheckKind::{
DuplicateArgumentName, FStringMissingPlaceholders, IfTuple, ImportStarUsage, LineTooLong, DuplicateArgumentName, FStringMissingPlaceholders, IfTuple, ImportStarUsage, LineTooLong,
}; };
@ -58,6 +81,7 @@ mod tests {
&settings::Settings { &settings::Settings {
line_length: 88, line_length: 88,
exclude: vec![], exclude: vec![],
select: HashSet::from([CheckCode::F831]),
}, },
&cache::Mode::None, &cache::Mode::None,
)?; )?;
@ -93,6 +117,7 @@ mod tests {
&settings::Settings { &settings::Settings {
line_length: 88, line_length: 88,
exclude: vec![], exclude: vec![],
select: HashSet::from([CheckCode::F541]),
}, },
&cache::Mode::None, &cache::Mode::None,
)?; )?;
@ -128,6 +153,7 @@ mod tests {
&settings::Settings { &settings::Settings {
line_length: 88, line_length: 88,
exclude: vec![], exclude: vec![],
select: HashSet::from([CheckCode::F634]),
}, },
&cache::Mode::None, &cache::Mode::None,
)?; )?;
@ -158,6 +184,7 @@ mod tests {
&settings::Settings { &settings::Settings {
line_length: 88, line_length: 88,
exclude: vec![], exclude: vec![],
select: HashSet::from([CheckCode::F403]),
}, },
&cache::Mode::None, &cache::Mode::None,
)?; )?;
@ -188,6 +215,7 @@ mod tests {
&settings::Settings { &settings::Settings {
line_length: 88, line_length: 88,
exclude: vec![], exclude: vec![],
select: HashSet::from([CheckCode::E501]),
}, },
&cache::Mode::None, &cache::Mode::None,
)?; )?;

View file

@ -48,7 +48,7 @@ impl Message {
.map(|code| code.trim()) .map(|code| code.trim())
.filter(|code| !code.is_empty()) .filter(|code| !code.is_empty())
{ {
if code == self.kind.code() { if code == self.kind.code().as_str() {
return true; return true;
} }
} }
@ -75,7 +75,7 @@ impl fmt::Display for Message {
":".cyan(), ":".cyan(),
self.location.column(), self.location.column(),
":".cyan(), ":".cyan(),
self.kind.code().red().bold(), self.kind.code().as_str().red().bold(),
self.kind.body() self.kind.body()
) )
} }

View file

@ -1,5 +1,7 @@
use std::collections::HashSet;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use crate::checks::CheckCode;
use anyhow::Result; use anyhow::Result;
use common_path::common_path_all; use common_path::common_path_all;
use log::debug; use log::debug;
@ -30,6 +32,7 @@ pub fn load_config<'a>(paths: impl IntoIterator<Item = &'a Path>) -> Result<(Pat
pub struct Config { pub struct Config {
pub line_length: Option<usize>, pub line_length: Option<usize>,
pub exclude: Option<Vec<PathBuf>>, pub exclude: Option<Vec<PathBuf>>,
pub select: Option<HashSet<CheckCode>>,
} }
#[derive(Debug, PartialEq, Eq, Deserialize)] #[derive(Debug, PartialEq, Eq, Deserialize)]
@ -82,8 +85,10 @@ fn find_project_root<'a>(sources: impl IntoIterator<Item = &'a Path>) -> Option<
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use std::collections::HashSet;
use std::path::Path; use std::path::Path;
use crate::checks::CheckCode;
use anyhow::Result; use anyhow::Result;
use crate::pyproject::{ use crate::pyproject::{
@ -113,7 +118,8 @@ mod tests {
Some(Tools { Some(Tools {
linter: Some(Config { linter: Some(Config {
line_length: None, line_length: None,
exclude: None exclude: None,
select: None,
}) })
}) })
); );
@ -130,7 +136,8 @@ line-length = 79
Some(Tools { Some(Tools {
linter: Some(Config { linter: Some(Config {
line_length: Some(79), line_length: Some(79),
exclude: None exclude: None,
select: None,
}) })
}) })
); );
@ -147,7 +154,26 @@ exclude = ["foo.py"]
Some(Tools { Some(Tools {
linter: Some(Config { linter: Some(Config {
line_length: None, 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#" r#"
[tool.black] [tool.black]
[tool.linter] [tool.linter]
select = ["E123"]
"#,
)
.is_err());
assert!(toml::from_str::<PyProject>(
r#"
[tool.black]
[tool.linter]
line-length = 79 line-length = 79
other-attribute = 1 other-attribute = 1
"#, "#,
@ -193,7 +228,8 @@ other-attribute = 1
config, config,
Config { Config {
line_length: Some(88), 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,
} }
); );

View file

@ -1,5 +1,7 @@
use std::collections::HashSet;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use crate::checks::CheckCode;
use anyhow::Result; use anyhow::Result;
use crate::pyproject::load_config; use crate::pyproject::load_config;
@ -7,16 +9,14 @@ use crate::pyproject::load_config;
pub struct Settings { pub struct Settings {
pub line_length: usize, pub line_length: usize,
pub exclude: Vec<PathBuf>, pub exclude: Vec<PathBuf>,
pub select: HashSet<CheckCode>,
} }
static DEFAULT_MAX_LINE_LENGTH: usize = 88;
impl Settings { impl Settings {
pub fn from_paths<'a>(paths: impl IntoIterator<Item = &'a Path>) -> Result<Self> { pub fn from_paths<'a>(paths: impl IntoIterator<Item = &'a Path>) -> Result<Self> {
let (project_root, config) = load_config(paths)?; let (project_root, config) = load_config(paths)?;
Ok(Settings { Ok(Settings {
line_length: config.line_length.unwrap_or(DEFAULT_MAX_LINE_LENGTH), line_length: config.line_length.unwrap_or(88),
exclude: config exclude: config
.exclude .exclude
.unwrap_or_default() .unwrap_or_default()
@ -29,6 +29,15 @@ impl Settings {
} }
}) })
.collect(), .collect(),
select: config.select.unwrap_or_else(|| {
HashSet::from([
CheckCode::F831,
CheckCode::F541,
CheckCode::F634,
CheckCode::F403,
CheckCode::E501,
])
}),
}) })
} }
} }