Skip to content

Commit

Permalink
fix(lint/noRedundantUseStrict/: keep leading comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Conaclos committed Oct 1, 2023
1 parent a75a9b7 commit 2e26cae
Show file tree
Hide file tree
Showing 10 changed files with 87 additions and 147 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::JsRuleAction;
use crate::{utils::batch::JsBatchMutation, JsRuleAction};
use biome_analyze::{
context::RuleContext, declare_rule, ActionCategory, Ast, FixKind, Rule, RuleDiagnostic,
};
Expand Down Expand Up @@ -133,7 +133,7 @@ impl Rule for NoRedundantUseStrict {
rule_category!(),
ctx.query().range(),
markup! {
"Redundant "<Emphasis>{"use strict"}</Emphasis>" directive."
"Redundant "<Emphasis>"use strict"</Emphasis>" directive."
},
);

Expand All @@ -143,28 +143,29 @@ impl Rule for NoRedundantUseStrict {
markup! {"All parts of a class's body are already in strict mode."},
) ,
AnyJsStrictModeNode::JsModule(_js_module) => diag= diag.note(
markup! {"The entire contents of "<Emphasis>{"JavaScript modules"}</Emphasis>" are automatically in strict mode, with no statement needed to initiate it."},
markup! {"The entire contents of "<Emphasis>"JavaScript modules"</Emphasis>" are automatically in strict mode, with no statement needed to initiate it."},
),
AnyJsStrictModeNode::JsDirective(js_directive) => diag= diag.detail(
js_directive.range(),
markup! {"This outer "<Emphasis>{"use strict"}</Emphasis>" directive already enables strict mode."},
markup! {"This outer "<Emphasis>"use strict"</Emphasis>" directive already enables strict mode."},
),
}

Some(diag)
}

