Skip to content

Commit

Permalink
Move Settings and ResolverSettings to ruff_workspace
Browse files Browse the repository at this point in the history
## 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`
  • Loading branch information
MichaReiser committed Sep 20, 2023
1 parent b34278e commit 6540321
Show file tree
Hide file tree
Showing 14 changed files with 135 additions and 123 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions crates/ruff_cli/src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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};
Expand All @@ -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")]
Expand Down
3 changes: 2 additions & 1 deletion crates/ruff_cli/src/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
3 changes: 2 additions & 1 deletion crates/ruff_cli/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down
3 changes: 2 additions & 1 deletion crates/ruff_cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_dev/src/format_dev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
])
Expand Down
108 changes: 3 additions & 105 deletions crates/ruff_linter/src/settings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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};
Expand All @@ -38,69 +35,6 @@ pub mod flags;
pub mod rule_table;
pub mod types;

pub static EXCLUDE: Lazy<Vec<FilePattern>> = 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<Vec<FilePattern>> = 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,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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<PerFileIgnore>,
Expand All @@ -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),
}
}
}
5 changes: 4 additions & 1 deletion crates/ruff_linter/src/settings/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,10 @@ pub struct FilePatternSet {
}

impl FilePatternSet {
pub fn try_from_vec(patterns: Vec<FilePattern>) -> Result<Self, anyhow::Error> {
pub fn try_from_iter<I>(patterns: I) -> Result<Self, anyhow::Error>
where
I: IntoIterator<Item = FilePattern>,
{
let mut builder = GlobSetBuilder::new();
let mut hasher = CacheKeyHasher::new();

Expand Down
3 changes: 2 additions & 1 deletion crates/ruff_wasm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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#"
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_workspace/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
16 changes: 8 additions & 8 deletions crates/ruff_workspace/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down
3 changes: 3 additions & 0 deletions crates/ruff_workspace/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ pub mod pyproject;
pub mod resolver;

pub mod options_base;
mod settings;

pub use settings::Settings;

#[cfg(test)]
mod tests {
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_workspace/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand All @@ -522,6 +521,7 @@ mod tests {
ConfigurationTransformer, PyprojectConfig, PyprojectDiscoveryStrategy, Relativity,
Resolver,
};
use crate::settings::Settings;
use crate::tests::test_resource_path;

struct NoOpTransformer;
Expand Down

0 comments on commit 6540321

Please sign in to comment.