feat(linter/no-new-wrapper): implement auto-fixer (#10680)

DonIsaac · DonIsaac · commit 432cd778f52e · 2025-04-28T18:42:34.000Z
- Implement auto fixer for `eslint/no-new-wrapper`
- Report `new Symbol(expr)` as a violation. Diagnostics for this case warn that `Symbol` is not a constructor.
- Limit length of report spans to 24, falling back to underlying only `new Builtin`
diff --git a/crates/oxc_linter/src/rules/eslint/no_new_wrappers.rs b/crates/oxc_linter/src/rules/eslint/no_new_wrappers.rs
@@ -1,16 +1,30 @@
-use oxc_ast::{AstKind, ast::Expression};
+use oxc_ast::{
+    AstKind,
+    ast::{Argument, Expression, NewExpression},
+};
 use oxc_diagnostics::OxcDiagnostic;
 use oxc_macros::declare_oxc_lint;
 use oxc_span::Span;
 
-use crate::{AstNode, context::LintContext, rule::Rule};
+use crate::{
+    AstNode,
+    context::LintContext,
+    fixer::{RuleFix, RuleFixer},
+    rule::Rule,
+};
 
 fn no_new_wrappers_diagnostic(builtin_name: &str, new_span: Span) -> OxcDiagnostic {
     OxcDiagnostic::warn(format!("Do not use `{builtin_name}` as a constructor"))
         .with_help("Remove the `new` operator.")
         .with_label(new_span)
 }
 
+fn not_a_constructor_diagnostic(builtin_name: &str, new_span: Span) -> OxcDiagnostic {
+    OxcDiagnostic::warn(format!("`{builtin_name}` is not a constructor"))
+        .with_help("Remove the `new` operator.")
+        .with_label(new_span)
+}
+
 #[derive(Debug, Default, Clone)]
 pub struct NoNewWrappers;
 
@@ -36,6 +50,7 @@ declare_oxc_lint!(
     /// var stringObject = new String('Hello world');
     /// var numberObject = new Number(33);
     /// var booleanObject = new Boolean(false);
+    /// var symbolObject = new Symbol('foo'); // symbol is not a constructor
     /// ```
     ///
     /// Examples of **correct** code for this rule:
@@ -44,11 +59,12 @@ declare_oxc_lint!(
     /// var stringObject2 = String(value);
     /// var numberObject = Number(value);
     /// var booleanObject = Boolean(value);
+    /// var symbolObject = Symbol('foo');
     /// ```
     NoNewWrappers,
     eslint,
     pedantic,
-    pending
+    fix,
 );
 
 impl Rule for NoNewWrappers {
@@ -59,14 +75,51 @@ impl Rule for NoNewWrappers {
         let Expression::Identifier(ident) = &expr.callee else {
             return;
         };
-        if (ident.name == "String" || ident.name == "Number" || ident.name == "Boolean")
-            && ctx.is_reference_to_global_variable(ident)
-        {
-            ctx.diagnostic(no_new_wrappers_diagnostic(ident.name.as_str(), expr.span));
+        if !ctx.is_reference_to_global_variable(ident) {
+            return;
+        }
+        let name = ident.name.as_str();
+        let span = if expr.span.size() > 24 {
+            Span::new(expr.span.start, ident.span.end)
+        } else {
+            expr.span
+        };
+        match name {
+            "String" | "Number" | "Boolean" => {
+                ctx.diagnostic_with_fix(no_new_wrappers_diagnostic(name, span), |fixer| {
+                    remove_new_operator(fixer, expr, name)
+                });
+            }
+            "Symbol" => {
+                ctx.diagnostic_with_fix(not_a_constructor_diagnostic(name, span), |fixer| {
+                    remove_new_operator(fixer, expr, name)
+                });
+            }
+            _ => {}
         }
     }
 }
 
+fn remove_new_operator<'a>(
+    fixer: RuleFixer<'_, 'a>,
+    expr: &NewExpression<'a>,
+    name: &'a str,
+) -> RuleFix<'a> {
+    debug_assert!(expr.callee.is_identifier_reference());
+    let remove_new_fix = fixer.delete_range(Span::sized(expr.span.start, 4 /* "new " */));
+
+    let Some(arg) = expr.arguments.first().and_then(Argument::as_expression) else {
+        return remove_new_fix;
+    };
+
+    match (name, arg.get_inner_expression()) {
+        ("Boolean", Expression::BooleanLiteral(lit)) => fixer.replace_with(expr, lit.as_ref()),
+        ("String", Expression::StringLiteral(lit)) => fixer.replace_with(expr, lit.as_ref()),
+        ("Number", Expression::NumericLiteral(lit)) => fixer.replace_with(expr, lit.as_ref()),
+        _ => remove_new_fix,
+    }
+}
+
 #[test]
 fn test() {
     use crate::tester::Tester;
@@ -107,7 +160,29 @@ fn test() {
                 const b = new String('foo');
             }
         ",
+        "
+            var a = new String('wow look at me im a really long string, ' +
+                               'it sure would be annoying if this whole thing ' +
+                               'was underlined instead of just the constructor');
+        ",
+    ];
+    let fix = vec![
+        ("var a = new Number(foo);", "var a = Number(foo);"),
+        ("var a = new Number(1 + 1);", "var a = Number(1 + 1);"),
+        ("var a = new String(foo);", "var a = String(foo);"),
+        ("var a = new Boolean(foo);", "var a = Boolean(foo);"),
+        ("var a = new Boolean(!!x);", "var a = Boolean(!!x);"),
+        ("var a = new Symbol('foo');", "var a = Symbol('foo');"),
+        // literals dont need to be wrapped
+        ("var a = new Boolean(false);", "var a = false;"),
+        ("var a = new Boolean(true);", "var a = true;"),
+        ("var a = new String('hello');", "var a = 'hello';"),
+        ("var a = new String((((('hello')))));", "var a = 'hello';"),
+        ("var a = new Number(10);", "var a = 10;"),
+        ("var a = new Number(10 as number);", "var a = 10;"),
     ];
 
-    Tester::new(NoNewWrappers::NAME, NoNewWrappers::PLUGIN, pass, fail).test_and_snapshot();
+    Tester::new(NoNewWrappers::NAME, NoNewWrappers::PLUGIN, pass, fail)
+        .expect_fix(fix)
+        .test_and_snapshot();
 }
diff --git a/crates/oxc_linter/src/snapshots/eslint_no_new_wrappers.snap b/crates/oxc_linter/src/snapshots/eslint_no_new_wrappers.snap
@@ -39,3 +39,12 @@ source: crates/oxc_linter/src/tester.rs
  6 │             }
    ╰────
   help: Remove the `new` operator.
+
+  ⚠ eslint(no-new-wrappers): Do not use `String` as a constructor
+   ╭─[no_new_wrappers.tsx:2:21]
+ 1 │ 
+ 2 │             var a = new String('wow look at me im a really long string, ' +
+   ·                     ──────────
+ 3 │                                'it sure would be annoying if this whole thing ' +
+   ╰────
+  help: Remove the `new` operator.