fn action(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<JsRuleAction> {
let root = ctx.root();
let mut batch = root.begin();

batch.remove_node(ctx.query().clone());

let node = ctx.query();
let mut mutation = ctx.root().begin();
mutation.transfer_leading_trivia_to_sibling(node.syntax());
mutation.remove_node(node.clone());
Some(JsRuleAction {
category: ActionCategory::QuickFix,
applicability: Applicability::Always,
message: markup! { "Remove the redundant \"use strict\" directive" }.to_owned(),
mutation: batch,
message:
markup! { "Remove the redundant "<Emphasis>"use strict"</Emphasis>" directive." }
.to_owned(),
mutation,
})
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{semantic_services::Semantic, JsRuleAction};
use crate::{semantic_services::Semantic, utils::batch::JsBatchMutation, JsRuleAction};
use biome_analyze::{
context::RuleContext, declare_rule, ActionCategory, FixKind, Rule, RuleDiagnostic,
};
Expand All @@ -8,7 +8,7 @@ use biome_js_factory::make;
use biome_js_semantic::ReferencesExtensions;
use biome_js_syntax::{
binding_ext::AnyJsBindingDeclaration, AnyJsImportClause, JsIdentifierBinding, JsImport,
JsImportNamedClause, JsLanguage, JsNamedImportSpecifierList, JsSyntaxNode, T,
JsImportNamedClause, JsLanguage, JsNamedImportSpecifierList, T,
};
use biome_rowan::{
AstNode, AstSeparatedList, BatchMutation, BatchMutationExt, NodeOrToken, SyntaxResult,
Expand Down Expand Up @@ -111,7 +111,7 @@ impl Rule for NoUnusedImports {
AnyJsBindingDeclaration::JsImportDefaultClause(_)
| AnyJsBindingDeclaration::JsImportNamespaceClause(_) => {
let import = declaration.parent::<JsImport>()?;
transfer_leading_trivia_to_sibling(&mut mutation, import.syntax());
mutation.transfer_leading_trivia_to_sibling(import.syntax());
mutation.remove_node(import);
}
AnyJsBindingDeclaration::JsShorthandNamedImportSpecifier(_)
Expand Down Expand Up @@ -177,31 +177,12 @@ fn remove_named_import_from_import_clause(
default_clause.into(),
);
} else if let Some(import) = import_clause.syntax().parent() {
transfer_leading_trivia_to_sibling(mutation, &import);
mutation.transfer_leading_trivia_to_sibling(&import);
mutation.remove_element(NodeOrToken::Node(import));
}
Ok(())
}

fn transfer_leading_trivia_to_sibling(
mutation: &mut BatchMutation<JsLanguage>,
node: &JsSyntaxNode,
) -> Option<()> {
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(())
}

const fn is_import(declaration: &AnyJsBindingDeclaration) -> bool {
matches!(
declaration,
Expand Down
26 changes: 26 additions & 0 deletions crates/biome_js_analyze/src/utils/batch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ pub trait JsBatchMutation {
/// 1 - removes commas around the member to keep the list valid.
fn remove_js_object_member(&mut self, parameter: &AnyJsObjectMember) -> bool;

/// Transfer leading trivia to the next sibling.
/// If there is no next sibling, then transfer to the previous sibling.
/// Otherwise do nothings.
fn transfer_leading_trivia_to_sibling(&mut self, node: &JsSyntaxNode);

/// It attempts to add a new element after the given element
///
/// The function appends the elements at the end if `after_element` isn't found
Expand Down Expand Up @@ -288,6 +293,27 @@ impl JsBatchMutation for BatchMutation<JsLanguage> {
false
}
}

fn transfer_leading_trivia_to_sibling(&mut self, node: &JsSyntaxNode) {
let Some(pieces) = node.first_leading_trivia().map(|trivia| trivia.pieces()) else {
return;
};
let (sibling, new_sibling) =
if let Some(next_sibling) = node.last_token().and_then(|x| x.next_token()) {
(
next_sibling.clone(),
next_sibling.prepend_trivia_pieces(pieces),
)
} else if let Some(prev_sibling) = node.first_token().and_then(|x| x.prev_token()) {
(
prev_sibling.clone(),
prev_sibling.append_trivia_pieces(pieces),
)
} else {
return;
};
self.replace_token_discard_trivia(sibling, new_sibling);
}
}

#[cfg(test)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,10 @@ invalid.cjs:2:1 lint/suspicious/noRedundantUseStrict FIXABLE ━━━━━
2 │ "use strict";
3 │
i Safe fix: Remove the redundant "use strict" directive
1 1 │ "use strict";
2 │ - "use·strict";
3 2 │
4 3 │ function test() {
i Safe fix: Remove the redundant use strict directive.
2 │ "use·strict";
│ -------------
```

Expand All @@ -68,16 +65,10 @@ invalid.cjs:5:2 lint/suspicious/noRedundantUseStrict FIXABLE ━━━━━
2 │ "use strict";
3 │
i Safe fix: Remove the redundant "use strict" directive
3 3 │
4 4 │ function test() {
5 │ - → "use·strict";
6 │ - → function·inner_a()·{
5 │ + → function·inner_a()·{
7 6 │ "use strict"; // redundant directive
8 7 │ }
i Safe fix: Remove the redundant use strict directive.
5 │ → "use·strict";
│ -------------
```

Expand All @@ -100,16 +91,10 @@ invalid.cjs:7:3 lint/suspicious/noRedundantUseStrict FIXABLE ━━━━━
2 │ "use strict";
3 │
i Safe fix: Remove the redundant "use strict" directive
5 5 │ "use strict";
6 6 │ function inner_a() {
7 │ - → → "use·strict";·//·redundant·directive
8 │ - → }
7 │ + → }
9 8 │ function inner_b() {
10 9 │ function inner_inner() {
i Safe fix: Remove the redundant use strict directive.
7 │ → → "use·strict";·//·redundant·directive
│ ------------------------------------
```

Expand All @@ -132,16 +117,10 @@ invalid.cjs:11:4 lint/suspicious/noRedundantUseStrict FIXABLE ━━━━━
2 │ "use strict";
3 │
i Safe fix: Remove the redundant "use strict" directive
9 9 │ function inner_b() {
10 10 │ function inner_inner() {
11 │ - → → → "use·strict";·//·additional·redundant·directive
12 │ - → → }
11 │ + → → }
13 12 │ }
14 13 │ }
i Safe fix: Remove the redundant use strict directive.
11 │ → → → "use·strict";·//·additional·redundant·directive
│ -----------------------------------------------
```

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// js module
"use strict";
"use strict"; // Associated comment

function foo() {
"use strict";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ expression: invalid.js
# Input
```js
// js module
"use strict";
"use strict"; // Associated comment

function foo() {
"use strict";
Expand Down Expand Up @@ -34,21 +34,17 @@ invalid.js:2:1 lint/suspicious/noRedundantUseStrict FIXABLE ━━━━━━
! Redundant use strict directive.
1 │ // js module
> 2 │ "use strict";
> 2 │ "use strict"; // Associated comment
│ ^^^^^^^^^^^^^
3 │
4 │ function foo() {
i The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
i Safe fix: Remove the redundant "use strict" directive
1 │ - //·js·module
2 │ - "use·strict";
1 │ +
3 2 │
4 3 │ function foo() {
i Safe fix: Remove the redundant use strict directive.
2 │ "use·strict";·//·Associated·comment
│ -----------------------------------
```

Expand All @@ -65,14 +61,10 @@ invalid.js:5:2 lint/suspicious/noRedundantUseStrict FIXABLE ━━━━━━
i The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
i Safe fix: Remove the redundant "use strict" directive
3 3 │
4 4 │ function foo() {
5 │ - → "use·strict";
6 5 │ }
7 6 │
i Safe fix: Remove the redundant use strict directive.
5 │ → "use·strict";
│ -------------
```

Expand All @@ -90,16 +82,10 @@ invalid.js:11:3 lint/suspicious/noRedundantUseStrict FIXABLE ━━━━━
i The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
i Safe fix: Remove the redundant "use strict" directive
9 9 │ // All code here is evaluated in strict mode
10 10 │ test() {
11 │ - → → "use·strict";
12 │ - → }
11 │ + → }
13 12 │ }
14 13 │
i Safe fix: Remove the redundant use strict directive.
11 │ → → "use·strict";
│ -------------
```

Expand All @@ -117,16 +103,10 @@ invalid.js:18:3 lint/suspicious/noRedundantUseStrict FIXABLE ━━━━━
i The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
i Safe fix: Remove the redundant "use strict" directive
16 16 │ // All code here is evaluated in strict mode
17 17 │ test() {
18 │ - → → "use·strict";
19 │ - → }
18 │ + → }
20 19 │ };
21 20 │
i Safe fix: Remove the redundant use strict directive.
18 │ → → "use·strict";
│ -------------
```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,10 @@ invalid.ts:2:2 lint/suspicious/noRedundantUseStrict FIXABLE ━━━━━━
i The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
i Safe fix: Remove the redundant "use strict" directive
1 1 │ function test(): void {
2 │ - → "use·strict";
3 2 │ }
4 3 │
i Safe fix: Remove the redundant use strict directive.
2 │ → "use·strict";
│ -------------
```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,10 @@ invalidClass.cjs:3:3 lint/suspicious/noRedundantUseStrict FIXABLE ━━━━
6 │
7 │ const C2 = class {
i Safe fix: Remove the redundant "use strict" directive
1 1 │ class C1 {
2 2 │ test() {
3 │ - → → "use·strict";
4 │ - → }
3 │ + → }
5 4 │ }
6 5 │
i Safe fix: Remove the redundant use strict directive.
3 │ → → "use·strict";
│ -------------
```

Expand Down Expand Up @@ -81,16 +75,10 @@ invalidClass.cjs:9:3 lint/suspicious/noRedundantUseStrict FIXABLE ━━━━
│ ^
12 │
i Safe fix: Remove the redundant "use strict" directive
7 7 │ const C2 = class {
8 8 │ test() {
9 │ - → → "use·strict";
10 │ - → }
9 │ + → }
11 10 │ };
12 11 │
i Safe fix: Remove the redundant use strict directive.
9 │ → → "use·strict";
│ -------------
```

Expand Down
Loading

0 comments on commit 2e26cae

Please sign in to comment.