-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Use Matchit to Resolve Per-File Settings #11111
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,6 @@ | |
//! filesystem. | ||
|
||
use std::cmp::Ordering; | ||
use std::collections::BTreeMap; | ||
use std::ffi::OsStr; | ||
use std::path::{Path, PathBuf}; | ||
use std::sync::RwLock; | ||
|
@@ -13,7 +12,9 @@ use globset::{Candidate, GlobSet}; | |
use ignore::{WalkBuilder, WalkState}; | ||
use itertools::Itertools; | ||
use log::debug; | ||
use matchit::{InsertError, Match, Router}; | ||
use path_absolutize::path_dedot; | ||
use path_slash::PathExt; | ||
use rustc_hash::{FxHashMap, FxHashSet}; | ||
|
||
use ruff_linter::fs; | ||
|
@@ -86,29 +87,32 @@ pub enum Relativity { | |
} | ||
|
||
impl Relativity { | ||
pub fn resolve(self, path: &Path) -> PathBuf { | ||
pub fn resolve(self, path: &Path) -> &Path { | ||
match self { | ||
Relativity::Parent => path | ||
.parent() | ||
.expect("Expected pyproject.toml file to be in parent directory") | ||
.to_path_buf(), | ||
Relativity::Cwd => path_dedot::CWD.clone(), | ||
.expect("Expected pyproject.toml file to be in parent directory"), | ||
Relativity::Cwd => &path_dedot::CWD, | ||
} | ||
} | ||
} | ||
|
||
#[derive(Debug)] | ||
pub struct Resolver<'a> { | ||
pyproject_config: &'a PyprojectConfig, | ||
settings: BTreeMap<PathBuf, Settings>, | ||
/// All [`Settings`] that have been added to the resolver. | ||
settings: Vec<Settings>, | ||
/// A router from path to index into the `settings` vector. | ||
router: Router<usize>, | ||
} | ||
|
||
impl<'a> Resolver<'a> { | ||
/// Create a new [`Resolver`] for the given [`PyprojectConfig`]. | ||
pub fn new(pyproject_config: &'a PyprojectConfig) -> Self { | ||
Self { | ||
pyproject_config, | ||
settings: BTreeMap::new(), | ||
settings: Vec::new(), | ||
router: Router::new(), | ||
} | ||
} | ||
|
||
|
@@ -140,19 +144,31 @@ impl<'a> Resolver<'a> { | |
} | ||
|
||
/// Add a resolved [`Settings`] under a given [`PathBuf`] scope. | ||
fn add(&mut self, path: PathBuf, settings: Settings) { | ||
self.settings.insert(path, settings); | ||
fn add(&mut self, path: &Path, settings: Settings) { | ||
self.settings.push(settings); | ||
|
||
// normalize the path to use `/` separators and escape the '{' and '}' characters, | ||
// which matchit uses for routing parameters | ||
let path = path.to_slash_lossy().replace('{', "{{").replace('}', "}}"); | ||
|
||
match self | ||
.router | ||
.insert(format!("{}/{{*filepath}}", path), self.settings.len() - 1) | ||
{ | ||
Ok(_) => {} | ||
Err(InsertError::Conflict { .. }) => {} | ||
Err(_) => unreachable!("file paths are escaped before being inserted in the router"), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was curious about this, and I think I convinced myself that this is correct because all other possible errors (aside from (It might make sense to add an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah after escaping we guarantee that we're inserting a static route, so none of the other errors are possible (barring a bug in |
||
} | ||
} | ||
|
||
/// Return the appropriate [`Settings`] for a given [`Path`]. | ||
pub fn resolve(&self, path: &Path) -> &Settings { | ||
match self.pyproject_config.strategy { | ||
PyprojectDiscoveryStrategy::Fixed => &self.pyproject_config.settings, | ||
PyprojectDiscoveryStrategy::Hierarchical => self | ||
.settings | ||
.iter() | ||
.rev() | ||
.find_map(|(root, settings)| path.starts_with(root).then_some(settings)) | ||
.router | ||
.at(path.to_slash_lossy().as_ref()) | ||
.map(|Match { value, .. }| &self.settings[*value]) | ||
.unwrap_or(&self.pyproject_config.settings), | ||
} | ||
} | ||
|
@@ -196,7 +212,7 @@ impl<'a> Resolver<'a> { | |
|
||
/// Return an iterator over the resolved [`Settings`] in this [`Resolver`]. | ||
pub fn settings(&self) -> impl Iterator<Item = &Settings> { | ||
std::iter::once(&self.pyproject_config.settings).chain(self.settings.values()) | ||
std::iter::once(&self.pyproject_config.settings).chain(self.settings.iter()) | ||
} | ||
} | ||
|
||
|
@@ -285,11 +301,11 @@ fn resolve_configuration( | |
|
||
/// Extract the project root (scope) and [`Settings`] from a given | ||
/// `pyproject.toml`. | ||
fn resolve_scoped_settings( | ||
pyproject: &Path, | ||
fn resolve_scoped_settings<'a>( | ||
pyproject: &'a Path, | ||
relativity: Relativity, | ||
transformer: &dyn ConfigurationTransformer, | ||
) -> Result<(PathBuf, Settings)> { | ||
) -> Result<(&'a Path, Settings)> { | ||
let configuration = resolve_configuration(pyproject, relativity, transformer)?; | ||
let project_root = relativity.resolve(pyproject); | ||
let settings = configuration.into_settings(&project_root)?; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any other characters that we need to watch out for here? In my previous PR I mentioned colons, which seems irrelevant, but e.g.
{*foo}
is a special routing parameter in matchit, right? I assume that's also irrelevant as long as we're escaping the{
and}
characters.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*
is only considered after a non-escaped{
, so that shouldn't be a problem.