Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Split CliSettings from Settings #1891

Merged
merged 2 commits into from
Jan 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions ruff_cli/src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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 },
Expand All @@ -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],
) {
Expand All @@ -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:?}");
Expand Down
14 changes: 7 additions & 7 deletions ruff_cli/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
);
}
}
Expand Down Expand Up @@ -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()))
}
Expand Down Expand Up @@ -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)
}
Expand Down
10 changes: 5 additions & 5 deletions ruff_cli/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand All @@ -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)?;
Expand All @@ -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)
};
Expand Down
41 changes: 19 additions & 22 deletions ruff_cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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))
}
}
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions src/lib_native.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
36 changes: 22 additions & 14 deletions src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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`.
Expand All @@ -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
Expand All @@ -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()
}

Expand All @@ -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()?;
}
}
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down
Loading