Skip to content

Commit 9eedb58

Browse files
camc314camc314
committed
fix(linter): false positive with negative matches in no-restricted-imports (#10976)
this was an absolute nightmare to fix. TLDR is that `GitignoreBuilder` nor `OverrideBuilder` do not contain the flexibility or have the behaviour we want to achieve this here. They don't go through the full list of patterns (for perf reasons), but that has the caveat of meaning it does not work correctly with eslint closes #10911
1 parent a4b5716 commit 9eedb58

File tree

4 files changed

+60
-21
lines changed

4 files changed

+60
-21
lines changed

‎Cargo.lock‎

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/oxc_linter/Cargo.toml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ convert_case = { workspace = true }
4949
cow-utils = { workspace = true }
5050
fast-glob = { workspace = true }
5151
globset = { workspace = true }
52-
ignore = { workspace = true }
5352
indexmap = { workspace = true, features = ["rayon"] }
5453
itertools = { workspace = true }
5554
javascript-globals = { workspace = true }

‎crates/oxc_linter/src/rules/eslint/no_restricted_imports.rs‎

Lines changed: 39 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
use ignore::gitignore::GitignoreBuilder;
1+
use std::borrow::Cow;
2+
3+
use globset::GlobBuilder;
24
use lazy_regex::Regex;
35
use oxc_ast::{
46
AstKind,
@@ -10,7 +12,6 @@ use oxc_span::{CompactStr, Span};
1012
use rustc_hash::FxHashMap;
1113
use serde::{Deserialize, Deserializer, de::Error};
1214
use serde_json::Value;
13-
use std::borrow::Cow;
1415

1516
use crate::{
1617
ModuleRecord,
@@ -856,30 +857,37 @@ impl RestrictedPattern {
856857
return GlobResult::None;
857858
};
858859

859-
let mut builder = GitignoreBuilder::new("");
860-
// returns always OK, will be fixed in the next version
861-
let _ = builder.case_insensitive(!self.case_sensitive.unwrap_or(false));
860+
let case_insensitive = !self.case_sensitive.unwrap_or(false);
862861

863-
for group in groups {
864-
// returns always OK
865-
let _ = builder.add_line(None, group.as_str());
866-
}
862+
let mut decision = GlobResult::None;
867863

868-
let Ok(gitignore) = builder.build() else {
869-
return GlobResult::None;
870-
};
864+
for raw_pat in groups {
865+
let (negated, pat) = match raw_pat.strip_prefix('!') {
866+
Some(rest) => (true, rest),
867+
None => (false, raw_pat.as_str()),
868+
};
871869

872-
let matched = gitignore.matched(name, false);
870+
// roughly based on https://github.com/BurntSushi/ripgrep/blob/6dfaec03e830892e787686917509c17860456db1/crates/ignore/src/gitignore.rs#L436-L516
871+
let mut pat = pat.to_string();
873872

874-
if matched.is_whitelist() {
875-
return GlobResult::Whitelist;
876-
}
873+
if !pat.starts_with('/') && !pat.chars().any(|c| c == '/') && (!pat.starts_with("**")) {
874+
pat = format!("**/{pat}");
875+
}
877876

878-
if matched.is_none() {
879-
return GlobResult::None;
877+
let Ok(glob) = GlobBuilder::new(&pat)
878+
.case_insensitive(case_insensitive)
879+
.build()
880+
.map(|g| g.compile_matcher())
881+
else {
882+
continue;
883+
};
884+
885+
if glob.is_match(name) {
886+
decision = if negated { GlobResult::Whitelist } else { GlobResult::Found };
887+
}
880888
}
881889

882-
GlobResult::Found
890+
decision
883891
}
884892

885893
fn get_regex_result(&self, name: &str) -> bool {
@@ -1762,6 +1770,12 @@ fn test() {
17621770
}]
17631771
}])),
17641772
),
1773+
(
1774+
r#"import a from "./index.mjs";"#,
1775+
Some(
1776+
serde_json::json!([{ "patterns": [{ "group": ["[@a-z]*", "!.*/**"], "message": "foo is forbidden, use bar instead" }] }]),
1777+
),
1778+
),
17651779
];
17661780

17671781
let pass_typescript = vec![
@@ -2984,6 +2998,12 @@ fn test() {
29842998
}]
29852999
}])),
29863000
),
3001+
(
3002+
r#"import {x} from "foo"; import {x2} from "./index.mjs"; import {x3} from "index";"#,
3003+
Some(
3004+
serde_json::json!([{ "patterns": [{ "group": ["[@a-z]*", "!.*/**","./index.mjs"], "message": "foo is forbidden, use bar instead" }] }]),
3005+
),
3006+
),
29873007
// (
29883008
// "
29893009
// // error

‎crates/oxc_linter/src/snapshots/eslint_no_restricted_imports.snap‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -787,6 +787,27 @@ source: crates/oxc_linter/src/tester.rs
787787
╰────
788788
help: foo is forbidden, use bar instead
789789

790+
⚠ eslint(no-restricted-imports): 'foo' import is restricted from being used by a pattern.
791+
╭─[no_restricted_imports.tsx:1:1]
792+
1 │ import {x} from "foo"; import {x2} from "./index.mjs"; import {x3} from "index";
793+
· ──────────────────────
794+
╰────
795+
help: foo is forbidden, use bar instead
796+
797+
⚠ eslint(no-restricted-imports): './index.mjs' import is restricted from being used by a pattern.
798+
╭─[no_restricted_imports.tsx:1:24]
799+
1 │ import {x} from "foo"; import {x2} from "./index.mjs"; import {x3} from "index";
800+
· ───────────────────────────────
801+
╰────
802+
help: foo is forbidden, use bar instead
803+
804+
⚠ eslint(no-restricted-imports): 'index' import is restricted from being used by a pattern.
805+
╭─[no_restricted_imports.tsx:1:56]
806+
1 │ import {x} from "foo"; import {x2} from "./index.mjs"; import {x3} from "index";
807+
· ─────────────────────────
808+
╰────
809+
help: foo is forbidden, use bar instead
810+
790811
⚠ eslint(no-restricted-imports): 'import1' import is restricted from being used.
791812
╭─[no_restricted_imports.tsx:1:1]
792813
1 │ import foo from 'import1';

0 commit comments

Comments
 (0)