diff --git a/.changeset/shaky-experts-sit.md b/.changeset/shaky-experts-sit.md new file mode 100644 index 000000000000..de1f4a480238 --- /dev/null +++ b/.changeset/shaky-experts-sit.md @@ -0,0 +1,25 @@ +--- +"@biomejs/biome": patch +--- + +Fixed [#6799](https://github.com/biomejs/biome/issues/6799): The [`noImportCycles`](https://biomejs.dev/linter/rules/no-import-cycles/) rule now ignores type-only imports if the new `ignoreTypes` option is enabled (enabled by default). + +> [!WARNING] +> **Breaking Change**: The `noImportCycles` rule no longer detects import cycles that include one or more type-only imports by default. +> To keep the old behaviour, you can turn off the `ignoreTypes` option explicitly: +> +> ```json +> { +> "linter": { +> "rules": { +> "nursery": { +> "noImportCycles": { +> "options": { +> "ignoreTypes": false +> } +> } +> } +> } +> } +> } +> ``` diff --git a/crates/biome_js_analyze/src/lint/correctness/no_private_imports.rs b/crates/biome_js_analyze/src/lint/correctness/no_private_imports.rs index 065eedbb42bf..09c2c25f00d1 100644 --- a/crates/biome_js_analyze/src/lint/correctness/no_private_imports.rs +++ b/crates/biome_js_analyze/src/lint/correctness/no_private_imports.rs @@ -9,7 +9,7 @@ use biome_js_syntax::{ AnyJsImportClause, AnyJsImportLike, AnyJsNamedImportSpecifier, JsModuleSource, JsSyntaxToken, }; use biome_jsdoc_comment::JsdocComment; -use biome_module_graph::{JsModuleInfo, ModuleGraph, ResolvedPath}; +use biome_module_graph::{JsImportPath, JsModuleInfo, ModuleGraph}; use biome_rowan::{AstNode, SyntaxResult, Text, TextRange}; use biome_rule_options::no_private_imports::{NoPrivateImportsOptions, Visibility}; use camino::{Utf8Path, Utf8PathBuf}; @@ -180,7 +180,7 @@ impl Rule for NoPrivateImports { .then(|| node.inner_string_text()) .flatten() .and_then(|specifier| module_info.static_import_paths.get(specifier.text())) - .and_then(ResolvedPath::as_path) + .and_then(JsImportPath::as_path) .filter(|path| !BiomePath::new(path).is_dependency()) else { return Vec::new(); diff --git a/crates/biome_js_analyze/src/lint/correctness/use_import_extensions.rs b/crates/biome_js_analyze/src/lint/correctness/use_import_extensions.rs index 2e1995ad0480..96c3b85a05f9 100644 --- a/crates/biome_js_analyze/src/lint/correctness/use_import_extensions.rs +++ b/crates/biome_js_analyze/src/lint/correctness/use_import_extensions.rs @@ -1,5 +1,5 @@ use biome_diagnostics::Severity; -use biome_module_graph::ResolvedPath; +use biome_module_graph::JsImportPath; use camino::{Utf8Component, Utf8Path}; use serde::{Deserialize, Serialize}; @@ -145,7 +145,7 @@ impl Rule for UseImportExtensions { let node = ctx.query(); let resolved_path = module_info .get_import_path_by_js_node(node) - .and_then(ResolvedPath::as_path)?; + .and_then(JsImportPath::as_path)?; get_extensionless_import(node, resolved_path, force_js_extensions) } diff --git a/crates/biome_js_analyze/src/lint/nursery/no_import_cycles.rs b/crates/biome_js_analyze/src/lint/nursery/no_import_cycles.rs index 7a19f160386b..7539ca0e7599 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_import_cycles.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_import_cycles.rs @@ -1,17 +1,16 @@ +use crate::services::module_graph::ResolvedImports; use biome_analyze::{ Rule, RuleDiagnostic, RuleDomain, RuleSource, context::RuleContext, declare_lint_rule, }; use biome_console::markup; use biome_diagnostics::Severity; use biome_js_syntax::AnyJsImportLike; -use biome_module_graph::{JsModuleInfo, ResolvedPath}; +use biome_module_graph::{JsImportPath, JsImportPhase, JsModuleInfo}; use biome_rowan::AstNode; use biome_rule_options::no_import_cycles::NoImportCyclesOptions; use camino::{Utf8Path, Utf8PathBuf}; use rustc_hash::FxHashSet; -use crate::services::module_graph::ResolvedImports; - declare_lint_rule! { /// Prevent import cycles. /// @@ -84,6 +83,62 @@ declare_lint_rule! { /// } /// ``` /// + /// **`types.ts`** + /// ```ts + /// import type { bar } from "./qux.ts"; + /// + /// export type Foo = { + /// bar: typeof bar; + /// }; + /// ``` + /// + /// **`qux.ts`** + /// ```ts + /// import type { Foo } from "./types.ts"; + /// + /// export function bar(foo: Foo) { + /// console.log(foo); + /// } + /// ``` + /// + /// ## Options + /// + /// The rule provides the options described below. + /// + /// ### `ignoreTypes` + /// + /// Ignores type-only imports when finding an import cycle. A type-only import (`import type`) + /// will be removed by the compiler, so it cuts an import cycle at runtime. Note that named type + /// imports (`import { type Foo }`) aren't considered as type-only because it's not removed by + /// the compiler if the `verbatimModuleSyntax` option is enabled. Enabled by default. + /// + /// ```json,options + /// { + /// "options": { + /// "ignoreTypes": false + /// } + /// } + /// ``` + /// + /// #### Invalid + /// + /// **`types.ts`** + /// ```ts + /// import type { bar } from "./qux.ts"; + /// + /// export type Foo = { + /// bar: typeof bar; + /// }; + /// ``` + /// + /// **`qux.ts`** + /// ```ts,use_options + /// import type { Foo } from "./types.ts"; + /// + /// export function bar(foo: Foo) { + /// console.log(foo); + /// } + /// ``` pub NoImportCycles { version: "2.0.0", name: "noImportCycles", @@ -105,13 +160,21 @@ impl Rule for NoImportCycles { fn run(ctx: &RuleContext) -> Self::Signals { let module_info = ctx.module_info_for_path(ctx.file_path())?; - let node = ctx.query(); - let resolved_path = module_info - .get_import_path_by_js_node(node) - .and_then(ResolvedPath::as_path)?; + let JsImportPath { + resolved_path, + phase, + } = module_info.get_import_path_by_js_node(node)?; + + let options = ctx.options(); + if options.ignore_types && *phase == JsImportPhase::Type { + return None; + } + + let resolved_path = resolved_path.as_path()?; let imports = ctx.module_info_for_path(resolved_path)?; + find_cycle(ctx, resolved_path, imports) } @@ -165,11 +228,20 @@ fn find_cycle( start_path: &Utf8Path, mut module_info: JsModuleInfo, ) -> Option]>> { + let options = ctx.options(); let mut seen = FxHashSet::default(); let mut stack: Vec<(Box, JsModuleInfo)> = Vec::new(); 'outer: loop { - for resolved_path in module_info.all_import_paths() { + for JsImportPath { + resolved_path, + phase, + } in module_info.all_import_paths() + { + if options.ignore_types && phase == JsImportPhase::Type { + continue; + } + let Some(path) = resolved_path.as_path() else { continue; }; diff --git a/crates/biome_js_analyze/src/lint/nursery/no_unresolved_imports.rs b/crates/biome_js_analyze/src/lint/nursery/no_unresolved_imports.rs index 33029a95d486..bec7212ce031 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_unresolved_imports.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_unresolved_imports.rs @@ -5,7 +5,7 @@ use biome_console::markup; use biome_js_syntax::{ AnyJsImportClause, AnyJsImportLike, AnyJsNamedImportSpecifier, JsModuleSource, JsSyntaxToken, }; -use biome_module_graph::{JsModuleInfo, ModuleGraph, SUPPORTED_EXTENSIONS}; +use biome_module_graph::{JsImportPath, JsModuleInfo, ModuleGraph, SUPPORTED_EXTENSIONS}; use biome_resolver::ResolveError; use biome_rowan::{AstNode, SyntaxResult, Text, TextRange, TokenText}; use biome_rule_options::no_unresolved_imports::NoUnresolvedImportsOptions; @@ -99,7 +99,8 @@ impl Rule for NoUnresolvedImports { }; let node = ctx.query(); - let Some(resolved_path) = module_info.get_import_path_by_js_node(node) else { + let Some(JsImportPath { resolved_path, .. }) = module_info.get_import_path_by_js_node(node) + else { return Vec::new(); }; diff --git a/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/ignoreTypes/a.ts b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/ignoreTypes/a.ts new file mode 100644 index 000000000000..f05667b33d8e --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/ignoreTypes/a.ts @@ -0,0 +1,7 @@ +/* should not generate diagnostics */ +import type { Foo } from "./b.ts"; +import type { Baz } from "./c.ts"; + +export type Bar = {}; + +export const baz: Baz = {}; diff --git a/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/ignoreTypes/a.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/ignoreTypes/a.ts.snap new file mode 100644 index 000000000000..e941e13a43d3 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/ignoreTypes/a.ts.snap @@ -0,0 +1,15 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: a.ts +--- +# Input +```ts +/* should not generate diagnostics */ +import type { Foo } from "./b.ts"; +import type { Baz } from "./c.ts"; + +export type Bar = {}; + +export const baz: Baz = {}; + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/ignoreTypes/b.ts b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/ignoreTypes/b.ts new file mode 100644 index 000000000000..41c4ec4457c2 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/ignoreTypes/b.ts @@ -0,0 +1,4 @@ +/* should not generate diagnostics */ +import type { Bar } from "./a.ts"; + +export type Foo = {}; diff --git a/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/ignoreTypes/b.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/ignoreTypes/b.ts.snap new file mode 100644 index 000000000000..41528f73a40c --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/ignoreTypes/b.ts.snap @@ -0,0 +1,12 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: b.ts +--- +# Input +```ts +/* should not generate diagnostics */ +import type { Bar } from "./a.ts"; + +export type Foo = {}; + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/ignoreTypes/c.ts b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/ignoreTypes/c.ts new file mode 100644 index 000000000000..76923ac9108d --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/ignoreTypes/c.ts @@ -0,0 +1,7 @@ +/* should not generate diagnostics */ + +// This is not a type-only import, but a.ts imports c.ts as type-only. +// It does not make an import cycle after the compiler erased type-only imports. +import { baz } from "./a.ts"; + +export type Baz = {}; diff --git a/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/ignoreTypes/c.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/ignoreTypes/c.ts.snap new file mode 100644 index 000000000000..9eddde4767a8 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/ignoreTypes/c.ts.snap @@ -0,0 +1,15 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: c.ts +--- +# Input +```ts +/* should not generate diagnostics */ + +// This is not a type-only import, but a.ts imports c.ts as type-only. +// It does not make an import cycle after the compiler erased type-only imports. +import { baz } from "./a.ts"; + +export type Baz = {}; + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/includeTypes.options.json b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/includeTypes.options.json new file mode 100644 index 000000000000..922904321df3 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/includeTypes.options.json @@ -0,0 +1,15 @@ +{ + "$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json", + "linter": { + "rules": { + "nursery": { + "noImportCycles": { + "level": "on", + "options": { + "ignoreTypes": false + } + } + } + } + } +} diff --git a/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/includeTypes.ts b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/includeTypes.ts new file mode 100644 index 000000000000..03f53f776bc1 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/includeTypes.ts @@ -0,0 +1,2 @@ +/* should generate diagnostics */ +import type { Foo } from "./types.ts"; diff --git a/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/includeTypes.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/includeTypes.ts.snap new file mode 100644 index 000000000000..4b8083da25bd --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/includeTypes.ts.snap @@ -0,0 +1,28 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: includeTypes.ts +--- +# Input +```ts +/* should generate diagnostics */ +import type { Foo } from "./types.ts"; + +``` + +# Diagnostics +``` +includeTypes.ts:2:26 lint/nursery/noImportCycles ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This import is part of a cycle. + + 1 │ /* should generate diagnostics */ + > 2 │ import type { Foo } from "./types.ts"; + │ ^^^^^^^^^^^^ + 3 │ + + i This import resolves to tests/specs/nursery/noImportCycles/types.ts + ... which imports tests/specs/nursery/noImportCycles/includeTypes.ts + ... which is the file we're importing from. + + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/types.ts b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/types.ts new file mode 100644 index 000000000000..624abfecfe7c --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/types.ts @@ -0,0 +1,5 @@ +/* should not generate diagnostics */ +import type {} from "./ignoreTypes.ts"; +import type {} from "./includeTypes.ts"; + +export type Foo = {}; diff --git a/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/types.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/types.ts.snap new file mode 100644 index 000000000000..f00855aa83ae --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noImportCycles/types.ts.snap @@ -0,0 +1,13 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: types.ts +--- +# Input +```ts +/* should not generate diagnostics */ +import type {} from "./ignoreTypes.ts"; +import type {} from "./includeTypes.ts"; + +export type Foo = {}; + +``` diff --git a/crates/biome_module_graph/src/js_module_info.rs b/crates/biome_module_graph/src/js_module_info.rs index b95c714d3380..da935f9c0c00 100644 --- a/crates/biome_module_graph/src/js_module_info.rs +++ b/crates/biome_module_graph/src/js_module_info.rs @@ -11,6 +11,7 @@ use biome_js_type_info::{BindingId, ImportSymbol, ResolvedTypeId, ScopeId, TypeD use biome_jsdoc_comment::JsdocComment; use biome_resolver::ResolvedPath; use biome_rowan::{Text, TextRange}; +use camino::Utf8Path; use indexmap::IndexMap; use rust_lapper::Lapper; use rustc_hash::FxHashMap; @@ -38,7 +39,7 @@ impl Deref for JsModuleInfo { impl JsModuleInfo { /// Returns an iterator over all the static and dynamic imports in this /// module. - pub fn all_import_paths(&self) -> impl Iterator + use<> { + pub fn all_import_paths(&self) -> impl Iterator + use<> { ImportPathIterator { module_info: self.clone(), index: 0, @@ -101,11 +102,11 @@ pub struct JsModuleInfoInner { /// Map of all the paths from static imports in the module. /// - /// Maps from the source specifier name to a [JsResolvedPath] with the + /// Maps from the source specifier name to a [JsImportPath] with the /// absolute path it resolves to. The resolved path may be looked up as key /// in the [ModuleGraph::data] map, although it is not required to exist /// (for instance, if the path is outside the project's scope). - pub static_import_paths: IndexMap, + pub static_import_paths: IndexMap, /// Map of all dynamic import paths found in the module for which the import /// specifier could be statically determined. @@ -114,14 +115,14 @@ pub struct JsModuleInfoInner { /// (for instance, because a template string with variables is used) will be /// omitted from this map. /// - /// Maps from the source specifier name to a [JsResolvedPath] with the + /// Maps from the source specifier name to a [JsImportPath] with the /// absolute path it resolves to. The resolved path may be looked up as key /// in the [ModuleGraph::data] map, although it is not required to exist /// (for instance, if the path is outside the project's scope). /// /// Paths found in `require()` expressions in CommonJS sources are also /// included with the dynamic import paths. - pub dynamic_import_paths: IndexMap, + pub dynamic_import_paths: IndexMap, /// Map of exports from the module. /// @@ -177,6 +178,31 @@ impl Deref for Imports { } } +#[derive(Copy, Clone, Debug, Default, Eq, PartialEq)] +pub enum JsImportPhase { + #[default] + Default, + /// https://tc39.es/proposal-defer-import-eval/ + Defer, + /// https://tc39.es/proposal-source-phase-imports/ + Source, + /// Technically this is not an import phase defined in ECMAScript, but type-only imports in + /// TypeScript cannot be imported in other phases. + Type, +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct JsImportPath { + pub resolved_path: ResolvedPath, + pub phase: JsImportPhase, +} + +impl JsImportPath { + pub fn as_path(&self) -> Option<&Utf8Path> { + self.resolved_path.as_path() + } +} + static_assertions::assert_impl_all!(JsModuleInfo: Send, Sync); impl JsModuleInfoInner { @@ -208,7 +234,7 @@ impl JsModuleInfoInner { } /// Returns the information about a given import by its syntax node. - pub fn get_import_path_by_js_node(&self, node: &AnyJsImportLike) -> Option<&ResolvedPath> { + pub fn get_import_path_by_js_node(&self, node: &AnyJsImportLike) -> Option<&JsImportPath> { let specifier_text = node.inner_string_text()?; let specifier = specifier_text.text(); if node.is_static_import() { @@ -313,7 +339,7 @@ struct ImportPathIterator { } impl Iterator for ImportPathIterator { - type Item = ResolvedPath; + type Item = JsImportPath; fn next(&mut self) -> Option { let num_static_imports = self.module_info.static_import_paths.len(); diff --git a/crates/biome_module_graph/src/js_module_info/collector.rs b/crates/biome_module_graph/src/js_module_info/collector.rs index 27079c99c1bb..365c20495dab 100644 --- a/crates/biome_module_graph/src/js_module_info/collector.rs +++ b/crates/biome_module_graph/src/js_module_info/collector.rs @@ -19,16 +19,16 @@ use indexmap::IndexMap; use rust_lapper::{Interval, Lapper}; use rustc_hash::FxHashMap; +use super::{ + Exports, ImportSymbol, Imports, JsExport, JsImport, JsModuleInfo, JsModuleInfoInner, + JsOwnExport, JsReexport, ResolvedPath, binding::JsBindingData, scope::JsScopeData, +}; use crate::js_module_info::{ binding::{JsBindingReference, JsBindingReferenceKind, JsDeclarationKind}, scope::TsBindingReference, scope_id_for_range, }; - -use super::{ - Exports, ImportSymbol, Imports, JsExport, JsImport, JsModuleInfo, JsModuleInfoInner, - JsOwnExport, JsReexport, ResolvedPath, binding::JsBindingData, scope::JsScopeData, -}; +use crate::{JsImportPath, JsImportPhase}; /// Responsible for collecting all the information from which to build the /// [`JsModuleInfo`]. @@ -74,11 +74,11 @@ pub(super) struct JsModuleInfoCollector { /// Map with all static import paths, from the source specifier to the /// resolved path. - static_import_paths: IndexMap, + static_import_paths: IndexMap, /// Map with all dynamic import paths, from the import source to the /// resolved path. - dynamic_import_paths: IndexMap, + dynamic_import_paths: IndexMap, /// All collected exports. /// @@ -194,7 +194,8 @@ impl JsModuleInfoCollector { let source = node.source().ok()?; let source_token = source.as_js_module_source()?.value_token().ok()?; let source = inner_string_text(&source_token); - let resolved_path = self.static_import_paths.get(source.text())?; + let JsImportPath { resolved_path, .. } = + self.static_import_paths.get(source.text())?; let default_specifier = node.default_specifier().ok()?; let local_name = default_specifier.local_name().ok()?; @@ -249,7 +250,8 @@ impl JsModuleInfoCollector { let source = node.source().ok()?; let source_token = source.as_js_module_source()?.value_token().ok()?; let source = inner_string_text(&source_token); - let resolved_path = self.static_import_paths.get(source.text())?; + let JsImportPath { resolved_path, .. } = + self.static_import_paths.get(source.text())?; let local_name = node.default_specifier().ok()?.local_name().ok()?; let local_name = local_name.as_js_identifier_binding()?; @@ -267,7 +269,8 @@ impl JsModuleInfoCollector { let source = node.source().ok()?; let source_token = source.as_js_module_source()?.value_token().ok()?; let source = inner_string_text(&source_token); - let resolved_path = self.static_import_paths.get(source.text())?; + let JsImportPath { resolved_path, .. } = + self.static_import_paths.get(source.text())?; for specifier in node.named_specifiers().ok()?.specifiers() { let specifier = specifier.ok()?; @@ -292,7 +295,8 @@ impl JsModuleInfoCollector { let source = node.source().ok()?; let source_token = source.as_js_module_source()?.value_token().ok()?; let source = inner_string_text(&source_token); - let resolved_path = self.static_import_paths.get(source.text())?; + let JsImportPath { resolved_path, .. } = + self.static_import_paths.get(source.text())?; let specifier = node.namespace_specifier().ok()?; let local_name = specifier.local_name().ok()?; @@ -335,18 +339,30 @@ impl JsModuleInfoCollector { &mut self, specifier: TokenText, resolved_path: ResolvedPath, + phase: JsImportPhase, ) { - self.static_import_paths - .insert(specifier.into(), resolved_path); + self.static_import_paths.insert( + specifier.into(), + JsImportPath { + resolved_path, + phase, + }, + ); } pub fn register_dynamic_import_path( &mut self, specifier: TokenText, resolved_path: ResolvedPath, + phase: JsImportPhase, ) { - self.dynamic_import_paths - .insert(specifier.into(), resolved_path); + self.dynamic_import_paths.insert( + specifier.into(), + JsImportPath { + resolved_path, + phase, + }, + ); } fn push_event(&mut self, event: SemanticEvent) { diff --git a/crates/biome_module_graph/src/js_module_info/module_resolver.rs b/crates/biome_module_graph/src/js_module_info/module_resolver.rs index 03c9d683f9c7..55f834edbd78 100644 --- a/crates/biome_module_graph/src/js_module_info/module_resolver.rs +++ b/crates/biome_module_graph/src/js_module_info/module_resolver.rs @@ -12,7 +12,7 @@ use biome_rowan::{AstNode, RawSyntaxKind, Text, TextRange, TokenText}; use rustc_hash::{FxHashMap, FxHashSet}; use crate::{ - JsExport, JsOwnExport, ModuleGraph, + JsExport, JsImportPath, JsOwnExport, ModuleGraph, js_module_info::{JsModuleInfoInner, scope::TsBindingReference}, }; @@ -184,7 +184,7 @@ impl ModuleResolver { let mut i = 0; while i < self.modules.len() { let module = self.modules[i].clone(); - for resolved_path in module.static_import_paths.values() { + for JsImportPath { resolved_path, .. } in module.static_import_paths.values() { self.register_module(resolved_path.clone()); } diff --git a/crates/biome_module_graph/src/js_module_info/visitor.rs b/crates/biome_module_graph/src/js_module_info/visitor.rs index 5031c868296a..7683dbdfa5b9 100644 --- a/crates/biome_module_graph/src/js_module_info/visitor.rs +++ b/crates/biome_module_graph/src/js_module_info/visitor.rs @@ -1,9 +1,9 @@ use biome_js_syntax::{ AnyJsArrayBindingPatternElement, AnyJsBinding, AnyJsBindingPattern, AnyJsDeclarationClause, - AnyJsExportClause, AnyJsExportDefaultDeclaration, AnyJsExpression, AnyJsImportLike, - AnyJsObjectBindingPatternMember, AnyJsRoot, AnyTsIdentifierBinding, AnyTsModuleName, - JsExportFromClause, JsExportNamedFromClause, JsExportNamedSpecifierList, JsIdentifierBinding, - JsVariableDeclaratorList, TsExportAssignmentClause, unescape_js_string, + AnyJsExportClause, AnyJsExportDefaultDeclaration, AnyJsExpression, AnyJsImportClause, + AnyJsImportLike, AnyJsObjectBindingPatternMember, AnyJsRoot, AnyTsIdentifierBinding, + AnyTsModuleName, JsExportFromClause, JsExportNamedFromClause, JsExportNamedSpecifierList, + JsIdentifierBinding, JsVariableDeclaratorList, TsExportAssignmentClause, unescape_js_string, }; use biome_js_type_info::{ImportSymbol, ScopeId, TypeData, TypeReference, TypeResolver}; use biome_jsdoc_comment::JsdocComment; @@ -12,7 +12,7 @@ use biome_rowan::{AstNode, TokenText, WalkEvent}; use camino::Utf8Path; use crate::{ - JsImport, JsModuleInfo, JsReexport, SUPPORTED_EXTENSIONS, + JsImport, JsImportPhase, JsModuleInfo, JsReexport, SUPPORTED_EXTENSIONS, js_module_info::collector::JsCollectedExport, module_graph::ModuleGraphFsProxy, }; @@ -65,11 +65,39 @@ impl<'a> JsModuleVisitor<'a> { let resolved_path = self.resolved_path_from_specifier(specifier.text()); match node { - AnyJsImportLike::JsModuleSource(_) => { - collector.register_static_import_path(specifier, resolved_path); + AnyJsImportLike::JsModuleSource(source) => { + // TODO: support defer or source imports + let phase = if let Some(clause) = source.parent::() { + match clause { + AnyJsImportClause::JsImportDefaultClause(clause) + if clause.type_token().is_some() => + { + JsImportPhase::Type + } + AnyJsImportClause::JsImportNamedClause(clause) + if clause.type_token().is_some() => + { + JsImportPhase::Type + } + AnyJsImportClause::JsImportNamespaceClause(clause) + if clause.type_token().is_some() => + { + JsImportPhase::Type + } + _ => JsImportPhase::Default, + } + } else { + JsImportPhase::Default + }; + + collector.register_static_import_path(specifier, resolved_path, phase); } AnyJsImportLike::JsCallExpression(_) | AnyJsImportLike::JsImportCallExpression(_) => { - collector.register_dynamic_import_path(specifier, resolved_path); + collector.register_dynamic_import_path( + specifier, + resolved_path, + JsImportPhase::Default, // TODO: support defer or source imports + ); } } } diff --git a/crates/biome_module_graph/src/lib.rs b/crates/biome_module_graph/src/lib.rs index b573bfcfbcb8..86e311ee1c0d 100644 --- a/crates/biome_module_graph/src/lib.rs +++ b/crates/biome_module_graph/src/lib.rs @@ -8,6 +8,7 @@ pub use biome_js_type_info::ImportSymbol; pub use biome_resolver::ResolvedPath; pub use js_module_info::{ - JsExport, JsImport, JsModuleInfo, JsOwnExport, JsReexport, ModuleResolver, + JsExport, JsImport, JsImportPath, JsImportPhase, JsModuleInfo, JsOwnExport, JsReexport, + ModuleResolver, }; pub use module_graph::{ModuleGraph, SUPPORTED_EXTENSIONS}; diff --git a/crates/biome_module_graph/tests/spec_tests.rs b/crates/biome_module_graph/tests/spec_tests.rs index 3e4f195edd2d..20b4e92618a8 100644 --- a/crates/biome_module_graph/tests/spec_tests.rs +++ b/crates/biome_module_graph/tests/spec_tests.rs @@ -13,9 +13,9 @@ use biome_js_type_info::{ScopeId, TypeData, TypeResolver}; use biome_jsdoc_comment::JsdocComment; use biome_json_parser::{JsonParserOptions, parse_json}; use biome_json_value::{JsonObject, JsonString}; -use biome_module_graph::JsExport; use biome_module_graph::{ - ImportSymbol, JsImport, JsReexport, ModuleGraph, ModuleResolver, ResolvedPath, + ImportSymbol, JsExport, JsImport, JsImportPath, JsImportPhase, JsReexport, ModuleGraph, + ModuleResolver, ResolvedPath, }; use biome_package::{Dependencies, PackageJson}; use biome_project_layout::ProjectLayout; @@ -1974,9 +1974,10 @@ fn test_resolve_swr_types() { .expect("module must exist"); assert_eq!( index_module.static_import_paths.get("swr"), - Some(&ResolvedPath::from_path(format!( - "{swr_path}/dist/index/index.d.mts" - ))) + Some(&JsImportPath { + resolved_path: ResolvedPath::from_path(format!("{swr_path}/dist/index/index.d.mts")), + phase: JsImportPhase::Default, + }) ); let swr_index_module = module_graph @@ -1986,9 +1987,12 @@ fn test_resolve_swr_types() { swr_index_module .static_import_paths .get("../_internal/index.mjs"), - Some(&ResolvedPath::from_path(format!( - "{swr_path}/dist/_internal/index.d.mts" - ))) + Some(&JsImportPath { + resolved_path: ResolvedPath::from_path(format!( + "{swr_path}/dist/_internal/index.d.mts" + )), + phase: JsImportPhase::Default, + }) ); let resolver = Arc::new(ModuleResolver::for_module( diff --git a/crates/biome_rule_options/src/no_import_cycles.rs b/crates/biome_rule_options/src/no_import_cycles.rs index 63b5bee94b1d..b3f7edb45f20 100644 --- a/crates/biome_rule_options/src/no_import_cycles.rs +++ b/crates/biome_rule_options/src/no_import_cycles.rs @@ -1,6 +1,27 @@ use biome_deserialize_macros::Deserializable; use serde::{Deserialize, Serialize}; -#[derive(Default, Clone, Debug, Deserialize, Deserializable, Eq, PartialEq, Serialize)] + +#[derive(Clone, Debug, Deserialize, Deserializable, Eq, PartialEq, Serialize)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase", deny_unknown_fields, default)] -pub struct NoImportCyclesOptions {} +pub struct NoImportCyclesOptions { + /// Ignores type-only imports when finding an import cycle. A type-only import (`import type`) + /// will be removed by the compiler, so it cuts an import cycle at runtime. Note that named type + /// imports (`import { type Foo }`) aren't considered as type-only because it's not removed by + /// the compiler if the `verbatimModuleSyntax` option is enabled. Enabled by default. + #[serde(default = "ignore_types_default")] + pub ignore_types: bool, +} + +impl Default for NoImportCyclesOptions { + fn default() -> Self { + Self { + ignore_types: ignore_types_default(), + } + } +} + +#[inline] +fn ignore_types_default() -> bool { + true +} diff --git a/packages/@biomejs/backend-jsonrpc/src/workspace.ts b/packages/@biomejs/backend-jsonrpc/src/workspace.ts index ba76f9e19ed3..d02634f7b75f 100644 --- a/packages/@biomejs/backend-jsonrpc/src/workspace.ts +++ b/packages/@biomejs/backend-jsonrpc/src/workspace.ts @@ -7860,7 +7860,12 @@ export interface NoExcessiveLinesPerFunctionOptions { export interface NoFloatingPromisesOptions {} export interface NoGlobalDirnameFilenameOptions {} export interface NoImplicitCoercionOptions {} -export interface NoImportCyclesOptions {} +export interface NoImportCyclesOptions { + /** + * Ignores type-only imports when finding an import cycle. A type-only import (`import type`) will be removed by the compiler, so it cuts an import cycle at runtime. Note that named type imports (`import { type Foo }`) aren't considered as type-only because it's not removed by the compiler if the `verbatimModuleSyntax` option is enabled. Enabled by default. + */ + ignoreTypes?: boolean; +} export interface NoImportantStylesOptions {} export interface NoMagicNumbersOptions {} export interface NoMisusedPromisesOptions {} diff --git a/packages/@biomejs/biome/configuration_schema.json b/packages/@biomejs/biome/configuration_schema.json index 17c268226be5..c8085943cdd2 100644 --- a/packages/@biomejs/biome/configuration_schema.json +++ b/packages/@biomejs/biome/configuration_schema.json @@ -3340,6 +3340,13 @@ }, "NoImportCyclesOptions": { "type": "object", + "properties": { + "ignoreTypes": { + "description": "Ignores type-only imports when finding an import cycle. A type-only import (`import type`) will be removed by the compiler, so it cuts an import cycle at runtime. Note that named type imports (`import { type Foo }`) aren't considered as type-only because it's not removed by the compiler if the `verbatimModuleSyntax` option is enabled. Enabled by default.", + "default": true, + "type": "boolean" + } + }, "additionalProperties": false }, "NoImportantInKeyframeConfiguration": {