Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin' into migrate-regexp-apis
Browse files Browse the repository at this point in the history
  • Loading branch information
leaysgur committed Oct 22, 2024
2 parents 843d6af + dbe1972 commit 96f3d01
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 31 deletions.
76 changes: 60 additions & 16 deletions crates/oxc_linter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,30 +121,74 @@ impl Linter {
.rules
.iter()
.filter(|rule| rule.should_run(&ctx_host))
.map(|rule| (rule, Rc::clone(&ctx_host).spawn(rule)))
.collect::<Vec<_>>();

for (rule, ctx) in &rules {
rule.run_once(ctx);
}
.map(|rule| (rule, Rc::clone(&ctx_host).spawn(rule)));

let semantic = ctx_host.semantic();
for symbol in semantic.symbols().symbol_ids() {

let should_run_on_jest_node =
self.options.plugins.has_test() && ctx_host.frameworks().is_test();

// IMPORTANT: We have two branches here for performance reasons:
//
// 1) Branch where we iterate over each node, then each rule
// 2) Branch where we iterate over each rule, then each node
//
// When the number of nodes is relatively small, most of them can fit
// in the cache and we can save iterating over the rules multiple times.
// But for large files, the number of nodes can be so large that it
// starts to not fit into the cache and pushes out other data, like the rules.
// So we end up thrashing the cache with each rule iteration. In this case,
// it's better to put rules in the inner loop, as the rules data is smaller
// and is more likely to fit in the cache.
//
// The threshold here is chosen to balance between performance improvement
// from not iterating over rules multiple times, but also ensuring that we
// don't thrash the cache too much. Feel free to tweak based on benchmarking.
//
// See https://github.com/oxc-project/oxc/pull/6600 for more context.
if semantic.stats().nodes > 200_000 {
// Collect rules into a Vec so that we can iterate over the rules multiple times
let rules = rules.collect::<Vec<_>>();

for (rule, ctx) in &rules {
rule.run_on_symbol(symbol, ctx);
rule.run_once(ctx);
}
}

for node in semantic.nodes() {
for (rule, ctx) in &rules {
rule.run(node, ctx);
for symbol in semantic.symbols().symbol_ids() {
for (rule, ctx) in &rules {
rule.run_on_symbol(symbol, ctx);
}
}
}

if ctx_host.frameworks().is_test() && self.options.plugins.has_test() {
for jest_node in iter_possible_jest_call_node(semantic) {
for node in semantic.nodes() {
for (rule, ctx) in &rules {
rule.run_on_jest_node(&jest_node, ctx);
rule.run(node, ctx);
}
}

if should_run_on_jest_node {
for jest_node in iter_possible_jest_call_node(semantic) {
for (rule, ctx) in &rules {
rule.run_on_jest_node(&jest_node, ctx);
}
}
}
} else {
for (rule, ref ctx) in rules {
rule.run_once(ctx);

for symbol in semantic.symbols().symbol_ids() {
rule.run_on_symbol(symbol, ctx);
}

for node in semantic.nodes() {
rule.run(node, ctx);
}

if should_run_on_jest_node {
for jest_node in iter_possible_jest_call_node(semantic) {
rule.run_on_jest_node(&jest_node, ctx);
}
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions crates/oxc_linter/src/rules/import/no_cycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl Default for NoCycle {
fn default() -> Self {
Self {
max_depth: u32::MAX,
ignore_types: false,
ignore_types: true,
ignore_external: false,
allow_unsafe_dynamic_cyclic_dependency: false,
}
Expand Down Expand Up @@ -93,24 +93,25 @@ declare_oxc_lint!(
impl Rule for NoCycle {
fn from_configuration(value: serde_json::Value) -> Self {
let obj = value.get(0);
let default = NoCycle::default();
Self {
max_depth: obj
.and_then(|v| v.get("maxDepth"))
.and_then(serde_json::Value::as_number)
.and_then(serde_json::Number::as_u64)
.map_or(u32::MAX, |n| n as u32),
.map_or(default.max_depth, |n| n as u32),
ignore_types: obj
.and_then(|v| v.get("ignoreTypes"))
.and_then(serde_json::Value::as_bool)
.unwrap_or_default(),
.unwrap_or(default.ignore_types),
ignore_external: obj
.and_then(|v| v.get("ignoreExternal"))
.and_then(serde_json::Value::as_bool)
.unwrap_or_default(),
.unwrap_or(default.ignore_external),
allow_unsafe_dynamic_cyclic_dependency: obj
.and_then(|v| v.get("allowUnsafeDynamicCyclicDependency"))
.and_then(serde_json::Value::as_bool)
.unwrap_or_default(),
.unwrap_or(default.allow_unsafe_dynamic_cyclic_dependency),
}
}

Expand Down Expand Up @@ -356,7 +357,6 @@ fn test() {
// (r#"import { bar } from "./flow-types-depth-one""#, None),
(r#"import { foo } from "./intermediate-ignore""#, None),
(r#"import { foo } from "./ignore""#, None),
(r#"import { foo } from "./typescript/ts-types-only-importing-type";"#, None),
(
r#"import { foo } from "./typescript/ts-types-some-type-imports";"#,
Some(json!([{"ignoreTypes":true}])),
Expand Down
9 changes: 0 additions & 9 deletions crates/oxc_linter/src/snapshots/no_cycle.snap
Original file line number Diff line number Diff line change
Expand Up @@ -262,15 +262,6 @@ source: crates/oxc_linter/src/tester.rs
-> ./ignore - fixtures/import/cycles/ignore/index.js
-> ../depth-zero - fixtures/import/cycles/depth-zero.js

eslint-plugin-import(no-cycle): Dependency cycle detected
╭─[cycles/depth-zero.js:1:21]
1import { foo } from "./typescript/ts-types-only-importing-type";
· ───────────────────────────────────────────
╰────
help: These paths form a cycle:
-> ./typescript/ts-types-only-importing-type - fixtures/import/cycles/typescript/ts-types-only-importing-type.ts
-> ../depth-zero - fixtures/import/cycles/depth-zero.js

eslint-plugin-import(no-cycle): Dependency cycle detected
╭─[cycles/depth-zero.js:1:21]
1import { foo } from "./typescript/ts-types-some-type-imports";
Expand Down

0 comments on commit 96f3d01

Please sign in to comment.