Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(js_grammar): improve representation of imports #1163

Merged
merged 2 commits into from
Dec 14, 2023
Merged

refactor(js_grammar): improve representation of imports #1163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d9c8c8f
refactor(js_grammar): improve imports representation
Conaclos Dec 12, 2023
8f858b7
refactor: DefaultExtra -> Combined
Conaclos Dec 13, 2023
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
41 changes: 15 additions & 26 deletions crates/biome_js_analyze/src/analyzers/nursery/use_grouped_type_import.rs
View file
Open in desktop
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
16 changes: 4 additions & 12 deletions crates/biome_js_analyze/src/assists/correctness/organize_imports.rs
View file
Open in desktop
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
View file
Open in desktop
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())
}
2 changes: 0 additions & 2 deletions crates/biome_js_analyze/src/semantic_analyzers/correctness/no_unused_variables.rs
View file
Open in desktop
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
4 changes: 1 addition & 3 deletions crates/biome_js_analyze/src/semantic_analyzers/correctness/use_exhaustive_dependencies.rs
View file
Open in desktop
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
109 changes: 68 additions & 41 deletions crates/biome_js_analyze/src/semantic_analyzers/nursery/no_unused_imports.rs
View file
Open in desktop
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
6 changes: 2 additions & 4 deletions crates/biome_js_analyze/src/semantic_analyzers/style/use_naming_convention.rs
View file
Open in desktop
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
15 changes: 7 additions & 8 deletions crates/biome_js_analyze/src/semantic_analyzers/suspicious/no_import_assign.rs
View file
Open in desktop
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
Loading