feat(rome_js_analyze): noPrototypeBuiltins (#4101)

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
Closes #3979

crates/rome_diagnostics_categories/src/categories.rs

@@ -95,6 +95,7 @@ define_dategories! {
     "lint/nursery/noDuplicateJsxProps": "https://docs.rome.tools/lint/rules/noDuplicateJsxProps",
     "lint/nursery/useYield": "https://docs.rome.tools/lint/rules/useYield",
     "lint/nursery/noGlobalObjectCalls": "https://docs.rome.tools/lint/rules/noGlobalObjectCalls",
+    "lint/nursery/noPrototypeBuiltins": "https://docs.rome.tools/lint/rules/noPrototypeBuiltins",
     // Insert new nursery rule here
 
     // performance

crates/rome_js_analyze/src/analyzers/nursery/no_prototype_builtins.rs

@@ -0,0 +1,136 @@
+use rome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic};
+use rome_console::markup;
+use rome_js_syntax::{AnyJsExpression, JsCallExpression, TextRange};
+use rome_rowan::AstNodeList;
+
+declare_rule! {
+    /// Disallow direct use of `Object.prototype` builtins.
+    ///
+    /// ECMAScript 5.1 added `Object.create` which allows the creation of an object with a custom prototype.
+    /// This pattern is often used for objects used as Maps. However, this pattern can lead to errors
+    /// if something else relies on prototype properties/methods.
+    /// Moreover, the methods could be shadowed, this can lead to random bugs and denial of service
+    /// vulnerabilities. For example, calling `hasOwnProperty` directly on parsed JSON like `{"hasOwnProperty": 1}` could lead to vulnerabilities.
+    /// To avoid subtle bugs like this, you should call these methods from `Object.prototype`.
+    /// For example, `foo.isPrototypeof(bar)` should be replaced with `Object.prototype.isPrototypeof.call(foo, "bar")`
+    /// As for the `hasOwn` method, `foo.hasOwn("bar")` should be replaced with `Object.hasOwn(foo, "bar")`.
+    ///
+    /// ## Examples
+    ///
+    /// ### Invalid
+    ///
+    /// ```js,expect_diagnostic
+    /// var invalid = foo.hasOwnProperty("bar");
+    /// ```
+    ///
+    /// ```js,expect_diagnostic
+    /// var invalid = foo.isPrototypeOf(bar);
+    /// ```
+    ///
+    /// ```js,expect_diagnostic
+    /// var invalid = foo.propertyIsEnumerable("bar");
+    /// ```
+    ///
+    /// ## Valid
+    ///
+    /// ```js
+    /// var valid = Object.hasOwn(foo, "bar");
+    /// var valid = Object.prototype.isPrototypeOf.call(foo, bar);
+    /// var valid = {}.propertyIsEnumerable.call(foo, "bar");
+    /// ```
+    ///
+    pub(crate) NoPrototypeBuiltins {
+        version: "next",
+        name: "noPrototypeBuiltins",
+        recommended: false,
+    }
+}
+
+pub(crate) struct RuleState {
+    prototype_builtins_method_name: String,
+    text_range: TextRange,
+}
+
+impl Rule for NoPrototypeBuiltins {
+    type Query = Ast<JsCallExpression>;
+    type State = RuleState;
+    type Signals = Option<Self::State>;
+    type Options = ();
+
+    fn run(ctx: &RuleContext<Self>) -> Self::Signals {
+        let call_expr = ctx.query();
+        let callee = call_expr.callee().ok()?.omit_parentheses();
+
+        match callee {
+            AnyJsExpression::JsComputedMemberExpression(expr) => {
+                let expr = expr.member().ok()?;
+                match expr {
+                    AnyJsExpression::AnyJsLiteralExpression(expr) => {
+                        let literal_expr = expr.as_js_string_literal_expression()?;
+                        let token_text = literal_expr.inner_string_text().ok()?;
+                        let token = literal_expr.value_token().ok()?;
+
+                        is_prototype_builtins(token_text.text()).then_some(RuleState {
+                            prototype_builtins_method_name: token_text.to_string(),
+                            text_range: token.text_range(),
+                        })
+                    }
+                    AnyJsExpression::JsTemplateExpression(expr) => {
+                        let template_element = expr.as_fields().elements.first()?;
+                        let template_chunk_token = template_element
+                            .as_js_template_chunk_element()?
+                            .template_chunk_token()
+                            .ok()?;
+                        let token_text = template_chunk_token.text();
+                        is_prototype_builtins(token_text).then_some(RuleState {
+                            prototype_builtins_method_name: token_text.to_string(),
+                            text_range: template_chunk_token.text_trimmed_range(),
+                        })
+                    }
+                    _ => None,
+                }
+            }
+            AnyJsExpression::JsStaticMemberExpression(expr) => {
+                let member = expr.as_fields().member.ok()?;
+                let value_token = member.as_js_name()?.value_token().ok()?;
+                let token_text = value_token.text();
+                is_prototype_builtins(token_text).then_some(RuleState {
+                    prototype_builtins_method_name: token_text.to_string(),
+                    text_range: value_token.text_range(),
+                })
+            }
+            _ => None,
+        }
+    }
+
+    fn diagnostic(_ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
+        let diag = RuleDiagnostic::new(
+            rule_category!(),
+            state.text_range,
+            markup! {
+                "Do not access Object.prototype method '"{&state.prototype_builtins_method_name}"' from target object."
+            },
+        );
+
+        if state.prototype_builtins_method_name == "hasOwnProperty" {
+            Some(
+                diag.note(markup! {
+                    "It's recommended using "<Emphasis>"Object.hasOwn()"</Emphasis>" instead of using "<Emphasis>"Object.hasOwnProperty()"</Emphasis>"."
+                })
+                .note(markup! {
+                    "See "<Hyperlink href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/hasOwn">"MDN web docs"</Hyperlink>" for more details."
+                }),
+            )
+        } else {
+            Some(diag)
+        }
+    }
+}
+
+/// Chekcks if the `Object.prototype` builtins called directly.
+fn is_prototype_builtins(token_text: &str) -> bool {
+    matches!(
+        token_text,
+        "hasOwnProperty" | "isPrototypeOf" | "propertyIsEnumerable"
+    )
+}

crates/rome_js_analyze/tests/specs/nursery/noPrototypeBuiltins/invalid.js

@@ -0,0 +1,11 @@
+foo.hasOwnProperty("bar");
+foo.isPrototypeOf(bar);
+foo.propertyIsEnumerable("bar");
+foo.bar.hasOwnProperty("bar");
+foo.bar.baz.isPrototypeOf("bar");
+foo["hasOwnProperty"]("bar");
+foo[`isPrototypeOf`]("bar").baz;
+foo?.hasOwnProperty(bar);
+(foo?.hasOwnProperty)("bar");
+foo?.["hasOwnProperty"]("bar");
+(foo?.[`hasOwnProperty`])("bar");

crates/rome_js_analyze/tests/specs/nursery/noPrototypeBuiltins/invalid.js.snap

@@ -0,0 +1,208 @@
+---
+source: crates/rome_js_analyze/tests/spec_tests.rs
+expression: invalid.js
+---
+# Input
+```js
+foo.hasOwnProperty("bar");
+foo.isPrototypeOf(bar);
+foo.propertyIsEnumerable("bar");
+foo.bar.hasOwnProperty("bar");
+foo.bar.baz.isPrototypeOf("bar");
+foo["hasOwnProperty"]("bar");
+foo[`isPrototypeOf`]("bar").baz;
+foo?.hasOwnProperty(bar);
+(foo?.hasOwnProperty)("bar");
+foo?.["hasOwnProperty"]("bar");
+(foo?.[`hasOwnProperty`])("bar");
+```
+
+# Diagnostics
+```
+invalid.js:1:5 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Do not access Object.prototype method 'hasOwnProperty' from target object.
+
+  > 1 │ foo.hasOwnProperty("bar");
+      │     ^^^^^^^^^^^^^^
+    2 │ foo.isPrototypeOf(bar);
+    3 │ foo.propertyIsEnumerable("bar");
+
+  i It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
+  
+  i See MDN web docs for more details.
+  
+
+```
+
+```
+invalid.js:2:5 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Do not access Object.prototype method 'isPrototypeOf' from target object.
+  
+    1 │ foo.hasOwnProperty("bar");
+  > 2 │ foo.isPrototypeOf(bar);
+      │     ^^^^^^^^^^^^^
+    3 │ foo.propertyIsEnumerable("bar");
+    4 │ foo.bar.hasOwnProperty("bar");
+  
+
+```
+
+```
+invalid.js:3:5 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Do not access Object.prototype method 'propertyIsEnumerable' from target object.
+  
+    1 │ foo.hasOwnProperty("bar");
+    2 │ foo.isPrototypeOf(bar);
+  > 3 │ foo.propertyIsEnumerable("bar");
+      │     ^^^^^^^^^^^^^^^^^^^^
+    4 │ foo.bar.hasOwnProperty("bar");
+    5 │ foo.bar.baz.isPrototypeOf("bar");
+  
+
+```
+
+```
+invalid.js:4:9 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Do not access Object.prototype method 'hasOwnProperty' from target object.
+  
+    2 │ foo.isPrototypeOf(bar);
+    3 │ foo.propertyIsEnumerable("bar");
+  > 4 │ foo.bar.hasOwnProperty("bar");
+      │         ^^^^^^^^^^^^^^
+    5 │ foo.bar.baz.isPrototypeOf("bar");
+    6 │ foo["hasOwnProperty"]("bar");
+  
+  i It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
+
+  i See MDN web docs for more details.
+
+
+```
+
+```
+invalid.js:5:13 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Do not access Object.prototype method 'isPrototypeOf' from target object.
+
+    3 │ foo.propertyIsEnumerable("bar");
+    4 │ foo.bar.hasOwnProperty("bar");
+  > 5 │ foo.bar.baz.isPrototypeOf("bar");
+      │             ^^^^^^^^^^^^^
+    6 │ foo["hasOwnProperty"]("bar");
+    7 │ foo[`isPrototypeOf`]("bar").baz;
+
+
+```
+
+```
+invalid.js:6:5 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Do not access Object.prototype method 'hasOwnProperty' from target object.
+
+    4 │ foo.bar.hasOwnProperty("bar");
+    5 │ foo.bar.baz.isPrototypeOf("bar");
+  > 6 │ foo["hasOwnProperty"]("bar");
+      │     ^^^^^^^^^^^^^^^^
+    7 │ foo[`isPrototypeOf`]("bar").baz;
+    8 │ foo?.hasOwnProperty(bar);
+
+  i It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
+  
+  i See MDN web docs for more details.
+  
+
+```
+
+```
+invalid.js:7:6 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Do not access Object.prototype method 'isPrototypeOf' from target object.
+  
+    5 │ foo.bar.baz.isPrototypeOf("bar");
+    6 │ foo["hasOwnProperty"]("bar");
+  > 7 │ foo[`isPrototypeOf`]("bar").baz;
+      │      ^^^^^^^^^^^^^
+    8 │ foo?.hasOwnProperty(bar);
+    9 │ (foo?.hasOwnProperty)("bar");
+  
+
+```
+
+```
+invalid.js:8:6 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Do not access Object.prototype method 'hasOwnProperty' from target object.
+  
+     6 │ foo["hasOwnProperty"]("bar");
+     7 │ foo[`isPrototypeOf`]("bar").baz;
+   > 8 │ foo?.hasOwnProperty(bar);
+       │      ^^^^^^^^^^^^^^
+     9 │ (foo?.hasOwnProperty)("bar");
+    10 │ foo?.["hasOwnProperty"]("bar");
+  
+  i It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
+
+  i See MDN web docs for more details.
+
+
+```
+
+```
+invalid.js:9:7 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Do not access Object.prototype method 'hasOwnProperty' from target object.
+
+     7 │ foo[`isPrototypeOf`]("bar").baz;
+     8 │ foo?.hasOwnProperty(bar);
+   > 9 │ (foo?.hasOwnProperty)("bar");
+       │       ^^^^^^^^^^^^^^
+    10 │ foo?.["hasOwnProperty"]("bar");
+    11 │ (foo?.[`hasOwnProperty`])("bar");
+
+  i It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
+  
+  i See MDN web docs for more details.
+  
+
+```
+
+```
+invalid.js:10:7 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Do not access Object.prototype method 'hasOwnProperty' from target object.
+  
+     8 │ foo?.hasOwnProperty(bar);
+     9 │ (foo?.hasOwnProperty)("bar");
+  > 10 │ foo?.["hasOwnProperty"]("bar");
+       │       ^^^^^^^^^^^^^^^^
+    11 │ (foo?.[`hasOwnProperty`])("bar");
+  
+  i It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
+
+  i See MDN web docs for more details.
+
+
+```
+
+```
+invalid.js:11:9 lint/nursery/noPrototypeBuiltins ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Do not access Object.prototype method 'hasOwnProperty' from target object.
+
+     9 │ (foo?.hasOwnProperty)("bar");
+    10 │ foo?.["hasOwnProperty"]("bar");
+  > 11 │ (foo?.[`hasOwnProperty`])("bar");
+       │         ^^^^^^^^^^^^^^
+
+  i It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
+  
+  i See MDN web docs for more details.
+  
+
+```
+
+