Skip to content

Commit

Permalink
refactor(js_grammar): improve representation of imports (#1163)
Browse files Browse the repository at this point in the history
  • Loading branch information
Conaclos authored Dec 14, 2023
1 parent 2928f95 commit f49e6df
Show file tree
Hide file tree
Showing 56 changed files with 1,246 additions and 1,264 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ use biome_analyze::{
use biome_console::markup;
use biome_diagnostics::Applicability;
use biome_js_factory::make;
use biome_js_syntax::{
AnyJsNamedImport, AnyJsNamedImportSpecifier, JsImportNamedClause, TriviaPieceKind, T,
};
use biome_js_syntax::{AnyJsNamedImportSpecifier, JsImportNamedClause, TriviaPieceKind, T};
use biome_rowan::{AstNode, AstSeparatedList, BatchMutationExt};

declare_rule! {
Expand Down Expand Up @@ -75,24 +73,16 @@ impl Rule for UseGroupedTypeImport {
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let node = ctx.query();
if node.type_token().is_some() || node.default_specifier().is_some() {
let import_clause = ctx.query();
if import_clause.type_token().is_some() {
return None;
}
if node
.named_import()
.ok()?
.as_js_named_import_specifiers()?
.specifiers()
.is_empty()
{
let specifiers = import_clause.named_specifiers().ok()?.specifiers();
if specifiers.is_empty() {
// import {} from ...
return None;
}
node.named_import()
.ok()?
.as_js_named_import_specifiers()?
.specifiers()
specifiers
.iter()
.all(|specifier| {
let Ok(specifier) = specifier else {
Expand All @@ -112,11 +102,11 @@ impl Rule for UseGroupedTypeImport {
}

fn diagnostic(ctx: &RuleContext<Self>, _: &Self::State) -> Option<RuleDiagnostic> {
let node = ctx.query();
let import_clause = ctx.query();
Some(
RuleDiagnostic::new(
rule_category!(),
node.named_import().ok()?.range(),
import_clause.named_specifiers().ok()?.range(),
markup! {
"The "<Emphasis>"type"</Emphasis>" qualifier can be moved just after "<Emphasis>"import"</Emphasis>" to completely remove the "<Emphasis>"import"</Emphasis>" at compile time."
},
Expand All @@ -128,10 +118,8 @@ impl Rule for UseGroupedTypeImport {
}

fn action(ctx: &RuleContext<Self>, _: &Self::State) -> Option<JsRuleAction> {
let node = ctx.query();
let mut mutation = ctx.root().begin();
let named_import = node.named_import().ok()?;
let named_import_specifiers = named_import.as_js_named_import_specifiers()?;
let import_clause = ctx.query();
let named_import_specifiers = import_clause.named_specifiers().ok()?;
let named_import_specifiers_list = named_import_specifiers.specifiers();
let new_named_import_specifiers_list = make::js_named_import_specifier_list(
named_import_specifiers_list
Expand All @@ -156,17 +144,18 @@ impl Rule for UseGroupedTypeImport {
.filter_map(|separator| separator.ok())
.collect::<Vec<_>>(),
);
let new_node = node
let new_node = import_clause
.clone()
.with_type_token(Some(
make::token(T![type]).with_trailing_trivia([(TriviaPieceKind::Whitespace, " ")]),
))
.with_named_import(AnyJsNamedImport::JsNamedImportSpecifiers(
.with_named_specifiers(
named_import_specifiers
.clone()
.with_specifiers(new_named_import_specifiers_list),
));
mutation.replace_node(node.clone(), new_node);
);
let mut mutation = ctx.root().begin();
mutation.replace_node(import_clause.clone(), new_node);
Some(JsRuleAction {
category: ActionCategory::QuickFix,
applicability: Applicability::MaybeIncorrect,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use biome_console::markup;
use biome_diagnostics::Applicability;
use biome_js_factory::make;
use biome_js_syntax::{
AnyJsImportClause, AnyJsModuleItem, AnyJsNamedImport, AnyJsNamedImportSpecifier, JsImport,
JsLanguage, JsModule, JsSyntaxToken, TextRange, TriviaPieceKind, T,
AnyJsImportClause, AnyJsModuleItem, AnyJsNamedImportSpecifier, JsImport, JsLanguage, JsModule,
JsSyntaxToken, TextRange, TriviaPieceKind, T,
};
use biome_rowan::{
chain_trivia_pieces, syntax::SyntaxTrivia, AstNode, AstNodeExt, AstNodeList, AstSeparatedList,
Expand Down Expand Up @@ -300,13 +300,7 @@ impl From<JsImport> for ImportNode {
let AnyJsImportClause::JsImportNamedClause(import_named_clause) = import_clause else {
return None;
};

let named_import = import_named_clause.named_import().ok()?;
let AnyJsNamedImport::JsNamedImportSpecifiers(named_import_specifiers) = named_import
else {
return None;
};

let named_import_specifiers = import_named_clause.named_specifiers().ok()?;
let mut result = BTreeMap::new();

for element in named_import_specifiers.specifiers().elements() {
Expand Down Expand Up @@ -353,9 +347,7 @@ impl ImportNode {
let Ok(AnyJsImportClause::JsImportNamedClause(import_named_clause)) = import_clause else {
return import;
};

let named_import = import_named_clause.named_import();
let Ok(AnyJsNamedImport::JsNamedImportSpecifiers(old_specifiers)) = named_import else {
let Ok(old_specifiers) = import_named_clause.named_specifiers() else {
return import;
};

Expand Down
9 changes: 2 additions & 7 deletions crates/biome_js_analyze/src/react.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ pub mod hooks;
use biome_js_semantic::{Binding, SemanticModel};
use biome_js_syntax::{
AnyJsCallArgument, AnyJsExpression, AnyJsMemberExpression, AnyJsNamedImportSpecifier,
AnyJsObjectMember, JsCallExpression, JsIdentifierBinding, JsImport, JsImportNamedClause,
JsNamedImportSpecifierList, JsNamedImportSpecifiers, JsObjectExpression,
AnyJsObjectMember, JsCallExpression, JsIdentifierBinding, JsImport, JsObjectExpression,
JsPropertyObjectMember, JsxMemberName, JsxReferenceIdentifier,
};
use biome_rowan::{AstNode, AstSeparatedList};
Expand Down Expand Up @@ -290,10 +289,6 @@ fn is_named_react_export(binding: &Binding, lib: ReactLibrary, name: &str) -> Op
return Some(false);
}

let import_specifier_list = import_specifier.parent::<JsNamedImportSpecifierList>()?;
let import_specifiers = import_specifier_list.parent::<JsNamedImportSpecifiers>()?;
let import_clause = import_specifiers.parent::<JsImportNamedClause>()?;
let import = import_clause.parent::<JsImport>()?;

let import = import_specifier.import_clause()?.parent::<JsImport>()?;
Some(import.source_text().ok()?.text() == lib.import_name())
}
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,6 @@ fn suggested_fix_if_unused(binding: &AnyJsIdentifierBinding) -> Option<Suggested
// Bindings under unknown parameter are never ok to be unused
AnyJsBindingDeclaration::JsBogusParameter(_)
// Imports are never ok to be unused
| AnyJsBindingDeclaration::JsImportDefaultClause(_)
| AnyJsBindingDeclaration::JsImportNamespaceClause(_)
| AnyJsBindingDeclaration::JsShorthandNamedImportSpecifier(_)
| AnyJsBindingDeclaration::JsNamedImportSpecifier(_)
| AnyJsBindingDeclaration::JsBogusNamedImportSpecifier(_)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -503,9 +503,7 @@ fn capture_needs_to_be_in_the_dependency_list(
| AnyJsBindingDeclaration::JsCatchDeclaration(_) => Some(capture),

// This should be unreachable because of the test if the capture is imported
AnyJsBindingDeclaration::JsImportDefaultClause(_)
| AnyJsBindingDeclaration::JsImportNamespaceClause(_)
| AnyJsBindingDeclaration::JsShorthandNamedImportSpecifier(_)
AnyJsBindingDeclaration::JsShorthandNamedImportSpecifier(_)
| AnyJsBindingDeclaration::JsNamedImportSpecifier(_)
| AnyJsBindingDeclaration::JsBogusNamedImportSpecifier(_)
| AnyJsBindingDeclaration::JsDefaultImportSpecifier(_)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,10 @@ use biome_diagnostics::Applicability;
use biome_js_factory::make;
use biome_js_semantic::ReferencesExtensions;
use biome_js_syntax::{
binding_ext::AnyJsBindingDeclaration, AnyJsImportClause, JsIdentifierBinding, JsImport,
JsImportNamedClause, JsLanguage, JsNamedImportSpecifierList, T,
};
use biome_rowan::{
AstNode, AstSeparatedList, BatchMutation, BatchMutationExt, NodeOrToken, SyntaxResult,
binding_ext::AnyJsBindingDeclaration, AnyJsCombinedSpecifier, AnyJsImportClause,
JsIdentifierBinding, JsImport, JsLanguage, JsNamedImportSpecifierList, JsSyntaxNode, T,
};
use biome_rowan::{AstNode, AstSeparatedList, BatchMutation, BatchMutationExt};

declare_rule! {
/// Disallow unused imports.
Expand Down Expand Up @@ -108,20 +106,12 @@ impl Rule for NoUnusedImports {
let declaration = ctx.query().declaration()?;
let mut mutation = ctx.root().begin();
match declaration {
AnyJsBindingDeclaration::JsImportDefaultClause(_)
| AnyJsBindingDeclaration::JsImportNamespaceClause(_) => {
let import = declaration.parent::<JsImport>()?;
mutation.transfer_leading_trivia_to_sibling(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()?;
if specifier_list.len() == 1 {
remove_import_specifier(&mut mutation, &specifier_list.syntax().parent()?)?;
} else {
let following_separator = specifier_list
.iter()
Expand All @@ -138,12 +128,9 @@ impl Rule for NoUnusedImports {
mutation.remove_node(declaration);
}
}
AnyJsBindingDeclaration::JsDefaultImportSpecifier(declaration) => {
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::JsDefaultImportSpecifier(_)
| AnyJsBindingDeclaration::JsNamespaceImportSpecifier(_) => {
remove_import_specifier(&mut mutation, declaration.syntax())?;
}
AnyJsBindingDeclaration::TsImportEqualsDeclaration(_) => {
mutation.remove_node(declaration);
Expand All @@ -161,34 +148,74 @@ impl Rule for NoUnusedImports {
}
}

fn remove_named_import_from_import_clause(
fn remove_import_specifier(
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() {
mutation.transfer_leading_trivia_to_sibling(&import);
mutation.remove_element(NodeOrToken::Node(import));
specifier: &JsSyntaxNode,
) -> Option<()> {
let clause = specifier.parent().and_then(AnyJsImportClause::cast)?;
match &clause {
AnyJsImportClause::JsImportCombinedClause(default_extra_clause) => {
let default_specifier = default_extra_clause.default_specifier().ok()?;
let from_token = default_extra_clause.from_token().ok()?;
let source = default_extra_clause.source().ok()?;
let assertion = default_extra_clause.assertion();
if default_specifier.syntax() == specifier {
let new_clause = match default_extra_clause.specifier().ok()? {
AnyJsCombinedSpecifier::JsNamedImportSpecifiers(named_specifier) => {
let named_clause =
make::js_import_named_clause(named_specifier, from_token, source);
let named_clause = if let Some(assertion) = assertion {
named_clause.with_assertion(assertion)
} else {
named_clause
};
AnyJsImportClause::JsImportNamedClause(named_clause.build())
}
AnyJsCombinedSpecifier::JsNamespaceImportSpecifier(namespace_specifier) => {
let namespace_clause = make::js_import_namespace_clause(
namespace_specifier,
from_token,
source,
);
let namespace_clause = if let Some(assertion) = assertion {
namespace_clause.with_assertion(assertion)
} else {
namespace_clause
};
AnyJsImportClause::JsImportNamespaceClause(namespace_clause.build())
}
};
mutation.replace_node(clause, new_clause);
} else {
let from_token = make::token_decorated_with_space(T![from])
.with_trailing_trivia_pieces(from_token.trailing_trivia().pieces());
let default_clause =
make::js_import_default_clause(default_specifier, from_token, source);
let default_clause = if let Some(assertion) = assertion {
default_clause.with_assertion(assertion)
} else {
default_clause
};
mutation.replace_node(clause, default_clause.build().into());
}
}
AnyJsImportClause::JsImportBareClause(_)
| AnyJsImportClause::JsImportDefaultClause(_)
| AnyJsImportClause::JsImportNamedClause(_)
| AnyJsImportClause::JsImportNamespaceClause(_) => {
// Remove the entire statement
let import = clause.parent::<JsImport>()?;
mutation.transfer_leading_trivia_to_sibling(import.syntax());
mutation.remove_node(import);
}
}
Ok(())
Some(())
}

const fn is_import(declaration: &AnyJsBindingDeclaration) -> bool {
matches!(
declaration,
AnyJsBindingDeclaration::JsDefaultImportSpecifier(_)
| AnyJsBindingDeclaration::JsImportDefaultClause(_)
| AnyJsBindingDeclaration::JsImportNamespaceClause(_)
| AnyJsBindingDeclaration::JsNamedImportSpecifier(_)
| AnyJsBindingDeclaration::JsNamespaceImportSpecifier(_)
| AnyJsBindingDeclaration::JsShorthandNamedImportSpecifier(_)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -840,17 +840,15 @@ impl Named {
AnyJsBindingDeclaration::JsCatchDeclaration(_) => Some(Named::CatchParameter),
AnyJsBindingDeclaration::TsPropertyParameter(_) => Some(Named::ParameterProperty),
AnyJsBindingDeclaration::TsIndexSignatureParameter(_) => Some(Named::IndexParameter),
AnyJsBindingDeclaration::JsNamespaceImportSpecifier(_)
| AnyJsBindingDeclaration::JsImportNamespaceClause(_) => Some(Named::ImportNamespace),
AnyJsBindingDeclaration::JsNamespaceImportSpecifier(_) => Some(Named::ImportNamespace),
AnyJsBindingDeclaration::JsFunctionDeclaration(_)
| AnyJsBindingDeclaration::JsFunctionExpression(_)
| AnyJsBindingDeclaration::JsFunctionExportDefaultDeclaration(_)
| AnyJsBindingDeclaration::TsDeclareFunctionDeclaration(_)
| AnyJsBindingDeclaration::TsDeclareFunctionExportDefaultDeclaration(_) => {
Some(Named::Function)
}
AnyJsBindingDeclaration::JsImportDefaultClause(_)
| AnyJsBindingDeclaration::TsImportEqualsDeclaration(_)
AnyJsBindingDeclaration::TsImportEqualsDeclaration(_)
| AnyJsBindingDeclaration::JsDefaultImportSpecifier(_)
| AnyJsBindingDeclaration::JsNamedImportSpecifier(_) => Some(Named::ImportAlias),
AnyJsBindingDeclaration::TsModuleDeclaration(_) => Some(Named::Namespace),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ use biome_analyze::{context::RuleContext, declare_rule, Rule, RuleDiagnostic};
use biome_console::markup;
use biome_js_semantic::ReferencesExtensions;
use biome_js_syntax::{
JsDefaultImportSpecifier, JsIdentifierAssignment, JsIdentifierBinding, JsImportDefaultClause,
JsImportNamespaceClause, JsNamedImportSpecifier, JsNamespaceImportSpecifier,
JsShorthandNamedImportSpecifier,
JsDefaultImportSpecifier, JsIdentifierAssignment, JsIdentifierBinding, JsNamedImportSpecifier,
JsNamespaceImportSpecifier, JsShorthandNamedImportSpecifier,
};

use biome_rowan::{declare_node_union, AstNode};
Expand Down Expand Up @@ -67,10 +66,6 @@ impl Rule for NoImportAssign {
let label_statement = ctx.query();
let mut invalid_assign_list = vec![];
let local_name_binding = match label_statement {
// `import xx from 'y'`
AnyJsImportLike::JsImportDefaultClause(clause) => clause.local_name().ok(),
// `import * as xxx from 'y'`
AnyJsImportLike::JsImportNamespaceClause(clause) => clause.local_name().ok(),
// `import {x as xx} from 'y'`
// ^^^^^^^
AnyJsImportLike::JsNamedImportSpecifier(specifier) => specifier.local_name().ok(),
Expand All @@ -79,9 +74,13 @@ impl Rule for NoImportAssign {
AnyJsImportLike::JsShorthandNamedImportSpecifier(specifier) => {
specifier.local_name().ok()
}
// `import * as xxx from 'y'`
// ^^^^^^^^
// `import a, * as b from 'y'`
// ^^^^^^
AnyJsImportLike::JsNamespaceImportSpecifier(specifier) => specifier.local_name().ok(),
// `import xx from 'y'`
// ^^
// `import a, * as b from 'y'`
// ^
AnyJsImportLike::JsDefaultImportSpecifier(specifier) => specifier.local_name().ok(),
Expand Down Expand Up @@ -125,5 +124,5 @@ impl Rule for NoImportAssign {
}

declare_node_union! {
pub(crate) AnyJsImportLike = JsImportDefaultClause | JsImportNamespaceClause | JsNamedImportSpecifier | JsShorthandNamedImportSpecifier | JsNamespaceImportSpecifier | JsDefaultImportSpecifier
pub(crate) AnyJsImportLike = JsNamedImportSpecifier | JsShorthandNamedImportSpecifier | JsNamespaceImportSpecifier | JsDefaultImportSpecifier
}
Loading

0 comments on commit f49e6df

Please sign in to comment.