feat(linter): add prefer-first-class-callable rule and new autofixes

azjezz · azjezz · commit a6b1917ada50 · 2025-08-27T23:03:32.000+01:00
This commit introduces a new linter rule to promote modern PHP syntax and adds autofix capabilities to several existing rules to improve code quality and developer experience.

A new, auto-fixable rule `prefer-first-class-callable` has been added to the `best_practices` category. It detects closures and arrow functions that simply forward their arguments and automatically converts them to the more concise first-class callable syntax (`...`), which is available from PHP 8.1 onwards.

Automated fixes have also been implemented for the following rules:

* **`no-empty-comment`**: Now automatically removes empty block comments.
* **`array-style`**: Can now automatically convert between short `[]` and long `array()` syntax.

These additions help automate code cleanup and enforce modern standards with minimal effort.

Signed-off-by: azjezz &lt;azjezz@protonmail.com&gt;
diff --git a/crates/linter/src/rule/best_practices/mod.rs b/crates/linter/src/rule/best_practices/mod.rs
@@ -3,6 +3,7 @@ pub mod loop_does_not_iterate;
 pub mod middleware_in_routes;
 pub mod no_sprintf_concat;
 pub mod prefer_anonymous_migration;
+pub mod prefer_first_class_callable;
 pub mod prefer_interface;
 pub mod prefer_view_array;
 pub mod prefer_while_loop;
@@ -22,6 +23,7 @@ pub use loop_does_not_iterate::*;
 pub use middleware_in_routes::*;
 pub use no_sprintf_concat::*;
 pub use prefer_anonymous_migration::*;
+pub use prefer_first_class_callable::*;
 pub use prefer_interface::*;
 pub use prefer_view_array::*;
 pub use prefer_while_loop::*;
