diff --git a/CHANGELOG.md b/CHANGELOG.md index ac4765b877f8..b9b867f1e308 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,8 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b ### Linter +#### Bug Fixes + - Biome no longer crashes when it encounters a string that contain a multibyte character ([#4181](https://github.com/biomejs/biome/issues/4181)). This fixes a regression introduced in Biome 1.9.3 @@ -49,6 +51,17 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b - Fix [#4041](https://github.com/biomejs/biome/issues/4041). Now the rule `useSortedClasses` won't be triggered if `className` is composed only by inlined variables. Contributed by @ematipico +- [useImportType](https://biomejs.dev/linter/rules/use-import-type/) now reports useless inline type qualifiers ([#4178](https://github.com/biomejs/biome/issues/4178)). + + The following fix is now proposed: + + ```diff + - import type { type A, B } from ""; + + import type { A, B } from ""; + ``` + + Contributed by @Conaclos + ### Parser #### Bug Fixes diff --git a/crates/biome_js_analyze/src/lint/style/use_import_type.rs b/crates/biome_js_analyze/src/lint/style/use_import_type.rs index 678fec49aa97..29b77ce026d5 100644 --- a/crates/biome_js_analyze/src/lint/style/use_import_type.rs +++ b/crates/biome_js_analyze/src/lint/style/use_import_type.rs @@ -136,10 +136,8 @@ impl Rule for UseImportType { if import_clause.assertion().is_some() { return None; } - if import_clause.type_token().is_some() || - // Import attributes and type-only imports are not compatible. - import_clause.assertion().is_some() - { + // Import attributes and type-only imports are not compatible. + if import_clause.assertion().is_some() { return None; } let model = ctx.model(); @@ -157,7 +155,7 @@ impl Rule for UseImportType { }; match clause.specifier().ok()? { AnyJsCombinedSpecifier::JsNamedImportSpecifiers(named_specifiers) => { - match named_import_type_fix(model, &named_specifiers) { + match named_import_type_fix(model, &named_specifiers, false) { Some(NamedImportTypeFix::UseImportType(specifiers)) => { if is_default_used_as_type { Some(ImportTypeFix::UseImportType) @@ -180,6 +178,10 @@ impl Rule for UseImportType { Some(ImportTypeFix::AddInlineTypeQualifiers(specifiers)) } } + Some(NamedImportTypeFix::RemoveInlineTypeQualifiers(_)) => { + // Should not be reached because we pass `false` to `named_import_type_fix`. + None + } None => is_default_used_as_type .then_some(ImportTypeFix::ExtractDefaultImportType(vec![])), } @@ -206,6 +208,9 @@ impl Rule for UseImportType { } } AnyJsImportClause::JsImportDefaultClause(clause) => { + if clause.type_token().is_some() { + return None; + } let default_binding = clause.default_specifier().ok()?.local_name().ok()?; let default_binding = default_binding.as_js_identifier_binding()?; if ctx.jsx_runtime() == JsxRuntime::ReactClassic @@ -217,14 +222,24 @@ impl Rule for UseImportType { is_only_used_as_type(model, default_binding).then_some(ImportTypeFix::UseImportType) } AnyJsImportClause::JsImportNamedClause(clause) => { - match named_import_type_fix(model, &clause.named_specifiers().ok()?)? { + match named_import_type_fix( + model, + &clause.named_specifiers().ok()?, + clause.type_token().is_some(), + )? { NamedImportTypeFix::UseImportType(_) => Some(ImportTypeFix::UseImportType), NamedImportTypeFix::AddInlineTypeQualifiers(specifiers) => { Some(ImportTypeFix::AddInlineTypeQualifiers(specifiers)) } + NamedImportTypeFix::RemoveInlineTypeQualifiers(type_tokens) => { + Some(ImportTypeFix::RemoveTypeQualifiers(type_tokens)) + } } } AnyJsImportClause::JsImportNamespaceClause(clause) => { + if clause.type_token().is_some() { + return None; + } let namespace_binding = clause.namespace_specifier().ok()?.local_name().ok()?; let namespace_binding = namespace_binding.as_js_identifier_binding()?; if ctx.jsx_runtime() == JsxRuntime::ReactClassic @@ -300,6 +315,20 @@ impl Rule for UseImportType { } diagnostic } + ImportTypeFix::RemoveTypeQualifiers(type_tokens) => { + let mut diagnostic = RuleDiagnostic::new( + rule_category!(), + import.import_clause().ok()?.type_token()?.text_trimmed_range(), + "The import has this type qualifier that makes all inline type qualifiers useless.", + ); + for type_token in type_tokens { + diagnostic = diagnostic.detail( + type_token.text_trimmed_range(), + "This inline type qualifier is useless.", + ) + } + return Some(diagnostic); + } }; Some(diagnostic.note(markup! { "Importing the types with ""import type"" ensures that they are removed by the transpilers and avoids loading unnecessary modules." @@ -527,6 +556,11 @@ impl Rule for UseImportType { mutation.replace_node(specifier.clone(), new_specifier); } } + ImportTypeFix::RemoveTypeQualifiers(type_tokens) => { + for type_token in type_tokens { + mutation.remove_token(type_token.clone()); + } + } } Some(JsRuleAction::new( ActionCategory::QuickFix, @@ -543,6 +577,7 @@ pub enum ImportTypeFix { ExtractDefaultImportType(Vec), ExtractCombinedImportType, AddInlineTypeQualifiers(Vec), + RemoveTypeQualifiers(Vec), } /// Returns `true` if all references of `binding` are only used as a type. @@ -564,50 +599,71 @@ fn is_only_used_as_type(model: &SemanticModel, binding: &JsIdentifierBinding) -> pub enum NamedImportTypeFix { UseImportType(Vec), AddInlineTypeQualifiers(Vec), + RemoveInlineTypeQualifiers(Vec), } fn named_import_type_fix( model: &SemanticModel, named_specifiers: &JsNamedImportSpecifiers, + has_type_token: bool, ) -> Option { let specifiers = named_specifiers.specifiers(); if specifiers.is_empty() { return None; }; - let mut imports_only_types = true; - let mut specifiers_requiring_type_marker = Vec::with_capacity(specifiers.len()); - for specifier in specifiers.iter() { - let Ok(specifier) = specifier else { - imports_only_types = false; - continue; - }; - if specifier.type_token().is_none() { - if specifier - .local_name() - .and_then(|local_name| { - Some(is_only_used_as_type( - model, - local_name.as_js_identifier_binding()?, - )) - }) - .unwrap_or(false) - { - specifiers_requiring_type_marker.push(specifier); - } else { - imports_only_types = false; + if has_type_token { + let mut useless_type_tokens = Vec::with_capacity(specifiers.len()); + for specifier in specifiers.iter() { + let Ok(specifier) = specifier else { + continue; + }; + if let Some(type_token) = specifier.type_token() { + useless_type_tokens.push(type_token); } } - } - if imports_only_types { - Some(NamedImportTypeFix::UseImportType( - specifiers_requiring_type_marker, - )) - } else if specifiers_requiring_type_marker.is_empty() { - None + if useless_type_tokens.is_empty() { + None + } else { + Some(NamedImportTypeFix::RemoveInlineTypeQualifiers( + useless_type_tokens, + )) + } } else { - Some(NamedImportTypeFix::AddInlineTypeQualifiers( - specifiers_requiring_type_marker, - )) + let mut imports_only_types = true; + let mut specifiers_requiring_type_marker = Vec::with_capacity(specifiers.len()); + for specifier in specifiers.iter() { + let Ok(specifier) = specifier else { + imports_only_types = false; + continue; + }; + if specifier.type_token().is_none() { + if specifier + .local_name() + .and_then(|local_name| { + Some(is_only_used_as_type( + model, + local_name.as_js_identifier_binding()?, + )) + }) + .unwrap_or(false) + { + specifiers_requiring_type_marker.push(specifier); + } else { + imports_only_types = false; + } + } + } + if imports_only_types { + Some(NamedImportTypeFix::UseImportType( + specifiers_requiring_type_marker, + )) + } else if specifiers_requiring_type_marker.is_empty() { + None + } else { + Some(NamedImportTypeFix::AddInlineTypeQualifiers( + specifiers_requiring_type_marker, + )) + } } } diff --git a/crates/biome_js_analyze/tests/specs/style/useImportType/invalid-named-imports.ts b/crates/biome_js_analyze/tests/specs/style/useImportType/invalid-named-imports.ts index b956804017cd..26589a020c15 100644 --- a/crates/biome_js_analyze/tests/specs/style/useImportType/invalid-named-imports.ts +++ b/crates/biome_js_analyze/tests/specs/style/useImportType/invalid-named-imports.ts @@ -9,11 +9,10 @@ import { X, Y } from ""; type XX = X; const YY = Y; -//import { type U, V } from ""; -//type VV = V; +import { type H, type I, type J } from ""; +export type { H, I, J }; -import { type X, type Y, type Z } from ""; -export type { X, Y, Z }; +import type { type M, N, type O } from ""; // multiline import { diff --git a/crates/biome_js_analyze/tests/specs/style/useImportType/invalid-named-imports.ts.snap b/crates/biome_js_analyze/tests/specs/style/useImportType/invalid-named-imports.ts.snap index 27f83ea298df..72d5c314de85 100644 --- a/crates/biome_js_analyze/tests/specs/style/useImportType/invalid-named-imports.ts.snap +++ b/crates/biome_js_analyze/tests/specs/style/useImportType/invalid-named-imports.ts.snap @@ -15,11 +15,10 @@ import { X, Y } from ""; type XX = X; const YY = Y; -//import { type U, V } from ""; -//type VV = V; +import { type H, type I, type J } from ""; +export type { H, I, J }; -import { type X, type Y, type Z } from ""; -export type { X, Y, Z }; +import type { type M, N, type O } from ""; // multiline import { @@ -104,81 +103,116 @@ invalid-named-imports.ts:8:1 lint/style/useImportType FIXABLE ━━━━━ ``` ``` -invalid-named-imports.ts:15:1 lint/style/useImportType FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid-named-imports.ts:12:1 lint/style/useImportType FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! All these imports are only used as types. - 13 │ //type VV = V; - 14 │ - > 15 │ import { type X, type Y, type Z } from ""; + 10 │ const YY = Y; + 11 │ + > 12 │ import { type H, type I, type J } from ""; │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - 16 │ export type { X, Y, Z }; - 17 │ + 13 │ export type { H, I, J }; + 14 │ i Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules. i Safe fix: Use import type. - 13 13 │ //type VV = V; + 10 10 │ const YY = Y; + 11 11 │ + 12 │ - import·{·type·H,·type·I,·type·J·}·from·""; + 12 │ + import·type·{·H,·I,·J·}·from·""; + 13 13 │ export type { H, I, J }; 14 14 │ - 15 │ - import·{·type·X,·type·Y,·type·Z·}·from·""; - 15 │ + import·type·{·X,·Y,·Z·}·from·""; - 16 16 │ export type { X, Y, Z }; - 17 17 │ ``` ``` -invalid-named-imports.ts:19:1 lint/style/useImportType FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid-named-imports.ts:15:8 lint/style/useImportType FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The import has this type qualifier that makes all inline type qualifiers useless. + + 13 │ export type { H, I, J }; + 14 │ + > 15 │ import type { type M, N, type O } from ""; + │ ^^^^ + 16 │ + 17 │ // multiline + + i This inline type qualifier is useless. + + 13 │ export type { H, I, J }; + 14 │ + > 15 │ import type { type M, N, type O } from ""; + │ ^^^^ + 16 │ + 17 │ // multiline + + i This inline type qualifier is useless. + + 13 │ export type { H, I, J }; + 14 │ + > 15 │ import type { type M, N, type O } from ""; + │ ^^^^ + 16 │ + 17 │ // multiline + + i Safe fix: Use import type. + + 15 │ import·type·{·type·M,·N,·type·O·}·from·""; + │ ----- ----- + +``` + +``` +invalid-named-imports.ts:18:1 lint/style/useImportType FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Some named imports are only used as types. - 18 │ // multiline - > 19 │ import { + 17 │ // multiline + > 18 │ import { │ ^^^^^^^^ - > 20 │ U, - > 21 │ V, - > 22 │ // leading comment - > 23 │ W, - > 24 │ } from ""; + > 19 │ U, + > 20 │ V, + > 21 │ // leading comment + > 22 │ W, + > 23 │ } from ""; │ ^^^^^^^^^^ - 25 │ export { U, type V, type W }; - 26 │ + 24 │ export { U, type V, type W }; + 25 │ i This import is only used as a type. - 19 │ import { - 20 │ U, - > 21 │ V, + 18 │ import { + 19 │ U, + > 20 │ V, │ ^ - 22 │ // leading comment - 23 │ W, + 21 │ // leading comment + 22 │ W, i This import is only used as a type. - 21 │ V, - 22 │ // leading comment - > 23 │ W, + 20 │ V, + 21 │ // leading comment + > 22 │ W, │ ^ - 24 │ } from ""; - 25 │ export { U, type V, type W }; + 23 │ } from ""; + 24 │ export { U, type V, type W }; i Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules. i Safe fix: Use import type. - 19 19 │ import { - 20 20 │ U, - 21 │ - ····V, - 21 │ + ····type·V, - 22 22 │ // leading comment - 23 │ - ····W, - 23 │ + ····type·W, - 24 24 │ } from ""; - 25 25 │ export { U, type V, type W }; + 18 18 │ import { + 19 19 │ U, + 20 │ - ····V, + 20 │ + ····type·V, + 21 21 │ // leading comment + 22 │ - ····W, + 22 │ + ····type·W, + 23 23 │ } from ""; + 24 24 │ export { U, type V, type W }; ``` - - diff --git a/crates/biome_js_analyze/tests/specs/style/useImportType/valid-named-imports.ts b/crates/biome_js_analyze/tests/specs/style/useImportType/valid-named-imports.ts index 7ffbbfade399..6e0384de814a 100644 --- a/crates/biome_js_analyze/tests/specs/style/useImportType/valid-named-imports.ts +++ b/crates/biome_js_analyze/tests/specs/style/useImportType/valid-named-imports.ts @@ -13,3 +13,10 @@ export type { C }; import { D } from ""; let a: D = new D(); + +import { type E, F } from ""; +export type { E }; +export { F }; + +import type { G } from ""; +export type { G }; diff --git a/crates/biome_js_analyze/tests/specs/style/useImportType/valid-named-imports.ts.snap b/crates/biome_js_analyze/tests/specs/style/useImportType/valid-named-imports.ts.snap index 6ef270364b09..2d9fb077771c 100644 --- a/crates/biome_js_analyze/tests/specs/style/useImportType/valid-named-imports.ts.snap +++ b/crates/biome_js_analyze/tests/specs/style/useImportType/valid-named-imports.ts.snap @@ -20,6 +20,11 @@ export type { C }; import { D } from ""; let a: D = new D(); -``` +import { type E, F } from ""; +export type { E }; +export { F }; +import type { G } from ""; +export type { G }; +```