Extract ResolverSettings

## 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 extracts a `ResolverSettings` struct that holds all the resolver-relevant fields (uninteresting for the `Formatter` or `Linter`). This will allow us to move the `ResolverSettings` out of `ruff_linter` further up in the stack.


## Test Plan

`cargo test`

(I'll to more extensive testing at the top of this stack)
This commit is contained in:
Micha Reiser 2023-09-20 16:37:49 +02:00 committed by GitHub
parent 83daddbeb7
commit 8f41eab0c7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 117 additions and 86 deletions

View file

@ -60,7 +60,7 @@ fn ruff_check_paths(
cli.stdin_filename.as_deref(),
)?;
// We don't want to format pyproject.toml
pyproject_config.settings.include = FilePatternSet::try_from_vec(vec![
pyproject_config.settings.file_resolver.include = FilePatternSet::try_from_vec(vec![
FilePattern::Builtin("*.py"),
FilePattern::Builtin("*.pyi"),
])

View file

@ -16,9 +16,12 @@ pub(crate) fn check_file_path(
// flake8-no-pep420
if settings.rules.enabled(Rule::ImplicitNamespacePackage) {
if let Some(diagnostic) =
implicit_namespace_package(path, package, &settings.project_root, &settings.src)
{
if let Some(diagnostic) = implicit_namespace_package(
path,
package,
&settings.file_resolver.project_root,
&settings.src,
) {
diagnostics.push(diagnostic);
}
}

View file

@ -5,7 +5,7 @@ use ruff_cache::cache_dir;
use rustc_hash::FxHashSet;
use std::collections::HashSet;
use super::types::{FilePattern, PreviewMode, PythonVersion};
use super::types::{PreviewMode, PythonVersion};
use super::Settings;
use crate::codes::{self, RuleCodePrefix};
use crate::line_width::{LineLength, TabSize};
@ -18,7 +18,8 @@ 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::{FilePatternSet, SerializationFormat};
use crate::settings::types::SerializationFormat;
use crate::settings::FileResolverSettings;
pub const PREFIXES: &[RuleSelector] = &[
RuleSelector::Prefix {
@ -33,44 +34,6 @@ pub const TASK_TAGS: &[&str] = &["TODO", "FIXME", "XXX"];
pub static DUMMY_VARIABLE_RGX: Lazy<Regex> =
Lazy::new(|| Regex::new("^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$").unwrap());
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"),
]
});
impl Default for Settings {
fn default() -> Self {
let project_root = path_dedot::CWD.clone();
@ -82,6 +45,8 @@ impl Default for Settings {
show_fixes: false,
show_source: false,
file_resolver: FileResolverSettings::with_project_root(project_root),
rules: PREFIXES
.iter()
.flat_map(|selector| selector.rules(PreviewMode::default()))
@ -89,20 +54,14 @@ impl Default for Settings {
allowed_confusables: FxHashSet::from_iter([]),
builtins: vec![],
dummy_variable_rgx: DUMMY_VARIABLE_RGX.clone(),
exclude: FilePatternSet::try_from_vec(EXCLUDE.clone()).unwrap(),
extend_exclude: FilePatternSet::default(),
extend_include: FilePatternSet::default(),
external: HashSet::default(),
force_exclude: false,
ignore_init_module_imports: false,
include: FilePatternSet::try_from_vec(INCLUDE.clone()).unwrap(),
line_length: LineLength::default(),
logger_objects: vec![],
namespace_packages: vec![],
preview: PreviewMode::default(),
per_file_ignores: vec![],
project_root,
respect_gitignore: true,
src: vec![path_dedot::CWD.clone()],
tab_size: TabSize::default(),
target_version: PythonVersion::default(),

View file

@ -6,6 +6,7 @@ use std::path::PathBuf;
use anyhow::Result;
use globset::{Glob, GlobMatcher};
use once_cell::sync::Lazy;
use regex::Regex;
use rustc_hash::FxHashSet;
@ -19,7 +20,9 @@ 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::{FilePatternSet, PerFileIgnore, PythonVersion, SerializationFormat};
use crate::settings::types::{
FilePattern, FilePatternSet, PerFileIgnore, PythonVersion, SerializationFormat,
};
use super::line_width::{LineLength, TabSize};
@ -31,6 +34,69 @@ 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: PathBuf) -> Self {
Self {
project_root,
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)]
#[allow(clippy::struct_excessive_bools)]
pub struct Settings {
@ -47,21 +113,14 @@ pub struct Settings {
#[cache_key(ignore)]
pub show_source: bool,
pub file_resolver: FileResolverSettings,
pub rules: RuleTable,
pub per_file_ignores: Vec<(GlobMatcher, GlobMatcher, RuleSet)>,
pub target_version: PythonVersion,
pub preview: PreviewMode,
// Resolver settings
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,
// Rule-specific settings
pub allowed_confusables: FxHashSet<char>,
pub builtins: Vec<String>,

View file

@ -27,7 +27,9 @@ use ruff_linter::settings::types::{
FilePattern, FilePatternSet, PerFileIgnore, PreviewMode, PythonVersion, SerializationFormat,
Version,
};
use ruff_linter::settings::{defaults, resolve_per_file_ignores, Settings};
use ruff_linter::settings::{
defaults, resolve_per_file_ignores, FileResolverSettings, Settings, EXCLUDE, INCLUDE,
};
use ruff_linter::{
fs, warn_user, warn_user_once, warn_user_once_by_id, RuleSelector, RUFF_PKG_VERSION,
};
@ -139,16 +141,20 @@ impl Configuration {
dummy_variable_rgx: self
.dummy_variable_rgx
.unwrap_or_else(|| defaults::DUMMY_VARIABLE_RGX.clone()),
exclude: FilePatternSet::try_from_vec(
self.exclude.unwrap_or_else(|| defaults::EXCLUDE.clone()),
)?,
extend_exclude: FilePatternSet::try_from_vec(self.extend_exclude)?,
extend_include: FilePatternSet::try_from_vec(self.extend_include)?,
file_resolver: FileResolverSettings {
exclude: FilePatternSet::try_from_vec(
self.exclude.unwrap_or_else(|| EXCLUDE.clone()),
)?,
extend_exclude: FilePatternSet::try_from_vec(self.extend_exclude)?,
extend_include: FilePatternSet::try_from_vec(self.extend_include)?,
force_exclude: self.force_exclude.unwrap_or(false),
include: FilePatternSet::try_from_vec(
self.include.unwrap_or_else(|| INCLUDE.clone()),
)?,
respect_gitignore: self.respect_gitignore.unwrap_or(true),
project_root: project_root.to_path_buf(),
},
external: FxHashSet::from_iter(self.external.unwrap_or_default()),
force_exclude: self.force_exclude.unwrap_or(false),
include: FilePatternSet::try_from_vec(
self.include.unwrap_or_else(|| defaults::INCLUDE.clone()),
)?,
ignore_init_module_imports: self.ignore_init_module_imports.unwrap_or_default(),
line_length: self.line_length.unwrap_or_default(),
tab_size: self.tab_size.unwrap_or_default(),
@ -160,9 +166,7 @@ impl Configuration {
.chain(self.extend_per_file_ignores)
.collect(),
)?,
respect_gitignore: self.respect_gitignore.unwrap_or(true),
src: self.src.unwrap_or_else(|| vec![project_root.to_path_buf()]),
project_root: project_root.to_path_buf(),
target_version: self.target_version.unwrap_or_default(),
task_tags: self.task_tags.unwrap_or_else(|| {
defaults::TASK_TAGS

View file

@ -299,7 +299,7 @@ pub fn python_files_in_path(
}
// Check if the paths themselves are excluded.
if pyproject_config.settings.force_exclude {
if pyproject_config.settings.file_resolver.force_exclude {
paths.retain(|path| !is_file_excluded(path, &resolver, pyproject_config));
if paths.is_empty() {
return Ok((vec![], resolver));
@ -315,7 +315,7 @@ pub fn python_files_in_path(
for path in &paths[1..] {
builder.add(path);
}
builder.standard_filters(pyproject_config.settings.respect_gitignore);
builder.standard_filters(pyproject_config.settings.file_resolver.respect_gitignore);
builder.hidden(false);
let walker = builder.build_parallel();
@ -333,13 +333,17 @@ pub fn python_files_in_path(
let resolver = resolver.read().unwrap();
let settings = resolver.resolve(path, pyproject_config);
if let Some(file_name) = path.file_name() {
if !settings.exclude.is_empty()
&& match_exclusion(path, file_name, &settings.exclude)
if !settings.file_resolver.exclude.is_empty()
&& match_exclusion(path, file_name, &settings.file_resolver.exclude)
{
debug!("Ignored path via `exclude`: {:?}", path);
return WalkState::Skip;
} else if !settings.extend_exclude.is_empty()
&& match_exclusion(path, file_name, &settings.extend_exclude)
} else if !settings.file_resolver.extend_exclude.is_empty()
&& match_exclusion(
path,
file_name,
&settings.file_resolver.extend_exclude,
)
{
debug!("Ignored path via `extend-exclude`: {:?}", path);
return WalkState::Skip;
@ -395,10 +399,10 @@ pub fn python_files_in_path(
let path = entry.path();
let resolver = resolver.read().unwrap();
let settings = resolver.resolve(path, pyproject_config);
if settings.include.is_match(path) {
if settings.file_resolver.include.is_match(path) {
debug!("Included path via `include`: {:?}", path);
true
} else if settings.extend_include.is_match(path) {
} else if settings.file_resolver.extend_include.is_match(path) {
debug!("Included path via `extend-include`: {:?}", path);
true
} else {
@ -424,7 +428,7 @@ pub fn python_file_at_path(
pyproject_config: &PyprojectConfig,
transformer: &dyn ConfigurationTransformer,
) -> Result<bool> {
if !pyproject_config.settings.force_exclude {
if !pyproject_config.settings.file_resolver.force_exclude {
return Ok(true);
}
@ -460,11 +464,13 @@ fn is_file_excluded(
}
let settings = resolver.resolve(path, pyproject_strategy);
if let Some(file_name) = path.file_name() {
if !settings.exclude.is_empty() && match_exclusion(path, file_name, &settings.exclude) {
if !settings.file_resolver.exclude.is_empty()
&& match_exclusion(path, file_name, &settings.file_resolver.exclude)
{
debug!("Ignored path via `exclude`: {:?}", path);
return true;
} else if !settings.extend_exclude.is_empty()
&& match_exclusion(path, file_name, &settings.extend_exclude)
} else if !settings.file_resolver.extend_exclude.is_empty()
&& match_exclusion(path, file_name, &settings.file_resolver.extend_exclude)
{
debug!("Ignored path via `extend-exclude`: {:?}", path);
return true;
@ -473,7 +479,7 @@ fn is_file_excluded(
debug!("Ignored path due to error in parsing: {:?}", path);
return true;
}
if path == settings.project_root {
if path == settings.file_resolver.project_root {
// Bail out; we'd end up past the project root on the next iteration
// (excludes etc. are thus "rooted" to the project).
break;