feat(lint): new rule: noImportCycles (biomejs#4948)

unvalley · Jan 26, 2025 · eb3f131 · eb3f131
1 parent 404e3a0
commit eb3f131
Show file tree

Hide file tree

Showing 39 changed files with 846 additions and 276 deletions.
diff --git a/.changeset/add_the_new_rule_noimportcycles.md b/.changeset/add_the_new_rule_noimportcycles.md
@@ -0,0 +1,5 @@
+---
+cli: minor
+---
+
+# Add the new rule [`noImportCycles`](https://biomejs.dev/linter/rules/no-import-cycles)
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs b/crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rs
diff --git a/crates/biome_configuration/src/analyzer/linter/rules.rs b/crates/biome_configuration/src/analyzer/linter/rules.rs
diff --git a/crates/biome_console/src/markup.rs b/crates/biome_console/src/markup.rs
@@ -182,6 +182,19 @@ impl Markup<'_> {
 pub struct MarkupBuf(pub Vec<MarkupNodeBuf>);
 
 impl MarkupBuf {
+    /// Extends the buffer with additional markup.
+    ///
+    /// ## Example
+    ///
+    /// ```rs
+    /// let mut markup = markup!(<Info>"Hello"</Info>).to_owned();
+    /// markup.extend_with(markup!(<Info>"world"</Info>));
+    /// ```
+    pub fn extend_with(&mut self, markup: Markup) {
+        // SAFETY: The implementation of Write for MarkupBuf below always returns Ok
+        Formatter::new(self).write_markup(markup).unwrap();
+    }
+
     pub fn is_empty(&self) -> bool {
         self.0.iter().all(|node| node.content.is_empty())
     }

diff --git a/crates/biome_dependency_graph/src/dependency_graph.rs b/crates/biome_dependency_graph/src/dependency_graph.rs
@@ -66,6 +66,24 @@ pub struct ModuleImports {
     pub dynamic_imports: BTreeMap<String, Import>,
 }
 
+impl ModuleImports {
+    /// Allows draining a single entry from the imports.
+    ///
+    /// Returns a `(specifier, import)` pair from either the static or dynamic
+    /// imports, whichever is non-empty. Returns `None` if both are empty.
+    ///
+    /// Using this method allows for consuming the struct while iterating over
+    /// it, without necessarily turning the entire struct into an iterator at
+    /// once.
+    pub fn drain_one(&mut self) -> Option<(String, Import)> {
+        if self.static_imports.is_empty() {
+            self.dynamic_imports.pop_first()
+        } else {
+            self.static_imports.pop_first()
+        }
+    }
+}
+
 #[derive(Clone, Debug, PartialEq)]
 pub struct Import {
     /// Absolute path of the resource being imported, if it can be resolved.

diff --git a/crates/biome_dependency_graph/src/lib.rs b/crates/biome_dependency_graph/src/lib.rs
@@ -2,4 +2,4 @@ mod dependency_graph;
 mod import_visitor;
 mod resolver_cache;
 
-pub use dependency_graph::DependencyGraph;
+pub use dependency_graph::{DependencyGraph, ModuleImports};
diff --git a/crates/biome_diagnostics_categories/src/categories.rs b/crates/biome_diagnostics_categories/src/categories.rs
@@ -154,6 +154,7 @@ define_categories! {
     "lint/nursery/noHeadElement": "https://biomejs.dev/linter/rules/no-head-element",
     "lint/nursery/noHeadImportInDocument": "https://biomejs.dev/linter/rules/no-head-import-in-document",
     "lint/nursery/noImgElement": "https://biomejs.dev/linter/rules/no-img-element",
+    "lint/nursery/noImportCycles": "https://biomejs.dev/linter/rules/no-import-cycles",
     "lint/nursery/noImportantInKeyframe": "https://biomejs.dev/linter/rules/no-important-in-keyframe",
     "lint/nursery/noInvalidDirectionInLinearGradient": "https://biomejs.dev/linter/rules/no-invalid-direction-in-linear-gradient",
     "lint/nursery/noInvalidGridAreas": "https://biomejs.dev/linter/rules/use-consistent-grid-areas",

diff --git a/crates/biome_js_analyze/Cargo.toml b/crates/biome_js_analyze/Cargo.toml
@@ -16,6 +16,7 @@ biome_aria               = { workspace = true }
 biome_aria_metadata      = { workspace = true }
 biome_console            = { workspace = true }
 biome_control_flow       = { workspace = true }
+biome_dependency_graph   = { workspace = true }
 biome_deserialize        = { workspace = true, features = ["smallvec"] }
 biome_deserialize_macros = { workspace = true }
 biome_diagnostics        = { workspace = true }

diff --git a/crates/biome_js_analyze/src/lib.rs b/crates/biome_js_analyze/src/lib.rs
@@ -7,6 +7,7 @@ use biome_analyze::{
     MatchQueryParams, MetadataRegistry, RuleAction, RuleRegistry,
 };
 use biome_aria::AriaRoles;
+use biome_dependency_graph::DependencyGraph;
 use biome_diagnostics::Error as DiagnosticError;
 use biome_js_syntax::{JsFileSource, JsLanguage};
 use biome_project_layout::ProjectLayout;
@@ -39,21 +40,42 @@ pub static METADATA: LazyLock<MetadataRegistry> = LazyLock::new(|| {
     metadata
 });
 
+#[derive(Default)]
+pub struct JsAnalyzerServices {
+    dependency_graph: Arc<DependencyGraph>,
+    project_layout: Arc<ProjectLayout>,
+    source_type: JsFileSource,
+}
+
+impl From<(Arc<DependencyGraph>, Arc<ProjectLayout>, JsFileSource)> for JsAnalyzerServices {
+    fn from(
+        (dependency_graph, project_layout, source_type): (
+            Arc<DependencyGraph>,
+            Arc<ProjectLayout>,
+            JsFileSource,
+        ),
+    ) -> Self {
+        Self {
+            dependency_graph,
+            project_layout,
+            source_type,
+        }
+    }
+}
+
 /// Run the analyzer on the provided `root`: this process will use the given `filter`
 /// to selectively restrict analysis to specific rules / a specific source range,
 /// then call `emit_signal` when an analysis rule emits a diagnostic or action.
 /// Additionally, this function takes a `inspect_matcher` function that can be
 /// used to inspect the "query matches" emitted by the analyzer before they are
 /// processed by the lint rules registry
-#[expect(clippy::too_many_arguments)]
 pub fn analyze_with_inspect_matcher<'a, V, F, B>(
     root: &LanguageRoot<JsLanguage>,
     filter: AnalysisFilter,
     inspect_matcher: V,
     options: &'a AnalyzerOptions,
     plugins: Vec<Box<dyn AnalyzerPlugin>>,
-    source_type: JsFileSource,
-    project_layout: Arc<ProjectLayout>,
+    services: JsAnalyzerServices,
     mut emit_signal: F,
 ) -> (Option<B>, Vec<DiagnosticError>)
 where
@@ -90,6 +112,12 @@ where
     let mut registry = RuleRegistry::builder(&filter, root);
     visit_registry(&mut registry);
 
+    let JsAnalyzerServices {
+        dependency_graph,
+        project_layout,
+        source_type,
+    } = services;
+
     let (registry, mut services, diagnostics, visitors) = registry.build();
 
     // Bail if we can't parse a rule option
@@ -117,7 +145,7 @@ where
 
     services.insert_service(Arc::new(AriaRoles));
     services.insert_service(source_type);
-
+    services.insert_service(dependency_graph);
     services.insert_service(project_layout.get_node_manifest_for_path(&options.file_path));
     services.insert_service(project_layout);
 
@@ -140,8 +168,7 @@ pub fn analyze<'a, F, B>(
     filter: AnalysisFilter,
     options: &'a AnalyzerOptions,
     plugins: Vec<Box<dyn AnalyzerPlugin>>,
-    source_type: JsFileSource,
-    project_layout: Arc<ProjectLayout>,
+    services: JsAnalyzerServices,
     emit_signal: F,
 ) -> (Option<B>, Vec<DiagnosticError>)
 where
@@ -154,8 +181,7 @@ where
         |_| {},
         options,
         plugins,
-        source_type,
-        project_layout,
+        services,
         emit_signal,
     )
 }
