Skip to content

Commit

Permalink
feat(lint/noUnusedImports): add rule
Browse files Browse the repository at this point in the history
  • Loading branch information
Conaclos committed Sep 28, 2023
1 parent ae1021c commit e97a4ae
Show file tree
Hide file tree
Showing 26 changed files with 1,474 additions and 34 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom
The rule reports `else` clauses that can be omitted because their `if` branches break.
Contributed by @Conaclos

- Add [noUnusedImports](https://biomejs.dev/linter/rules/no-unused-imports) rule.
The rule reports unused imports and suggests to remove them.
Contributed by @Conaclos

[noUnusedVariables](https://biomejs.dev/linter/rules/no-unused-variables) reports also unused imports, but don't suggest their removal.
Once [noUnusedImports](https://biomejs.dev/linter/rules/no-unused-imports) stabilized,
[noUnusedVariables](https://biomejs.dev/linter/rules/no-unused-variables) will not report unused imports.

#### Enhancements

- The following rules have now safe code fixes:
Expand Down
1 change: 1 addition & 0 deletions crates/biome_diagnostics_categories/src/categories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ define_categories! {
"lint/nursery/noGlobalIsNan": "https://biomejs.dev/linter/rules/no-global-is-nan",
"lint/nursery/noInvalidNewBuiltin": "https://biomejs.dev/lint/rules/no-invalid-new-builtin",
"lint/nursery/noMisleadingInstantiator": "https://biomejs.dev/linter/rules/no-misleading-instantiator",
"lint/nursery/noUnusedImports": "https://biomejs.dev/lint/rules/no-unused-imports",
"lint/nursery/noUselessElse": "https://biomejs.dev/lint/rules/no-useless-else",
"lint/nursery/noVoid": "https://biomejs.dev/linter/rules/no-void",
"lint/nursery/useArrowFunction": "https://biomejs.dev/linter/rules/use-arrow-function",
Expand Down
57 changes: 52 additions & 5 deletions crates/biome_js_analyze/src/react.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ pub mod hooks;

use biome_js_semantic::{Binding, SemanticModel};
use biome_js_syntax::{
AnyJsCallArgument, AnyJsExpression, AnyJsMemberExpression, AnyJsNamedImportSpecifier,
JsCallExpression, JsIdentifierBinding, JsImport, JsImportNamedClause,
JsNamedImportSpecifierList, JsNamedImportSpecifiers, JsObjectExpression,
AnyJsCallArgument, AnyJsExpression, AnyJsImportClause, AnyJsMemberExpression, AnyJsNamedImport,
AnyJsNamedImportSpecifier, JsCallExpression, JsIdentifierBinding, JsImport,
JsImportNamedClause, JsNamedImportSpecifierList, JsNamedImportSpecifiers, JsObjectExpression,
JsPropertyObjectMember, JsxMemberName, JsxReferenceIdentifier,
};
use biome_rowan::{AstNode, AstSeparatedList};
Expand Down Expand Up @@ -124,21 +124,68 @@ pub(crate) enum ReactLibrary {
}

impl ReactLibrary {
const fn import_name(self) -> &'static str {
pub const fn import_name(self) -> &'static str {
match self {
ReactLibrary::React => "react",
ReactLibrary::ReactDOM => "react-dom",
}
}

const fn global_name(self) -> &'static str {
pub const fn global_name(self) -> &'static str {
match self {
ReactLibrary::React => "React",
ReactLibrary::ReactDOM => "ReactDOM",
}
}
}

/// Return `Some(true)` if `import` imports the global react name from react.
pub(crate) fn is_global_react_import(import: &JsImport, lib: ReactLibrary) -> Option<bool> {
let is_react_import = import.source_is(lib.import_name()).ok()?;
if !is_react_import {
return None;
}
let local_name = match import.import_clause().ok()? {
AnyJsImportClause::JsImportBareClause(_) => {
return None;
}
AnyJsImportClause::JsImportDefaultClause(import_clause) => import_clause.local_name(),
AnyJsImportClause::JsImportNamedClause(import_clause) => {
if let Some(default_specifier) = import_clause.default_specifier() {
default_specifier.local_name()
} else {
match import_clause.named_import().ok()? {
AnyJsNamedImport::JsNamedImportSpecifiers(import_clause) => {
import_clause.specifiers().iter().find_map(|specifier| {
let specifier = specifier.ok()?;
let specifier = specifier.as_js_named_import_specifier()?;
specifier
.name()
.ok()?
.is_default()
.ok()?
.then_some(specifier.local_name())
})?
}
AnyJsNamedImport::JsNamespaceImportSpecifier(import_clause) => {
import_clause.local_name()
}
}
}
}
AnyJsImportClause::JsImportNamespaceClause(import_clause) => import_clause.local_name(),
};
Some(
local_name
.ok()?
.as_js_identifier_binding()?
.name_token()
.ok()?
.text_trimmed()
== lib.global_name(),
)
}

/// List of valid [`React` API]
///
/// [`React` API]: https://reactjs.org/docs/react-api.html
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,8 @@ impl Rule for NoUnusedVariables {
let name = binding.name_token().ok()?;
let name = name.text_trimmed();

// Old code import React but do not used directly
// only indirectly after transpiling JSX.
// Legacy React framework requires to import `React`, even if it is not used.
// This is required for transpiling JSX.
if name.starts_with('_') || name == "React" {
return None;
}
Expand Down
2 changes: 2 additions & 0 deletions crates/biome_js_analyze/src/semantic_analyzers/nursery.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
use crate::{
react::{is_global_react_import, ReactLibrary},
semantic_services::Semantic,
JsRuleAction,
};
use biome_analyze::{context::RuleContext, declare_rule, ActionCategory, Rule, RuleDiagnostic};
use biome_console::markup;
use biome_diagnostics::Applicability;
use biome_js_factory::make;
use biome_js_semantic::ReferencesExtensions;
use biome_js_syntax::{
binding_ext::AnyJsBindingDeclaration, AnyJsImportClause, JsFileSource, JsIdentifierBinding,
JsImport, JsImportNamedClause, JsLanguage, JsNamedImportSpecifierList, JsSyntaxNode, T,
};
use biome_rowan::{
AstNode, AstSeparatedList, BatchMutation, BatchMutationExt, NodeOrToken, SyntaxResult,
};

declare_rule! {
/// Disallow unused imports.
///
/// Unused imports usually are result of incomplete refactoring.
///
/// There is one exception to the rule: the `React` import.
/// Importing the `React` variable was a mandatory pattern until some time ago:
/// For the time being this rule will ignore it,
/// but this **might change in the future releases**.
///
/// ## Examples
///
/// ### Invalid
///
/// ```js,expect_diagnostic
/// // Comment
/// import A from 'mod';
/// ```
///
/// ```js,expect_diagnostic
/// // Comment
/// import * as A from 'mod';
/// ```
///
/// ```ts,expect_diagnostic
/// import { type A, B } from 'mod';
///
/// export { B }
/// ```
///
/// ## Valid
///
/// ```ts
/// import { A, type B } from 'mod';
///
/// function f(arg: B): A {
/// return new A(arg);
/// }
/// ```
///
/// ```jsx
/// import React from 'react';
///
/// function foo() {
/// return <div />;
/// };
///
/// foo();
/// ```
pub(crate) NoUnusedImports {
version: "next",
name: "noUnusedImports",
recommended: false,
}
}

impl Rule for NoUnusedImports {
type Query = Semantic<JsIdentifierBinding>;
type State = ();
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let binding = ctx.query();
let declaration = binding.declaration()?;
if !is_import(&declaration) {
return None;
}

if ctx.source_type::<JsFileSource>().variant().is_jsx() {
let js_import = declaration.syntax().ancestors().find_map(JsImport::cast)?;
// Legacy React framework requires to import `React`, even if it is not used.
// This is required for transpiling JSX.
if is_global_react_import(&js_import, ReactLibrary::React).unwrap_or(false) {
return None;
}
}

let model = ctx.model();
binding.all_references(model).next().is_none().then_some(())
}

fn diagnostic(ctx: &RuleContext<Self>, _: &Self::State) -> Option<RuleDiagnostic> {
let binding = ctx.query();
Some(
RuleDiagnostic::new(
rule_category!(),
binding.range(),
markup! {
"This "<Emphasis>"import"</Emphasis>" is unused."
},
)
.note(markup! {
"Unused imports usually are result of incomplete refactoring."
}),
)
}

fn action(ctx: &RuleContext<Self>, _: &Self::State) -> Option<JsRuleAction> {
let declaration = ctx.query().declaration()?;
let mut mutation = ctx.root().begin();
match declaration {
AnyJsBindingDeclaration::JsImportDefaultClause(_)
| AnyJsBindingDeclaration::JsImportNamespaceClause(_) => {
let import = declaration.parent::<JsImport>()?;
transfer_leading_trivia_to_sibling(&mut mutation, import.syntax());
mutation.remove_node(import);
}
AnyJsBindingDeclaration::JsShorthandNamedImportSpecifier(_)
| AnyJsBindingDeclaration::JsNamedImportSpecifier(_)
| AnyJsBindingDeclaration::JsBogusNamedImportSpecifier(_) => {
let specifier_list = declaration.parent::<JsNamedImportSpecifierList>()?;
if specifier_list.iter().count() == 1 {
let import_clause =
JsImportNamedClause::cast(specifier_list.syntax().parent()?.parent()?)?;
remove_named_import_from_import_clause(&mut mutation, import_clause).ok()?;
} else {
let following_separator = specifier_list
.iter()
.zip(specifier_list.separators().map(|separator| separator.ok()))
.find(|(specifier, _)| {
specifier
.as_ref()
.is_ok_and(|x| x.syntax() == declaration.syntax())
})
.and_then(|(_, separator)| separator);
if let Some(separator) = following_separator {
mutation.remove_token(separator);
}
mutation.remove_node(declaration);
}
}
AnyJsBindingDeclaration::JsDefaultImportSpecifier(_) => {
mutation.remove_node(declaration);
}
AnyJsBindingDeclaration::JsNamespaceImportSpecifier(_) => {
let import_clause = JsImportNamedClause::cast(declaration.syntax().parent()?)?;
remove_named_import_from_import_clause(&mut mutation, import_clause).ok()?;
}
AnyJsBindingDeclaration::TsImportEqualsDeclaration(_) => {
mutation.remove_node(declaration);
}
_ => {
return None;
}
}
Some(JsRuleAction {
mutation,
category: ActionCategory::QuickFix,
applicability: Applicability::Always,
message: markup! { "Remove the unused import." }.to_owned(),
})
}
}

fn remove_named_import_from_import_clause(
mutation: &mut BatchMutation<JsLanguage>,
import_clause: JsImportNamedClause,
) -> SyntaxResult<()> {
if let Some(default_specifier) = import_clause.default_specifier() {
let default_clause = make::js_import_default_clause(
default_specifier.local_name()?,
make::token_decorated_with_space(T![from]),
import_clause.source()?,
)
.build();
mutation.replace_node(
AnyJsImportClause::from(import_clause),
default_clause.into(),
);
} else if let Some(import) = import_clause.syntax().parent() {
transfer_leading_trivia_to_sibling(mutation, &import);
mutation.remove_element(NodeOrToken::Node(import));
}
Ok(())
}

fn transfer_leading_trivia_to_sibling(
mutation: &mut BatchMutation<JsLanguage>,
node: &JsSyntaxNode,
) -> Option<()> {
node.first_leading_trivia();
let pieces = node.first_leading_trivia()?.pieces();
let (sibling, new_sibling) = if let Some(next_sibling) = node.next_sibling() {
let new_next_sibling = next_sibling.clone().prepend_trivia_pieces(pieces)?;
(next_sibling, new_next_sibling)
} else if let Some(prev_sibling) = node.prev_sibling() {
let new_prev_sibling = prev_sibling.clone().append_trivia_pieces(pieces)?;
(prev_sibling, new_prev_sibling)
} else {
return None;
};
mutation
.replace_element_discard_trivia(NodeOrToken::Node(sibling), NodeOrToken::Node(new_sibling));
Some(())
}

fn is_import(declaration: &AnyJsBindingDeclaration) -> bool {
matches!(
declaration,
AnyJsBindingDeclaration::JsBogusNamedImportSpecifier(_)
| AnyJsBindingDeclaration::JsDefaultImportSpecifier(_)
| AnyJsBindingDeclaration::JsImportDefaultClause(_)
| AnyJsBindingDeclaration::JsImportNamespaceClause(_)
| AnyJsBindingDeclaration::JsNamedImportSpecifier(_)
| AnyJsBindingDeclaration::JsNamespaceImportSpecifier(_)
| AnyJsBindingDeclaration::JsShorthandNamedImportSpecifier(_)
| AnyJsBindingDeclaration::TsImportEqualsDeclaration(_)
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Header comment
import A from "mod";

// Header comment
import * as B from "mod"; // Import comment

// Header comment
import { C } from "mod"; // Import comment

// Header comment
import /*a*/ D /*b*/, /*c*/{ E }/*d*/ from "mod"; // Import comment

import /*a*/ F /*b*/, /*c*/ * as G /*d*/ from "mod";

import {
// Comment
H,
I,
} from "mod";

import {/*a*/J/*b*/, /*c*/K/*d*/} from "mod";

// Header comment
import { L as M, } from "mod"; // Import comment
Loading

0 comments on commit e97a4ae

Please sign in to comment.