fix(useArrowFunction): make useArrowFunction rule fixer safer

.changeset/yellow-mangos-clean.md

@@ -0,0 +1,19 @@
+---
+"@biomejs/biome": patch
+---
+
+Fixed an edge case in the [`useArrowFunction`](https://biomejs.dev/linter/rules/use-arrow-function/) rule.
+
+The rule no longer emits diagnostics for or offers to fix functions that reference
+the [arguments object](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/arguments),
+because that object is undefined for arrow functions.
+
+**Valid example:**
+
+```ts
+// Valid: this function cannot be transformed into an arrow function because
+// arguments is not defined for arrow functions.
+const getFirstArg = function () {
+  return arguments[0];
+}
+```

crates/biome_js_analyze/src/lint/complexity/use_arrow_function.rs

@@ -1,21 +1,21 @@
-use crate::JsRuleAction;
+use crate::{JsRuleAction, services::semantic::Semantic};
 use biome_analyze::{
-    AddVisitor, FixKind, Phases, QueryMatch, Queryable, Rule, RuleDiagnostic, RuleSource,
-    ServiceBag, Visitor, VisitorContext, context::RuleContext, declare_lint_rule,
+    FixKind, Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule,
 };
 use biome_console::markup;
 use biome_diagnostics::Severity;
 use biome_js_factory::make;
+use biome_js_semantic::BindingExtensions;
 use biome_js_syntax::{
     AnyJsExpression, AnyJsFunctionBody, AnyJsStatement, JsConstructorClassMember, JsFileSource,
     JsFunctionBody, JsFunctionDeclaration, JsFunctionExportDefaultDeclaration,
-    JsFunctionExpression, JsGetterClassMember, JsGetterObjectMember, JsLanguage,
-    JsMethodClassMember, JsMethodObjectMember, JsModule, JsScript, JsSetterClassMember,
+    JsFunctionExpression, JsGetterClassMember, JsGetterObjectMember, JsMethodClassMember,
+    JsMethodObjectMember, JsModule, JsReferenceIdentifier, JsScript, JsSetterClassMember,
     JsSetterObjectMember, JsStaticInitializationBlockClassMember, JsSyntaxKind, T,
 };
 use biome_rowan::{
-    AstNode, AstNodeList, AstSeparatedList, BatchMutationExt, Language, SyntaxNode, TextRange,
-    TriviaPieceKind, WalkEvent, declare_node_union,
+    AstNode, AstNodeList, AstSeparatedList, BatchMutationExt, TextRange, TriviaPieceKind,
+    WalkEvent, declare_node_union,
 };
 use biome_rule_options::use_arrow_function::UseArrowFunctionOptions;
 
@@ -60,6 +60,17 @@ declare_lint_rule! {
     /// }
     /// ```
     ///
+    /// Functions that reference the [arguments
+    /// object](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/arguments)
+    /// are ignored because the arguments object is not available to arrow
+    /// functions.
+    ///
+    /// ```js
+    /// const q = function () {
+    ///     return arguments[0];
+    /// }
+    /// ```
+    ///
     /// Function expressions that declare the type of `this` are  also ignored:
     ///
     /// ```ts
@@ -79,16 +90,17 @@ declare_lint_rule! {
 }
 
 impl Rule for UseArrowFunction {
-    type Query = ActualThisScope;
+    type Query = Semantic<AnyThisScope>;
     type State = ();
     type Signals = Option<Self::State>;
     type Options = UseArrowFunctionOptions;
 
     fn run(ctx: &RuleContext<Self>) -> Self::Signals {
-        let AnyThisScopeMetadata { scope, has_this } = ctx.query();
-        if *has_this {
-            return None;
-        }
+        let scope = ctx.query();
+        let model = ctx.model();
+        let root = scope.syntax();
+        let mut preorder = root.preorder();
+
         let AnyThisScope::JsFunctionExpression(function_expression) = scope else {
             return None;
         };
@@ -123,12 +135,44 @@ impl Rule for UseArrowFunction {
             // Ignore cases where a prototype is required
             return None;
         }
+
+        while let Some(event) = preorder.next() {
+            match event {
+                WalkEvent::Enter(node) => {
+                    if node != *root
+                        && let Some(_) = AnyThisScope::cast_ref(&node)
+                    {
+                        // Stop crawling when we hit another function scope.
+                        preorder.skip_subtree();
+                    } else if matches!(
+                        node.kind(),
+                        JsSyntaxKind::JS_THIS_EXPRESSION | JsSyntaxKind::JS_NEW_TARGET_EXPRESSION
+                    ) {
+                        // If this function contains a this expression, it is not convertible
+                        // into an arrow function
+                        return None;
+                    }
+
+                    if let Some(reference) = JsReferenceIdentifier::cast_ref(&node)
+                        && reference.name().is_ok_and(|name| name == "arguments")
+                        && reference.binding(model).is_none()
+                    {
+                        // This method refers to arguments, and arguments has not
+                        // been reassigned as a variable (which is only permitted in
+                        // script mode, not module mode)
+                        return None;
+                    }
+                }
+                WalkEvent::Leave(_) => {}
+            }
+        }
+
         Some(())
     }
 
     fn text_range(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<TextRange> {
         let node = ctx.query();
-        let AnyThisScopeMetadata { scope, .. } = node;
+        let scope = node;
         let AnyThisScope::JsFunctionExpression(function_expression) = scope else {
             return None;
         };
@@ -139,7 +183,7 @@ impl Rule for UseArrowFunction {
         Some(
             RuleDiagnostic::new(
                 rule_category!(),
-                ctx.query().scope.range(),
+                ctx.query().range(),
                 markup! {
                     "This "<Emphasis>"function expression"</Emphasis>" can be turned into an "<Emphasis>"arrow function"</Emphasis>"."
                 },
@@ -151,7 +195,7 @@ impl Rule for UseArrowFunction {
     }
 
     fn action(ctx: &RuleContext<Self>, _: &Self::State) -> Option<JsRuleAction> {
-        let AnyThisScopeMetadata { scope, .. } = ctx.query();
+        let scope = ctx.query();
         let AnyThisScope::JsFunctionExpression(function_expression) = scope else {
             return None;
         };
@@ -263,83 +307,6 @@ declare_node_union! {
         | JsStaticInitializationBlockClassMember
 }
 
-#[derive(Debug, Clone)]
-pub struct AnyThisScopeMetadata {
-    scope: AnyThisScope,
-    has_this: bool,
-}
-
-pub struct ActualThisScope(AnyThisScopeMetadata);
-
-impl QueryMatch for ActualThisScope {
-    fn text_range(&self) -> TextRange {
-        self.0.scope.range()
-    }
-}
-
-impl Queryable for ActualThisScope {
-    type Input = Self;
-    type Language = JsLanguage;
-    type Output = AnyThisScopeMetadata;
-    type Services = ();
-
-    fn build_visitor(
-        analyzer: &mut impl AddVisitor<Self::Language>,
-        _: &<Self::Language as Language>::Root,
-    ) {
-        analyzer.add_visitor(Phases::Syntax, AnyThisScopeVisitor::default);
-    }
-
-    fn unwrap_match(_: &ServiceBag, query: &Self::Input) -> Self::Output {
-        query.0.clone()
-    }
-}
-
-#[derive(Default)]
-struct AnyThisScopeVisitor {
-    /// Vector to hold a function or block where `this` is scoped.
-    /// The function or block is associated to a boolean indicating whether it contains `this`.
-    stack: Vec<AnyThisScopeMetadata>,
-}
-
-impl Visitor for AnyThisScopeVisitor {
-    type Language = JsLanguage;
-
-    fn visit(
-        &mut self,
-        event: &WalkEvent<SyntaxNode<Self::Language>>,
-        mut ctx: VisitorContext<Self::Language>,
-    ) {
-        match event {
-            WalkEvent::Enter(node) => {
-                // When the visitor enters a function node, push a new entry on the stack
-                if let Some(scope) = AnyThisScope::cast_ref(node) {
-                    self.stack.push(AnyThisScopeMetadata {
-                        scope,
-                        has_this: false,
-                    });
-                } else if matches!(
-                    node.kind(),
-                    JsSyntaxKind::JS_THIS_EXPRESSION | JsSyntaxKind::JS_NEW_TARGET_EXPRESSION
-                ) {
-                    // When the visitor enters a `this` expression or `new.target`,
-                    // set the `has_this` flag for the top entry on the stack to `true`.
-                    if let Some(AnyThisScopeMetadata { has_this, .. }) = self.stack.last_mut() {
-                        *has_this = true;
-                    }
-                }
-            }
-            WalkEvent::Leave(node) => {
-                if AnyThisScope::can_cast(node.kind())
-                    && let Some(scope_metadata) = self.stack.pop()
-                {
-                    ctx.match_query(ActualThisScope(scope_metadata));
-                }
-            }
-        }
-    }
-}
-
 /// Get a minimal arrow function body from a regular function body.
 fn to_arrow_body(body: JsFunctionBody) -> AnyJsFunctionBody {
     let directives = body.directives();

crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/invalid.cjs

@@ -0,0 +1,4 @@
+const withBoundArguments = function() {
+	const arguments = 10;
+	return arguments;
+}

crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/invalid.cjs.snap

@@ -0,0 +1,38 @@
+---
+source: crates/biome_js_analyze/tests/spec_tests.rs
+expression: invalid.cjs
+---
+# Input
+```cjs
+const withBoundArguments = function() {
+	const arguments = 10;
+	return arguments;
+}
+
+```
+
+# Diagnostics
+```
+invalid.cjs:1:28 lint/complexity/useArrowFunction  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! This function expression can be turned into an arrow function.
+
+  > 1 │ const withBoundArguments = function() {
+      │                            ^^^^^^^^^^^^
+  > 2 │ 	const arguments = 10;
+  > 3 │ 	return arguments;
+  > 4 │ }
+      │ ^
+    5 │ 
+
+  i Function expressions that don't use this can be turned into arrow functions.
+
+  i Safe fix: Use an arrow function instead.
+
+    1   │ - const·withBoundArguments·=·function()·{
+      1 │ + const·withBoundArguments·=·()·=>·{
+    2 2 │   	const arguments = 10;
+    3 3 │   	return arguments;
+
+
+```

crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/valid.ts

@@ -15,6 +15,10 @@ const named = function named (this: Number) {
     return 0;
 }
 
+const withArguments = function () {
+    return arguments[0];
+}
+
 const withThisParameter = function(this: Number) {
     return 0;
 }

crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/valid.ts.snap

@@ -21,6 +21,10 @@ const named = function named (this: Number) {
     return 0;
 }
 
+const withArguments = function () {
+    return arguments[0];
+}
+
 const withThisParameter = function(this: Number) {
     return 0;
 }