diff --git a/crates/oxc_linter/src/rules/oxc/no_barrel_file.rs b/crates/oxc_linter/src/rules/oxc/no_barrel_file.rs index f4c5ba031c938..4431337e800a4 100644 --- a/crates/oxc_linter/src/rules/oxc/no_barrel_file.rs +++ b/crates/oxc_linter/src/rules/oxc/no_barrel_file.rs @@ -1,81 +1,122 @@ -use oxc_ast::AstKind; -use oxc_diagnostics::OxcDiagnostic; - +use oxc_diagnostics::{LabeledSpan, OxcDiagnostic}; use oxc_macros::declare_oxc_lint; -use oxc_span::Span; +use oxc_semantic::ModuleRecord; use oxc_syntax::module_graph_visitor::{ModuleGraphVisitorBuilder, VisitFoldWhile}; use crate::{context::LintContext, rule::Rule}; -fn no_barrel_file_diagnostic(span: Span, x1: u32) -> OxcDiagnostic { - OxcDiagnostic::warning(format!( - "oxc(no-barrel-file): \ - Avoid barrel files, they slow down performance, \ - and cause large module graphs with modules that go unused.\n\ - Loading this barrel file results in importing {x1} modules." - )) - .with_help("For more information visit this link: ") - .with_label(span) +#[derive(Debug, Clone)] +pub struct NoBarrelFile { + threshold: usize, } -/// Minimum amount of exports to consider module as barrelfile -const AMOUNT_OF_EXPORTS_TO_CONSIDER_MODULE_AS_BARREL: u8 = 3; - -/// -#[derive(Debug, Default, Clone)] -pub struct NoBarrelFile; +impl Default for NoBarrelFile { + fn default() -> Self { + Self { threshold: 100 } + } +} declare_oxc_lint!( /// ### What it does /// - /// Disallow the use of barrel files. + /// Disallow the use of barrel files where the file contains `export *` statements, + /// and the total number of modules exceed a threshold. + /// + /// The default threshold is 100; + /// + /// References: + /// + /// * + /// * /// /// ### Example /// /// Invalid: + /// /// ```javascript - /// export { foo } from 'foo'; - /// export { bar } from 'bar'; - /// export { baz } from 'baz'; - /// export { qux } from 'qux'; + /// export * from 'foo'; // where `foo` loads a subtree of 100 modules + /// import * as ns from 'foo'; // where `foo` loads a subtree of 100 modules /// ``` + /// /// Valid: + /// /// ```javascript - /// export type { foo } from './foo.js'; + /// export { foo } from 'foo'; /// ``` NoBarrelFile, - nursery + restriction ); impl Rule for NoBarrelFile { + #[allow(clippy::cast_possible_truncation)] + fn from_configuration(value: serde_json::Value) -> Self { + Self { + threshold: value + .get(0) + .and_then(|config| config.get("threshold")) + .and_then(serde_json::Value::as_u64) + .map_or(NoBarrelFile::default().threshold, |n| n as usize), + } + } + fn run_once(&self, ctx: &LintContext<'_>) { let semantic = ctx.semantic(); let module_record = semantic.module_record(); - let Some(root) = semantic.nodes().root_node() else { - // Return early if the semantic's root node isn't set. - // It usually means we are running on an empty or invalid file. + + if module_record.not_esm { return; - }; - - let AstKind::Program(program) = root.kind() else { unreachable!() }; - - let declarations = - program - .body - .iter() - .fold(0, |acc, node| if node.is_declaration() { acc + 1 } else { acc }); - let exports = - module_record.star_export_entries.len() + module_record.indirect_export_entries.len(); - - if exports > declarations - && exports > AMOUNT_OF_EXPORTS_TO_CONSIDER_MODULE_AS_BARREL as usize - { - let loaded_modules_count = ModuleGraphVisitorBuilder::default() - .visit_fold(0, module_record, |acc, _, _| VisitFoldWhile::Next(acc + 1)) - .result; - ctx.diagnostic(no_barrel_file_diagnostic(program.span, loaded_modules_count)); } + + let module_requests = module_record + .indirect_export_entries + .iter() + .chain(module_record.star_export_entries.iter()) + .filter_map(|export_entry| { + if let Some(module_request) = &export_entry.module_request { + let import_name = &export_entry.import_name; + if import_name.is_all() || import_name.is_all_but_default() { + return Some(module_request); + } + } + None + }) + .collect::>(); + + let mut labels = vec![]; + let mut total: usize = 0; + + for module_request in module_requests { + if let Some(remote_module) = module_record.loaded_modules.get(module_request.name()) { + if let Some(count) = count_loaded_modules(remote_module.value()) { + total += count; + labels.push(LabeledSpan::new_with_span( + Some(format!("{count} modules")), + module_request.span(), + )); + } + }; + } + + if total >= self.threshold { + let diagnostic = OxcDiagnostic::warning(format!( + "oxc(no-barrel-file): Barrel file detected, {total} modules are loaded." + )) + .with_help(format!("Loading {total} modules is slow for runtimes and bundlers.\nSee also: .")) + .with_labels(labels); + ctx.diagnostic(diagnostic); + } + } +} + +fn count_loaded_modules(module_record: &ModuleRecord) -> Option { + if module_record.loaded_modules.is_empty() { + return None; } + Some( + ModuleGraphVisitorBuilder::default() + .visit_fold(0, module_record, |acc, _, _| VisitFoldWhile::Next(acc + 1)) + .result, + ) } #[test] @@ -83,33 +124,21 @@ fn test() { use crate::tester::Tester; let pass = vec![ - r#"export type * from "foo";"#, - r#"export type { foo } from "foo";"#, - r#"export type * from "foo"; - export type { bar } from "bar";"#, - r#"import { foo, bar, baz } from "../feature"; - export { foo }; - export { bar };"#, + (r#"export type * from "foo";"#, None), + (r#"export type { foo } from "foo";"#, None), + (r#"export type * from "foo"; export type { bar } from "bar";"#, None), + (r#"import { foo, bar, baz } from "../import/export-star/models";"#, None), ]; - let fail = vec![ + let settings = Some(serde_json::json!([{"threshold": 1}])); + + let fail = vec![( r#"export * from "./deep/a.js"; export * from "./deep/b.js"; export * from "./deep/c.js"; export * from "./deep/d.js";"#, - r#"export { foo } from "foo"; - export { bar } from "bar"; - export { baz } from "baz"; - export { qux } from "qux";"#, - r#"export { default as module1 } from "./module1"; - export { default as module2 } from "./module2"; - export { default as module3 } from "./module3"; - export { default as module4 } from "./module4";"#, - r#"export { foo, type Foo } from "foo"; - export { bar, type Bar } from "bar"; - export { baz, type Baz } from "baz"; - export { qux, type Qux } from "qux";"#, - ]; + settings, + )]; Tester::new(NoBarrelFile::NAME, pass, fail) .change_rule_path("index.ts") diff --git a/crates/oxc_linter/src/snapshots/no_barrel_file.snap b/crates/oxc_linter/src/snapshots/no_barrel_file.snap index 38fb1c178e536..eca407d4d70e2 100644 --- a/crates/oxc_linter/src/snapshots/no_barrel_file.snap +++ b/crates/oxc_linter/src/snapshots/no_barrel_file.snap @@ -2,42 +2,18 @@ source: crates/oxc_linter/src/tester.rs expression: no_barrel_file --- - ⚠ oxc(no-barrel-file): Avoid barrel files, they slow down performance, and cause large module graphs with modules that go unused. - │ Loading this barrel file results in importing 4 modules. - ╭─[index.ts:1:1] - 1 │ ╭─▶ export * from "./deep/a.js"; - 2 │ │ export * from "./deep/b.js"; - 3 │ │ export * from "./deep/c.js"; - 4 │ ╰─▶ export * from "./deep/d.js"; + ⚠ oxc(no-barrel-file): Barrel file detected, 6 modules are loaded. + ╭─[index.ts:1:15] + 1 │ export * from "./deep/a.js"; + · ──────┬────── + · ╰── 3 modules + 2 │ export * from "./deep/b.js"; + · ──────┬────── + · ╰── 2 modules + 3 │ export * from "./deep/c.js"; + · ──────┬────── + · ╰── 1 modules + 4 │ export * from "./deep/d.js"; ╰──── - help: For more information visit this link: - - ⚠ oxc(no-barrel-file): Avoid barrel files, they slow down performance, and cause large module graphs with modules that go unused. - │ Loading this barrel file results in importing 0 modules. - ╭─[index.ts:1:1] - 1 │ ╭─▶ export { foo } from "foo"; - 2 │ │ export { bar } from "bar"; - 3 │ │ export { baz } from "baz"; - 4 │ ╰─▶ export { qux } from "qux"; - ╰──── - help: For more information visit this link: - - ⚠ oxc(no-barrel-file): Avoid barrel files, they slow down performance, and cause large module graphs with modules that go unused. - │ Loading this barrel file results in importing 0 modules. - ╭─[index.ts:1:1] - 1 │ ╭─▶ export { default as module1 } from "./module1"; - 2 │ │ export { default as module2 } from "./module2"; - 3 │ │ export { default as module3 } from "./module3"; - 4 │ ╰─▶ export { default as module4 } from "./module4"; - ╰──── - help: For more information visit this link: - - ⚠ oxc(no-barrel-file): Avoid barrel files, they slow down performance, and cause large module graphs with modules that go unused. - │ Loading this barrel file results in importing 0 modules. - ╭─[index.ts:1:1] - 1 │ ╭─▶ export { foo, type Foo } from "foo"; - 2 │ │ export { bar, type Bar } from "bar"; - 3 │ │ export { baz, type Baz } from "baz"; - 4 │ ╰─▶ export { qux, type Qux } from "qux"; - ╰──── - help: For more information visit this link: + help: Loading 6 modules is slow for runtimes and bundlers. + See also: . diff --git a/crates/oxc_syntax/src/module_graph_visitor.rs b/crates/oxc_syntax/src/module_graph_visitor.rs index 4450313af6562..614e1e9d69647 100644 --- a/crates/oxc_syntax/src/module_graph_visitor.rs +++ b/crates/oxc_syntax/src/module_graph_visitor.rs @@ -1,4 +1,6 @@ -use std::{collections::HashSet, marker::PhantomData, path::PathBuf, sync::Arc}; +use std::{marker::PhantomData, path::PathBuf, sync::Arc}; + +use rustc_hash::FxHashSet; use oxc_span::CompactStr; @@ -92,8 +94,11 @@ impl<'a, T> ModuleGraphVisitorBuilder<'a, T> { module: &ModuleRecord, visit: V, ) -> ModuleGraphVisitResult { - let mut visitor = - ModuleGraphVisitor { traversed: HashSet::new(), depth: 0, max_depth: self.max_depth }; + let mut visitor = ModuleGraphVisitor { + traversed: FxHashSet::default(), + depth: 0, + max_depth: self.max_depth, + }; let filter = self.filter.unwrap_or_else(|| Box::new(|_, _| true)); let event = self.event.unwrap_or_else(|| Box::new(|_, _, _| {})); let enter = self.enter.unwrap_or_else(|| Box::new(|_, _| {})); @@ -120,7 +125,7 @@ impl<'a, T> Default for ModuleGraphVisitorBuilder<'a, T> { pub struct ModuleGraphVisitResult { pub result: T, - pub traversed: HashSet, + pub traversed: FxHashSet, pub max_depth: u32, } @@ -132,7 +137,7 @@ impl ModuleGraphVisitResult { #[derive(Debug)] struct ModuleGraphVisitor { - traversed: HashSet, + traversed: FxHashSet, depth: u32, max_depth: u32, }