Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions .changeset/shaky-experts-sit.md
Original file line number Diff line number Diff line change
@@ -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
> }
> }
> }
> }
> }
> }
> ```
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -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)
}
Expand Down
88 changes: 80 additions & 8 deletions crates/biome_js_analyze/src/lint/nursery/no_import_cycles.rs
Original file line number Diff line number Diff line change
@@ -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.
///
Expand Down Expand Up @@ -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",
Expand All @@ -105,13 +160,21 @@ impl Rule for NoImportCycles {

fn run(ctx: &RuleContext<Self>) -> 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)
}

Expand Down Expand Up @@ -165,11 +228,20 @@ fn find_cycle(
start_path: &Utf8Path,
mut module_info: JsModuleInfo,
) -> Option<Box<[Box<str>]>> {
let options = ctx.options();
let mut seen = FxHashSet::default();
let mut stack: Vec<(Box<str>, 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;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
};

Expand Down
Original file line number Diff line number Diff line change
@@ -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 = {};
Original file line number Diff line number Diff line change
@@ -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 = {};

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/* should not generate diagnostics */
import type { Bar } from "./a.ts";

export type Foo = {};
Original file line number Diff line number Diff line change
@@ -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 = {};

```
Original file line number Diff line number Diff line change
@@ -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 = {};
Original file line number Diff line number Diff line change
@@ -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 = {};

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json",
"linter": {
"rules": {
"nursery": {
"noImportCycles": {
"level": "on",
"options": {
"ignoreTypes": false
}
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
/* should generate diagnostics */
import type { Foo } from "./types.ts";
Original file line number Diff line number Diff line change
@@ -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.


```
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/* should not generate diagnostics */
import type {} from "./ignoreTypes.ts";
import type {} from "./includeTypes.ts";

export type Foo = {};
Original file line number Diff line number Diff line change
@@ -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 = {};

```
Loading
Loading