refactor(noOctalEscape): fixes and improvements (#5243)

crates/biome_js_analyze/src/lint/nursery/no_octal_escape.rs

@@ -1,9 +1,12 @@
 use biome_analyze::{
-    Ast, Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule,
+    Ast, FixKind, Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule,
 };
 use biome_console::markup;
-use biome_js_syntax::JsStringLiteralExpression;
-use biome_rowan::AstNode;
+use biome_diagnostics::Severity;
+use biome_js_syntax::{JsLiteralMemberName, JsStringLiteralExpression, JsSyntaxToken};
+use biome_rowan::{BatchMutationExt, SyntaxResult, TextRange, declare_node_union};
+
+use crate::JsRuleAction;
 
 declare_lint_rule! {
     /// Disallow octal escape sequences in string literals
@@ -16,69 +19,123 @@ declare_lint_rule! {
     /// ### Invalid
     ///
     /// ```js,expect_diagnostic
-    /// var foo = "Copyright \251";
+    /// const foo = "Copyright \251";
     /// ```
     ///
     /// ### Valid
     ///
     /// ```js
-    /// var foo = "Copyright \u00A9";   // unicode
-    ///
-    /// var foo = "Copyright \xA9";     // hexadecimal
+    /// const foo = "Copyright \u00A9"; // unicode escape
+    /// const bar = "Copyright \xA9"; // hexadecimal escape
     /// ```
     ///
     pub NoOctalEscape {
         version: "1.9.3",
         name: "noOctalEscape",
         language: "js",
         sources: &[RuleSource::Eslint("no-octal-escape")],
-        recommended: false,
+        recommended: true,
+        severity: Severity::Warning,
+        fix_kind: FixKind::Safe,
     }
 }
 
 impl Rule for NoOctalEscape {
-    type Query = Ast<JsStringLiteralExpression>;
-    type State = ();
+    type Query = Ast<AnyJsStringLiteral>;
+    type State = RuleState;
     type Signals = Option<Self::State>;
     type Options = ();
 
     fn run(ctx: &RuleContext<Self>) -> Self::Signals {
-        let node = ctx.query();
-        let token = node.value_token().ok()?;
-        let text = token.text();
-
-        let bytes = text.as_bytes();
-        let mut byte_it = bytes.iter();
-        while let Some(&byte) = byte_it.next() {
+        let token = ctx.query().value_token().ok()?;
+        let mut it = token.text_trimmed().bytes().enumerate();
+        while let Some((index, byte)) = it.next() {
             if byte == b'\\' {
-                if let Some(&next_byte) = byte_it.next() {
-                    if (b'0'..=b'7').contains(&next_byte) {
-                        return Some(());
+                if let Some((_, byte)) = it.next() {
+                    if matches!(byte, b'0'..=b'7') {
+                        let len = 2 + it
+                            .clone()
+                            .take(5)
+                            .take_while(|(_, byte)| matches!(byte, b'0'..=b'7'))
+                            .count();
+                        // Ignore the non-deprecated `\0`
+                        if byte != b'0' || len > 2 {
+                            return Some(RuleState { index, len });
+                        }
                     }
                 }
             }
         }
         None
     }
 
-    fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {
-        let node = ctx.query();
-        let token = node.value_token().ok()?;
-        let text = token.text();
-        Some(
-            RuleDiagnostic::new(
-                rule_category!(),
-                node.range(),
-                markup! {
-                    "Don't use "<Emphasis>"octal"</Emphasis>
-                },
+    fn diagnostic(
+        ctx: &RuleContext<Self>,
+        RuleState { index, len }: &Self::State,
+    ) -> Option<RuleDiagnostic> {
+        let token = ctx.query().value_token().ok()?;
+        let escape_start = token
+            .text_trimmed_range()
+            .start()
+            .checked_add((*index as u32).into())?;
+        Some(RuleDiagnostic::new(
+            rule_category!(),
+            TextRange::at(escape_start, (*len as u32).into()),
+            markup! {
+                "Don't use deprecated "<Emphasis>"octal escape sequences"</Emphasis>"."
+            },
+        ))
+    }
+
+    fn action(
+        ctx: &RuleContext<Self>,
+        RuleState { index, len }: &Self::State,
+    ) -> Option<JsRuleAction> {
+        let token = ctx.query().value_token().ok()?;
+        let text = token.text_trimmed();
+        let octal = &text[(index + 1)..(index + len)];
+        let codepoint = u32::from_str_radix(octal, 8).ok()?;
+        let before_octal = &text[..(index + 1)];
+        let after_octal = &text[(index + len)..];
+        let (new_text, unicode_or_hexa) = if codepoint <= 0xff {
+            (
+                format!("{before_octal}x{codepoint:02x}{after_octal}"),
+                "hexadecimal",
+            )
+        } else {
+            (
+                format!("{before_octal}u{codepoint:04x}{after_octal}"),
+                "unicode",
             )
-            .note(markup! {
-                "Don't use octal escape sequences: " {text}
-            })
-            .note(markup! {
-                "Use unicode or hexidecimal escape sequences instead."
-            }),
-        )
+        };
+        let new_token = JsSyntaxToken::new_detached(token.kind(), &new_text, [], []);
+        let mut mutation = ctx.root().begin();
+        mutation.replace_token_transfer_trivia(token.clone(), new_token);
+        Some(JsRuleAction::new(
+            ctx.metadata().action_category(ctx.category(), ctx.group()),
+            ctx.metadata().applicability(),
+            markup! { "Use "{unicode_or_hexa}" escape sequences instead." }.to_owned(),
+            mutation,
+        ))
     }
 }
+
+declare_node_union! {
+    /// Any string literal excluding JsxString.
+    pub AnyJsStringLiteral = JsStringLiteralExpression | JsLiteralMemberName
+}
+impl AnyJsStringLiteral {
+    pub fn value_token(&self) -> SyntaxResult<JsSyntaxToken> {
+        match self {
+            AnyJsStringLiteral::JsStringLiteralExpression(node) => node.value_token(),
+            AnyJsStringLiteral::JsLiteralMemberName(node) => node.value(),
+        }
+    }
+}
+
+pub struct RuleState {
+    // Index of the escape sequence (starts with `\`)
+    index: usize,
+    // Length of the escape sequence
+    len: usize,
+}

crates/biome_js_analyze/src/lint/nursery/no_useless_escape_in_string.rs

@@ -62,20 +62,20 @@ declare_lint_rule! {
 }
 
 impl Rule for NoUselessEscapeInString {
-    type Query = Ast<AnyString>;
+    type Query = Ast<AnyJsString>;
     type State = (JsSyntaxToken, usize);
     type Signals = Option<Self::State>;
     type Options = ();
 
     fn run(ctx: &RuleContext<Self>) -> Self::Signals {
         let node = ctx.query();
         match node {
-            AnyString::JsStringLiteralExpression(literal) => {
+            AnyJsString::JsStringLiteralExpression(literal) => {
                 let token = literal.value_token().ok()?;
                 let text = token.text_trimmed();
                 next_useless_escape(text, text.bytes().next()?).map(|index| (token, index))
             }
-            AnyString::JsTemplateExpression(template) => {
+            AnyJsString::JsTemplateExpression(template) => {
                 if template.tag().is_some() {
                     return None;
                 }
@@ -94,7 +94,7 @@ impl Rule for NoUselessEscapeInString {
                 }
                 None
             }
-            AnyString::JsLiteralMemberName(member_name) => {
+            AnyJsString::JsLiteralMemberName(member_name) => {
                 let Ok(token) = member_name.value() else {
                     return None;
                 };
@@ -141,7 +141,7 @@ impl Rule for NoUselessEscapeInString {
 
 declare_node_union! {
     /// Any string literal excluding JsxString.
-    pub AnyString = JsStringLiteralExpression | JsTemplateExpression | JsLiteralMemberName
+    pub AnyJsString = JsStringLiteralExpression | JsTemplateExpression | JsLiteralMemberName
 }
 
 /// Returns the index in `str` of the first useless escape.

crates/biome_js_analyze/tests/specs/nursery/noOctalEscape/invalid.js

@@ -1,64 +1,15 @@
-var foo = "foo \01 bar";
-var foo = "foo \000 bar";
-var foo = "foo \377 bar";
-var foo = "foo \378 bar";
-var foo = "foo \37a bar";
-var foo = "foo \381 bar";
-var foo = "foo \3a1 bar";
-var foo = "foo \251 bar";
-var foo = "foo \258 bar";
-var foo = "foo \25a bar";
-var foo = "\3s51";
-var foo = "\77";
-var foo = "\78";
-var foo = "\5a";
-var foo = "\751";
-var foo = "foo \400 bar";
-var foo = "foo \400 bar";
-var foo = "\t\1";
-var foo = "\\\751";
-'\0\1'
-'\0 \1'
-'\0\01'
-'\0 \01'
-'\0a\1'
-'\0a\01'
-'\0\08'
-'\1'
-'\2'
-'\7'
-'\00'
-'\01'
-'\02'
-'\07'
-'\08'
-'\09'
-'\10'
-'\12'
-' \1'
-'\1 '
-'a\1'
-'\1a'
-'a\1a'
-' \01'
-'\01 '
-'a\01'
-'\01a'
-'a\01a'
-'a\08a'
-'\n\1'
-'\n\01'
-'\n\08'
-'\\\1'
-'\\\01'
-'\\\08'
-'\\n\1'
-'\01\02'
-'\02\01'
-'\01\2'
-'\2\01'
-'\08\1'
-'foo \1 bar \2'
-
-
+0;
+"foo \01 bar";
+"foo \000 bar";
+"foo \377 bar";
+"foo \378 bar";
+"foo \37a bar";
+"foo \381 bar";
+"foo \3a1 bar";
+"foo \751 bar";
+"foo \258 bar";
+"foo \25a bar";
 
+const o = {
+    '\31': 0,
+};