Skip to content

Commit

Permalink
feat(linter): Add support for ignorePatterns property within config f…
Browse files Browse the repository at this point in the history
…ile (#7092)

This could probably use some tests, but I'm not really sure what exactly
should be tested.

Will leave a review with a few comments on things that might need a
different approach.

Closes #7032.
  • Loading branch information
nrayburn-tech authored Nov 28, 2024
1 parent cc078d6 commit 32f860d
Show file tree
Hide file tree
Showing 16 changed files with 116 additions and 27 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"ignorePatterns": [
"tests/**"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"ignorePatterns": [
"*.js"
]
}
Empty file.
Empty file.
3 changes: 2 additions & 1 deletion apps/oxlint/fixtures/print_config/ban_rules/expect.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,6 @@
"env": {
"builtin": true
},
"globals": {}
"globals": {},
"ignorePatterns": []
}
3 changes: 2 additions & 1 deletion apps/oxlint/fixtures/print_config/normal/expect.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,6 @@
"env": {
"builtin": true
},
"globals": {}
"globals": {},
"ignorePatterns": []
}
36 changes: 31 additions & 5 deletions apps/oxlint/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,6 @@ impl Runner for LintRunner {
.copied()
.collect::<Vec<&'static str>>();

let paths =
Walk::new(&paths, &ignore_options).with_extensions(Extensions(extensions)).paths();

let number_of_files = paths.len();

let config_search_result = Self::find_oxlint_config(&self.cwd, &basic_options.config);

if let Err(err) = config_search_result {
Expand All @@ -115,6 +110,17 @@ impl Runner for LintRunner {

let mut oxlintrc = config_search_result.unwrap();

let ignore_paths = oxlintrc
.ignore_patterns
.iter()
.map(|value| oxlintrc.path.parent().unwrap().join(value))
.collect::<Vec<_>>();
let paths = Walk::new(&paths, &ignore_options, &ignore_paths)
.with_extensions(Extensions(extensions))
.paths();

let number_of_files = paths.len();

enable_plugins.apply_overrides(&mut oxlintrc.plugins);

let oxlintrc_for_print =
Expand Down Expand Up @@ -750,4 +756,24 @@ mod test {
assert_eq!(result.number_of_warnings, 2);
assert_eq!(result.number_of_errors, 2);
}

#[test]
fn test_config_ignore_patterns_extension() {
let result = test(&[
"-c",
"fixtures/config_ignore_patterns/ignore_extension/eslintrc.json",
"fixtures/config_ignore_patterns/ignore_extension",
]);
assert_eq!(result.number_of_files, 1);
}

#[test]
fn test_config_ignore_patterns_directory() {
let result = test(&[
"-c",
"fixtures/config_ignore_patterns/ignore_directory/eslintrc.json",
"fixtures/config_ignore_patterns/ignore_directory",
]);
assert_eq!(result.number_of_files, 1);
}
}
21 changes: 16 additions & 5 deletions apps/oxlint/src/walk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,11 @@ impl ignore::ParallelVisitor for WalkCollector {
impl Walk {
/// Will not canonicalize paths.
/// # Panics
pub fn new(paths: &[PathBuf], options: &IgnoreOptions) -> Self {
pub fn new(
paths: &[PathBuf],
options: &IgnoreOptions,
config_ignore_patterns: &[PathBuf],
) -> Self {
assert!(!paths.is_empty(), "At least one path must be provided to Walk::new");

let mut inner = ignore::WalkBuilder::new(
Expand All @@ -89,17 +93,24 @@ impl Walk {
if !options.no_ignore {
inner.add_custom_ignore_filename(&options.ignore_path);

let mut override_builder = OverrideBuilder::new(Path::new("/"));
if !options.ignore_pattern.is_empty() {
let mut override_builder = OverrideBuilder::new(Path::new("/"));
for pattern in &options.ignore_pattern {
// Meaning of ignore pattern is reversed
// <https://docs.rs/ignore/latest/ignore/overrides/struct.OverrideBuilder.html#method.add>
let pattern = format!("!{pattern}");
override_builder.add(&pattern).unwrap();
}
let overrides = override_builder.build().unwrap();
inner.overrides(overrides);
}

if !config_ignore_patterns.is_empty() {
for pattern in config_ignore_patterns {
let pattern = format!("!{}", pattern.to_str().unwrap());
override_builder.add(&pattern).unwrap();
}
}

inner.overrides(override_builder.build().unwrap());
}
// Turning off `follow_links` because:
// * following symlinks is a really slow syscall
Expand Down Expand Up @@ -155,7 +166,7 @@ mod test {
symlinks: false,
};

let mut paths = Walk::new(&fixtures, &ignore_options)
let mut paths = Walk::new(&fixtures, &ignore_options, &[])
.with_extensions(Extensions(["js", "vue"].to_vec()))
.paths()
.into_iter()
Expand Down
38 changes: 25 additions & 13 deletions crates/oxc_language_server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ enum SyntheticRunLevel {
impl LanguageServer for Backend {
async fn initialize(&self, params: InitializeParams) -> Result<InitializeResult> {
self.init(params.root_uri)?;
self.init_ignore_glob().await;
let options = params.initialization_options.and_then(|mut value| {
let settings = value.get_mut("settings")?.take();
serde_json::from_value::<Options>(settings).ok()
Expand Down Expand Up @@ -117,7 +116,8 @@ impl LanguageServer for Backend {
None
};

self.init_linter_config().await;
let oxlintrc = self.init_linter_config().await;
self.init_ignore_glob(oxlintrc).await;
Ok(InitializeResult {
server_info: Some(ServerInfo { name: "oxc".into(), version: None }),
offset_encoding: None,
Expand Down Expand Up @@ -411,7 +411,7 @@ impl Backend {
Ok(())
}

async fn init_ignore_glob(&self) {
async fn init_ignore_glob(&self, oxlintrc: Option<Oxlintrc>) {
let uri = self
.root_uri
.get()
Expand Down Expand Up @@ -447,6 +447,17 @@ impl Backend {
}
}
}

if let Some(oxlintrc) = oxlintrc {
if !oxlintrc.ignore_patterns.is_empty() {
let mut builder =
ignore::gitignore::GitignoreBuilder::new(oxlintrc.path.parent().unwrap());
for entry in &oxlintrc.ignore_patterns {
builder.add_line(None, entry).expect("Failed to add ignore line");
}
gitignore_globs.push(builder.build().unwrap());
}
}
}

#[allow(clippy::ptr_arg)]
Expand All @@ -470,12 +481,12 @@ impl Backend {
.await;
}

async fn init_linter_config(&self) {
async fn init_linter_config(&self) -> Option<Oxlintrc> {
let Some(Some(uri)) = self.root_uri.get() else {
return;
return None;
};
let Ok(root_path) = uri.to_file_path() else {
return;
return None;
};
let mut config_path = None;
let config = root_path.join(self.options.lock().await.get_config_path().unwrap());
Expand All @@ -484,16 +495,17 @@ impl Backend {
}
if let Some(config_path) = config_path {
let mut linter = self.server_linter.write().await;
let config = Oxlintrc::from_file(&config_path)
.expect("should have initialized linter with new options");
*linter = ServerLinter::new_with_linter(
LinterBuilder::from_oxlintrc(
true,
Oxlintrc::from_file(&config_path)
.expect("should have initialized linter with new options"),
)
.with_fix(FixKind::SafeFix)
.build(),
LinterBuilder::from_oxlintrc(true, config.clone())
.with_fix(FixKind::SafeFix)
.build(),
);
return Some(config);
}

None
}

async fn handle_file_update(&self, uri: Url, content: Option<String>, version: Option<i32>) {
Expand Down
1 change: 1 addition & 0 deletions crates/oxc_linter/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ impl LinterBuilder {
rules: oxlintrc_rules,
overrides,
path,
ignore_patterns: _,
} = oxlintrc;

let config = LintConfig { plugins, settings, env, globals, path: Some(path) };
Expand Down
3 changes: 3 additions & 0 deletions crates/oxc_linter/src/config/oxlintrc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ pub struct Oxlintrc {
/// Absolute path to the configuration file.
#[serde(skip)]
pub path: PathBuf,
/// Globs to ignore during linting. These are resolved from the configuration file path.
#[serde(rename = "ignorePatterns")]
pub ignore_patterns: Vec<String>,
}

impl Oxlintrc {
Expand Down
9 changes: 8 additions & 1 deletion crates/oxc_linter/src/snapshots/schema_json.snap
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
---
source: crates/oxc_linter/src/lib.rs
expression: json
snapshot_kind: text
---
{
"$schema": "http://json-schema.org/draft-07/schema#",
Expand Down Expand Up @@ -37,6 +36,14 @@ snapshot_kind: text
}
]
},
"ignorePatterns": {
"description": "Globs to ignore during linting. These are resolved from the configuration file path.",
"default": [],
"type": "array",
"items": {
"type": "string"
}
},
"overrides": {
"description": "Add, remove, or otherwise reconfigure rules for specific files or groups of files.",
"allOf": [
Expand Down
8 changes: 8 additions & 0 deletions npm/oxlint/configuration_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@
}
]
},
"ignorePatterns": {
"description": "Globs to ignore during linting. These are resolved from the configuration file path.",
"default": [],
"type": "array",
"items": {
"type": "string"
}
},
"overrides": {
"description": "Add, remove, or otherwise reconfigure rules for specific files or groups of files.",
"allOf": [
Expand Down
10 changes: 9 additions & 1 deletion tasks/website/src/linter/snapshots/schema_markdown.snap
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
---
source: tasks/website/src/linter/json_schema.rs
expression: snapshot
snapshot_kind: text
---
# Oxlint Configuration File

Expand Down Expand Up @@ -161,6 +160,15 @@ Globals can be disabled by setting their value to `"off"`. For example, in an en
You may also use `"readable"` or `false` to represent `"readonly"`, and `"writeable"` or `true` to represent `"writable"`.


## ignorePatterns

type: `string[]`

default: `[]`

Globs to ignore during linting. These are resolved from the configuration file path.


## overrides

type: `array`
Expand Down

0 comments on commit 32f860d

Please sign in to comment.