From 654032196620b2a75d25767d144f643345c8ee20 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 20 Sep 2023 17:24:28 +0200 Subject: [PATCH] Move `Settings` and `ResolverSettings` to `ruff_workspace` ## Summary ## Stack Summary This stack splits `Settings` into `FormatterSettings` and `LinterSettings` and moves it into `ruff_workspace`. This change is necessary to add the `FormatterSettings` to `Settings` without adding `ruff_python_formatter` as a dependency to `ruff_linter` (and the linter should not contain the formatter settings). A quick overview of our settings struct at play: * `Options`: 1:1 representation of the options in the `pyproject.toml` or `ruff.toml`. Used for deserialization. * `Configuration`: Resolved `Options`, potentially merged from multiple configurations (when using `extend`). The representation is very close if not identical to the `Options`. * `Settings`: The resolved configuration that uses a data format optimized for reading. Optional fields are initialized with their default values. Initialized by `Configuration::into_settings` . The goal of this stack is to split `Settings` into tool-specific resolved `Settings` that are independent of each other. This comes at the advantage that the individual crates don't need to know anything about the other tools. The downside is that information gets duplicated between `Settings`. Right now the duplication is minimal (`line-length`, `tab-width`) but we may need to come up with a solution if more expensive data needs sharing. This stack focuses on `Settings`. Splitting `Configuration` into some smaller structs is something I'll follow up on later. ## PR Summary This PR moves the `ResolverSettings` and `Settings` struct to `ruff_workspace`. `LinterSettings` remains in `ruff_linter` because it gets passed to lint rules, the `Checker` etc. ## Test Plan `cargo test` --- Cargo.lock | 1 + crates/ruff_cli/src/cache.rs | 5 +- crates/ruff_cli/src/commands/check.rs | 3 +- crates/ruff_cli/src/diagnostics.rs | 3 +- crates/ruff_cli/src/lib.rs | 3 +- crates/ruff_dev/src/format_dev.rs | 2 +- crates/ruff_linter/src/settings/mod.rs | 108 +-------------------- crates/ruff_linter/src/settings/types.rs | 5 +- crates/ruff_wasm/src/lib.rs | 3 +- crates/ruff_workspace/Cargo.toml | 1 + crates/ruff_workspace/src/configuration.rs | 16 +-- crates/ruff_workspace/src/lib.rs | 3 + crates/ruff_workspace/src/resolver.rs | 4 +- crates/ruff_workspace/src/settings.rs | 101 +++++++++++++++++++ 14 files changed, 135 insertions(+), 123 deletions(-) create mode 100644 crates/ruff_workspace/src/settings.rs diff --git a/Cargo.lock b/Cargo.lock index a9d3035cf6..d202d037e9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2523,6 +2523,7 @@ dependencies = [ "ignore", "itertools 0.11.0", "log", + "once_cell", "path-absolutize", "pep440_rs", "regex", diff --git a/crates/ruff_cli/src/cache.rs b/crates/ruff_cli/src/cache.rs index c721bffcf2..3a2b851451 100644 --- a/crates/ruff_cli/src/cache.rs +++ b/crates/ruff_cli/src/cache.rs @@ -14,12 +14,12 @@ use serde::{Deserialize, Serialize}; use ruff_cache::{CacheKey, CacheKeyHasher}; use ruff_diagnostics::{DiagnosticKind, Fix}; use ruff_linter::message::Message; -use ruff_linter::settings::Settings; use ruff_linter::warn_user; use ruff_notebook::NotebookIndex; use ruff_python_ast::imports::ImportMap; use ruff_source_file::SourceFileBuilder; use ruff_text_size::{TextRange, TextSize}; +use ruff_workspace::Settings; use crate::diagnostics::Diagnostics; @@ -347,7 +347,7 @@ mod tests { use itertools::Itertools; use ruff_cache::CACHE_DIR_NAME; - use ruff_linter::settings::{flags, Settings}; + use ruff_linter::settings::flags; use crate::cache::RelativePathBuf; use crate::cache::{self, Cache, FileCache}; @@ -358,6 +358,7 @@ mod tests { use anyhow::Result; use ruff_python_ast::imports::ImportMap; + use ruff_workspace::Settings; use test_case::test_case; #[test_case("../ruff_linter/resources/test/fixtures", "ruff_tests/cache_same_results_ruff_linter"; "ruff_linter_fixtures")] diff --git a/crates/ruff_cli/src/commands/check.rs b/crates/ruff_cli/src/commands/check.rs index 1662b36af5..a16f9fef9a 100644 --- a/crates/ruff_cli/src/commands/check.rs +++ b/crates/ruff_cli/src/commands/check.rs @@ -238,8 +238,9 @@ mod test { use ruff_linter::message::{Emitter, EmitterContext, TextEmitter}; use ruff_linter::registry::Rule; - use ruff_linter::settings::{flags, LinterSettings, Settings}; + use ruff_linter::settings::{flags, LinterSettings}; use ruff_workspace::resolver::{PyprojectConfig, PyprojectDiscoveryStrategy}; + use ruff_workspace::Settings; use crate::args::CliOverrides; diff --git a/crates/ruff_cli/src/diagnostics.rs b/crates/ruff_cli/src/diagnostics.rs index 18acb45d55..13de795a86 100644 --- a/crates/ruff_cli/src/diagnostics.rs +++ b/crates/ruff_cli/src/diagnostics.rs @@ -22,7 +22,7 @@ use ruff_linter::logging::DisplayParseError; use ruff_linter::message::Message; use ruff_linter::pyproject_toml::lint_pyproject_toml; use ruff_linter::registry::AsRule; -use ruff_linter::settings::{flags, LinterSettings, Settings}; +use ruff_linter::settings::{flags, LinterSettings}; use ruff_linter::source_kind::SourceKind; use ruff_linter::{fs, IOError, SyntaxError}; use ruff_macros::CacheKey; @@ -31,6 +31,7 @@ use ruff_python_ast::imports::ImportMap; use ruff_python_ast::{PySourceType, SourceType, TomlSourceType}; use ruff_source_file::{LineIndex, SourceCode, SourceFileBuilder}; use ruff_text_size::{TextRange, TextSize}; +use ruff_workspace::Settings; use crate::cache::Cache; diff --git a/crates/ruff_cli/src/lib.rs b/crates/ruff_cli/src/lib.rs index 1f7e60b409..c7f0c34729 100644 --- a/crates/ruff_cli/src/lib.rs +++ b/crates/ruff_cli/src/lib.rs @@ -10,9 +10,10 @@ use log::warn; use notify::{recommended_watcher, RecursiveMode, Watcher}; use ruff_linter::logging::{set_up_logging, LogLevel}; +use ruff_linter::settings::flags; use ruff_linter::settings::types::SerializationFormat; -use ruff_linter::settings::{flags, Settings}; use ruff_linter::{fs, warn_user, warn_user_once}; +use ruff_workspace::Settings; use crate::args::{Args, CheckCommand, Command, FormatCommand}; use crate::printer::{Flags as PrinterFlags, Printer}; diff --git a/crates/ruff_dev/src/format_dev.rs b/crates/ruff_dev/src/format_dev.rs index 4a170bbca9..fb6d4650d3 100644 --- a/crates/ruff_dev/src/format_dev.rs +++ b/crates/ruff_dev/src/format_dev.rs @@ -60,7 +60,7 @@ fn ruff_check_paths( cli.stdin_filename.as_deref(), )?; // We don't want to format pyproject.toml - pyproject_config.settings.file_resolver.include = FilePatternSet::try_from_vec(vec![ + pyproject_config.settings.file_resolver.include = FilePatternSet::try_from_iter([ FilePattern::Builtin("*.py"), FilePattern::Builtin("*.pyi"), ]) diff --git a/crates/ruff_linter/src/settings/mod.rs b/crates/ruff_linter/src/settings/mod.rs index ba35f4969b..6ced3e3be0 100644 --- a/crates/ruff_linter/src/settings/mod.rs +++ b/crates/ruff_linter/src/settings/mod.rs @@ -10,7 +10,6 @@ use globset::{Glob, GlobMatcher}; use once_cell::sync::Lazy; use path_absolutize::path_dedot; use regex::Regex; -use ruff_cache::cache_dir; use rustc_hash::FxHashSet; use crate::codes::RuleCodePrefix; @@ -24,9 +23,7 @@ use crate::rules::{ flake8_tidy_imports, flake8_type_checking, flake8_unused_arguments, isort, mccabe, pep8_naming, pycodestyle, pydocstyle, pyflakes, pylint, pyupgrade, }; -use crate::settings::types::{ - FilePattern, FilePatternSet, PerFileIgnore, PythonVersion, SerializationFormat, -}; +use crate::settings::types::{PerFileIgnore, PythonVersion}; use crate::{codes, RuleSelector}; use super::line_width::{LineLength, TabSize}; @@ -38,69 +35,6 @@ pub mod flags; pub mod rule_table; pub mod types; -pub static EXCLUDE: Lazy> = Lazy::new(|| { - vec![ - FilePattern::Builtin(".bzr"), - FilePattern::Builtin(".direnv"), - FilePattern::Builtin(".eggs"), - FilePattern::Builtin(".git"), - FilePattern::Builtin(".git-rewrite"), - FilePattern::Builtin(".hg"), - FilePattern::Builtin(".ipynb_checkpoints"), - FilePattern::Builtin(".mypy_cache"), - FilePattern::Builtin(".nox"), - FilePattern::Builtin(".pants.d"), - FilePattern::Builtin(".pyenv"), - FilePattern::Builtin(".pytest_cache"), - FilePattern::Builtin(".pytype"), - FilePattern::Builtin(".ruff_cache"), - FilePattern::Builtin(".svn"), - FilePattern::Builtin(".tox"), - FilePattern::Builtin(".venv"), - FilePattern::Builtin(".vscode"), - FilePattern::Builtin("__pypackages__"), - FilePattern::Builtin("_build"), - FilePattern::Builtin("buck-out"), - FilePattern::Builtin("build"), - FilePattern::Builtin("dist"), - FilePattern::Builtin("node_modules"), - FilePattern::Builtin("venv"), - ] -}); - -pub static INCLUDE: Lazy> = Lazy::new(|| { - vec![ - FilePattern::Builtin("*.py"), - FilePattern::Builtin("*.pyi"), - FilePattern::Builtin("**/pyproject.toml"), - ] -}); - -#[derive(Debug, CacheKey)] -pub struct FileResolverSettings { - pub exclude: FilePatternSet, - pub extend_exclude: FilePatternSet, - pub force_exclude: bool, - pub include: FilePatternSet, - pub extend_include: FilePatternSet, - pub respect_gitignore: bool, - pub project_root: PathBuf, -} - -impl FileResolverSettings { - fn with_project_root(project_root: &Path) -> Self { - Self { - project_root: project_root.to_path_buf(), - exclude: FilePatternSet::try_from_vec(EXCLUDE.clone()).unwrap(), - extend_exclude: FilePatternSet::default(), - extend_include: FilePatternSet::default(), - force_exclude: false, - respect_gitignore: true, - include: FilePatternSet::try_from_vec(INCLUDE.clone()).unwrap(), - } - } -} - #[derive(Debug, CacheKey)] pub struct LinterSettings { pub project_root: PathBuf, @@ -181,7 +115,7 @@ impl LinterSettings { } } - pub fn with_project_root(project_root: &Path) -> Self { + pub fn new(project_root: &Path) -> Self { Self { target_version: PythonVersion::default(), project_root: project_root.to_path_buf(), @@ -246,30 +180,10 @@ impl LinterSettings { impl Default for LinterSettings { fn default() -> Self { - Self::with_project_root(path_dedot::CWD.as_path()) + Self::new(path_dedot::CWD.as_path()) } } -#[derive(Debug, CacheKey)] -#[allow(clippy::struct_excessive_bools)] -pub struct Settings { - #[cache_key(ignore)] - pub cache_dir: PathBuf, - #[cache_key(ignore)] - pub fix: bool, - #[cache_key(ignore)] - pub fix_only: bool, - #[cache_key(ignore)] - pub output_format: SerializationFormat, - #[cache_key(ignore)] - pub show_fixes: bool, - #[cache_key(ignore)] - pub show_source: bool, - - pub file_resolver: FileResolverSettings, - pub linter: LinterSettings, -} - /// Given a list of patterns, create a `GlobSet`. pub fn resolve_per_file_ignores( per_file_ignores: Vec, @@ -288,19 +202,3 @@ pub fn resolve_per_file_ignores( }) .collect() } - -impl Default for Settings { - fn default() -> Self { - let project_root = path_dedot::CWD.as_path(); - Self { - cache_dir: cache_dir(project_root), - fix: false, - fix_only: false, - output_format: SerializationFormat::default(), - show_fixes: false, - show_source: false, - linter: LinterSettings::with_project_root(project_root), - file_resolver: FileResolverSettings::with_project_root(project_root), - } - } -} diff --git a/crates/ruff_linter/src/settings/types.rs b/crates/ruff_linter/src/settings/types.rs index 9d3cbe0a00..2971ca30fc 100644 --- a/crates/ruff_linter/src/settings/types.rs +++ b/crates/ruff_linter/src/settings/types.rs @@ -152,7 +152,10 @@ pub struct FilePatternSet { } impl FilePatternSet { - pub fn try_from_vec(patterns: Vec) -> Result { + pub fn try_from_iter(patterns: I) -> Result + where + I: IntoIterator, + { let mut builder = GlobSetBuilder::new(); let mut hasher = CacheKeyHasher::new(); diff --git a/crates/ruff_wasm/src/lib.rs b/crates/ruff_wasm/src/lib.rs index ea9b5dff70..22774f27ba 100644 --- a/crates/ruff_wasm/src/lib.rs +++ b/crates/ruff_wasm/src/lib.rs @@ -11,7 +11,7 @@ use ruff_linter::line_width::{LineLength, TabSize}; use ruff_linter::linter::{check_path, LinterResult}; use ruff_linter::registry::AsRule; use ruff_linter::settings::types::{PreviewMode, PythonVersion}; -use ruff_linter::settings::{flags, Settings, DUMMY_VARIABLE_RGX, PREFIXES}; +use ruff_linter::settings::{flags, DUMMY_VARIABLE_RGX, PREFIXES}; use ruff_linter::source_kind::SourceKind; use ruff_python_ast::{Mod, PySourceType}; use ruff_python_codegen::Stylist; @@ -24,6 +24,7 @@ use ruff_source_file::{Locator, SourceLocation}; use ruff_text_size::Ranged; use ruff_workspace::configuration::Configuration; use ruff_workspace::options::Options; +use ruff_workspace::Settings; #[wasm_bindgen(typescript_custom_section)] const TYPES: &'static str = r#" diff --git a/crates/ruff_workspace/Cargo.toml b/crates/ruff_workspace/Cargo.toml index 3240b49124..dd741254eb 100644 --- a/crates/ruff_workspace/Cargo.toml +++ b/crates/ruff_workspace/Cargo.toml @@ -25,6 +25,7 @@ itertools = { workspace = true } log = { workspace = true } glob = { workspace = true } globset = { workspace = true } +once_cell = { workspace = true } path-absolutize = { workspace = true } pep440_rs = { version = "0.3.1", features = ["serde"] } regex = { workspace = true } diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index dd13b9cdcd..c1445276f7 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -14,6 +14,7 @@ use crate::options::{ Flake8UnusedArgumentsOptions, IsortOptions, McCabeOptions, Options, Pep8NamingOptions, PyUpgradeOptions, PycodestyleOptions, PydocstyleOptions, PyflakesOptions, PylintOptions, }; +use crate::settings::{FileResolverSettings, Settings, EXCLUDE, INCLUDE}; use anyhow::{anyhow, Result}; use glob::{glob, GlobError, Paths, PatternError}; use regex::Regex; @@ -28,8 +29,7 @@ use ruff_linter::settings::types::{ Version, }; use ruff_linter::settings::{ - resolve_per_file_ignores, FileResolverSettings, LinterSettings, Settings, DUMMY_VARIABLE_RGX, - EXCLUDE, INCLUDE, PREFIXES, TASK_TAGS, + resolve_per_file_ignores, LinterSettings, DUMMY_VARIABLE_RGX, PREFIXES, TASK_TAGS, }; use ruff_linter::{ fs, warn_user, warn_user_once, warn_user_once_by_id, RuleSelector, RUFF_PKG_VERSION, @@ -137,14 +137,14 @@ impl Configuration { show_source: self.show_source.unwrap_or(false), file_resolver: FileResolverSettings { - exclude: FilePatternSet::try_from_vec( - self.exclude.unwrap_or_else(|| EXCLUDE.clone()), + exclude: FilePatternSet::try_from_iter( + self.exclude.unwrap_or_else(|| EXCLUDE.to_vec()), )?, - extend_exclude: FilePatternSet::try_from_vec(self.extend_exclude)?, - extend_include: FilePatternSet::try_from_vec(self.extend_include)?, + extend_exclude: FilePatternSet::try_from_iter(self.extend_exclude)?, + extend_include: FilePatternSet::try_from_iter(self.extend_include)?, force_exclude: self.force_exclude.unwrap_or(false), - include: FilePatternSet::try_from_vec( - self.include.unwrap_or_else(|| INCLUDE.clone()), + include: FilePatternSet::try_from_iter( + self.include.unwrap_or_else(|| INCLUDE.to_vec()), )?, respect_gitignore: self.respect_gitignore.unwrap_or(true), project_root: project_root.to_path_buf(), diff --git a/crates/ruff_workspace/src/lib.rs b/crates/ruff_workspace/src/lib.rs index cc464ffd49..a18e151a35 100644 --- a/crates/ruff_workspace/src/lib.rs +++ b/crates/ruff_workspace/src/lib.rs @@ -4,6 +4,9 @@ pub mod pyproject; pub mod resolver; pub mod options_base; +mod settings; + +pub use settings::Settings; #[cfg(test)] mod tests { diff --git a/crates/ruff_workspace/src/resolver.rs b/crates/ruff_workspace/src/resolver.rs index dbe564167f..5075bb7153 100644 --- a/crates/ruff_workspace/src/resolver.rs +++ b/crates/ruff_workspace/src/resolver.rs @@ -14,12 +14,12 @@ use path_absolutize::path_dedot; use rustc_hash::{FxHashMap, FxHashSet}; use ruff_linter::packaging::is_package; -use ruff_linter::settings::Settings; use ruff_linter::{fs, warn_user_once}; use crate::configuration::Configuration; use crate::pyproject; use crate::pyproject::settings_toml; +use crate::settings::Settings; /// The configuration information from a `pyproject.toml` file. pub struct PyprojectConfig { @@ -513,7 +513,6 @@ mod tests { use tempfile::TempDir; use ruff_linter::settings::types::FilePattern; - use ruff_linter::settings::Settings; use crate::configuration::Configuration; use crate::pyproject::find_settings_toml; @@ -522,6 +521,7 @@ mod tests { ConfigurationTransformer, PyprojectConfig, PyprojectDiscoveryStrategy, Relativity, Resolver, }; + use crate::settings::Settings; use crate::tests::test_resource_path; struct NoOpTransformer; diff --git a/crates/ruff_workspace/src/settings.rs b/crates/ruff_workspace/src/settings.rs new file mode 100644 index 0000000000..f166a62ff4 --- /dev/null +++ b/crates/ruff_workspace/src/settings.rs @@ -0,0 +1,101 @@ +use path_absolutize::path_dedot; +use ruff_cache::cache_dir; +use ruff_linter::settings::types::{FilePattern, FilePatternSet, SerializationFormat}; +use ruff_linter::settings::LinterSettings; +use ruff_macros::CacheKey; +use std::path::{Path, PathBuf}; + +#[derive(Debug, CacheKey)] +#[allow(clippy::struct_excessive_bools)] +pub struct Settings { + #[cache_key(ignore)] + pub cache_dir: PathBuf, + #[cache_key(ignore)] + pub fix: bool, + #[cache_key(ignore)] + pub fix_only: bool, + #[cache_key(ignore)] + pub output_format: SerializationFormat, + #[cache_key(ignore)] + pub show_fixes: bool, + #[cache_key(ignore)] + pub show_source: bool, + + pub file_resolver: FileResolverSettings, + pub linter: LinterSettings, +} + +impl Default for Settings { + fn default() -> Self { + let project_root = path_dedot::CWD.as_path(); + Self { + cache_dir: cache_dir(project_root), + fix: false, + fix_only: false, + output_format: SerializationFormat::default(), + show_fixes: false, + show_source: false, + linter: LinterSettings::new(project_root), + file_resolver: FileResolverSettings::new(project_root), + } + } +} + +#[derive(Debug, CacheKey)] +pub struct FileResolverSettings { + pub exclude: FilePatternSet, + pub extend_exclude: FilePatternSet, + pub force_exclude: bool, + pub include: FilePatternSet, + pub extend_include: FilePatternSet, + pub respect_gitignore: bool, + pub project_root: PathBuf, +} + +pub(crate) static EXCLUDE: &[FilePattern] = &[ + FilePattern::Builtin(".bzr"), + FilePattern::Builtin(".direnv"), + FilePattern::Builtin(".eggs"), + FilePattern::Builtin(".git"), + FilePattern::Builtin(".git-rewrite"), + FilePattern::Builtin(".hg"), + FilePattern::Builtin(".ipynb_checkpoints"), + FilePattern::Builtin(".mypy_cache"), + FilePattern::Builtin(".nox"), + FilePattern::Builtin(".pants.d"), + FilePattern::Builtin(".pyenv"), + FilePattern::Builtin(".pytest_cache"), + FilePattern::Builtin(".pytype"), + FilePattern::Builtin(".ruff_cache"), + FilePattern::Builtin(".svn"), + FilePattern::Builtin(".tox"), + FilePattern::Builtin(".venv"), + FilePattern::Builtin(".vscode"), + FilePattern::Builtin("__pypackages__"), + FilePattern::Builtin("_build"), + FilePattern::Builtin("buck-out"), + FilePattern::Builtin("build"), + FilePattern::Builtin("dist"), + FilePattern::Builtin("node_modules"), + FilePattern::Builtin("venv"), +]; + +pub(crate) static INCLUDE: &[FilePattern] = &[ + FilePattern::Builtin("*.py"), + FilePattern::Builtin("*.pyi"), + FilePattern::Builtin("**/pyproject.toml"), +]; + +impl FileResolverSettings { + fn new(project_root: &Path) -> Self { + Self { + project_root: project_root.to_path_buf(), + exclude: FilePatternSet::try_from_iter(EXCLUDE.iter().cloned()).unwrap(), + extend_exclude: FilePatternSet::default(), + extend_include: FilePatternSet::default(), + force_exclude: false, + respect_gitignore: true, + include: FilePatternSet::try_from_iter(INCLUDE.iter().cloned()).unwrap(), + } + } +}