From 7359e862c1b82b3ce86e8b2d8eaff4aa11c76e50 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 20 Aug 2022 13:00:34 -0400 Subject: [PATCH] Add pyproject.toml support (#17) --- .gitignore | 2 +- Cargo.lock | 29 ++++- Cargo.toml | 3 + resources/test/src/pyproject.toml | 2 + src/bin/rust_python_linter.rs | 17 ++- src/check_lines.rs | 8 +- src/fs.rs | 5 +- src/lib.rs | 3 +- src/linter.rs | 12 +- src/pyproject.rs | 179 ++++++++++++++++++++++++++++++ src/settings.rs | 26 ++++- 11 files changed, 267 insertions(+), 19 deletions(-) create mode 100644 resources/test/src/pyproject.toml create mode 100644 src/pyproject.rs diff --git a/.gitignore b/.gitignore index 9082f5d7cfcadd..9a7b7b59812249 100644 --- a/.gitignore +++ b/.gitignore @@ -1,7 +1,7 @@ # Local cache .cache -resources/test !resources/test/src +resources/test ### # Rust.gitignore diff --git a/Cargo.lock b/Cargo.lock index 39858cc62d7944..3fe6496706b819 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -390,6 +390,12 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "common-path" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2382f75942f4b3be3690fe4f86365e9c853c1587d6ee58212cebf6e2a9ccd101" + [[package]] name = "concurrent-queue" version = "1.2.4" @@ -513,6 +519,15 @@ dependencies = [ "dirs-sys", ] +[[package]] +name = "dirs" +version = "4.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ca3aa72a6f96ea37bbc5aa912f6788242832f75369bdfdadcb0e38423f100059" +dependencies = [ + "dirs-sys", +] + [[package]] name = "dirs-sys" version = "0.3.7" @@ -1528,6 +1543,8 @@ dependencies = [ "clap", "clearscreen", "colored", + "common-path", + "dirs 4.0.0", "fern", "lazy_static", "log", @@ -1538,6 +1555,7 @@ dependencies = [ "rustpython-parser", "serde", "serde_json", + "toml", "walkdir", ] @@ -1787,7 +1805,7 @@ version = "0.7.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "76971977e6121664ec1b960d1313aacfa75642adc93b9d4d53b247bd4cb1747e" dependencies = [ - "dirs", + "dirs 2.0.2", "fnv", "nom", "phf 0.8.0", @@ -1840,6 +1858,15 @@ dependencies = [ "crunchy", ] +[[package]] +name = "toml" +version = "0.5.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8d82e1a7758622a465f8cee077614c73484dac5b836c02ff6a40d5d1010324d7" +dependencies = [ + "serde", +] + [[package]] name = "typenum" version = "1.15.0" diff --git a/Cargo.toml b/Cargo.toml index d99fa044e65f0e..d5295e46d3a14f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,8 @@ chrono = { version = "0.4.21" } clap = { version = "3.2.16", features = ["derive"] } clearscreen = { version = "1.0.10" } colored = { version = "2.0.0" } +common-path = "1.0.0" +dirs = "4.0.0" fern = { version = "0.6.1" } lazy_static = { version = "1.4.0" } log = { version = "0.4.17" } @@ -25,6 +27,7 @@ regex = { version = "1.6.0" } rustpython-parser = { git = "https://github.com/RustPython/RustPython.git", rev = "dff916d45c5d13074d21ad329a5ab68a6499426a" } serde = { version = "1.0.143", features = ["derive"] } serde_json = { version = "1.0.83" } +toml = { version = "0.5.9"} walkdir = { version = "2.3.2" } [profile.release] diff --git a/resources/test/src/pyproject.toml b/resources/test/src/pyproject.toml new file mode 100644 index 00000000000000..5293d5396a32ca --- /dev/null +++ b/resources/test/src/pyproject.toml @@ -0,0 +1,2 @@ +[tool.linter] +line-length = 88 diff --git a/src/bin/rust_python_linter.rs b/src/bin/rust_python_linter.rs index 573218a4586a08..f25a5602501f4f 100644 --- a/src/bin/rust_python_linter.rs +++ b/src/bin/rust_python_linter.rs @@ -1,4 +1,4 @@ -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::mpsc::channel; use std::time::Instant; @@ -15,6 +15,7 @@ use ::rust_python_linter::linter::check_path; use ::rust_python_linter::logging::set_up_logging; use ::rust_python_linter::message::Message; use ::rust_python_linter::tell_user; +use rust_python_linter::settings::Settings; #[derive(Debug, Parser)] #[clap(name = "rust-python-linter")] @@ -30,7 +31,7 @@ struct Cli { no_cache: bool, } -fn run_once(files: &[PathBuf], cache: bool) -> Result> { +fn run_once(files: &[PathBuf], settings: &Settings, cache: bool) -> Result> { // Collect all the files to check. let start = Instant::now(); let files: Vec = files.iter().flat_map(iter_python_files).collect(); @@ -41,7 +42,7 @@ fn run_once(files: &[PathBuf], cache: bool) -> Result> { let messages: Vec = files .par_iter() .map(|entry| { - check_path(entry.path(), &cache.into()).unwrap_or_else(|e| { + check_path(entry.path(), settings, &cache.into()).unwrap_or_else(|e| { error!("Failed to check {}: {e:?}", entry.path().to_string_lossy()); vec![] }) @@ -88,12 +89,16 @@ fn main() -> Result<()> { set_up_logging(cli.verbose)?; + // TODO(charlie): Avoid this cast. + let paths: Vec<&Path> = cli.files.iter().map(PathBuf::as_path).collect(); + let settings = Settings::from_paths(paths)?; + if cli.watch { // Perform an initial run instantly. clearscreen::clear()?; tell_user!("Starting linter in watch mode...\n"); - let messages = run_once(&cli.files, !cli.no_cache)?; + let messages = run_once(&cli.files, &settings, !cli.no_cache)?; report_continuously(&messages)?; // Configure the file watcher. @@ -111,7 +116,7 @@ fn main() -> Result<()> { clearscreen::clear()?; tell_user!("File change detected...\n"); - let messages = run_once(&cli.files, !cli.no_cache)?; + let messages = run_once(&cli.files, &settings, !cli.no_cache)?; report_continuously(&messages)?; } } @@ -120,7 +125,7 @@ fn main() -> Result<()> { } } } else { - let messages = run_once(&cli.files, !cli.no_cache)?; + let messages = run_once(&cli.files, &settings, !cli.no_cache)?; report_once(&messages)?; } diff --git a/src/check_lines.rs b/src/check_lines.rs index 8d5235a5a61794..c67210e982a598 100644 --- a/src/check_lines.rs +++ b/src/check_lines.rs @@ -2,21 +2,21 @@ use rustpython_parser::ast::Location; use crate::checks::Check; use crate::checks::CheckKind::LineTooLong; -use crate::settings::MAX_LINE_LENGTH; +use crate::settings::Settings; -pub fn check_lines(contents: &str) -> Vec { +pub fn check_lines(contents: &str, settings: &Settings) -> Vec { contents .lines() .enumerate() .filter_map(|(row, line)| { - if line.len() > *MAX_LINE_LENGTH { + if 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, - location: Location::new(row + 1, MAX_LINE_LENGTH + 1), + location: Location::new(row + 1, settings.line_length + 1), }) } } else { diff --git a/src/fs.rs b/src/fs.rs index 8407e40a58e5f8..bfc4fc6ba8a25b 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -1,10 +1,11 @@ -use anyhow::Result; -use lazy_static::lazy_static; use std::collections::HashSet; use std::fs::File; use std::io::{BufRead, BufReader, Read}; use std::ops::Deref; use std::path::{Path, PathBuf}; + +use anyhow::Result; +use lazy_static::lazy_static; use walkdir::{DirEntry, WalkDir}; lazy_static! { diff --git a/src/lib.rs b/src/lib.rs index e15e1a3d7c4587..a5c2d7803ba072 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -6,5 +6,6 @@ pub mod fs; pub mod linter; pub mod logging; pub mod message; -mod settings; +mod pyproject; +pub mod settings; mod visitor; diff --git a/src/linter.rs b/src/linter.rs index 577949bd0d0570..f2eb1a9e0b9ee7 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -7,9 +7,10 @@ use rustpython_parser::parser; use crate::check_ast::check_ast; use crate::check_lines::check_lines; use crate::message::Message; +use crate::settings::Settings; use crate::{cache, fs}; -pub fn check_path(path: &Path, mode: &cache::Mode) -> Result> { +pub fn check_path(path: &Path, settings: &Settings, mode: &cache::Mode) -> Result> { // Check the cache. if let Some(messages) = cache::get(path, mode) { debug!("Cache hit for: {}", path.to_string_lossy()); @@ -23,7 +24,7 @@ pub fn check_path(path: &Path, mode: &cache::Mode) -> Result> { let python_ast = parser::parse_program(&contents)?; let messages: Vec = check_ast(&python_ast) .into_iter() - .chain(check_lines(&contents)) + .chain(check_lines(&contents, settings)) .map(|check| Message { kind: check.kind, location: check.location, @@ -43,17 +44,18 @@ mod tests { use anyhow::Result; use rustpython_parser::ast::Location; - use crate::cache; use crate::checks::CheckKind::{ DuplicateArgumentName, FStringMissingPlaceholders, IfTuple, ImportStarUsage, LineTooLong, }; use crate::linter::check_path; use crate::message::Message; + use crate::{cache, settings}; #[test] fn duplicate_argument_name() -> Result<()> { let actual = check_path( &Path::new("./resources/test/src/duplicate_argument_name.py"), + &settings::Settings { line_length: 88 }, &cache::Mode::None, )?; let expected = vec![ @@ -85,6 +87,7 @@ mod tests { fn f_string_missing_placeholders() -> Result<()> { let actual = check_path( &Path::new("./resources/test/src/f_string_missing_placeholders.py"), + &settings::Settings { line_length: 88 }, &cache::Mode::None, )?; let expected = vec![ @@ -116,6 +119,7 @@ mod tests { fn if_tuple() -> Result<()> { let actual = check_path( &Path::new("./resources/test/src/if_tuple.py"), + &settings::Settings { line_length: 88 }, &cache::Mode::None, )?; let expected = vec![ @@ -142,6 +146,7 @@ mod tests { fn import_star_usage() -> Result<()> { let actual = check_path( &Path::new("./resources/test/src/import_star_usage.py"), + &settings::Settings { line_length: 88 }, &cache::Mode::None, )?; let expected = vec![ @@ -168,6 +173,7 @@ mod tests { fn line_too_long() -> Result<()> { let actual = check_path( &Path::new("./resources/test/src/line_too_long.py"), + &settings::Settings { line_length: 88 }, &cache::Mode::None, )?; let expected = vec![Message { diff --git a/src/pyproject.rs b/src/pyproject.rs new file mode 100644 index 00000000000000..64a27a8b5522e2 --- /dev/null +++ b/src/pyproject.rs @@ -0,0 +1,179 @@ +use std::path::{Path, PathBuf}; + +use anyhow::Result; +use common_path::common_path_all; +use log::debug; +use serde::Deserialize; + +use crate::fs; + +pub fn load_config<'a>(paths: impl IntoIterator) -> Result { + match find_project_root(paths) { + Some(project_root) => match find_pyproject_toml(&project_root) { + Some(path) => { + debug!("Found pyproject.toml at: {}", path.to_string_lossy()); + let pyproject = parse_pyproject_toml(&path)?; + let config = pyproject + .tool + .and_then(|tool| tool.linter) + .unwrap_or_default(); + Ok(config) + } + None => Ok(Default::default()), + }, + None => Ok(Default::default()), + } +} + +#[derive(Debug, PartialEq, Eq, Deserialize, Default)] +#[serde(deny_unknown_fields, rename_all = "kebab-case")] +pub struct Config { + pub line_length: Option, +} + +#[derive(Debug, PartialEq, Eq, Deserialize)] +struct Tools { + linter: Option, +} + +#[derive(Debug, PartialEq, Eq, Deserialize)] +struct PyProject { + tool: Option, +} + +fn parse_pyproject_toml(path: &Path) -> Result { + let contents = fs::read_file(path)?; + toml::from_str(&contents).map_err(|e| e.into()) +} + +// https://github.com/psf/black/blob/44d5da00b520a05cd56e58b3998660f64ea59ebd/src/black/files.py#L84 +fn find_pyproject_toml(path: &Path) -> Option { + let path_pyproject_toml = path.join("pyproject.toml"); + if path_pyproject_toml.is_file() { + return Some(path_pyproject_toml); + } + find_user_pyproject_toml() +} + +// https://github.com/psf/black/blob/44d5da00b520a05cd56e58b3998660f64ea59ebd/src/black/files.py#L117 +fn find_user_pyproject_toml() -> Option { + dirs::home_dir().map(|path| path.join(".linter")) +} + +// https://github.com/psf/black/blob/44d5da00b520a05cd56e58b3998660f64ea59ebd/src/black/files.py#L42 +fn find_project_root<'a>(sources: impl IntoIterator) -> Option { + if let Some(prefix) = common_path_all(sources) { + for directory in prefix.ancestors() { + if directory.join(".git").is_dir() { + return Some(directory.to_path_buf()); + } + if directory.join(".hg").is_dir() { + return Some(directory.to_path_buf()); + } + if directory.join("pyproject.toml").is_file() { + return Some(directory.to_path_buf()); + } + } + } + + None +} + +#[cfg(test)] +mod tests { + use std::path::Path; + + use anyhow::Result; + + use crate::pyproject::{ + find_project_root, find_pyproject_toml, parse_pyproject_toml, Config, PyProject, Tools, + }; + + #[test] + fn deserialize() -> Result<()> { + let pyproject: PyProject = toml::from_str(r#""#)?; + assert_eq!(pyproject.tool, None); + + let pyproject: PyProject = toml::from_str( + r#" +[tool.black] +"#, + )?; + assert_eq!(pyproject.tool, Some(Tools { linter: None })); + + let pyproject: PyProject = toml::from_str( + r#" +[tool.black] +[tool.linter] +"#, + )?; + assert_eq!( + pyproject.tool, + Some(Tools { + linter: Some(Config { line_length: None }) + }) + ); + + let pyproject: PyProject = toml::from_str( + r#" +[tool.black] +[tool.linter] +line-length = 79 +"#, + )?; + assert_eq!( + pyproject.tool, + Some(Tools { + linter: Some(Config { + line_length: Some(79) + }) + }) + ); + + assert!(toml::from_str::( + r#" +[tool.black] +[tool.linter] +line_length = 79 +"#, + ) + .is_err()); + + assert!(toml::from_str::( + r#" +[tool.black] +[tool.linter] +line-length = 79 +other-attribute = 1 +"#, + ) + .is_err()); + + Ok(()) + } + + #[test] + fn find_and_parse_pyproject_toml() -> Result<()> { + let project_root = find_project_root([Path::new("resources/test/src/__init__.py")]) + .expect("Unable to find project root."); + assert_eq!(project_root, Path::new("resources/test/src")); + + let path = find_pyproject_toml(&project_root).expect("Unable to find pyproject.toml."); + assert_eq!(path, Path::new("resources/test/src/pyproject.toml")); + + let pyproject = parse_pyproject_toml(&path)?; + let config = pyproject + .tool + .map(|tool| tool.linter) + .flatten() + .expect("Unable to find tool.linter."); + assert_eq!( + config, + Config { + line_length: Some(88) + } + ); + + Ok(()) + } +} diff --git a/src/settings.rs b/src/settings.rs index f21a4348f38b9a..015ffd516636e1 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -1 +1,25 @@ -pub static MAX_LINE_LENGTH: &usize = &88; +use std::path::Path; + +use anyhow::Result; + +use crate::pyproject::{load_config, Config}; + +pub struct Settings { + pub line_length: usize, +} + +static DEFAULT_MAX_LINE_LENGTH: usize = 88; + +impl From for Settings { + fn from(config: Config) -> Settings { + Settings { + line_length: config.line_length.unwrap_or(DEFAULT_MAX_LINE_LENGTH), + } + } +} + +impl Settings { + pub fn from_paths<'a>(paths: impl IntoIterator) -> Result { + load_config(paths).map(|config| config.into()) + } +}