Skip to content

Commit a89fc81

Browse files
committed
feat(lint): add ignoreTypes option to the noImportCycles rule
1 parent be42745 commit a89fc81

File tree

25 files changed

+366
-57
lines changed

25 files changed

+366
-57
lines changed

.changeset/shaky-experts-sit.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@biomejs/biome": patch
3+
---
4+
5+
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).

crates/biome_js_analyze/src/lint/correctness/no_private_imports.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use biome_js_syntax::{
99
AnyJsImportClause, AnyJsImportLike, AnyJsNamedImportSpecifier, JsModuleSource, JsSyntaxToken,
1010
};
1111
use biome_jsdoc_comment::JsdocComment;
12-
use biome_module_graph::{JsModuleInfo, ModuleGraph, ResolvedPath};
12+
use biome_module_graph::{JsImportPath, JsModuleInfo, ModuleGraph};
1313
use biome_rowan::{AstNode, SyntaxResult, Text, TextRange};
1414
use biome_rule_options::no_private_imports::{NoPrivateImportsOptions, Visibility};
1515
use camino::{Utf8Path, Utf8PathBuf};
@@ -180,7 +180,7 @@ impl Rule for NoPrivateImports {
180180
.then(|| node.inner_string_text())
181181
.flatten()
182182
.and_then(|specifier| module_info.static_import_paths.get(specifier.text()))
183-
.and_then(ResolvedPath::as_path)
183+
.and_then(JsImportPath::as_path)
184184
.filter(|path| !BiomePath::new(path).is_dependency())
185185
else {
186186
return Vec::new();

crates/biome_js_analyze/src/lint/correctness/use_import_extensions.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use biome_diagnostics::Severity;
2-
use biome_module_graph::ResolvedPath;
2+
use biome_module_graph::JsImportPath;
33
use camino::{Utf8Component, Utf8Path};
44
use serde::{Deserialize, Serialize};
55

@@ -145,7 +145,7 @@ impl Rule for UseImportExtensions {
145145
let node = ctx.query();
146146
let resolved_path = module_info
147147
.get_import_path_by_js_node(node)
148-
.and_then(ResolvedPath::as_path)?;
148+
.and_then(JsImportPath::as_path)?;
149149

150150
get_extensionless_import(node, resolved_path, force_js_extensions)
151151
}

crates/biome_js_analyze/src/lint/nursery/no_import_cycles.rs

Lines changed: 80 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,16 @@
1+
use crate::services::module_graph::ResolvedImports;
12
use biome_analyze::{
23
Rule, RuleDiagnostic, RuleDomain, RuleSource, context::RuleContext, declare_lint_rule,
34
};
45
use biome_console::markup;
56
use biome_diagnostics::Severity;
67
use biome_js_syntax::AnyJsImportLike;
7-
use biome_module_graph::{JsModuleInfo, ResolvedPath};
8+
use biome_module_graph::{JsImportPath, JsImportPhase, JsModuleInfo};
89
use biome_rowan::AstNode;
910
use biome_rule_options::no_import_cycles::NoImportCyclesOptions;
1011
use camino::{Utf8Path, Utf8PathBuf};
1112
use rustc_hash::FxHashSet;
1213

13-
use crate::services::module_graph::ResolvedImports;
14-
1514
declare_lint_rule! {
1615
/// Prevent import cycles.
1716
///
@@ -84,6 +83,62 @@ declare_lint_rule! {
8483
/// }
8584
/// ```
8685
///
86+
/// **`types.ts`**
87+
/// ```ts
88+
/// import type { bar } from "./qux.ts";
89+
///
90+
/// export type Foo = {
91+
/// bar: typeof bar;
92+
/// };
93+
/// ```
94+
///
95+
/// **`qux.ts`**
96+
/// ```ts
97+
/// import type { Foo } from "./types.ts";
98+
///
99+
/// export function bar(foo: Foo) {
100+
/// console.log(foo);
101+
/// }
102+
/// ```
103+
///
104+
/// ## Options
105+
///
106+
/// The rule provides the options described below.
107+
///
108+
/// ### `ignoreTypes`
109+
///
110+
/// Ignores type-only imports when finding an import cycle. A type-only import (`import type`)
111+
/// will be removed by the compiler, so it cuts an import cycle at runtime. Note that named type
112+
/// imports (`import { type Foo }`) aren't considered as type-only because it's not removed by
113+
/// the compiler if the `verbatimModuleSyntax` option is enabled. Enabled by default.
114+
///
115+
/// ```json,options
116+
/// {
117+
/// "options": {
118+
/// "ignoreTypes": false
119+
/// }
120+
/// }
121+
/// ```
122+
///
123+
/// #### Invalid
124+
///
125+
/// **`types.ts`**
126+
/// ```ts
127+
/// import type { bar } from "./qux.ts";
128+
///
129+
/// export type Foo = {
130+
/// bar: typeof bar;
131+
/// };
132+
/// ```
133+
///
134+
/// **`qux.ts`**
135+
/// ```ts,use_options
136+
/// import type { Foo } from "./types.ts";
137+
///
138+
/// export function bar(foo: Foo) {
139+
/// console.log(foo);
140+
/// }
141+
/// ```
87142
pub NoImportCycles {
88143
version: "2.0.0",
89144
name: "noImportCycles",
@@ -105,13 +160,21 @@ impl Rule for NoImportCycles {
105160

106161
fn run(ctx: &RuleContext<Self>) -> Self::Signals {
107162
let module_info = ctx.module_info_for_path(ctx.file_path())?;
108-
109163
let node = ctx.query();
110-
let resolved_path = module_info
111-
.get_import_path_by_js_node(node)
112-
.and_then(ResolvedPath::as_path)?;
113164

165+
let JsImportPath {
166+
resolved_path,
167+
phase,
168+
} = module_info.get_import_path_by_js_node(node)?;
169+
170+
let options = ctx.options();
171+
if options.ignore_types && *phase == JsImportPhase::Type {
172+
return None;
173+
}
174+
175+
let resolved_path = resolved_path.as_path()?;
114176
let imports = ctx.module_info_for_path(resolved_path)?;
177+
115178
find_cycle(ctx, resolved_path, imports)
116179
}
117180

@@ -165,11 +228,20 @@ fn find_cycle(
165228
start_path: &Utf8Path,
166229
mut module_info: JsModuleInfo,
167230
) -> Option<Box<[Box<str>]>> {
231+
let options = ctx.options();
168232
let mut seen = FxHashSet::default();
169233
let mut stack: Vec<(Box<str>, JsModuleInfo)> = Vec::new();
170234

171235
'outer: loop {
172-
for resolved_path in module_info.all_import_paths() {
236+
for JsImportPath {
237+
resolved_path,
238+
phase,
239+
} in module_info.all_import_paths()
240+
{
241+
if options.ignore_types && phase == JsImportPhase::Type {
242+
continue;
243+
}
244+
173245
let Some(path) = resolved_path.as_path() else {
174246
continue;
175247
};

crates/biome_js_analyze/src/lint/nursery/no_unresolved_imports.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use biome_console::markup;
55
use biome_js_syntax::{
66
AnyJsImportClause, AnyJsImportLike, AnyJsNamedImportSpecifier, JsModuleSource, JsSyntaxToken,
77
};
8-
use biome_module_graph::{JsModuleInfo, ModuleGraph, SUPPORTED_EXTENSIONS};
8+
use biome_module_graph::{JsImportPath, JsModuleInfo, ModuleGraph, SUPPORTED_EXTENSIONS};
99
use biome_resolver::ResolveError;
1010
use biome_rowan::{AstNode, SyntaxResult, Text, TextRange, TokenText};
1111
use biome_rule_options::no_unresolved_imports::NoUnresolvedImportsOptions;
@@ -99,7 +99,8 @@ impl Rule for NoUnresolvedImports {
9999
};
100100

101101
let node = ctx.query();
102-
let Some(resolved_path) = module_info.get_import_path_by_js_node(node) else {
102+
let Some(JsImportPath { resolved_path, .. }) = module_info.get_import_path_by_js_node(node)
103+
else {
103104
return Vec::new();
104105
};
105106

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
/* should not generate diagnostics */
2+
import type { Foo } from "./b.ts";
3+
import type { Baz } from "./c.ts";
4+
5+
export type Bar = {};
6+
7+
export const baz: Baz = {};
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
source: crates/biome_js_analyze/tests/spec_tests.rs
3+
expression: a.ts
4+
---
5+
# Input
6+
```ts
7+
/* should not generate diagnostics */
8+
import type { Foo } from "./b.ts";
9+
import type { Baz } from "./c.ts";
10+
11+
export type Bar = {};
12+
13+
export const baz: Baz = {};
14+
15+
```
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
/* should not generate diagnostics */
2+
import type { Bar } from "./a.ts";
3+
4+
export type Foo = {};
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
source: crates/biome_js_analyze/tests/spec_tests.rs
3+
expression: b.ts
4+
---
5+
# Input
6+
```ts
7+
/* should not generate diagnostics */
8+
import type { Bar } from "./a.ts";
9+
10+
export type Foo = {};
11+
12+
```
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
/* should not generate diagnostics */
2+
3+
// This is not a type-only import, but a.ts imports c.ts as type-only.
4+
// It does not make an import cycle after the compiler erased type-only imports.
5+
import { baz } from "./a.ts";
6+
7+
export type Baz = {};

0 commit comments

Comments
 (0)