fix(lint/noRedundantUseStrict/: keep leading comments

CHANGELOG.md

@@ -109,6 +109,8 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom
 
 - Fix [#294](https://github.com/biomejs/biome/issues/294). [noConfusingVoidType](https://biomejs.dev/linter/rules/no-confusing-void-type/) no longer reports false positives for return types. Contributed by @b4s36t4
 
+- Fix [#313](https://github.com/biomejs/biome/issues/313). [noRedundantUseStrict](https://biomejs.dev/linter/rules/no-redundant-use-strict/) now keeps leading comments.
+
 - Fix [#383](https://github.com/biomejs/biome/issues/383). [noMultipleSpacesInRegularExpressionLiterals](https://biomejs.dev/linter/rules/no-multiple-spaces-in-regular-expression-literals) now provides correct code fixes when consecutive spaces are followed by a quantifier. Contributed by @Conaclos
 
 - Fix [#397](https://github.com/biomejs/biome/issues/397). [useNumericLiterals](https://biomejs.dev/linter/rules/use-numeric-literals) now provides correct code fixes for signed numbers. Contributed by @Conaclos

crates/biome_js_analyze/src/analyzers/suspicious/no_redundant_use_strict.rs

@@ -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,
 };
@@ -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."
             },
         );
 
@@ -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,
         })
     }
 }

crates/biome_js_analyze/src/semantic_analyzers/nursery/no_unused_imports.rs

@@ -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,
 };
@@ -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,
@@ -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(_)
@@ -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,

crates/biome_js_analyze/src/utils/batch.rs

@@ -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
@@ -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)]

crates/biome_js_analyze/tests/specs/suspicious/noRedundantUseStrict/invalid.cjs.snap

@@ -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";
+      │ -------------
 
 ```
 
@@ -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";
+      │   -------------
 
 ```
 
@@ -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
+      │     ------------------------------------
 
 ```
 
@@ -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
+       │       -----------------------------------------------
 
 ```
 

crates/biome_js_analyze/tests/specs/suspicious/noRedundantUseStrict/invalid.js

@@ -1,5 +1,5 @@
 // js module
-"use strict";
+"use strict"; // Associated comment
 
 function foo() {
 	"use strict";

crates/biome_js_analyze/tests/specs/suspicious/noRedundantUseStrict/invalid.js.snap

@@ -5,7 +5,7 @@ expression: invalid.js
 # Input
 ```js
 // js module
-"use strict";
+"use strict"; // Associated comment
 
 function foo() {
 	"use strict";
@@ -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
+      │ -----------------------------------
 
 ```
 
@@ -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";
+      │   -------------
 
 ```
 
@@ -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";
+       │     -------------
 
 ```
 
@@ -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";
+       │     -------------
 
 ```
 

crates/biome_js_analyze/tests/specs/suspicious/noRedundantUseStrict/invalid.ts.snap

@@ -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";
+      │   -------------
 
 ```
 

crates/biome_js_analyze/tests/specs/suspicious/noRedundantUseStrict/invalidClass.cjs.snap

@@ -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";
+      │     -------------
 
 ```
 
@@ -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";
+      │     -------------
 
 ```