Skip to content

Commit

Permalink
fix(lint/useSingleVarDeclarator): output valid code when declarators …
Browse files Browse the repository at this point in the history 
…are on multilines (#1070)
  • Loading branch information
@Conaclos
Conaclos authored Dec 6, 2023
1 parent c8aab47 commit b5fa5af
Show file tree
Hide file tree
Showing 8 changed files with 237 additions and 124 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom
}
```

- Fix [#728](https://github.com/biomejs/biome/issues/728). [useSingleVarDeclarator](https://biomejs.dev/linter/rules/use-single-var-declarator) no longer outputs invalid code. Contributed by @Conaclos

### Parser


Expand Down
205 changes: 88 additions & 117 deletions crates/biome_js_analyze/src/analyzers/style/use_single_var_declarator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,41 @@ use biome_console::markup;
use biome_diagnostics::Applicability;
use biome_js_factory::make;
use biome_js_syntax::{
JsModuleItemList, JsStatementList, JsSyntaxToken, JsVariableDeclarationFields,
JsVariableDeclaratorList, JsVariableStatement, JsVariableStatementFields, TextSize,
TriviaPieceKind, T,
JsSyntaxKind, JsSyntaxToken, JsVariableDeclarationFields, JsVariableStatement,
JsVariableStatementFields, TextSize, TriviaPieceKind, T,
};
use biome_rowan::{
trim_leading_trivia_pieces, AstNode, AstSeparatedList, BatchMutationExt, TriviaPiece,
};
use biome_rowan::{AstNode, AstNodeExt, AstSeparatedList, BatchMutationExt, TriviaPiece};

use crate::JsRuleAction;

declare_rule! {
/// Disallow multiple variable declarations in the same variable statement
///
/// In JavaScript, multiple variables can be declared within a single `var`, `const` or `let` declaration.
/// It is often considered a best practice to declare every variable separately.
/// That is what this rule enforces.
///
/// Source: https://eslint.org/docs/latest/rules/one-var
///
/// ## Examples
///
/// ### Invalid
///
/// ```js,expect_diagnostic
/// let foo, bar;
/// let foo = 0, bar, baz;
/// ```
///
/// ### Valid
///
/// ```js
/// const foo = 0;
/// let bar;
/// let baz;
/// ```
///
/// ```js
/// for (let i = 0, x = 1; i < arr.length; i++) {}
/// ```
pub(crate) UseSingleVarDeclarator {
Expand All @@ -39,192 +52,150 @@ declare_rule! {

impl Rule for UseSingleVarDeclarator {
type Query = Ast<JsVariableStatement>;
type State = (
Option<JsSyntaxToken>,
JsSyntaxToken,
JsVariableDeclaratorList,
Option<JsSyntaxToken>,
);
type State = ();
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Option<Self::State> {
let node = ctx.query();

let JsVariableStatementFields {
declaration,
semicolon_token,
} = node.as_fields();

let JsVariableDeclarationFields {
await_token,
kind,
declarators,
} = declaration.ok()?.as_fields();

let kind = kind.ok()?;

if declarators.len() < 2 {
return None;
}

Some((await_token, kind, declarators, semicolon_token))
(ctx.query().declaration().ok()?.declarators().len() > 1).then_some(())
}

fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {
let node = ctx.query();

Some(RuleDiagnostic::new(
rule_category!(),
node.range(),
"Declare variables separately",
))
}

fn action(ctx: &RuleContext<Self>, state: &Self::State) -> Option<JsRuleAction> {
fn action(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<JsRuleAction> {
let node = ctx.query();
let prev_parent = node.syntax().parent()?;

if !JsStatementList::can_cast(prev_parent.kind())
&& !JsModuleItemList::can_cast(prev_parent.kind())
{
if !matches!(
prev_parent.kind(),
JsSyntaxKind::JS_STATEMENT_LIST | JsSyntaxKind::JS_MODULE_ITEM_LIST
) {
return None;
}

let (await_token, kind, declarators, semicolon_token) = state;

let JsVariableStatementFields {
declaration,
semicolon_token,
} = node.as_fields();
let JsVariableDeclarationFields {
await_token,
kind,
declarators,
} = declaration.ok()?.as_fields();
let kind = kind.ok()?;
let index = prev_parent
.children()
.position(|slot| &slot == node.syntax())?;

// Extract the indentation part from the leading trivia of the kind
// token, defined as all whitespace and newline trivia pieces starting
// from the token going backwards up to the first newline (included).
// If the leading trivia for the token is empty, a single newline trivia
// piece is created. For efficiency, the trivia pieces are stored in
// reverse order (the vector is then reversed again on iteration)
let mut has_newline = false;
let leading_trivia: Vec<_> = kind
.leading_trivia()
.pieces()
.rev()
.take_while(|piece| {
let has_previous_newline = has_newline;
has_newline |= piece.is_newline();
!has_previous_newline
})
.filter_map(|piece| {
if piece.is_whitespace() || piece.is_newline() {
Some((piece.kind(), piece.text().to_string()))
} else {
None
}
})
.collect();

let kind_indent = if !leading_trivia.is_empty() {
leading_trivia
} else {
let kind_indent = kind
.indentation_trivia_pieces()
.map(|piece| (piece.kind(), piece.text().to_string()))
.collect::<Vec<_>>();
let kind_indent = if kind_indent.is_empty() {
vec![(TriviaPieceKind::Newline, String::from("\n"))]
} else {
kind_indent
};

let last_semicolon_token = semicolon_token.as_ref();
let remaining_semicolon_token = semicolon_token.clone().map(|_| make::token(T![;]));

let declarators_len = declarators.len();

let mut separators = declarators.separators();
let last_semicolon_token = semicolon_token;
let next_parent = prev_parent.clone().splice_slots(
index..=index,
declarators
.iter()
.enumerate()
.filter_map(|(index, declarator)| {
let mut declarator = declarator.ok()?;

// Remove the leading trivia for the declarators
let first_token = declarator.syntax().first_token()?;
let first_token_leading_trivia = first_token.leading_trivia();

declarator = declarator
.replace_token_discard_trivia(
first_token.clone(),
first_token.with_leading_trivia([]),
)
// SAFETY: first_token is a known child of declarator
.unwrap();
let declarator = declarator.ok()?;
let declarator_leading_trivia = declarator.syntax().first_leading_trivia()?;
let declarator = declarator.with_leading_trivia_pieces([])?;

let kind = if index == 0 {
let kind = kind.clone();
// Clone the kind token with its entire leading trivia
// for the first statement
kind.clone()
if let Some(last_piece) = kind.trailing_trivia().last() {
if last_piece.kind().is_single_line_comment() {
kind.prepend_trivia_pieces(trim_leading_trivia_pieces(
kind.trailing_trivia().pieces(),
))
.with_trailing_trivia([(TriviaPieceKind::Whitespace, " ")])
} else {
kind
}
} else {
// Add a trailing space if the kind has no trailing trivia.
kind.with_trailing_trivia([(TriviaPieceKind::Whitespace, " ")])
}
} else {
// For the remaining statements, clone the kind token
// with the leading trivia pieces previously removed
// from the first token of the declarator node, with
// the indentation fixed up to match the original kind
// token
// from the declarator node, with the indentation
// fixed up to match the original kind token
let indent: &[(TriviaPieceKind, String)] = &kind_indent;
let mut trivia_pieces = Vec::new();
let mut token_text = String::new();

for piece in first_token_leading_trivia.pieces() {
for piece in declarator_leading_trivia.pieces() {
if !piece.is_comments() {
continue;
}

for (kind, text) in indent.iter().rev() {
for (kind, text) in indent {
trivia_pieces.push(TriviaPiece::new(*kind, TextSize::of(text)));
token_text.push_str(text);
}

trivia_pieces.push(TriviaPiece::new(piece.kind(), piece.text_len()));
token_text.push_str(piece.text());
}

for (kind, text) in indent.iter().rev() {
for (kind, text) in indent {
trivia_pieces.push(TriviaPiece::new(*kind, TextSize::of(text)));
token_text.push_str(text);
}

token_text.push_str(kind.text_trimmed());

for piece in kind.trailing_trivia().pieces() {
token_text.push_str(piece.text());
}

token_text.push(' ');
JsSyntaxToken::new_detached(
kind.kind(),
&token_text,
trivia_pieces,
kind.trailing_trivia().pieces().map(|piece| {
TriviaPiece::new(piece.kind(), TextSize::of(piece.text()))
}),
[TriviaPiece::new(
TriviaPieceKind::Whitespace,
TextSize::from(1),
)],
)
};

let mut variable_declaration = make::js_variable_declaration(
kind,
make::js_variable_declarator_list([declarator], []),
);

if let Some(await_token) = await_token {
variable_declaration =
variable_declaration.with_await_token(await_token.clone());
if let Some(await_token) = await_token.clone() {
variable_declaration = variable_declaration.with_await_token(await_token);
}

let mut builder = make::js_variable_statement(variable_declaration.build());

let semicolon_token = if index + 1 == declarators_len {
last_semicolon_token
} else {
remaining_semicolon_token.as_ref()
};

if let Some(semicolon_token) = semicolon_token {
builder = builder.with_semicolon_token(semicolon_token.clone());
if let Some(last_semicolon_token) = last_semicolon_token.as_ref() {
let semicolon_token = if index + 1 == declarators_len {
last_semicolon_token.clone()
} else {
make::token(T![;])
};
builder = builder.with_semicolon_token(semicolon_token)
}
let mut result = builder.build();
if let Some(Ok(separator)) = separators.next() {
if separator.has_trailing_comments() {
result = result
.append_trivia_pieces(separator.trailing_trivia().pieces())?;
}
}

Some(Some(builder.build().into_syntax().into()))
Some(Some(result.into_syntax().into()))
}),
);

Expand Down
10 changes: 10 additions & 0 deletions crates/biome_js_analyze/tests/specs/style/useSingleVarDeclarator/invalid.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,13 @@ function test() {
var x = 1,
// comment
y = 2

let
a = 0,
b = 1;

let // comment
c = 0, // trailing comment
/*0*/d;

while(true) var v = 0, u = 1;
Loading

0 comments on commit b5fa5af

Please sign in to comment.