From 78f38161d367668b7e4465b3c70d1b43a6fee9a1 Mon Sep 17 00:00:00 2001
From: Hao Cheng <hcheng636@gmail.com>
Date: Tue, 12 Dec 2023 23:51:12 +0100
Subject: [PATCH 1/2] feat(linter):
 eslint-plugin-unicorn/prefer-prototype-methods

---
 crates/oxc_linter/src/rules.rs                |   2 +
 .../rules/unicorn/prefer_prototype_methods.rs | 294 ++++++++++++++++++
 .../snapshots/prefer_prototype_methods.snap   | 143 +++++++++
 crates/oxc_linter/src/utils/unicorn.rs        |   8 +
 4 files changed, 447 insertions(+)
 create mode 100644 crates/oxc_linter/src/rules/unicorn/prefer_prototype_methods.rs
 create mode 100644 crates/oxc_linter/src/snapshots/prefer_prototype_methods.snap

diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs
index 78f867560535e..f1b0e0def5fad 100644
--- a/crates/oxc_linter/src/rules.rs
+++ b/crates/oxc_linter/src/rules.rs
@@ -205,6 +205,7 @@ mod unicorn {
     pub mod prefer_node_protocol;
     pub mod prefer_number_properties;
     pub mod prefer_optional_catch_binding;
+    pub mod prefer_prototype_methods;
     pub mod prefer_query_selector;
     pub mod prefer_reflect_apply;
     pub mod prefer_regexp_test;
@@ -402,6 +403,7 @@ oxc_macros::declare_all_lint_rules! {
     unicorn::no_useless_spread,
     unicorn::prefer_number_properties,
     unicorn::prefer_optional_catch_binding,
+    unicorn::prefer_prototype_methods,
     unicorn::prefer_query_selector,
     unicorn::prefer_reflect_apply,
     unicorn::prefer_regexp_test,
diff --git a/crates/oxc_linter/src/rules/unicorn/prefer_prototype_methods.rs b/crates/oxc_linter/src/rules/unicorn/prefer_prototype_methods.rs
new file mode 100644
index 0000000000000..a01cb3575fb7d
--- /dev/null
+++ b/crates/oxc_linter/src/rules/unicorn/prefer_prototype_methods.rs
@@ -0,0 +1,294 @@
+use oxc_ast::{
+    ast::{Argument, Expression},
+    AstKind,
+};
+use oxc_diagnostics::{
+    miette::{self, Diagnostic},
+    thiserror::{self, Error},
+};
+use oxc_macros::declare_oxc_lint;
+use oxc_span::{GetSpan, Span};
+
+use crate::{
+    ast_util::is_method_call,
+    context::LintContext,
+    rule::Rule,
+    utils::{is_empty_array_expression, is_empty_object_expression},
+    AstNode, Fix,
+};
+
+#[derive(Debug, Error, Diagnostic)]
+enum PreferPrototypeMethodsDiagnostic {
+    #[error("eslint-plugin-unicorn(prefer-prototype-methods): Prefer using `{1}.prototype.{2}`.")]
+    #[diagnostic(severity(warning))]
+    KnownMethod(#[label] Span, &'static str, String),
+    #[error("eslint-plugin-unicorn(prefer-prototype-methods): Prefer using method from `{1}.prototype`.")]
+    #[diagnostic(severity(warning))]
+    UnknownMethod(#[label] Span, &'static str),
+}
+
+#[derive(Debug, Default, Clone)]
+pub struct PreferPrototypeMethods;
+
+declare_oxc_lint!(
+    /// ### What it does
+    /// This rule prefers borrowing methods from the prototype instead of the instance.
+    ///
+    /// ### Why is this bad?
+    /// “Borrowing” a method from an instance of `Array` or `Object` is less clear than getting it from the corresponding prototype.
+    ///
+    /// ### Example
+    /// ```javascript
+    /// // Fail
+    /// const array = [].slice.apply(bar);
+    /// const type = {}.toString.call(foo);
+    /// Reflect.apply([].forEach, arrayLike, [callback]);
+    ///
+    /// // Pass
+    /// const array = Array.prototype.slice.apply(bar);
+    /// const type = Object.prototype.toString.call(foo);
+    /// Reflect.apply(Array.prototype.forEach, arrayLike, [callback]);
+    /// const maxValue = Math.max.apply(Math, numbers);
+    /// ```
+    PreferPrototypeMethods,
+    pedantic
+);
+
+impl Rule for PreferPrototypeMethods {
+    fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
+        let AstKind::CallExpression(call_expr) = node.kind() else { return };
+
+        if call_expr.optional {
+            return;
+        }
+        if let Expression::MemberExpression(member_expr) = &call_expr.callee.without_parenthesized()
+        {
+            if member_expr.is_computed() || member_expr.optional() {
+                return;
+            }
+        }
+
+        let mut method_expr: Option<&Expression> = None;
+
+        // `Reflect.apply([].foo, …)`
+        // `Reflect.apply({}.foo, …)`
+        if is_method_call(call_expr, Some(&["Reflect"]), Some(&["apply"]), Some(1), None) {
+            if let Argument::Expression(argument_expr) = &call_expr.arguments[0] {
+                method_expr = Some(argument_expr.without_parenthesized());
+            }
+        }
+        // `[].foo.{apply,bind,call}(…)`
+        // `({}).foo.{apply,bind,call}(…)`
+        else if is_method_call(call_expr, None, Some(&["apply", "bind", "call"]), None, None) {
+            method_expr = call_expr
+                .callee
+                .get_member_expr()
+                .map(|member_expr| member_expr.object().without_parenthesized());
+        }
+
+        let Some(method_expr) = method_expr else { return };
+        let Expression::MemberExpression(method_expr) = method_expr else { return };
+        let object_expr = method_expr.object().without_parenthesized();
+
+        if !is_empty_array_expression(object_expr) && !is_empty_object_expression(object_expr) {
+            return;
+        }
+
+        let constructor_name = match object_expr {
+            Expression::ArrayExpression(_) => "Array",
+            Expression::ObjectExpression(_) => "Object",
+            _ => unreachable!(),
+        };
+        // TODO: Replace `static_property_name` with a function similar to `getPropertyName`
+        // in @eslint-community/eslint-utils to generate better error messages for some cases.
+        let method_name = method_expr.static_property_name();
+
+        ctx.diagnostic_with_fix(
+            method_name.map_or_else(
+                || {
+                    PreferPrototypeMethodsDiagnostic::UnknownMethod(
+                        method_expr.span(),
+                        constructor_name,
+                    )
+                },
+                |method_name| {
+                    PreferPrototypeMethodsDiagnostic::KnownMethod(
+                        method_expr.span(),
+                        constructor_name,
+                        method_name.into(),
+                    )
+                },
+            ),
+            || {
+                let span = object_expr.span();
+                let need_padding = span.start >= 1
+                    && ctx.source_text().as_bytes()[span.start as usize - 1].is_ascii_alphabetic();
+                Fix::new(
+                    format!(
+                        "{}{}.prototype",
+                        if need_padding { " " } else { "" },
+                        constructor_name
+                    ),
+                    span,
+                )
+            },
+        );
+    }
+}
+
+#[test]
+fn test() {
+    use crate::tester::Tester;
+
+    let pass = vec![
+        "const foo = Array.prototype.push.apply(bar, elements);",
+        "const foo = Array.prototype.slice.call(bar);",
+        "const foo = Object.prototype.toString.call(bar);",
+        r#"const foo = Object.prototype.hasOwnProperty.call(bar, "property");"#,
+        r#"const foo = Object.prototype.propertyIsEnumerable.call(bar, "property");"#,
+        r#"const foo = "".charCodeAt.call(bar, 0);"#,
+        "const foo = [notEmpty].push.apply(bar, elements);",
+        "const foo = {notEmpty}.toString.call(bar);",
+        "Array.prototype.forEach.call(foo, () => {})",
+        "const push = Array.prototype.push.bind(foo)",
+        "const push = [].push",
+        "const {push} = []",
+        "Math.max.apply(null, numbers)",
+        "foo.apply(null, args)",
+        "Reflect.apply(...[].slice)",
+        "Reflect.apply(foo, [].slice)",
+        "Reflect.apply(Math.max, Math, numbers)",
+        "Reflect.apply()",
+        r#"Reflect["apply"]([].slice, foo, [])"#,
+        "NotReflect.apply([].slice, foo, [])",
+        "Reflect.notApply([].slice, foo, [])",
+        "Reflect.apply([]?.slice, foo, [])",
+        "Reflect.apply(foo, [].bar, [].bar)",
+        "class Foo {bar() { this.baz.bind(this) }}",
+        "const a = {bar() { this.baz.bind(this) }}",
+        "foo.bar.bind(foo)",
+        "foo.bar.bind(bar)",
+        "foo[{}].call(bar)",
+        "Object.hasOwn(bar)",
+    ];
+
+    let fail = vec![
+        "const foo = [].push.apply(bar, elements);",
+        "const foo = [].slice.call(bar);",
+        "const foo = {}.toString.call(bar);",
+        r#"const foo = {}.hasOwnProperty.call(bar, "property");"#,
+        r#"const foo = {}.propertyIsEnumerable.call(bar, "property");"#,
+        "[].forEach.call(foo, () => {})",
+        "const push = [].push.bind(foo)",
+        "const foo = [][method].call(foo)",
+        r#"const method = "realMethodName";const foo = [][method].call(foo)"#, // todo
+        "const array = Reflect.apply([].slice, foo, [])",
+        "Reflect.apply([].bar, baz, [])",
+        "const foo = ({}).toString.call(bar);",
+        "const foo = ({}.toString).call(bar);",
+        "const foo = ({}.toString.call)(bar);",
+        "function foo(){return[].slice.call(bar);}",
+        "function foo(){return{}.toString.call(bar)}",
+        "Reflect.apply({}[Symbol()], baz, [])",
+        r#"Reflect.apply({}[Symbol("symbol description")], baz, [])"#,
+        "Reflect.apply([][Symbol()], baz, [])",
+        r#"Reflect.apply({}[Symbol("symbol description")], baz, [])"#,
+        "[][Symbol.iterator].call(foo)",
+        "const foo = [].at.call(bar)",
+        "const foo = [].findLast.call(bar)",
+    ];
+
+    let fix = vec![
+        (
+            "const foo = [].push.apply(bar, elements);",
+            "const foo = Array.prototype.push.apply(bar, elements);",
+            None,
+        ),
+        ("const foo = [].slice.call(bar);", "const foo = Array.prototype.slice.call(bar);", None),
+        (
+            "const foo = {}.toString.call(bar);",
+            "const foo = Object.prototype.toString.call(bar);",
+            None,
+        ),
+        (
+            r#"const foo = {}.hasOwnProperty.call(bar, "property");"#,
+            r#"const foo = Object.prototype.hasOwnProperty.call(bar, "property");"#,
+            None,
+        ),
+        (
+            r#"const foo = {}.propertyIsEnumerable.call(bar, "property");"#,
+            r#"const foo = Object.prototype.propertyIsEnumerable.call(bar, "property");"#,
+            None,
+        ),
+        ("[].forEach.call(foo, () => {})", "Array.prototype.forEach.call(foo, () => {})", None),
+        ("const push = [].push.bind(foo)", "const push = Array.prototype.push.bind(foo)", None),
+        ("const foo = [][method].call(foo)", "const foo = Array.prototype[method].call(foo)", None),
+        (
+            r#"const method = "realMethodName";const foo = [][method].call(foo)"#,
+            r#"const method = "realMethodName";const foo = Array.prototype[method].call(foo)"#,
+            None,
+        ),
+        (
+            "const array = Reflect.apply([].slice, foo, [])",
+            "const array = Reflect.apply(Array.prototype.slice, foo, [])",
+            None,
+        ),
+        ("Reflect.apply([].bar, baz, [])", "Reflect.apply(Array.prototype.bar, baz, [])", None),
+        (
+            "const foo = ({}).toString.call(bar);",
+            "const foo = (Object.prototype).toString.call(bar);",
+            None,
+        ),
+        (
+            "const foo = ({}.toString).call(bar);",
+            "const foo = (Object.prototype.toString).call(bar);",
+            None,
+        ),
+        (
+            "const foo = ({}.toString.call)(bar);",
+            "const foo = (Object.prototype.toString.call)(bar);",
+            None,
+        ),
+        (
+            "function foo(){return[].slice.call(bar);}",
+            "function foo(){return Array.prototype.slice.call(bar);}",
+            None,
+        ),
+        (
+            "function foo(){return{}.toString.call(bar)}",
+            "function foo(){return Object.prototype.toString.call(bar)}",
+            None,
+        ),
+        (
+            "Reflect.apply({}[Symbol()], baz, [])",
+            "Reflect.apply(Object.prototype[Symbol()], baz, [])",
+            None,
+        ),
+        (
+            r#"Reflect.apply({}[Symbol("symbol description")], baz, [])"#,
+            r#"Reflect.apply(Object.prototype[Symbol("symbol description")], baz, [])"#,
+            None,
+        ),
+        (
+            "Reflect.apply([][Symbol()], baz, [])",
+            "Reflect.apply(Array.prototype[Symbol()], baz, [])",
+            None,
+        ),
+        (
+            r#"Reflect.apply({}[Symbol("symbol description")], baz, [])"#,
+            r#"Reflect.apply(Object.prototype[Symbol("symbol description")], baz, [])"#,
+            None,
+        ),
+        ("[][Symbol.iterator].call(foo)", "Array.prototype[Symbol.iterator].call(foo)", None),
+        ("const foo = [].at.call(bar)", "const foo = Array.prototype.at.call(bar)", None),
+        (
+            "const foo = [].findLast.call(bar)",
+            "const foo = Array.prototype.findLast.call(bar)",
+            None,
+        ),
+    ];
+
+    Tester::new_without_config(PreferPrototypeMethods::NAME, pass, fail)
+        .expect_fix(fix)
+        .test_and_snapshot();
+}
diff --git a/crates/oxc_linter/src/snapshots/prefer_prototype_methods.snap b/crates/oxc_linter/src/snapshots/prefer_prototype_methods.snap
new file mode 100644
index 0000000000000..848962772f64c
--- /dev/null
+++ b/crates/oxc_linter/src/snapshots/prefer_prototype_methods.snap
@@ -0,0 +1,143 @@
+---
+source: crates/oxc_linter/src/tester.rs
+expression: prefer_prototype_methods
+---
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using `Array.prototype.push`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ const foo = [].push.apply(bar, elements);
+   ·             ───────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using `Array.prototype.slice`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ const foo = [].slice.call(bar);
+   ·             ────────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using `Object.prototype.toString`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ const foo = {}.toString.call(bar);
+   ·             ───────────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using `Object.prototype.hasOwnProperty`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ const foo = {}.hasOwnProperty.call(bar, "property");
+   ·             ─────────────────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using `Object.prototype.propertyIsEnumerable`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ const foo = {}.propertyIsEnumerable.call(bar, "property");
+   ·             ───────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using `Array.prototype.forEach`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ [].forEach.call(foo, () => {})
+   · ──────────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using `Array.prototype.push`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ const push = [].push.bind(foo)
+   ·              ───────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using method from `Array.prototype`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ const foo = [][method].call(foo)
+   ·             ──────────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using method from `Array.prototype`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ const method = "realMethodName";const foo = [][method].call(foo)
+   ·                                             ──────────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using `Array.prototype.slice`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ const array = Reflect.apply([].slice, foo, [])
+   ·                             ────────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using `Array.prototype.bar`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ Reflect.apply([].bar, baz, [])
+   ·               ──────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using `Object.prototype.toString`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ const foo = ({}).toString.call(bar);
+   ·             ─────────────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using `Object.prototype.toString`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ const foo = ({}.toString).call(bar);
+   ·              ───────────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using `Object.prototype.toString`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ const foo = ({}.toString.call)(bar);
+   ·              ───────────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using `Array.prototype.slice`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ function foo(){return[].slice.call(bar);}
+   ·                      ────────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using `Object.prototype.toString`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ function foo(){return{}.toString.call(bar)}
+   ·                      ───────────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using method from `Object.prototype`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ Reflect.apply({}[Symbol()], baz, [])
+   ·               ────────────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using method from `Object.prototype`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ Reflect.apply({}[Symbol("symbol description")], baz, [])
+   ·               ────────────────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using method from `Array.prototype`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ Reflect.apply([][Symbol()], baz, [])
+   ·               ────────────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using method from `Object.prototype`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ Reflect.apply({}[Symbol("symbol description")], baz, [])
+   ·               ────────────────────────────────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using method from `Array.prototype`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ [][Symbol.iterator].call(foo)
+   · ───────────────────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using `Array.prototype.at`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ const foo = [].at.call(bar)
+   ·             ─────
+   ╰────
+
+  ⚠ eslint-plugin-unicorn(prefer-prototype-methods): Prefer using `Array.prototype.findLast`.
+   ╭─[prefer_prototype_methods.tsx:1:1]
+ 1 │ const foo = [].findLast.call(bar)
+   ·             ───────────
+   ╰────
+
+
diff --git a/crates/oxc_linter/src/utils/unicorn.rs b/crates/oxc_linter/src/utils/unicorn.rs
index a17dfc851914e..b2336993e9cc6 100644
--- a/crates/oxc_linter/src/utils/unicorn.rs
+++ b/crates/oxc_linter/src/utils/unicorn.rs
@@ -86,6 +86,14 @@ pub fn is_empty_array_expression(expr: &Expression) -> bool {
     }
 }
 
+pub fn is_empty_object_expression(expr: &Expression) -> bool {
+    if let Expression::ObjectExpression(object_expr) = expr {
+        object_expr.properties.len() == 0
+    } else {
+        false
+    }
+}
+
 pub fn is_logical_expression(node: &AstNode) -> bool {
     matches!(
         node.kind(),

From 1fc46b9c53ef09dae7cfb1f1ad99894aa431e6b4 Mon Sep 17 00:00:00 2001
From: Hao Cheng <hcheng636@gmail.com>
Date: Wed, 13 Dec 2023 23:13:40 +0100
Subject: [PATCH 2/2] chore: update todo comment

---
 .../oxc_linter/src/rules/unicorn/prefer_prototype_methods.rs  | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/crates/oxc_linter/src/rules/unicorn/prefer_prototype_methods.rs b/crates/oxc_linter/src/rules/unicorn/prefer_prototype_methods.rs
index a01cb3575fb7d..642d0764a05c6 100644
--- a/crates/oxc_linter/src/rules/unicorn/prefer_prototype_methods.rs
+++ b/crates/oxc_linter/src/rules/unicorn/prefer_prototype_methods.rs
@@ -181,7 +181,7 @@ fn test() {
         "[].forEach.call(foo, () => {})",
         "const push = [].push.bind(foo)",
         "const foo = [][method].call(foo)",
-        r#"const method = "realMethodName";const foo = [][method].call(foo)"#, // todo
+        r#"const method = "realMethodName";const foo = [][method].call(foo)"#, // TODO: Improve error message for this case.
         "const array = Reflect.apply([].slice, foo, [])",
         "Reflect.apply([].bar, baz, [])",
         "const foo = ({}).toString.call(bar);",
@@ -193,7 +193,7 @@ fn test() {
         r#"Reflect.apply({}[Symbol("symbol description")], baz, [])"#,
         "Reflect.apply([][Symbol()], baz, [])",
         r#"Reflect.apply({}[Symbol("symbol description")], baz, [])"#,
-        "[][Symbol.iterator].call(foo)",
+        "[][Symbol.iterator].call(foo)", // TODO: Improve error message for this case.
         "const foo = [].at.call(bar)",
         "const foo = [].findLast.call(bar)",
     ];