@@ -195,6 +221,12 @@ let bar = 33;
         let mut dependencies = Dependencies::default();
         dependencies.add("buffer", "latest");
 
+        let services = JsAnalyzerServices::from((
+            Default::default(),
+            project_layout_with_top_level_dependencies(dependencies),
+            JsFileSource::tsx(),
+        ));
+
         analyze(
             &parsed.tree(),
             AnalysisFilter {
@@ -203,8 +235,7 @@ let bar = 33;
             },
             &options,
             Vec::new(),
-            JsFileSource::tsx(),
-            project_layout_with_top_level_dependencies(dependencies),
+            services,
             |signal| {
                 if let Some(diag) = signal.diagnostic() {
                     error_ranges.push(diag.location().span.unwrap());
@@ -252,7 +283,6 @@ let bar = 33;
             AnalysisFilter::default(),
             &options,
             Vec::new(),
-            JsFileSource::js_module(),
             Default::default(),
             |signal| {
                 if let Some(diag) = signal.diagnostic() {
@@ -338,7 +368,6 @@ let bar = 33;
             AnalysisFilter::default(),
             &options,
             Vec::new(),
-            JsFileSource::js_module(),
             Default::default(),
             |signal| {
                 if let Some(diag) = signal.diagnostic() {
@@ -410,7 +439,6 @@ let bar = 33;
             filter,
             &options,
             Vec::new(),
-            JsFileSource::js_module(),
             Default::default(),
             |signal| {
                 if let Some(diag) = signal.diagnostic() {
@@ -455,7 +483,6 @@ let bar = 33;
             filter,
             &options,
             Vec::new(),
-            JsFileSource::js_module(),
             Default::default(),
             |signal| {
                 if let Some(diag) = signal.diagnostic() {
@@ -507,7 +534,6 @@ debugger;
             filter,
             &options,
             Vec::new(),
-            JsFileSource::js_module(),
             Default::default(),
             |signal| {
                 if let Some(diag) = signal.diagnostic() {
@@ -554,7 +580,6 @@ debugger;
             filter,
             &options,
             Vec::new(),
-            JsFileSource::js_module(),
             Default::default(),
             |signal| {
                 if let Some(diag) = signal.diagnostic() {
@@ -603,7 +628,6 @@ debugger;
             filter,
             &options,
             Vec::new(),
-            JsFileSource::js_module(),
             Default::default(),
             |signal| {
                 if let Some(diag) = signal.diagnostic() {
@@ -653,7 +677,6 @@ let bar = 33;
             filter,
             &options,
             Vec::new(),
-            JsFileSource::js_module(),
             Default::default(),
             |signal| {
                 if let Some(diag) = signal.diagnostic() {
@@ -701,7 +724,6 @@ let bar = 33;
             filter,
             &options,
             Vec::new(),
-            JsFileSource::js_module(),
             Default::default(),
             |signal| {
                 if let Some(diag) = signal.diagnostic() {
@@ -752,7 +774,6 @@ let c;
             filter,
             &options,
             Vec::new(),
-            JsFileSource::js_module(),
             Default::default(),
             |signal| {
                 if let Some(diag) = signal.diagnostic() {
@@ -804,7 +825,6 @@ debugger;
             filter,
             &options,
             Vec::new(),
-            JsFileSource::js_module(),
             Default::default(),
             |signal| {
                 if let Some(diag) = signal.diagnostic() {
@@ -857,7 +877,6 @@ let d;
             filter,
             &options,
             Vec::new(),
-            JsFileSource::js_module(),
             Default::default(),
             |signal| {
                 if let Some(diag) = signal.diagnostic() {
@@ -896,26 +915,22 @@ const foo0 = function (bar: string) {
         };
         let options = AnalyzerOptions::default();
         let root = parsed.tree();
-        analyze(
-            &root,
-            filter,
-            &options,
-            Vec::new(),
-            JsFileSource::ts(),
-            Default::default(),
-            |signal| {
-                if let Some(diag) = signal.diagnostic() {
-                    let error = diag
-                        .with_file_path("dummyFile")
-                        .with_file_source_code(SOURCE);
-                    let text = print_diagnostic_to_string(&error);
-                    eprintln!("{text}");
-                    panic!("Unexpected diagnostic");
-                }
 
-                ControlFlow::<Never>::Continue(())
-            },
-        );
+        let services =
+            JsAnalyzerServices::from((Default::default(), Default::default(), JsFileSource::ts()));
+
+        analyze(&root, filter, &options, Vec::new(), services, |signal| {
+            if let Some(diag) = signal.diagnostic() {
+                let error = diag
+                    .with_file_path("dummyFile")
+                    .with_file_source_code(SOURCE);
+                let text = print_diagnostic_to_string(&error);
+                eprintln!("{text}");
+                panic!("Unexpected diagnostic");
+            }
+
+            ControlFlow::<Never>::Continue(())
+        });
     }
 
     #[test]
@@ -949,7 +964,6 @@ a == b;
             filter,
             &options,
             Vec::new(),
-            JsFileSource::js_module(),
             Default::default(),
             |signal| {
                 if let Some(diag) = signal.diagnostic() {

diff --git a/crates/biome_js_analyze/src/lint/nursery.rs b/crates/biome_js_analyze/src/lint/nursery.rs
@@ -14,6 +14,7 @@ pub mod no_global_dirname_filename;
 pub mod no_head_element;
 pub mod no_head_import_in_document;
 pub mod no_img_element;
+pub mod no_import_cycles;
 pub mod no_irregular_whitespace;
 pub mod no_nested_ternary;
 pub mod no_noninteractive_element_interactions;
@@ -66,6 +67,7 @@ declare_lint_group! {
             self :: no_head_element :: NoHeadElement ,
             self :: no_head_import_in_document :: NoHeadImportInDocument ,
             self :: no_img_element :: NoImgElement ,
+            self :: no_import_cycles :: NoImportCycles ,
             self :: no_irregular_whitespace :: NoIrregularWhitespace ,
             self :: no_nested_ternary :: NoNestedTernary ,
             self :: no_noninteractive_element_interactions :: NoNoninteractiveElementInteractions ,