From a36ce585ce47787be3db2042113850ed50009059 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 8 Apr 2023 11:14:42 -0400 Subject: [PATCH] Remove `extract_path_names` helper (#3920) --- crates/ruff/src/fs.rs | 21 ++------ crates/ruff/src/resolver.rs | 95 ++++++++++++++++++------------------- 2 files changed, 49 insertions(+), 67 deletions(-) diff --git a/crates/ruff/src/fs.rs b/crates/ruff/src/fs.rs index 898c45129b..c8ff195468 100644 --- a/crates/ruff/src/fs.rs +++ b/crates/ruff/src/fs.rs @@ -1,36 +1,21 @@ use std::path::{Path, PathBuf}; -use anyhow::{anyhow, Result}; use globset::GlobMatcher; use log::debug; use path_absolutize::{path_dedot, Absolutize}; use crate::registry::RuleSet; -/// Extract the absolute path and basename (as strings) from a Path. -pub fn extract_path_names(path: &Path) -> Result<(&str, &str)> { - let file_path = path - .to_str() - .ok_or_else(|| anyhow!("Unable to parse filename: {:?}", path))?; - let file_basename = path - .file_name() - .ok_or_else(|| anyhow!("Unable to parse filename: {:?}", path))? - .to_str() - .ok_or_else(|| anyhow!("Unable to parse filename: {:?}", path))?; - Ok((file_path, file_basename)) -} - /// Create a set with codes matching the pattern/code pairs. pub(crate) fn ignores_from_path( path: &Path, pattern_code_pairs: &[(GlobMatcher, GlobMatcher, RuleSet)], ) -> RuleSet { - let (file_path, file_basename) = extract_path_names(path).expect("Unable to parse filename"); - + let file_name = path.file_name().expect("Unable to parse filename"); pattern_code_pairs .iter() .filter_map(|(absolute, basename, rules)| { - if basename.is_match(file_basename) { + if basename.is_match(file_name) { debug!( "Adding per-file ignores for {:?} due to basename match on {:?}: {:?}", path, @@ -38,7 +23,7 @@ pub(crate) fn ignores_from_path( rules ); Some(rules) - } else if absolute.is_match(file_path) { + } else if absolute.is_match(path) { debug!( "Adding per-file ignores for {:?} due to absolute match on {:?}: {:?}", path, diff --git a/crates/ruff/src/resolver.rs b/crates/ruff/src/resolver.rs index 6ddbccefb1..f21ad1a135 100644 --- a/crates/ruff/src/resolver.rs +++ b/crates/ruff/src/resolver.rs @@ -10,9 +10,10 @@ use ignore::{DirEntry, WalkBuilder, WalkState}; use itertools::Itertools; use log::debug; use path_absolutize::path_dedot; -use ruff_python_stdlib::path::is_python_file; use rustc_hash::FxHashSet; +use ruff_python_stdlib::path::is_python_file; + use crate::fs; use crate::jupyter::is_jupyter_notebook; use crate::settings::configuration::Configuration; @@ -191,7 +192,11 @@ pub fn resolve_settings_with_processor( /// Return `true` if the given file should be ignored based on the exclusion /// criteria. -fn match_exclusion(file_path: &str, file_basename: &str, exclusion: &globset::GlobSet) -> bool { +fn match_exclusion, R: AsRef>( + file_path: P, + file_basename: R, + exclusion: &globset::GlobSet, +) -> bool { exclusion.is_match(file_path) || exclusion.is_match(file_basename) } @@ -268,28 +273,21 @@ pub fn python_files_in_path( let path = entry.path(); let resolver = resolver.read().unwrap(); let settings = resolver.resolve(path, pyproject_strategy); - match fs::extract_path_names(path) { - Ok((file_path, file_basename)) => { - if !settings.exclude.is_empty() - && match_exclusion(file_path, file_basename, &settings.exclude) - { - debug!("Ignored path via `exclude`: {:?}", path); - return WalkState::Skip; - } else if !settings.extend_exclude.is_empty() - && match_exclusion( - file_path, - file_basename, - &settings.extend_exclude, - ) - { - debug!("Ignored path via `extend-exclude`: {:?}", path); - return WalkState::Skip; - } - } - Err(err) => { - debug!("Ignored path due to error in parsing: {:?}: {}", path, err); + if let Some(file_name) = path.file_name() { + if !settings.exclude.is_empty() + && match_exclusion(path, file_name, &settings.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) + { + debug!("Ignored path via `extend-exclude`: {:?}", path); return WalkState::Skip; } + } else { + debug!("Ignored path due to error in parsing: {:?}", path); + return WalkState::Skip; } } } @@ -385,24 +383,19 @@ fn is_file_excluded( break; } let settings = resolver.resolve(path, pyproject_strategy); - match fs::extract_path_names(path) { - Ok((file_path, file_basename)) => { - if !settings.exclude.is_empty() - && match_exclusion(file_path, file_basename, &settings.exclude) - { - debug!("Ignored path via `exclude`: {:?}", path); - return true; - } else if !settings.extend_exclude.is_empty() - && match_exclusion(file_path, file_basename, &settings.extend_exclude) - { - debug!("Ignored path via `extend-exclude`: {:?}", path); - return true; - } - } - Err(err) => { - debug!("Ignored path due to error in parsing: {:?}: {}", path, err); + if let Some(file_name) = path.file_name() { + if !settings.exclude.is_empty() && match_exclusion(path, file_name, &settings.exclude) { + debug!("Ignored path via `exclude`: {:?}", path); + return true; + } else if !settings.extend_exclude.is_empty() + && match_exclusion(path, file_name, &settings.extend_exclude) + { + debug!("Ignored path via `extend-exclude`: {:?}", path); return true; } + } else { + debug!("Ignored path due to error in parsing: {:?}", path); + return true; } if path == settings.project_root { // Bail out; we'd end up past the project root on the next iteration @@ -421,7 +414,6 @@ mod tests { use globset::GlobSet; use path_absolutize::Absolutize; - use crate::fs; use crate::resolver::{ is_file_excluded, match_exclusion, resolve_settings_with_processor, NoOpProcessor, PyprojectDiscovery, Relativity, Resolver, @@ -437,7 +429,7 @@ mod tests { } #[test] - fn exclusions() -> Result<()> { + fn exclusions() { let project_root = Path::new("/tmp/"); let path = Path::new("foo").absolutize_from(project_root).unwrap(); @@ -448,7 +440,8 @@ mod tests { .unwrap() .to_path_buf(), ); - let (file_path, file_basename) = fs::extract_path_names(&path)?; + let file_path = &path; + let file_basename = path.file_name().unwrap(); assert!(match_exclusion( file_path, file_basename, @@ -463,7 +456,8 @@ mod tests { .unwrap() .to_path_buf(), ); - let (file_path, file_basename) = fs::extract_path_names(&path)?; + let file_path = &path; + let file_basename = path.file_name().unwrap(); assert!(match_exclusion( file_path, file_basename, @@ -480,7 +474,8 @@ mod tests { .unwrap() .to_path_buf(), ); - let (file_path, file_basename) = fs::extract_path_names(&path)?; + let file_path = &path; + let file_basename = path.file_name().unwrap(); assert!(match_exclusion( file_path, file_basename, @@ -495,7 +490,8 @@ mod tests { .unwrap() .to_path_buf(), ); - let (file_path, file_basename) = fs::extract_path_names(&path)?; + let file_path = &path; + let file_basename = path.file_name().unwrap(); assert!(match_exclusion( file_path, file_basename, @@ -512,7 +508,8 @@ mod tests { .unwrap() .to_path_buf(), ); - let (file_path, file_basename) = fs::extract_path_names(&path)?; + let file_path = &path; + let file_basename = path.file_name().unwrap(); assert!(match_exclusion( file_path, file_basename, @@ -529,7 +526,8 @@ mod tests { .unwrap() .to_path_buf(), ); - let (file_path, file_basename) = fs::extract_path_names(&path)?; + let file_path = &path; + let file_basename = path.file_name().unwrap(); assert!(match_exclusion( file_path, file_basename, @@ -546,14 +544,13 @@ mod tests { .unwrap() .to_path_buf(), ); - let (file_path, file_basename) = fs::extract_path_names(&path)?; + let file_path = &path; + let file_basename = path.file_name().unwrap(); assert!(!match_exclusion( file_path, file_basename, &make_exclusion(exclude), )); - - Ok(()) } #[test]