diff --git a/crates/linter/src/rule/best_practices/prefer_first_class_callable.rs b/crates/linter/src/rule/best_practices/prefer_first_class_callable.rs
@@ -0,0 +1,203 @@
+use indoc::indoc;
+use mago_fixer::SafetyClassification;
+use mago_php_version::PHPVersion;
+use serde::Deserialize;
+use serde::Serialize;
+
+use mago_php_version::PHPVersionRange;
+use mago_reporting::Annotation;
+use mago_reporting::Issue;
+use mago_reporting::Level;
+use mago_span::HasSpan;
+use mago_syntax::ast::*;
+
+use crate::category::Category;
+use crate::context::LintContext;
+use crate::integration::IntegrationSet;
+use crate::rule::Config;
+use crate::rule::LintRule;
+use crate::rule_meta::RuleMeta;
+use crate::settings::RuleSettings;
+
+#[derive(Debug, Clone)]
+pub struct PreferFirstClassCallableRule {
+    meta: &'static RuleMeta,
+    cfg: PreferFirstClassCallableConfig,
+}
+
+#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, Serialize, Deserialize)]
+#[serde(default, rename_all = "kebab-case", deny_unknown_fields)]
+pub struct PreferFirstClassCallableConfig {
+    pub level: Level,
+}
+
+impl Default for PreferFirstClassCallableConfig {
+    fn default() -> Self {
+        Self { level: Level::Warning }
+    }
+}
+
+impl Config for PreferFirstClassCallableConfig {
+    fn level(&self) -> Level {
+        self.level
+    }
+}
+
+impl LintRule for PreferFirstClassCallableRule {
+    type Config = PreferFirstClassCallableConfig;
+
+    fn meta() -> &'static RuleMeta {
+        const META: RuleMeta = RuleMeta {
+            name: "Prefer First Class Callable",
+            code: "prefer-first-class-callable",
+            description: indoc! {r#"
+                Promotes the use of first-class callable syntax (`...`) for creating closures.
+
+                This rule identifies closures and arrow functions that do nothing but forward their arguments to another function or method.
+                In such cases, the more concise and modern first-class callable syntax, introduced in PHP 8.1, can be used instead.
+                This improves readability by reducing boilerplate code.
+            "#},
+            good_example: indoc! {r#"
+                <?php
+
+                $names = ['Alice', 'Bob', 'Charlie'];
+                $uppercased_names = array_map(strtoupper(...), $names);
+            "#},
+            bad_example: indoc! {r#"
+                <?php
+
+                $names = ['Alice', 'Bob', 'Charlie'];
+                $uppercased_names = array_map(fn($name) => strtoupper($name), $names);
+            "#},
+            category: Category::BestPractices,
+            php: PHPVersionRange::from(PHPVersion::PHP81),
+            requires: IntegrationSet::empty(),
+        };
+        &META
+    }
+
+    fn targets() -> &'static [NodeKind] {
+        const TARGETS: &[NodeKind] = &[NodeKind::ArrowFunction, NodeKind::Closure];
+
+        TARGETS
+    }
+
+    fn build(settings: RuleSettings<Self::Config>) -> Self {
+        Self { meta: Self::meta(), cfg: settings.config }
+    }
+
+    fn check<'ast, 'arena>(&self, ctx: &mut LintContext<'_, 'arena>, node: Node<'ast, 'arena>) {
+        if let Node::ArrowFunction(arrow_function) = node {
+            let Expression::Call(call) = arrow_function.expression else {
+                return;
+            };
+
+            if !is_call_forwarding(&arrow_function.parameter_list, call) {
+                return;
+            }
+
+            let span = arrow_function.span();
+
+            let issue = Issue::new(
+                self.cfg.level(),
+                "Use first-class callable syntax `...` instead of a arrow function.",
+            )
+            .with_code(self.meta.code)
+            .with_annotation(Annotation::primary(span).with_message("This arrow function can be simplified to the `...` syntax."))
+            .with_annotation(Annotation::secondary(arrow_function.parameter_list.span()).with_message("These parameters..."))
+            .with_annotation(Annotation::secondary(call.get_argument_list().span()).with_message("...are directly forwarded here."))
+            .with_note("This closure only forwards its arguments to another function or method, which can be expressed more concisely.")
+            .with_help("Replace the arrow function with the first-class callable syntax (e.g., `strlen(...)`).");
+
+            ctx.collector.propose(issue, |p| {
+                p.delete(span.to_end(call.start_position()).to_range(), SafetyClassification::Safe);
+                p.replace(call.get_argument_list().span().to_range(), "(...)", SafetyClassification::Safe);
+            });
+        };
+
+        if let Node::Closure(closure) = node {
+            let Some(return_stmt) = get_single_return_statement(&closure.body) else {
+                return;
+            };
+
+            let Some(value) = &return_stmt.value else {
+                return;
+            };
+
+            let Expression::Call(call) = value else {
+                return;
+            };
+
+            if !is_call_forwarding(&closure.parameter_list, call) {
+                return;
+            }
+
+            let issue = Issue::new(
+                self.cfg.level(),
+                "Use first-class callable syntax `...` instead of a closure.",
+            )
+            .with_code(self.meta.code)
+            .with_annotation(Annotation::primary(node.span()).with_message("This closure can be simplified to the `...` syntax."))
+            .with_annotation(Annotation::secondary(closure.parameter_list.span()).with_message("These parameters..."))
+            .with_annotation(Annotation::secondary(call.get_argument_list().span()).with_message("...are directly forwarded here."))
+            .with_note("This closure only forwards its arguments to another function or method, which can be expressed more concisely.")
+            .with_help("Replace the closure with the first-class callable syntax (e.g., `strlen(...)`).");
+
+            ctx.collector.propose(issue, |p| {
+                let closure_end = closure.end_position();
+
+                p.delete(closure.span().to_end(value.start_position()).to_range(), SafetyClassification::Safe);
+                p.delete(return_stmt.terminator.span().to_end(closure_end).to_range(), SafetyClassification::Safe);
+                p.replace(call.get_argument_list().span().to_range(), "(...)", SafetyClassification::Safe);
+            });
+        }
+    }
+}
+
+fn is_call_forwarding<'ast, 'arena>(
+    parameter_list: &'ast FunctionLikeParameterList<'arena>,
+    call: &'ast Call<'arena>,
+) -> bool {
+    let argument_list = call.get_argument_list();
+
+    if parameter_list.parameters.len() != argument_list.arguments.len() {
+        return false;
+    }
+
+    for (idx, parameter) in parameter_list.parameters.iter().enumerate() {
+        let Some(argument) = argument_list.arguments.get(idx) else {
+            return false;
+        };
+
+        let Argument::Positional(PositionalArgument { value, .. }) = argument else {
+            return false;
+        };
+
+        let Expression::Variable(Variable::Direct(direct_variable)) = value else {
+            return false;
+        };
+
+        if direct_variable.name != parameter.variable.name {
+            return false;
+        }
+    }
+
+    // Same number of parameters and arguments, and all arguments are direct references to the corresponding parameters
+    // -> it's a call forwarding
+    true
+}
+
+#[inline]
+fn get_single_return_statement<'ast, 'arena>(block: &'ast Block<'arena>) -> Option<&'ast Return<'arena>> {
+    let statements = block.statements.as_slice();
+
+    if statements.len() != 1 {
+        return None;
+    }
+
+    let Statement::Return(return_stmt) = &statements[0] else {
+        return None;
+    };
+
+    Some(return_stmt)
+}
diff --git a/crates/linter/src/rule/consistency/array_style.rs b/crates/linter/src/rule/consistency/array_style.rs
@@ -2,6 +2,7 @@ use indoc::indoc;
 use serde::Deserialize;
 use serde::Serialize;
 
+use mago_fixer::SafetyClassification;
 use mago_php_version::PHPVersionRange;
 use mago_reporting::Annotation;
 use mago_reporting::Issue;
@@ -94,26 +95,32 @@ impl LintRule for ArrayStyleRule {
     fn check<'ast, 'arena>(&self, ctx: &mut LintContext<'_, 'arena>, node: Node<'ast, 'arena>) {
         match node {
             Node::LegacyArray(arr) if ArrayStyleOption::Short == self.cfg.style => {
-                ctx.collector.report(
-                    Issue::new(self.cfg.level(), "Short array style `[..]` is preferred over `array(..)`.")
-                        .with_code(self.meta.code)
-                        .with_annotation(
-                            Annotation::primary(arr.span())
-                                .with_message("This array uses the long array style `array(..)`"),
-                        )
-                        .with_help("Use the short array style `[..]` instead."),
-                );
+                let issue = Issue::new(self.cfg.level(), "Short array style `[..]` is preferred over `array(..)`.")
+                    .with_code(self.meta.code)
+                    .with_annotation(
+                        Annotation::primary(arr.span())
+                            .with_message("This array uses the long array style `array(..)`"),
+                    )
+                    .with_help("Use the short array style `[..]` instead.");
+
+                ctx.collector.propose(issue, |plan| {
+                    plan.delete(arr.array.span.to_range(), SafetyClassification::Safe);
+                    plan.replace(arr.left_parenthesis.to_range(), "[", SafetyClassification::Safe);
+                    plan.replace(arr.right_parenthesis.to_range(), "]", SafetyClassification::Safe);
+                });
             }
             Node::Array(arr) if ArrayStyleOption::Long == self.cfg.style => {
-                ctx.collector.report(
-                    Issue::new(self.cfg.level(), "Long array style `array(..)` is preferred over `[..]`.")
-                        .with_code(self.meta.code)
-                        .with_annotation(
-                            Annotation::primary(arr.span())
-                                .with_message("This array uses the short array style `[..]`"),
-                        )
-                        .with_help("Use the long array style `array(..)` instead."),
-                );
+                let issue = Issue::new(self.cfg.level(), "Long array style `array(..)` is preferred over `[..]`.")
+                    .with_code(self.meta.code)
+                    .with_annotation(
+                        Annotation::primary(arr.span()).with_message("This array uses the short array style `[..]`"),
+                    )
+                    .with_help("Use the long array style `array(..)` instead.");
+
+                ctx.collector.propose(issue, |plan| {
+                    plan.replace(arr.left_bracket.to_range(), "array(", SafetyClassification::Safe);
+                    plan.replace(arr.right_bracket.to_range(), ")", SafetyClassification::Safe);
+                });
             }
             _ => {}
         }
diff --git a/crates/linter/src/rule/mod.rs b/crates/linter/src/rule/mod.rs
@@ -155,6 +155,7 @@ define_rules! {
     OptionalParamOrder(optional_param_order @ OptionalParamOrderRule),
     PreferInterface(prefer_interface @ PreferInterfaceRule),
     PreferAnonymousMigration(prefer_anonymous_migration @ PreferAnonymousMigrationRule),
+    PreferFirstClassCallable(prefer_first_class_callable @ PreferFirstClassCallableRule),
     NoVoidReferenceReturn(no_void_reference_return @ NoVoidReferenceReturnRule),
     NoUnderscoreClass(no_underscore_class @ NoUnderscoreClassRule),
     NoTrailingSpace(no_trailing_space @ NoTrailingSpaceRule),
diff --git a/crates/linter/src/rule/redundancy/no_empty_comment.rs b/crates/linter/src/rule/redundancy/no_empty_comment.rs
@@ -2,6 +2,7 @@ use indoc::indoc;
 use serde::Deserialize;
 use serde::Serialize;
 
+use mago_fixer::SafetyClassification;
 use mago_php_version::PHPVersionRange;
 use mago_reporting::Annotation;
 use mago_reporting::Issue;
@@ -103,15 +104,24 @@ impl LintRule for NoEmptyCommentRule {
             }
 
             let is_empty = comment_lines(trivia).iter().all(|(_, line)| line.trim().is_empty());
+            if !is_empty {
+                continue;
+            }
 
-            if is_empty {
-                let issue = Issue::new(self.cfg.level(), "Empty comments are not allowed.")
-                    .with_code(self.meta.code)
-                    .with_annotation(Annotation::primary(trivia.span).with_message("This is an empty comment"))
-                    .with_help("Consider removing this comment.");
+            let issue = Issue::new(self.cfg.level(), "Empty comments are not allowed.")
+                .with_code(self.meta.code)
+                .with_annotation(Annotation::primary(trivia.span).with_message("This is an empty comment"))
+                .with_help("Consider removing this comment.");
 
+            if trivia.kind.is_single_line_comment() {
                 ctx.collector.report(issue);
+
+                continue;
             }
+
+            ctx.collector.propose(issue, |plan| {
+                plan.delete(trivia.span.to_range(), SafetyClassification::Safe);
+            });
         }
     }
 }
diff --git a/crates/linter/src/settings.rs b/crates/linter/src/settings.rs
@@ -55,6 +55,7 @@ pub struct RulesSettings {
     pub no_sprintf_concat: RuleSettings<NoSprintfConcatConfig>,
     pub optional_param_order: RuleSettings<OptionalParamOrderConfig>,
     pub prefer_anonymous_migration: RuleSettings<PreferAnonymousMigrationConfig>,
+    pub prefer_first_class_callable: RuleSettings<PreferFirstClassCallableConfig>,
     pub no_void_reference_return: RuleSettings<NoVoidReferenceReturnConfig>,
     pub no_underscore_class: RuleSettings<NoUnderscoreClassConfig>,
     pub no_trailing_space: RuleSettings<NoTrailingSpaceConfig>,
diff --git a/docs/tools/linter/rules/best-practices.md b/docs/tools/linter/rules/best-practices.md
@@ -15,6 +15,7 @@ This document details the rules available in the `BestPractices` category.
 | Middleware In Routes | [`middleware-in-routes`](#middleware-in-routes) |
 | No Sprintf Concat | [`no-sprintf-concat`](#no-sprintf-concat) |
 | Prefer Anonymous Migration | [`prefer-anonymous-migration`](#prefer-anonymous-migration) |
+| Prefer First Class Callable | [`prefer-first-class-callable`](#prefer-first-class-callable) |
 | Prefer Interface | [`prefer-interface`](#prefer-interface) |
 | Prefer View Array | [`prefer-view-array`](#prefer-view-array) |
 | Prefer While Loop | [`prefer-while-loop`](#prefer-while-loop) |
@@ -275,6 +276,48 @@ return new MyMigration();
 
 ---
 
+## <a id="prefer-first-class-callable"></a>`prefer-first-class-callable`
+
+Promotes the use of first-class callable syntax (`...`) for creating closures.
+
+This rule identifies closures and arrow functions that do nothing but forward their arguments to another function or method.
+In such cases, the more concise and modern first-class callable syntax, introduced in PHP 8.1, can be used instead.
+This improves readability by reducing boilerplate code.
+
+
+### Requirements
+
+- **PHP Version:** PHP `>= 8.1.0`
+
+### Configuration
+
+| Option | Type | Default |
+| :--- | :--- | :--- |
+| `enabled` | `boolean` | `true` |
+| `level` | `string` | `"warning"` |
+
+### Examples
+
+#### Correct Code
+
+```php
+<?php
+
+$names = ['Alice', 'Bob', 'Charlie'];
+$uppercased_names = array_map(strtoupper(...), $names);
+```
+
+#### Incorrect Code
+
+```php
+<?php
+
+$names = ['Alice', 'Bob', 'Charlie'];
+$uppercased_names = array_map(fn($name) => strtoupper($name), $names);
+```
+
+---
+
 ## <a id="prefer-interface"></a>`prefer-interface`
 
 Detects when an implementation class is used instead of the interface.
