From 012ff580de967ed9d38eea31a7d7238e856a6c7d Mon Sep 17 00:00:00 2001 From: Victorien Elvinger Date: Fri, 26 Jan 2024 17:58:07 +0100 Subject: [PATCH] refactor(biome_service): move gitignore from matcher to files settings (#1686) --- .../src/configuration/formatter.rs | 6 +- .../src/configuration/linter/mod.rs | 6 +- .../src/configuration/organize_imports.rs | 16 +--- .../src/configuration/overrides.rs | 16 +--- crates/biome_service/src/matcher/mod.rs | 59 +------------ crates/biome_service/src/settings.rs | 84 ++++++++++--------- crates/biome_service/src/workspace/server.rs | 22 ++++- 7 files changed, 75 insertions(+), 134 deletions(-) diff --git a/crates/biome_service/src/configuration/formatter.rs b/crates/biome_service/src/configuration/formatter.rs index b7480295791d..e6bbaac7ade4 100644 --- a/crates/biome_service/src/configuration/formatter.rs +++ b/crates/biome_service/src/configuration/formatter.rs @@ -102,8 +102,6 @@ impl FromStr for FormatterConfiguration { pub fn to_format_settings( working_directory: Option, conf: FormatterConfiguration, - _vcs_path: Option, - _gitignore_matches: &[String], ) -> Result { let indent_style = match conf.indent_style { Some(PlainIndentStyle::Tab) => IndentStyle::Tab, @@ -123,8 +121,8 @@ pub fn to_format_settings( line_ending: conf.line_ending, line_width: conf.line_width, format_with_errors: conf.format_with_errors.unwrap_or_default(), - ignored_files: to_matcher(working_directory.clone(), conf.ignore.as_ref(), None, &[])?, - included_files: to_matcher(working_directory, conf.include.as_ref(), None, &[])?, + ignored_files: to_matcher(working_directory.clone(), conf.ignore.as_ref())?, + included_files: to_matcher(working_directory, conf.include.as_ref())?, }) } diff --git a/crates/biome_service/src/configuration/linter/mod.rs b/crates/biome_service/src/configuration/linter/mod.rs index 6aad86a37733..e5c358b62382 100644 --- a/crates/biome_service/src/configuration/linter/mod.rs +++ b/crates/biome_service/src/configuration/linter/mod.rs @@ -69,14 +69,12 @@ impl Default for LinterConfiguration { pub fn to_linter_settings( working_directory: Option, conf: LinterConfiguration, - _vcs_path: Option, - _gitignore_matches: &[String], ) -> Result { Ok(LinterSettings { enabled: conf.enabled.unwrap_or_default(), rules: conf.rules, - ignored_files: to_matcher(working_directory.clone(), conf.ignore.as_ref(), None, &[])?, - included_files: to_matcher(working_directory.clone(), conf.include.as_ref(), None, &[])?, + ignored_files: to_matcher(working_directory.clone(), conf.ignore.as_ref())?, + included_files: to_matcher(working_directory.clone(), conf.include.as_ref())?, }) } diff --git a/crates/biome_service/src/configuration/organize_imports.rs b/crates/biome_service/src/configuration/organize_imports.rs index 61323087e6da..76e666d70ac5 100644 --- a/crates/biome_service/src/configuration/organize_imports.rs +++ b/crates/biome_service/src/configuration/organize_imports.rs @@ -55,23 +55,11 @@ impl OrganizeImports { pub fn to_organize_imports_settings( working_directory: Option, organize_imports: OrganizeImports, - _vcs_base_path: Option, - _gitignore_matches: &[String], ) -> Result { Ok(OrganizeImportsSettings { enabled: organize_imports.enabled.unwrap_or_default(), - ignored_files: to_matcher( - working_directory.clone(), - organize_imports.ignore.as_ref(), - None, - &[], - )?, - included_files: to_matcher( - working_directory, - organize_imports.include.as_ref(), - None, - &[], - )?, + ignored_files: to_matcher(working_directory.clone(), organize_imports.ignore.as_ref())?, + included_files: to_matcher(working_directory, organize_imports.include.as_ref())?, }) } diff --git a/crates/biome_service/src/configuration/overrides.rs b/crates/biome_service/src/configuration/overrides.rs index 8651895b59f2..126b01be0753 100644 --- a/crates/biome_service/src/configuration/overrides.rs +++ b/crates/biome_service/src/configuration/overrides.rs @@ -169,8 +169,6 @@ pub struct OverrideOrganizeImportsConfiguration { pub fn to_override_settings( working_directory: Option, overrides: Overrides, - _vcs_base_path: Option, - _gitignore_matches: &[String], current_settings: &WorkspaceSettings, ) -> Result { let mut override_settings = OverrideSettings::default(); @@ -196,18 +194,8 @@ pub fn to_override_settings( languages.css = to_css_language_settings(css, ¤t_settings.languages.css); let pattern_setting = OverrideSettingPattern { - include: to_matcher( - working_directory.clone(), - pattern.include.as_ref(), - None, - &[], - )?, - exclude: to_matcher( - working_directory.clone(), - pattern.ignore.as_ref(), - None, - &[], - )?, + include: to_matcher(working_directory.clone(), pattern.include.as_ref())?, + exclude: to_matcher(working_directory.clone(), pattern.ignore.as_ref())?, formatter, linter, organize_imports, diff --git a/crates/biome_service/src/matcher/mod.rs b/crates/biome_service/src/matcher/mod.rs index cdbf007bcf0d..5446e1e4c152 100644 --- a/crates/biome_service/src/matcher/mod.rs +++ b/crates/biome_service/src/matcher/mod.rs @@ -1,10 +1,7 @@ pub mod pattern; -use crate::configuration::diagnostics::InvalidIgnorePattern; -use crate::{ConfigurationDiagnostic, WorkspaceError}; use biome_console::markup; use biome_diagnostics::Diagnostic; -use ignore::gitignore::{Gitignore, GitignoreBuilder}; pub use pattern::{MatchOptions, Pattern, PatternError}; use std::collections::HashMap; use std::path::{Path, PathBuf}; @@ -15,7 +12,6 @@ use std::sync::RwLock; #[derive(Debug)] pub struct Matcher { root: Option, - git_ignore: Option, patterns: Vec, options: MatchOptions, /// Whether the string was already checked @@ -29,7 +25,6 @@ impl Matcher { pub fn new(options: MatchOptions) -> Self { Self { root: None, - git_ignore: None, patterns: Vec::new(), options, already_checked: RwLock::new(HashMap::default()), @@ -39,7 +34,6 @@ impl Matcher { pub fn empty() -> Self { Self { root: None, - git_ignore: None, patterns: Vec::new(), options: MatchOptions::default(), already_checked: RwLock::new(HashMap::default()), @@ -50,36 +44,6 @@ impl Matcher { self.root = Some(root); } - pub fn add_gitignore_matches( - &mut self, - path: PathBuf, - matches: &[String], - ) -> Result<(), WorkspaceError> { - let mut gitignore_builder = GitignoreBuilder::new(path.clone()); - - for the_match in matches { - gitignore_builder - .add_line(Some(path.clone()), the_match) - .map_err(|err| { - WorkspaceError::Configuration(ConfigurationDiagnostic::InvalidIgnorePattern( - InvalidIgnorePattern { - message: err.to_string(), - }, - )) - })?; - } - let gitignore = gitignore_builder.build().map_err(|err| { - WorkspaceError::Configuration(ConfigurationDiagnostic::InvalidIgnorePattern( - InvalidIgnorePattern { - message: err.to_string(), - }, - )) - })?; - self.git_ignore = Some(gitignore); - - Ok(()) - } - /// It adds a unix shell style pattern pub fn add_pattern(&mut self, pattern: &str) -> Result<(), PatternError> { let pattern = Pattern::new(pattern)?; @@ -107,11 +71,6 @@ impl Matcher { pub fn is_empty(&self) -> bool { self.patterns.is_empty() - && self - .git_ignore - .as_ref() - .map(|ignore| ignore.is_empty()) - .unwrap_or(true) } /// It matches the given path against the stored patterns @@ -160,23 +119,7 @@ impl Matcher { return true; } } - - self.git_ignore - .as_ref() - .map(|ignore| { - // `matched_path_or_any_parents` panics if `source` is not under the gitignore root. - // This checks excludes absolute paths that are not a prefix of the base root. - if !source.has_root() || source.starts_with(ignore.path()) { - // Because Biome passes a list of paths, - // we use `matched_path_or_any_parents` instead of `matched`. - ignore - .matched_path_or_any_parents(source, source.is_dir()) - .is_ignore() - } else { - false - } - }) - .unwrap_or_default() + false } } diff --git a/crates/biome_service/src/settings.rs b/crates/biome_service/src/settings.rs index ef74b9c1a10c..fb9837990b2c 100644 --- a/crates/biome_service/src/settings.rs +++ b/crates/biome_service/src/settings.rs @@ -1,3 +1,4 @@ +use crate::configuration::diagnostics::InvalidIgnorePattern; use crate::configuration::formatter::to_format_settings; use crate::configuration::linter::to_linter_settings; use crate::configuration::organize_imports::to_organize_imports_settings; @@ -24,6 +25,7 @@ use biome_js_syntax::JsLanguage; use biome_json_formatter::context::JsonFormatOptions; use biome_json_parser::JsonParserOptions; use biome_json_syntax::JsonLanguage; +use ignore::gitignore::{Gitignore, GitignoreBuilder}; use indexmap::IndexSet; use std::borrow::Cow; use std::path::{Path, PathBuf}; @@ -94,41 +96,27 @@ impl WorkspaceSettings { ) -> Result<(), WorkspaceError> { // formatter part if let Some(formatter) = configuration.formatter { - self.formatter = to_format_settings( - working_directory.clone(), - formatter, - vcs_path.clone(), - gitignore_matches, - )?; + self.formatter = to_format_settings(working_directory.clone(), formatter)?; } // linter part if let Some(linter) = configuration.linter { - self.linter = to_linter_settings( - working_directory.clone(), - linter, - vcs_path.clone(), - gitignore_matches, - )?; + self.linter = to_linter_settings(working_directory.clone(), linter)?; } // Filesystem settings if let Some(files) = to_file_settings( working_directory.clone(), configuration.files, - vcs_path.clone(), + vcs_path, gitignore_matches, )? { self.files = files; } if let Some(organize_imports) = configuration.organize_imports { - self.organize_imports = to_organize_imports_settings( - working_directory.clone(), - organize_imports, - vcs_path.clone(), - gitignore_matches, - )?; + self.organize_imports = + to_organize_imports_settings(working_directory.clone(), organize_imports)?; } // javascript settings @@ -146,13 +134,8 @@ impl WorkspaceSettings { // NOTE: keep this last. Computing the overrides require reading the settings computed by the parent settings. if let Some(overrides) = configuration.overrides { - self.override_settings = to_override_settings( - working_directory.clone(), - overrides, - vcs_path, - gitignore_matches, - self, - )?; + self.override_settings = + to_override_settings(working_directory.clone(), overrides, self)?; } Ok(()) @@ -448,6 +431,9 @@ pub struct FilesSettings { /// File size limit in bytes pub max_size: NonZeroU64, + /// gitignore file patterns + pub git_ignore: Option, + /// List of paths/files to matcher pub ignored_files: Matcher, @@ -467,6 +453,7 @@ impl Default for FilesSettings { fn default() -> Self { Self { max_size: DEFAULT_FILE_SIZE_LIMIT, + git_ignore: None, ignored_files: Matcher::empty(), included_files: Matcher::empty(), ignore_unknown: false, @@ -487,17 +474,17 @@ fn to_file_settings( } else { None }; - + let git_ignore = if let Some(vcs_config_path) = vcs_config_path { + Some(to_git_ignore(vcs_config_path, gitignore_matches)?) + } else { + None + }; Ok(if let Some(config) = config { Some(FilesSettings { max_size: config.max_size.unwrap_or(DEFAULT_FILE_SIZE_LIMIT), - ignored_files: to_matcher( - working_directory.clone(), - config.ignore.as_ref(), - vcs_config_path.clone(), - gitignore_matches, - )?, - included_files: to_matcher(working_directory, config.include.as_ref(), None, &[])?, + git_ignore, + ignored_files: to_matcher(working_directory.clone(), config.ignore.as_ref())?, + included_files: to_matcher(working_directory, config.include.as_ref())?, ignore_unknown: config.ignore_unknown.unwrap_or_default(), }) } else { @@ -841,8 +828,6 @@ pub struct OverrideSettingPattern { pub fn to_matcher( working_directory: Option, string_set: Option<&StringSet>, - vcs_path: Option, - git_ignore_matches: &[String], ) -> Result { let mut matcher = Matcher::empty(); if let Some(working_directory) = working_directory { @@ -858,8 +843,29 @@ pub fn to_matcher( })?; } } - if let Some(vcs_path) = vcs_path { - matcher.add_gitignore_matches(vcs_path, git_ignore_matches)?; - } Ok(matcher) } + +fn to_git_ignore(path: PathBuf, matches: &[String]) -> Result { + let mut gitignore_builder = GitignoreBuilder::new(path.clone()); + + for the_match in matches { + gitignore_builder + .add_line(Some(path.clone()), the_match) + .map_err(|err| { + WorkspaceError::Configuration(ConfigurationDiagnostic::InvalidIgnorePattern( + InvalidIgnorePattern { + message: err.to_string(), + }, + )) + })?; + } + let gitignore = gitignore_builder.build().map_err(|err| { + WorkspaceError::Configuration(ConfigurationDiagnostic::InvalidIgnorePattern( + InvalidIgnorePattern { + message: err.to_string(), + }, + )) + })?; + Ok(gitignore) +} diff --git a/crates/biome_service/src/workspace/server.rs b/crates/biome_service/src/workspace/server.rs index 6dd78dc23a6f..d8c9707c6098 100644 --- a/crates/biome_service/src/workspace/server.rs +++ b/crates/biome_service/src/workspace/server.rs @@ -199,7 +199,27 @@ impl WorkspaceServer { let settings = self.settings(); let is_included = settings.as_ref().files.included_files.is_empty() || settings.as_ref().files.included_files.matches_path(path); - !is_included || settings.as_ref().files.ignored_files.matches_path(path) + !is_included + || settings.as_ref().files.ignored_files.matches_path(path) + || settings + .as_ref() + .files + .git_ignore + .as_ref() + .map(|ignore| { + // `matched_path_or_any_parents` panics if `source` is not under the gitignore root. + // This checks excludes absolute paths that are not a prefix of the base root. + if !path.has_root() || path.starts_with(ignore.path()) { + // Because Biome passes a list of paths, + // we use `matched_path_or_any_parents` instead of `matched`. + ignore + .matched_path_or_any_parents(path, path.is_dir()) + .is_ignore() + } else { + false + } + }) + .unwrap_or_default() } /// Check whether a file is ignored in the feature `ignore`/`include`