mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-09-26 11:59:35 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

refactor: Split CliSettings from Settings

Browse source 
We want to automatically derive Hash for the library settings, which
requires us to split off all the settings unused by the library
(since these shouldn't affect the hash used by ruff_cli::cache).
This commit is contained in:
Martin Fischer 2023-01-15 10:28:00 +01:00 • committed by Charlie Marsh
parent ef80ab205c
commit d75d6d7c7c
7 changed files with 100 additions and 76 deletions
Download patch file Download diff file Expand all files Collapse all files

16
ruff_cli/src/cache.rs
View file

@ -9,7 +9,7 @@ use filetime::FileTime;
use log::error;
use path_absolutize::Absolutize;
use ruff::message::Message;
use ruff::settings::{flags, Settings};
use ruff::settings::{flags, AllSettings, Settings};
use serde::{Deserialize, Serialize};
const CARGO_PKG_VERSION: &str = env!("CARGO_PKG_VERSION");
@ -80,10 +80,14 @@ fn read_sync(cache_dir: &Path, key: u64) -> Result<Vec<u8>, std::io::Error> {
pub fn get<P: AsRef<Path>>(
path: P,
metadata: &fs::Metadata,
settings: &Settings,
settings: &AllSettings,
autofix: flags::Autofix,
) -> Option<Vec<Message>> {
let encoded = read_sync(&settings.cache_dir, cache_key(path, settings, autofix)).ok()?;
let encoded = read_sync(
&settings.cli.cache_dir,
cache_key(path, &settings.lib, autofix),
)
.ok()?;
let (mtime, messages) = match bincode::deserialize::<CheckResult>(&encoded[..]) {
Ok(CheckResult {
metadata: CacheMetadata { mtime },
@ -104,7 +108,7 @@ pub fn get<P: AsRef<Path>>(
pub fn set<P: AsRef<Path>>(
path: P,
metadata: &fs::Metadata,
settings: &Settings,
settings: &AllSettings,
autofix: flags::Autofix,
messages: &[Message],
) {
@ -115,8 +119,8 @@ pub fn set<P: AsRef<Path>>(
messages,
};
if let Err(e) = write_sync(
&settings.cache_dir,
cache_key(path, settings, autofix),
&settings.cli.cache_dir,
cache_key(path, &settings.lib, autofix),
&bincode::serialize(&check_result).unwrap(),
) {
error!("Failed to write to cache: {e:?}");

14
ruff_cli/src/commands.rs
View file

@ -56,19 +56,19 @@ pub fn run(
if matches!(cache, flags::Cache::Enabled) {
match &pyproject_strategy {
PyprojectDiscovery::Fixed(settings) => {
if let Err(e) = cache::init(&settings.cache_dir) {
if let Err(e) = cache::init(&settings.cli.cache_dir) {
error!(
"Failed to initialize cache at {}: {e:?}",
settings.cache_dir.to_string_lossy()
settings.cli.cache_dir.to_string_lossy()
);
}
}
PyprojectDiscovery::Hierarchical(default) => {
for settings in std::iter::once(default).chain(resolver.iter()) {
if let Err(e) = cache::init(&settings.cache_dir) {
if let Err(e) = cache::init(&settings.cli.cache_dir) {
error!(
"Failed to initialize cache at {}: {e:?}",
settings.cache_dir.to_string_lossy()
settings.cli.cache_dir.to_string_lossy()
);
}
}
@ -97,7 +97,7 @@ pub fn run(
.parent()
.and_then(|parent| package_roots.get(parent))
.and_then(|package| *package);
let settings = resolver.resolve(path, pyproject_strategy);
let settings = resolver.resolve_all(path, pyproject_strategy);
lint_path(path, package, settings, cache, autofix)
.map_err(|e| (Some(path.to_owned()), e.to_string()))
}
@ -171,9 +171,9 @@ pub fn run_stdin(
};
let package_root = filename
.and_then(Path::parent)
.and_then(|path| packaging::detect_package_root(path, &settings.namespace_packages));
.and_then(|path| packaging::detect_package_root(path, &settings.lib.namespace_packages));
let stdin = read_from_stdin()?;
let mut diagnostics = lint_stdin(filename, package_root, &stdin, settings, autofix)?;
let mut diagnostics = lint_stdin(filename, package_root, &stdin, &settings.lib, autofix)?;
diagnostics.messages.sort_unstable();
Ok(diagnostics)
}

10
ruff_cli/src/diagnostics.rs
View file

@ -9,7 +9,7 @@ use anyhow::Result;
use log::debug;
use ruff::linter::{lint_fix, lint_only};
use ruff::message::Message;
use ruff::settings::{flags, Settings};
use ruff::settings::{flags, AllSettings, Settings};
use ruff::{fix, fs};
use similar::TextDiff;
@ -38,12 +38,12 @@ impl AddAssign for Diagnostics {
pub fn lint_path(
path: &Path,
package: Option<&Path>,
settings: &Settings,
settings: &AllSettings,
cache: flags::Cache,
autofix: fix::FixMode,
) -> Result<Diagnostics> {
// Validate the `Settings` and return any errors.
settings.validate()?;
settings.lib.validate()?;
// Check the cache.
// TODO(charlie): `fixer::Mode::Apply` and `fixer::Mode::Diff` both have
@ -69,7 +69,7 @@ pub fn lint_path(
// Lint the file.
let (messages, fixed) = if matches!(autofix, fix::FixMode::Apply | fix::FixMode::Diff) {
let (transformed, fixed, messages) = lint_fix(&contents, path, package, settings)?;
let (transformed, fixed, messages) = lint_fix(&contents, path, package, &settings.lib)?;
if fixed > 0 {
if matches!(autofix, fix::FixMode::Apply) {
write(path, transformed)?;
@ -85,7 +85,7 @@ pub fn lint_path(
}
(messages, fixed)
} else {
let messages = lint_only(&contents, path, package, settings, autofix.into())?;
let messages = lint_only(&contents, path, package, &settings.lib, autofix.into())?;
let fixed = 0;
(messages, fixed)
};

41
ruff_cli/src/main.rs
View file

@ -16,8 +16,8 @@ use ::ruff::resolver::{
resolve_settings_with_processor, ConfigProcessor, FileDiscovery, PyprojectDiscovery, Relativity,
};
use ::ruff::settings::configuration::Configuration;
use ::ruff::settings::pyproject;
use ::ruff::settings::types::SerializationFormat;
use ::ruff::settings::{pyproject, Settings};
use ::ruff::{fix, fs, warn_user_once};
use anyhow::Result;
use clap::{CommandFactory, Parser};
@ -26,6 +26,7 @@ use colored::Colorize;
use notify::{recommended_watcher, RecursiveMode, Watcher};
use path_absolutize::path_dedot;
use printer::{Printer, Violations};
use ruff::settings::{AllSettings, CliSettings};
mod cache;
mod cli;
@ -48,7 +49,7 @@ fn resolve(
// First priority: if we're running in isolated mode, use the default settings.
let mut config = Configuration::default();
overrides.process_config(&mut config);
let settings = Settings::from_configuration(config, &path_dedot::CWD)?;
let settings = AllSettings::from_configuration(config, &path_dedot::CWD)?;
Ok(PyprojectDiscovery::Fixed(settings))
} else if let Some(pyproject) = config {
// Second priority: the user specified a `pyproject.toml` file. Use that
@ -82,7 +83,7 @@ fn resolve(
// as the "default" settings.)
let mut config = Configuration::default();
overrides.process_config(&mut config);
let settings = Settings::from_configuration(config, &path_dedot::CWD)?;
let settings = AllSettings::from_configuration(config, &path_dedot::CWD)?;
Ok(PyprojectDiscovery::Hierarchical(settings))
}
}
@ -113,35 +114,31 @@ pub fn main() -> Result<ExitCode> {
// Validate the `Settings` and return any errors.
match &pyproject_strategy {
PyprojectDiscovery::Fixed(settings) => settings.validate()?,
PyprojectDiscovery::Hierarchical(settings) => settings.validate()?,
PyprojectDiscovery::Fixed(settings) => settings.lib.validate()?,
PyprojectDiscovery::Hierarchical(settings) => settings.lib.validate()?,
};
// Extract options that are included in `Settings`, but only apply at the top
// level.
let file_strategy = FileDiscovery {
force_exclude: match &pyproject_strategy {
PyprojectDiscovery::Fixed(settings) => settings.force_exclude,
PyprojectDiscovery::Hierarchical(settings) => settings.force_exclude,
PyprojectDiscovery::Fixed(settings) => settings.lib.force_exclude,
PyprojectDiscovery::Hierarchical(settings) => settings.lib.force_exclude,
},
respect_gitignore: match &pyproject_strategy {
PyprojectDiscovery::Fixed(settings) => settings.respect_gitignore,
PyprojectDiscovery::Hierarchical(settings) => settings.respect_gitignore,
PyprojectDiscovery::Fixed(settings) => settings.lib.respect_gitignore,
PyprojectDiscovery::Hierarchical(settings) => settings.lib.respect_gitignore,
},
};
let (fix, fix_only, format, update_check) = match &pyproject_strategy {
PyprojectDiscovery::Fixed(settings) => (
settings.fix,
settings.fix_only,
settings.format,
settings.update_check,
),
PyprojectDiscovery::Hierarchical(settings) => (
settings.fix,
settings.fix_only,
settings.format,
settings.update_check,
),
let CliSettings {
fix,
fix_only,
format,
update_check,
..
} = match &pyproject_strategy {
PyprojectDiscovery::Fixed(settings) => settings.cli.clone(),
PyprojectDiscovery::Hierarchical(settings) => settings.cli.clone(),
};
if let Some(code) = cli.explain {

4
src/lib_native.rs
View file

@ -17,10 +17,10 @@ use crate::{directives, packaging, resolver};
fn resolve(path: &Path) -> Result<Settings> {
if let Some(pyproject) = pyproject::find_settings_toml(path)? {
// First priority: `pyproject.toml` in the current `Path`.
resolver::resolve_settings(&pyproject, &Relativity::Parent)
Ok(resolver::resolve_settings(&pyproject, &Relativity::Parent)?.lib)
} else if let Some(pyproject) = pyproject::find_user_settings_toml() {
// Second priority: user-specific `pyproject.toml`.
resolver::resolve_settings(&pyproject, &Relativity::Cwd)
Ok(resolver::resolve_settings(&pyproject, &Relativity::Cwd)?.lib)
} else {
// Fallback: default settings.
Settings::from_configuration(Configuration::default(), &path_dedot::CWD)

36
src/resolver.rs
View file

@ -14,7 +14,7 @@ use rustc_hash::FxHashSet;
use crate::fs;
use crate::settings::configuration::Configuration;
use crate::settings::pyproject::settings_toml;
use crate::settings::{pyproject, Settings};
use crate::settings::{pyproject, AllSettings, Settings};
/// The strategy used to discover Python files in the filesystem..
#[derive(Debug)]
@ -29,10 +29,10 @@ pub struct FileDiscovery {
pub enum PyprojectDiscovery {
/// Use a fixed `pyproject.toml` file for all Python files (i.e., one
/// provided on the command-line).
Fixed(Settings),
Fixed(AllSettings),
/// Use the closest `pyproject.toml` file in the filesystem hierarchy, or
/// the default settings.
Hierarchical(Settings),
Hierarchical(AllSettings),
}
/// The strategy for resolving file paths in a `pyproject.toml`.
@ -58,17 +58,21 @@ impl Relativity {
#[derive(Default)]
pub struct Resolver {
settings: BTreeMap<PathBuf, Settings>,
settings: BTreeMap<PathBuf, AllSettings>,
}
impl Resolver {
/// Add a resolved `Settings` under a given `PathBuf` scope.
pub fn add(&mut self, path: PathBuf, settings: Settings) {
pub fn add(&mut self, path: PathBuf, settings: AllSettings) {
self.settings.insert(path, settings);
}
/// Return the appropriate `Settings` for a given `Path`.
pub fn resolve<'a>(&'a self, path: &Path, strategy: &'a PyprojectDiscovery) -> &'a Settings {
/// Return the appropriate `AllSettings` for a given `Path`.
pub fn resolve_all<'a>(
&'a self,
path: &Path,
strategy: &'a PyprojectDiscovery,
) -> &'a AllSettings {
match strategy {
PyprojectDiscovery::Fixed(settings) => settings,
PyprojectDiscovery::Hierarchical(default) => self
@ -86,8 +90,12 @@ impl Resolver {
}
}
pub fn resolve<'a>(&'a self, path: &Path, strategy: &'a PyprojectDiscovery) -> &'a Settings {
&self.resolve_all(path, strategy).lib
}
/// Return an iterator over the resolved `Settings` in this `Resolver`.
pub fn iter(&self) -> impl Iterator<Item = &Settings> {
pub fn iter(&self) -> impl Iterator<Item = &AllSettings> {
self.settings.values()
}
@ -100,11 +108,11 @@ impl Resolver {
// `Settings` for each path, but that's more expensive.
match &strategy {
PyprojectDiscovery::Fixed(settings) => {
settings.validate()?;
settings.lib.validate()?;
}
PyprojectDiscovery::Hierarchical(default) => {
for settings in std::iter::once(default).chain(self.iter()) {
settings.validate()?;
settings.lib.validate()?;
}
}
}
@ -176,15 +184,15 @@ pub fn resolve_scoped_settings(
pyproject: &Path,
relativity: &Relativity,
processor: impl ConfigProcessor,
) -> Result<(PathBuf, Settings)> {
) -> Result<(PathBuf, AllSettings)> {
let project_root = relativity.resolve(pyproject);
let configuration = resolve_configuration(pyproject, relativity, processor)?;
let settings = Settings::from_configuration(configuration, &project_root)?;
let settings = AllSettings::from_configuration(configuration, &project_root)?;
Ok((project_root, settings))
}
/// Extract the `Settings` from a given `pyproject.toml`.
pub fn resolve_settings(pyproject: &Path, relativity: &Relativity) -> Result<Settings> {
pub fn resolve_settings(pyproject: &Path, relativity: &Relativity) -> Result<AllSettings> {
let (_project_root, settings) = resolve_scoped_settings(pyproject, relativity, &NoOpProcessor)?;
Ok(settings)
}
@ -195,7 +203,7 @@ pub fn resolve_settings_with_processor(
pyproject: &Path,
relativity: &Relativity,
processor: impl ConfigProcessor,
) -> Result<Settings> {
) -> Result<AllSettings> {
let (_project_root, settings) = resolve_scoped_settings(pyproject, relativity, processor)?;
Ok(settings)
}

55
src/settings/mod.rs
View file

@ -39,22 +39,53 @@ pub mod types;
const CARGO_PKG_VERSION: &str = env!("CARGO_PKG_VERSION");
#[derive(Debug)]
pub struct AllSettings {
pub cli: CliSettings,
pub lib: Settings,
}
impl AllSettings {
pub fn from_configuration(config: Configuration, project_root: &Path) -> Result<Self> {
Ok(Self {
cli: CliSettings {
cache_dir: config
.cache_dir
.clone()
.unwrap_or_else(|| cache_dir(project_root)),
fix: config.fix.unwrap_or(false),
fix_only: config.fix_only.unwrap_or(false),
format: config.format.unwrap_or_default(),
update_check: config.update_check.unwrap_or_default(),
},
lib: Settings::from_configuration(config, project_root)?,
})
}
}
#[derive(Debug, Default, Clone)]
/// Settings that are not used by this library and
/// only here so that `ruff_cli` can use them.
pub struct CliSettings {
pub cache_dir: PathBuf,
pub fix: bool,
pub fix_only: bool,
pub format: SerializationFormat,
pub update_check: bool,
}
#[derive(Debug)]
#[allow(clippy::struct_excessive_bools)]
pub struct Settings {
pub allowed_confusables: FxHashSet<char>,
pub builtins: Vec<String>,
pub cache_dir: PathBuf,
pub dummy_variable_rgx: Regex,
pub enabled: FxHashSet<RuleCode>,
pub exclude: GlobSet,
pub extend_exclude: GlobSet,
pub external: FxHashSet<String>,
pub fix: bool,
pub fix_only: bool,
pub fixable: FxHashSet<RuleCode>,
pub force_exclude: bool,
pub format: SerializationFormat,
pub ignore_init_module_imports: bool,
pub line_length: usize,
pub namespace_packages: Vec<PathBuf>,
@ -66,7 +97,6 @@ pub struct Settings {
pub target_version: PythonVersion,
pub task_tags: Vec<String>,
pub typing_modules: Vec<String>,
pub update_check: bool,
// Plugins
pub flake8_annotations: flake8_annotations::settings::Settings,
pub flake8_bandit: flake8_bandit::settings::Settings,
@ -120,7 +150,6 @@ impl Settings {
.map(FxHashSet::from_iter)
.unwrap_or_default(),
builtins: config.builtins.unwrap_or_default(),
cache_dir: config.cache_dir.unwrap_or_else(|| cache_dir(project_root)),
dummy_variable_rgx: config
.dummy_variable_rgx
.unwrap_or_else(|| DEFAULT_DUMMY_VARIABLE_RGX.clone()),
@ -159,8 +188,6 @@ impl Settings {
exclude: resolve_globset(config.exclude.unwrap_or_else(|| DEFAULT_EXCLUDE.clone()))?,
extend_exclude: resolve_globset(config.extend_exclude)?,
external: FxHashSet::from_iter(config.external.unwrap_or_default()),
fix: config.fix.unwrap_or(false),
fix_only: config.fix_only.unwrap_or(false),
fixable: resolve_codes(
[RuleCodeSpec {
select: &config.fixable.unwrap_or_else(|| CATEGORIES.to_vec()),
@ -168,7 +195,6 @@ impl Settings {
}]
.into_iter(),
),
format: config.format.unwrap_or_default(),
force_exclude: config.force_exclude.unwrap_or(false),
ignore_init_module_imports: config.ignore_init_module_imports.unwrap_or_default(),
line_length: config.line_length.unwrap_or(88),
@ -187,7 +213,6 @@ impl Settings {
vec!["TODO".to_string(), "FIXME".to_string(), "XXX".to_string()]
}),
typing_modules: config.typing_modules.unwrap_or_default(),
update_check: config.update_check.unwrap_or_default(),
// Plugins
flake8_annotations: config
.flake8_annotations
@ -227,17 +252,13 @@ impl Settings {
Self {
allowed_confusables: FxHashSet::from_iter([]),
builtins: vec![],
cache_dir: cache_dir(path_dedot::CWD.as_path()),
dummy_variable_rgx: Regex::new("^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$").unwrap(),
enabled: FxHashSet::from_iter([rule_code.clone()]),
exclude: GlobSet::empty(),
extend_exclude: GlobSet::empty(),
external: FxHashSet::default(),
fix: false,
fix_only: false,
fixable: FxHashSet::from_iter([rule_code]),
force_exclude: false,
format: SerializationFormat::Text,
ignore_init_module_imports: false,
line_length: 88,
namespace_packages: vec![],
@ -249,7 +270,6 @@ impl Settings {
target_version: PythonVersion::Py310,
task_tags: vec!["TODO".to_string(), "FIXME".to_string(), "XXX".to_string()],
typing_modules: vec![],
update_check: false,
flake8_annotations: flake8_annotations::settings::Settings::default(),
flake8_bandit: flake8_bandit::settings::Settings::default(),
flake8_bugbear: flake8_bugbear::settings::Settings::default(),
@ -273,17 +293,13 @@ impl Settings {
Self {
allowed_confusables: FxHashSet::from_iter([]),
builtins: vec![],
cache_dir: cache_dir(path_dedot::CWD.as_path()),
dummy_variable_rgx: Regex::new("^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$").unwrap(),
enabled: FxHashSet::from_iter(rule_codes.clone()),
exclude: GlobSet::empty(),
extend_exclude: GlobSet::empty(),
external: FxHashSet::default(),
fix: false,
fix_only: false,
fixable: FxHashSet::from_iter(rule_codes),
force_exclude: false,
format: SerializationFormat::Text,
ignore_init_module_imports: false,
line_length: 88,
namespace_packages: vec![],
@ -295,7 +311,6 @@ impl Settings {
target_version: PythonVersion::Py310,
task_tags: vec!["TODO".to_string(), "FIXME".to_string(), "XXX".to_string()],
typing_modules: vec![],
update_check: false,
flake8_annotations: flake8_annotations::settings::Settings::default(),
flake8_bandit: flake8_bandit::settings::Settings::default(),
flake8_bugbear: flake8_bugbear::settings::Settings::default(),