From 2bd95dccfedf66d51ba17eaa65101165c40f0013 Mon Sep 17 00:00:00 2001 From: Victorien Elvinger Date: Thu, 4 Apr 2024 17:41:30 +0200 Subject: [PATCH] feat(lint/noUnusedImports): add ignoreReact option (#2306) --- CHANGELOG.md | 11 +- .../src/lint/correctness/no_unused_imports.rs | 44 +++- crates/biome_js_analyze/src/react.rs | 41 +++- .../noUnusedImports/invalid-unused-react.jsx | 8 + .../invalid-unused-react.jsx.snap | 199 ++++++++++++++++++ .../invalid-unused-react.options.json | 14 ++ .../noUnusedImports/valid-unused-react.jsx | 3 + .../valid-unused-react.jsx.snap | 11 + .../valid-unused-react.options.json | 14 ++ .../@biomejs/backend-jsonrpc/src/workspace.ts | 15 +- .../@biomejs/biome/configuration_schema.json | 28 ++- .../src/content/docs/internals/changelog.md | 11 +- .../docs/linter/rules/no-unused-imports.md | 19 ++ 13 files changed, 404 insertions(+), 14 deletions(-) create mode 100644 crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/invalid-unused-react.jsx create mode 100644 crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/invalid-unused-react.jsx.snap create mode 100644 crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/invalid-unused-react.options.json create mode 100644 crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/valid-unused-react.jsx create mode 100644 crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/valid-unused-react.jsx.snap create mode 100644 crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/valid-unused-react.options.json diff --git a/CHANGELOG.md b/CHANGELOG.md index e279cf8863a0..218ddd4ee43d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,16 +43,21 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b ### Linter -#### Bug fixes +#### New features -- Lint rules `useNodejsImportProtocol`, `useNodeAssertStrict`, `noRestrictedImports`, `noNodejsModules` will no longer check `declare module` statements anymore. Contributed by @Sec-ant +- Add a new option `ignoreReact` to [noUnusedImports](https://biomejs.dev/linter/rules/no-unused-imports). -#### New features + When `ignoreReact` is enabled, Biome ignores imports of `React` from the `react` package. + The option is disabled by default. + + Contributed by @Conaclos #### Enhancements #### Bug fixes +- Lint rules `useNodejsImportProtocol`, `useNodeAssertStrict`, `noRestrictedImports`, `noNodejsModules` will no longer check `declare module` statements anymore. Contributed by @Sec-ant + ### Parser diff --git a/crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs b/crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs index b902b1a2b990..ce9d82971aae 100644 --- a/crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs +++ b/crates/biome_js_analyze/src/lint/correctness/no_unused_imports.rs @@ -1,8 +1,13 @@ -use crate::{services::semantic::Semantic, JsRuleAction}; +use crate::{ + react::{is_global_react_import, ReactLibrary}, + services::semantic::Semantic, + JsRuleAction, +}; use biome_analyze::{ context::RuleContext, declare_rule, ActionCategory, FixKind, Rule, RuleDiagnostic, }; use biome_console::markup; +use biome_deserialize_macros::Deserializable; use biome_diagnostics::Applicability; use biome_js_factory::make; use biome_js_semantic::ReferencesExtensions; @@ -11,6 +16,10 @@ use biome_js_syntax::{ JsIdentifierBinding, JsImport, JsLanguage, JsNamedImportSpecifierList, JsSyntaxNode, T, }; use biome_rowan::{AstNode, AstSeparatedList, BatchMutation, BatchMutationExt}; +use serde::{Deserialize, Serialize}; + +#[cfg(feature = "schemars")] +use schemars::JsonSchema; declare_rule! { /// Disallow unused imports. @@ -23,6 +32,25 @@ declare_rule! { /// the unused imports will also be removed. So that comment directives /// like `@ts-expect-error` won't be transferred to a wrong place. /// + /// ## Options + /// + /// The rule provides a single option `ignoreReact`. + /// When this option is set to `true`, imports named `React` from the package `react` are ignored. + /// `ignoreReact` is disabled by default. + /// + /// ```json + /// { + /// "//": "...", + /// "options": { + /// "ignoreReact": true + /// } + /// } + /// ``` + /// + /// This option should only be necessary if you cannot upgrade to a React version that supports the new JSX runtime. + /// In the new JSX runtime, you no longer need to import `React`. + /// You can find more details in [this comment](https://github.com/biomejs/biome/issues/571#issuecomment-1774026734). + /// /// ## Examples /// /// ### Invalid @@ -77,7 +105,7 @@ impl Rule for NoUnusedImports { type Query = Semantic; type State = (); type Signals = Option; - type Options = (); + type Options = UnusedImportsOptions; fn run(ctx: &RuleContext) -> Self::Signals { let binding = ctx.query(); @@ -85,7 +113,9 @@ impl Rule for NoUnusedImports { if !is_import(&declaration) { return None; } - + if ctx.options().ignore_react && is_global_react_import(binding, ReactLibrary::React) { + return None; + } let model = ctx.model(); binding.all_references(model).next().is_none().then_some(()) } @@ -152,6 +182,14 @@ impl Rule for NoUnusedImports { } } +#[derive(Clone, Debug, Default, Deserializable, Deserialize, Eq, PartialEq, Serialize)] +#[cfg_attr(feature = "schemars", derive(JsonSchema))] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct UnusedImportsOptions { + /// Ignore `React` imports from the `react` package when set to `true`. + ignore_react: bool, +} + fn remove_import_specifier( mutation: &mut BatchMutation, specifier: &JsSyntaxNode, diff --git a/crates/biome_js_analyze/src/react.rs b/crates/biome_js_analyze/src/react.rs index 12580c92f4dd..84530953dbe4 100644 --- a/crates/biome_js_analyze/src/react.rs +++ b/crates/biome_js_analyze/src/react.rs @@ -4,9 +4,10 @@ pub mod hooks; use biome_js_semantic::{Binding, SemanticModel}; use biome_js_syntax::{ - AnyJsCallArgument, AnyJsExpression, AnyJsMemberExpression, AnyJsNamedImportSpecifier, - AnyJsObjectMember, JsCallExpression, JsIdentifierBinding, JsImport, JsObjectExpression, - JsPropertyObjectMember, JsxMemberName, JsxReferenceIdentifier, + binding_ext::AnyJsBindingDeclaration, AnyJsCallArgument, AnyJsExpression, + AnyJsMemberExpression, AnyJsNamedImportSpecifier, AnyJsObjectMember, JsCallExpression, + JsIdentifierBinding, JsImport, JsObjectExpression, JsPropertyObjectMember, JsxMemberName, + JsxReferenceIdentifier, }; use biome_rowan::{AstNode, AstSeparatedList}; @@ -291,3 +292,37 @@ fn is_named_react_export(binding: &Binding, lib: ReactLibrary, name: &str) -> Op let import = import_specifier.import_clause()?.parent::()?; Some(import.source_text().ok()?.text() == lib.import_name()) } + +/// Checks if `binding` is an import of the global name of `lib`. +pub(crate) fn is_global_react_import(binding: &JsIdentifierBinding, lib: ReactLibrary) -> bool { + if !binding + .name_token() + .is_ok_and(|name| name.text_trimmed() == lib.global_name()) + { + return false; + }; + let Some(decl) = binding.declaration() else { + return false; + }; + // This must be a default import or a namespace import + let syntax = match decl { + AnyJsBindingDeclaration::JsNamedImportSpecifier(specifier) => { + if !specifier.name().is_ok_and(|name| name.is_default()) { + return false; + } + specifier.into_syntax() + } + AnyJsBindingDeclaration::JsDefaultImportSpecifier(specifier) => specifier.into_syntax(), + AnyJsBindingDeclaration::JsNamespaceImportSpecifier(specifier) => specifier.into_syntax(), + _ => { + return false; + } + }; + // Check import source + syntax + .ancestors() + .skip(1) + .find_map(JsImport::cast) + .and_then(|import| import.source_text().ok()) + .is_some_and(|source| source.text() == lib.import_name()) +} diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/invalid-unused-react.jsx b/crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/invalid-unused-react.jsx new file mode 100644 index 000000000000..a9b7e46659eb --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/invalid-unused-react.jsx @@ -0,0 +1,8 @@ +import X from "react" +import * as X from "react" +import { default as X } from "react" + +import React from "x" +import * as React from "x" +import { default as React } from "x" +import React, { useEffect } from "x" diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/invalid-unused-react.jsx.snap b/crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/invalid-unused-react.jsx.snap new file mode 100644 index 000000000000..f8ad82df81fa --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/invalid-unused-react.jsx.snap @@ -0,0 +1,199 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: invalid-unused-react.jsx +--- +# Input +```jsx +import X from "react" +import * as X from "react" +import { default as X } from "react" + +import React from "x" +import * as React from "x" +import { default as React } from "x" +import React, { useEffect } from "x" + +``` + +# Diagnostics +``` +invalid-unused-react.jsx:1:8 lint/correctness/noUnusedImports FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This import is unused. + + > 1 │ import X from "react" + │ ^ + 2 │ import * as X from "react" + 3 │ import { default as X } from "react" + + i Unused imports might be the result of an incomplete refactoring. + + i Safe fix: Remove the unused import. + + 1 │ import·X·from·"react" + │ --------------------- + +``` + +``` +invalid-unused-react.jsx:2:13 lint/correctness/noUnusedImports FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This import is unused. + + 1 │ import X from "react" + > 2 │ import * as X from "react" + │ ^ + 3 │ import { default as X } from "react" + 4 │ + + i Unused imports might be the result of an incomplete refactoring. + + i Safe fix: Remove the unused import. + + 1 1 │ import X from "react" + 2 │ - import·*·as·X·from·"react" + 3 2 │ import { default as X } from "react" + 4 3 │ + + +``` + +``` +invalid-unused-react.jsx:3:21 lint/correctness/noUnusedImports FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This import is unused. + + 1 │ import X from "react" + 2 │ import * as X from "react" + > 3 │ import { default as X } from "react" + │ ^ + 4 │ + 5 │ import React from "x" + + i Unused imports might be the result of an incomplete refactoring. + + i Safe fix: Remove the unused import. + + 1 1 │ import X from "react" + 2 2 │ import * as X from "react" + 3 │ - import·{·default·as·X·}·from·"react" + 4 3 │ + 5 4 │ import React from "x" + + +``` + +``` +invalid-unused-react.jsx:5:8 lint/correctness/noUnusedImports FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This import is unused. + + 3 │ import { default as X } from "react" + 4 │ + > 5 │ import React from "x" + │ ^^^^^ + 6 │ import * as React from "x" + 7 │ import { default as React } from "x" + + i Unused imports might be the result of an incomplete refactoring. + + i Safe fix: Remove the unused import. + + 2 2 │ import * as X from "react" + 3 3 │ import { default as X } from "react" + 4 │ - + 5 │ - import·React·from·"x" + 6 4 │ import * as React from "x" + 7 5 │ import { default as React } from "x" + + +``` + +``` +invalid-unused-react.jsx:6:13 lint/correctness/noUnusedImports FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This import is unused. + + 5 │ import React from "x" + > 6 │ import * as React from "x" + │ ^^^^^ + 7 │ import { default as React } from "x" + 8 │ import React, { useEffect } from "x" + + i Unused imports might be the result of an incomplete refactoring. + + i Safe fix: Remove the unused import. + + 4 4 │ + 5 5 │ import React from "x" + 6 │ - import·*·as·React·from·"x" + 7 6 │ import { default as React } from "x" + 8 7 │ import React, { useEffect } from "x" + + +``` + +``` +invalid-unused-react.jsx:7:21 lint/correctness/noUnusedImports FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This import is unused. + + 5 │ import React from "x" + 6 │ import * as React from "x" + > 7 │ import { default as React } from "x" + │ ^^^^^ + 8 │ import React, { useEffect } from "x" + 9 │ + + i Unused imports might be the result of an incomplete refactoring. + + i Safe fix: Remove the unused import. + + 5 5 │ import React from "x" + 6 6 │ import * as React from "x" + 7 │ - import·{·default·as·React·}·from·"x" + 8 7 │ import React, { useEffect } from "x" + 9 8 │ + + +``` + +``` +invalid-unused-react.jsx:8:8 lint/correctness/noUnusedImports FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This import is unused. + + 6 │ import * as React from "x" + 7 │ import { default as React } from "x" + > 8 │ import React, { useEffect } from "x" + │ ^^^^^ + 9 │ + + i Unused imports might be the result of an incomplete refactoring. + + i Safe fix: Remove the unused import. + + 8 │ import·React,·{·useEffect·}·from·"x" + │ ------- + +``` + +``` +invalid-unused-react.jsx:8:17 lint/correctness/noUnusedImports FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This import is unused. + + 6 │ import * as React from "x" + 7 │ import { default as React } from "x" + > 8 │ import React, { useEffect } from "x" + │ ^^^^^^^^^ + 9 │ + + i Unused imports might be the result of an incomplete refactoring. + + i Safe fix: Remove the unused import. + + 8 │ import·React,·{·useEffect·}·from·"x" + │ --------------- + +``` diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/invalid-unused-react.options.json b/crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/invalid-unused-react.options.json new file mode 100644 index 000000000000..9dc81bdf9b0b --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/invalid-unused-react.options.json @@ -0,0 +1,14 @@ +{ + "linter": { + "rules": { + "correctness": { + "noUnusedImports": { + "level": "error", + "options": { + "ignoreReact": true + } + } + } + } + } +} \ No newline at end of file diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/valid-unused-react.jsx b/crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/valid-unused-react.jsx new file mode 100644 index 000000000000..15daa84798e0 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/valid-unused-react.jsx @@ -0,0 +1,3 @@ +import React from "react" +import { default as React } from "react" +import * as React from "react" diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/valid-unused-react.jsx.snap b/crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/valid-unused-react.jsx.snap new file mode 100644 index 000000000000..6686741021b5 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/valid-unused-react.jsx.snap @@ -0,0 +1,11 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: valid-unused-react.jsx +--- +# Input +```jsx +import React from "react" +import { default as React } from "react" +import * as React from "react" + +``` diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/valid-unused-react.options.json b/crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/valid-unused-react.options.json new file mode 100644 index 000000000000..9dc81bdf9b0b --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/noUnusedImports/valid-unused-react.options.json @@ -0,0 +1,14 @@ +{ + "linter": { + "rules": { + "correctness": { + "noUnusedImports": { + "level": "error", + "options": { + "ignoreReact": true + } + } + } + } + } +} \ No newline at end of file diff --git a/packages/@biomejs/backend-jsonrpc/src/workspace.ts b/packages/@biomejs/backend-jsonrpc/src/workspace.ts index eafe91479eb3..cf5356a8cb50 100644 --- a/packages/@biomejs/backend-jsonrpc/src/workspace.ts +++ b/packages/@biomejs/backend-jsonrpc/src/workspace.ts @@ -834,7 +834,7 @@ export interface Correctness { /** * Disallow unused imports. */ - noUnusedImports?: RuleConfiguration_for_Null; + noUnusedImports?: RuleConfiguration_for_UnusedImportsOptions; /** * Disallow unused labels. */ @@ -1481,6 +1481,9 @@ export type RuleConfiguration_for_ValidAriaRoleOptions = export type RuleConfiguration_for_ComplexityOptions = | RulePlainConfiguration | RuleWithOptions_for_ComplexityOptions; +export type RuleConfiguration_for_UnusedImportsOptions = + | RulePlainConfiguration + | RuleWithOptions_for_UnusedImportsOptions; export type RuleConfiguration_for_HooksOptions = | RulePlainConfiguration | RuleWithOptions_for_HooksOptions; @@ -1518,6 +1521,10 @@ export interface RuleWithOptions_for_ComplexityOptions { level: RulePlainConfiguration; options: ComplexityOptions; } +export interface RuleWithOptions_for_UnusedImportsOptions { + level: RulePlainConfiguration; + options: UnusedImportsOptions; +} export interface RuleWithOptions_for_HooksOptions { level: RulePlainConfiguration; options: HooksOptions; @@ -1563,6 +1570,12 @@ export interface ComplexityOptions { */ maxAllowedComplexity: number; } +export interface UnusedImportsOptions { + /** + * Ignore `React` imports from the `react` package when set to `true`. + */ + ignoreReact: boolean; +} /** * Options for the rule `useExhaustiveDependencies` */ diff --git a/packages/@biomejs/biome/configuration_schema.json b/packages/@biomejs/biome/configuration_schema.json index 0fdfbd7996f5..6914fcdc5ed5 100644 --- a/packages/@biomejs/biome/configuration_schema.json +++ b/packages/@biomejs/biome/configuration_schema.json @@ -737,7 +737,7 @@ "noUnusedImports": { "description": "Disallow unused imports.", "anyOf": [ - { "$ref": "#/definitions/RuleConfiguration" }, + { "$ref": "#/definitions/UnusedImportsConfiguration" }, { "type": "null" } ] }, @@ -1872,6 +1872,15 @@ }, "additionalProperties": false }, + "RuleWithUnusedImportsOptions": { + "type": "object", + "required": ["level", "options"], + "properties": { + "level": { "$ref": "#/definitions/RulePlainConfiguration" }, + "options": { "$ref": "#/definitions/UnusedImportsOptions" } + }, + "additionalProperties": false + }, "RuleWithUtilityClassSortingOptions": { "type": "object", "required": ["level", "options"], @@ -2697,6 +2706,23 @@ } ] }, + "UnusedImportsConfiguration": { + "anyOf": [ + { "$ref": "#/definitions/RulePlainConfiguration" }, + { "$ref": "#/definitions/RuleWithUnusedImportsOptions" } + ] + }, + "UnusedImportsOptions": { + "type": "object", + "required": ["ignoreReact"], + "properties": { + "ignoreReact": { + "description": "Ignore `React` imports from the `react` package when set to `true`.", + "type": "boolean" + } + }, + "additionalProperties": false + }, "UtilityClassSortingConfiguration": { "anyOf": [ { "$ref": "#/definitions/RulePlainConfiguration" }, diff --git a/website/src/content/docs/internals/changelog.md b/website/src/content/docs/internals/changelog.md index 32093e424063..1536957ecc38 100644 --- a/website/src/content/docs/internals/changelog.md +++ b/website/src/content/docs/internals/changelog.md @@ -49,16 +49,21 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b ### Linter -#### Bug fixes +#### New features -- Lint rules `useNodejsImportProtocol`, `useNodeAssertStrict`, `noRestrictedImports`, `noNodejsModules` will no longer check `declare module` statements anymore. Contributed by @Sec-ant +- Add a new option `ignoreReact` to [noUnusedImports](https://biomejs.dev/linter/rules/no-unused-imports). -#### New features + When `ignoreReact` is enabled, Biome ignores imports of `React` from the `react` package. + The option is disabled by default. + + Contributed by @Conaclos #### Enhancements #### Bug fixes +- Lint rules `useNodejsImportProtocol`, `useNodeAssertStrict`, `noRestrictedImports`, `noNodejsModules` will no longer check `declare module` statements anymore. Contributed by @Sec-ant + ### Parser diff --git a/website/src/content/docs/linter/rules/no-unused-imports.md b/website/src/content/docs/linter/rules/no-unused-imports.md index e995e0cf8d82..9c3c4125054b 100644 --- a/website/src/content/docs/linter/rules/no-unused-imports.md +++ b/website/src/content/docs/linter/rules/no-unused-imports.md @@ -14,6 +14,25 @@ Note that the leading trivia, e.g., comments or newlines preceding the unused imports will also be removed. So that comment directives like `@ts-expect-error` won't be transferred to a wrong place. +## Options + +The rule provides a single option `ignoreReact`. +When this option is set to `true`, imports named `React` from the package `react` are ignored. +`ignoreReact` is disabled by default. + +```json +{ + "//": "...", + "options": { + "ignoreReact": true + } +} +``` + +This option should only be necessary if you cannot upgrade to a React version that supports the new JSX runtime. +In the new JSX runtime, you no longer need to import `React`. +You can find more details in [this comment](https://github.com/biomejs/biome/issues/571#issuecomment-1774026734). + ## Examples ### Invalid