From 7c65740dcee1dad4263bca1c08a57b55175cb7f9 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Wed, 18 Sep 2024 11:02:19 -0400 Subject: [PATCH 01/16] wip --- crates/oxc_linter/src/rules.rs | 2 + .../src/rules/eslint/no_extend_native.rs | 207 ++++++++++++++++++ 2 files changed, 209 insertions(+) create mode 100644 crates/oxc_linter/src/rules/eslint/no_extend_native.rs diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 73b0b8663d348..d1286ffc42029 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -71,6 +71,7 @@ mod eslint { pub mod no_eq_null; pub mod no_eval; pub mod no_ex_assign; + pub mod no_extend_native; pub mod no_extra_boolean_cast; pub mod no_fallthrough; pub mod no_func_assign; @@ -530,6 +531,7 @@ oxc_macros::declare_all_lint_rules! { eslint::no_eq_null, eslint::no_eval, eslint::no_ex_assign, + eslint::no_extend_native, eslint::no_extra_boolean_cast, eslint::no_fallthrough, eslint::no_func_assign, diff --git a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs new file mode 100644 index 0000000000000..b8327252e039f --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs @@ -0,0 +1,207 @@ +use oxc_ast::{ast::MemberExpression, AstKind}; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::{CompactStr, Span}; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +#[derive(Debug, Default, Clone)] +pub struct NoExtendNative { + /// A list of objects which are allowed to be exceptions to the rule. + exceptions: Vec, +} + +declare_oxc_lint!( + /// ### What it does + /// + /// + /// ### Why is this bad? + /// + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// FIXME: Tests will fail if examples are missing or syntactically incorrect. + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// FIXME: Tests will fail if examples are missing or syntactically incorrect. + /// ``` + NoExtendNative, + suspicious, +); + +impl Rule for NoExtendNative { + fn from_configuration(value: serde_json::Value) -> Self { + let obj = value.get(0); + + Self { + exceptions: obj + .and_then(|v| v.get("exceptions")) + .and_then(serde_json::Value::as_array) + .unwrap_or(&vec![]) + .iter() + .map(serde_json::Value::as_str) + .filter(Option::is_some) + .map(|x| x.unwrap().into()) + .collect::>(), + } + } + + fn run_once(&self, ctx: &LintContext) { + let symbols = ctx.symbols(); + for reference_id_list in ctx.scopes().root_unresolved_references_ids() { + for reference_id in reference_id_list { + let reference = symbols.get_reference(reference_id); + let name = ctx.semantic().reference_name(reference); + // If the referenced name is explicitly allowed, skip it. + let compact_name = CompactStr::from(name); + if self.exceptions.contains(&compact_name) { + continue; + } + // If the first letter is capital, like `Object`, we will assume it is a native obejct + let Some(first_char) = name.chars().next() else { + continue; + }; + if first_char.is_lowercase() { + continue; + } + let node = ctx.nodes().get_node(reference.node_id()); + dbg!(name); + dbg!(node); + // If this is not `*.prototype` access, skip it. + let Some(prop_access) = get_prototype_property_accessed(ctx, node) else { + continue; + }; + dbg!(prop_access); + let Some(prop_assign) = get_property_assignment(ctx, prop_access) else { + continue; + }; + dbg!(prop_assign); + if ctx.env_contains_var(name) { + ctx.diagnostic( + OxcDiagnostic::error(format!( + "{} prototype is read only, properties should not be added.", + name + )) + .with_label(ctx.semantic().reference_span(reference)), + ); + } + } + } + } +} + +/// Get an assignment to the property of the given node. +/// Example: `*.prop = 0` where `*.prop` is the given node. +fn get_property_assignment<'a>( + ctx: &'a LintContext, + node: &AstNode<'a>, +) -> Option<&'a AstNode<'a>> { + let mut parent = ctx.nodes().parent_node(node.id())?; + loop { + if let AstKind::AssignmentExpression(_) | AstKind::SimpleAssignmentTarget(_) = parent.kind() + { + return Some(parent); + } else if let AstKind::MemberExpression(expr) = parent.kind() { + // Ignore computed member expressions like `obj[Object.prototype] = 0` + if let MemberExpression::ComputedMemberExpression(_) = expr { + return None; + } + parent = ctx.nodes().parent_node(parent.id())?; + } else { + return None; + } + } +} + +fn get_prototype_property_accessed<'a>( + ctx: &'a LintContext, + node: &AstNode<'a>, +) -> Option<&'a AstNode<'a>> { + let AstKind::IdentifierReference(ident) = node.kind() else { + return None; + }; + let Some(parent) = ctx.nodes().parent_node(node.id()) else { + return None; + }; + let mut prototype_node = Some(parent); + let AstKind::MemberExpression(prop_access) = parent.kind() else { + return None; + }; + let MemberExpression::StaticMemberExpression(prop_access) = prop_access else { + return None; + }; + let prop_name = prop_access.property.name.as_str(); + if prop_name != "prototype" { + return None; + } + let Some(grandparent) = ctx.nodes().parent_node(parent.id()) else { + return None; + }; + dbg!(grandparent); + // Expand the search to include computed member expressions like `Object.prototype['p']` + if let AstKind::MemberExpression(expr) = grandparent.kind() { + if let MemberExpression::ComputedMemberExpression(computed) = expr { + if computed.object == prop_access { + prototype_node = Some(grandparent); + } + } + } + prototype_node +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ("x.prototype.p = 0", None), + ("x.prototype['p'] = 0", None), + ("Object.p = 0", None), + ("Object.toString.bind = 0", None), + ("Object['toString'].bind = 0", None), + ("Object.defineProperty(x, 'p', {value: 0})", None), + ("Object.defineProperties(x, {p: {value: 0}})", None), + ("global.Object.prototype.toString = 0", None), + ("this.Object.prototype.toString = 0", None), + ("with(Object) { prototype.p = 0; }", None), + ("o = Object; o.prototype.toString = 0", None), + ("eval('Object.prototype.toString = 0')", None), + ("parseFloat.prototype.x = 1", None), + ("Object.prototype.g = 0", Some(serde_json::json!([{ "exceptions": ["Object"] }]))), + ("obj[Object.prototype] = 0", None), + ("Object.defineProperty()", None), + ("Object.defineProperties()", None), + ("function foo() { var Object = function() {}; Object.prototype.p = 0 }", None), + ("{ let Object = function() {}; Object.prototype.p = 0 }", None), // { "ecmaVersion": 6 } + ]; + + let fail = vec![ + ("Object.prototype.p = 0", None), + ("BigInt.prototype.p = 0", None), // { "ecmaVersion": 2020 }, + ("WeakRef.prototype.p = 0", None), // { "ecmaVersion": 2021 }, + ("FinalizationRegistry.prototype.p = 0", None), // { "ecmaVersion": 2021 }, + ("AggregateError.prototype.p = 0", None), // { "ecmaVersion": 2021 }, + ("Function.prototype['p'] = 0", None), + ("String['prototype'].p = 0", None), + ("Number['prototype']['p'] = 0", None), + ("Object.defineProperty(Array.prototype, 'p', {value: 0})", None), + ("Object.defineProperties(Array.prototype, {p: {value: 0}})", None), + ("Object.defineProperties(Array.prototype, {p: {value: 0}, q: {value: 0}})", None), + ("Number['prototype']['p'] = 0", Some(serde_json::json!([{ "exceptions": ["Object"] }]))), + ("Object.prototype.p = 0; Object.prototype.q = 0", None), + ("function foo() { Object.prototype.p = 0 }", None), + ("(Object?.prototype).p = 0", None), // { "ecmaVersion": 2020 }, + ("Object.defineProperty(Object?.prototype, 'p', { value: 0 })", None), // { "ecmaVersion": 2020 }, + ("Object?.defineProperty(Object.prototype, 'p', { value: 0 })", None), // { "ecmaVersion": 2020 }, + ("(Object?.defineProperty)(Object.prototype, 'p', { value: 0 })", None), // { "ecmaVersion": 2020 }, + ("Array.prototype.p &&= 0", None), // { "ecmaVersion": 2021 }, + ("Array.prototype.p ||= 0", None), // { "ecmaVersion": 2021 }, + ("Array.prototype.p ??= 0", None), // { "ecmaVersion": 2021 } + ]; + + Tester::new(NoExtendNative::NAME, pass, fail).test_and_snapshot(); +} From cb48dbc23365de55102b09f903150cfc91707dfd Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Wed, 18 Sep 2024 11:18:07 -0400 Subject: [PATCH 02/16] Support computed members of prototype --- .../src/rules/eslint/no_extend_native.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs index b8327252e039f..75bea5d9c852e 100644 --- a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs +++ b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs @@ -1,6 +1,7 @@ use oxc_ast::{ast::MemberExpression, AstKind}; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; +use oxc_span::cmp::ContentEq; use oxc_span::{CompactStr, Span}; use crate::{context::LintContext, rule::Rule, AstNode}; @@ -121,20 +122,22 @@ fn get_prototype_property_accessed<'a>( ctx: &'a LintContext, node: &AstNode<'a>, ) -> Option<&'a AstNode<'a>> { - let AstKind::IdentifierReference(ident) = node.kind() else { + let AstKind::IdentifierReference(_) = node.kind() else { return None; }; let Some(parent) = ctx.nodes().parent_node(node.id()) else { return None; }; let mut prototype_node = Some(parent); - let AstKind::MemberExpression(prop_access) = parent.kind() else { + let AstKind::MemberExpression(prop_access_expr) = parent.kind() else { return None; }; - let MemberExpression::StaticMemberExpression(prop_access) = prop_access else { + let MemberExpression::StaticMemberExpression(_) = prop_access_expr else { + return None; + }; + let Some(prop_name) = prop_access_expr.static_property_name() else { return None; }; - let prop_name = prop_access.property.name.as_str(); if prop_name != "prototype" { return None; } @@ -145,7 +148,11 @@ fn get_prototype_property_accessed<'a>( // Expand the search to include computed member expressions like `Object.prototype['p']` if let AstKind::MemberExpression(expr) = grandparent.kind() { if let MemberExpression::ComputedMemberExpression(computed) = expr { - if computed.object == prop_access { + if computed + .object + .as_member_expression() + .is_some_and(|object| object.content_eq(prop_access_expr)) + { prototype_node = Some(grandparent); } } From ff067a051941d5deaaa3243fc9f1ae1e2dd3764c Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Wed, 18 Sep 2024 11:25:32 -0400 Subject: [PATCH 03/16] Support exprs like `String['prototype']` --- crates/oxc_linter/src/rules/eslint/no_extend_native.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs index 75bea5d9c852e..0916d6c0a0454 100644 --- a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs +++ b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs @@ -107,6 +107,7 @@ fn get_property_assignment<'a>( { return Some(parent); } else if let AstKind::MemberExpression(expr) = parent.kind() { + dbg!(expr); // Ignore computed member expressions like `obj[Object.prototype] = 0` if let MemberExpression::ComputedMemberExpression(_) = expr { return None; @@ -132,9 +133,7 @@ fn get_prototype_property_accessed<'a>( let AstKind::MemberExpression(prop_access_expr) = parent.kind() else { return None; }; - let MemberExpression::StaticMemberExpression(_) = prop_access_expr else { - return None; - }; + dbg!(prop_access_expr); let Some(prop_name) = prop_access_expr.static_property_name() else { return None; }; From e5d6f388d730c70565c411aa7eab95291b1491d2 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Wed, 18 Sep 2024 12:00:53 -0400 Subject: [PATCH 04/16] Support defineProperty calls --- .../src/rules/eslint/no_extend_native.rs | 69 +++++++++++++++++-- 1 file changed, 62 insertions(+), 7 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs index 0916d6c0a0454..bd59b8d96ca1d 100644 --- a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs +++ b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs @@ -1,8 +1,9 @@ +use oxc_ast::ast::{CallExpression, Expression}; use oxc_ast::{ast::MemberExpression, AstKind}; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; use oxc_span::cmp::ContentEq; -use oxc_span::{CompactStr, Span}; +use oxc_span::{CompactStr, GetSpan, Span}; use crate::{context::LintContext, rule::Rule, AstNode}; @@ -76,18 +77,31 @@ impl Rule for NoExtendNative { let Some(prop_access) = get_prototype_property_accessed(ctx, node) else { continue; }; - dbg!(prop_access); - let Some(prop_assign) = get_property_assignment(ctx, prop_access) else { + if !ctx.env_contains_var(name) { continue; - }; - dbg!(prop_assign); - if ctx.env_contains_var(name) { + } + dbg!(prop_access); + // Check if being used like `String.prototype.xyz = 0` + if let Some(prop_assign) = get_property_assignment(ctx, prop_access) { + dbg!(prop_assign); + ctx.diagnostic( + OxcDiagnostic::error(format!( + "{} prototype is read only, properties should not be added.", + name + )) + .with_label(prop_assign.span()), + ); + } + // Check if being used like `Object.defineProperty(String.prototype, 'xyz', 0)` + else if let Some(define_property_call) = + get_define_property_call(ctx, prop_access) + { ctx.diagnostic( OxcDiagnostic::error(format!( "{} prototype is read only, properties should not be added.", name )) - .with_label(ctx.semantic().reference_span(reference)), + .with_label(define_property_call.span()), ); } } @@ -95,6 +109,44 @@ impl Rule for NoExtendNative { } } +fn get_define_property_call<'a>( + ctx: &'a LintContext, + node: &AstNode<'a>, +) -> Option<&'a AstNode<'a>> { + dbg!(node); + let Some(parent) = ctx.nodes().parent_node(node.id()) else { + return None; + }; + dbg!(parent); + let AstKind::Argument(_) = parent.kind() else { + return None; + }; + let Some(grandparent) = ctx.nodes().parent_node(parent.id()) else { + return None; + }; + let AstKind::CallExpression(call_expr) = grandparent.kind() else { + return None; + }; + if !is_define_property_call(call_expr) { + return None; + } + Some(grandparent) +} + +fn is_define_property_call(call_expr: &CallExpression) -> bool { + match call_expr.callee.as_member_expression() { + Some(me) => { + let prop_name = me.static_property_name(); + me.object() + .get_identifier_reference() + .map_or(false, |ident_ref| ident_ref.name == "Object") + && (prop_name == Some("defineProperty") || prop_name == Some("defineProperties")) + && !me.optional() + } + _ => false, + } +} + /// Get an assignment to the property of the given node. /// Example: `*.prop = 0` where `*.prop` is the given node. fn get_property_assignment<'a>( @@ -195,6 +247,8 @@ fn test() { ("String['prototype'].p = 0", None), ("Number['prototype']['p'] = 0", None), ("Object.defineProperty(Array.prototype, 'p', {value: 0})", None), + ("Object['defineProperty'](Array.prototype, 'p', {value: 0})", None), + ("Object['defineProperty'](Array['prototype'], 'p', {value: 0})", None), ("Object.defineProperties(Array.prototype, {p: {value: 0}})", None), ("Object.defineProperties(Array.prototype, {p: {value: 0}, q: {value: 0}})", None), ("Number['prototype']['p'] = 0", Some(serde_json::json!([{ "exceptions": ["Object"] }]))), @@ -203,6 +257,7 @@ fn test() { ("(Object?.prototype).p = 0", None), // { "ecmaVersion": 2020 }, ("Object.defineProperty(Object?.prototype, 'p', { value: 0 })", None), // { "ecmaVersion": 2020 }, ("Object?.defineProperty(Object.prototype, 'p', { value: 0 })", None), // { "ecmaVersion": 2020 }, + ("Object?.['defineProperty'](Object?.['prototype'], 'p', {value: 0})", None), ("(Object?.defineProperty)(Object.prototype, 'p', { value: 0 })", None), // { "ecmaVersion": 2020 }, ("Array.prototype.p &&= 0", None), // { "ecmaVersion": 2021 }, ("Array.prototype.p ||= 0", None), // { "ecmaVersion": 2021 }, From 9ead3e8bb9e1a4c64139ee4182fedc9f73135c86 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Wed, 18 Sep 2024 13:26:11 -0400 Subject: [PATCH 05/16] Add basic support for chained expression --- .../src/rules/eslint/no_extend_native.rs | 63 ++++++++++++++----- 1 file changed, 46 insertions(+), 17 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs index bd59b8d96ca1d..0811f6c61eb89 100644 --- a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs +++ b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs @@ -1,4 +1,4 @@ -use oxc_ast::ast::{CallExpression, Expression}; +use oxc_ast::ast::{CallExpression, ChainElement, Expression}; use oxc_ast::{ast::MemberExpression, AstKind}; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; @@ -58,6 +58,10 @@ impl Rule for NoExtendNative { for reference_id in reference_id_list { let reference = symbols.get_reference(reference_id); let name = ctx.semantic().reference_name(reference); + // If the referenced name does not appear to be a global object, skip it. + if !ctx.env_contains_var(name) { + continue; + } // If the referenced name is explicitly allowed, skip it. let compact_name = CompactStr::from(name); if self.exceptions.contains(&compact_name) { @@ -77,9 +81,6 @@ impl Rule for NoExtendNative { let Some(prop_access) = get_prototype_property_accessed(ctx, node) else { continue; }; - if !ctx.env_contains_var(name) { - continue; - } dbg!(prop_access); // Check if being used like `String.prototype.xyz = 0` if let Some(prop_assign) = get_property_assignment(ctx, prop_access) { @@ -113,11 +114,9 @@ fn get_define_property_call<'a>( ctx: &'a LintContext, node: &AstNode<'a>, ) -> Option<&'a AstNode<'a>> { - dbg!(node); let Some(parent) = ctx.nodes().parent_node(node.id()) else { return None; }; - dbg!(parent); let AstKind::Argument(_) = parent.kind() else { return None; }; @@ -153,16 +152,27 @@ fn get_property_assignment<'a>( ctx: &'a LintContext, node: &AstNode<'a>, ) -> Option<&'a AstNode<'a>> { + dbg!(node); let mut parent = ctx.nodes().parent_node(node.id())?; loop { + dbg!(parent); if let AstKind::AssignmentExpression(_) | AstKind::SimpleAssignmentTarget(_) = parent.kind() { return Some(parent); - } else if let AstKind::MemberExpression(expr) = parent.kind() { - dbg!(expr); - // Ignore computed member expressions like `obj[Object.prototype] = 0` - if let MemberExpression::ComputedMemberExpression(_) = expr { - return None; + } else if let AstKind::MemberExpression(member_expr) = parent.kind() { + if let MemberExpression::ComputedMemberExpression(computed) = member_expr { + if let AstKind::MemberExpression(node_expr) = node.kind() { + // Ignore computed member expressions like `obj[Object.prototype] = 0` (i.e., the + // given node is the `expression` of the computed member expression) + if computed + .expression + .as_member_expression() + .is_some_and(|expression| expression.content_eq(node_expr)) + { + return None; + } + return None; + } } parent = ctx.nodes().parent_node(parent.id())?; } else { @@ -171,6 +181,8 @@ fn get_property_assignment<'a>( } } +/// Returns the ASTNode that represents a prototype property access, such as +/// `Object?.['prototype']` fn get_prototype_property_accessed<'a>( ctx: &'a LintContext, node: &AstNode<'a>, @@ -181,30 +193,46 @@ fn get_prototype_property_accessed<'a>( let Some(parent) = ctx.nodes().parent_node(node.id()) else { return None; }; + dbg!(parent); let mut prototype_node = Some(parent); let AstKind::MemberExpression(prop_access_expr) = parent.kind() else { return None; }; - dbg!(prop_access_expr); let Some(prop_name) = prop_access_expr.static_property_name() else { return None; }; if prop_name != "prototype" { return None; } - let Some(grandparent) = ctx.nodes().parent_node(parent.id()) else { + let Some(grandparent_node) = ctx.nodes().parent_node(parent.id()) else { return None; }; - dbg!(grandparent); - // Expand the search to include computed member expressions like `Object.prototype['p']` - if let AstKind::MemberExpression(expr) = grandparent.kind() { + dbg!(grandparent_node); + if let AstKind::ChainExpression(chain_expr) = grandparent_node.kind() { + prototype_node = Some(grandparent_node); + if let Some(grandparent_parent) = ctx.nodes().parent_node(grandparent_node.id()) { + dbg!(grandparent_parent); + prototype_node = Some(grandparent_parent); + }; + if let ChainElement::ComputedMemberExpression(computed) = &chain_expr.expression { + if computed + .object + .as_member_expression() + .is_some_and(|object| object.content_eq(prop_access_expr)) + { + prototype_node = Some(grandparent_node); + } + } + } + // Expand the search to include computed member expressions like `Object['prototype']` + else if let AstKind::MemberExpression(expr) = grandparent_node.kind() { if let MemberExpression::ComputedMemberExpression(computed) = expr { if computed .object .as_member_expression() .is_some_and(|object| object.content_eq(prop_access_expr)) { - prototype_node = Some(grandparent); + prototype_node = Some(grandparent_node); } } } @@ -255,6 +283,7 @@ fn test() { ("Object.prototype.p = 0; Object.prototype.q = 0", None), ("function foo() { Object.prototype.p = 0 }", None), ("(Object?.prototype).p = 0", None), // { "ecmaVersion": 2020 }, + ("(Object?.['prototype'])['p'] = 0", None), ("Object.defineProperty(Object?.prototype, 'p', { value: 0 })", None), // { "ecmaVersion": 2020 }, ("Object?.defineProperty(Object.prototype, 'p', { value: 0 })", None), // { "ecmaVersion": 2020 }, ("Object?.['defineProperty'](Object?.['prototype'], 'p', {value: 0})", None), From 7ad6878edd911d376630761d4f07c127f0d36e67 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Wed, 18 Sep 2024 14:48:53 -0400 Subject: [PATCH 06/16] Add chained expression support for defineProperty --- .../src/rules/eslint/no_extend_native.rs | 53 +++--- .../src/snapshots/no_extend_native.snap | 158 ++++++++++++++++++ 2 files changed, 190 insertions(+), 21 deletions(-) create mode 100644 crates/oxc_linter/src/snapshots/no_extend_native.snap diff --git a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs index 0811f6c61eb89..f7da5407c9c33 100644 --- a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs +++ b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs @@ -3,7 +3,7 @@ use oxc_ast::{ast::MemberExpression, AstKind}; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; use oxc_span::cmp::ContentEq; -use oxc_span::{CompactStr, GetSpan, Span}; +use oxc_span::{CompactStr, GetSpan}; use crate::{context::LintContext, rule::Rule, AstNode}; @@ -87,7 +87,7 @@ impl Rule for NoExtendNative { dbg!(prop_assign); ctx.diagnostic( OxcDiagnostic::error(format!( - "{} prototype is read only, properties should not be added.", + "{} prototype is read-only, properties should not be added.", name )) .with_label(prop_assign.span()), @@ -99,7 +99,7 @@ impl Rule for NoExtendNative { { ctx.diagnostic( OxcDiagnostic::error(format!( - "{} prototype is read only, properties should not be added.", + "{} prototype is read-only, properties should not be added.", name )) .with_label(define_property_call.span()), @@ -114,33 +114,41 @@ fn get_define_property_call<'a>( ctx: &'a LintContext, node: &AstNode<'a>, ) -> Option<&'a AstNode<'a>> { - let Some(parent) = ctx.nodes().parent_node(node.id()) else { - return None; - }; - let AstKind::Argument(_) = parent.kind() else { - return None; - }; - let Some(grandparent) = ctx.nodes().parent_node(parent.id()) else { + dbg!("get_define_property_call1", node); + let Some(mut ancestor) = ctx.nodes().parent_node(node.id()) else { return None; }; - let AstKind::CallExpression(call_expr) = grandparent.kind() else { - return None; - }; - if !is_define_property_call(call_expr) { - return None; + loop { + dbg!(ancestor); + if let AstKind::CallExpression(call_expr) = ancestor.kind() { + if !is_define_property_call(call_expr) { + return None; + } + return Some(ancestor); + } else if let AstKind::ChainExpression(_) | AstKind::Argument(_) = ancestor.kind() { + ancestor = ctx.nodes().parent_node(ancestor.id())?; + } else { + return None; + } } - Some(grandparent) } fn is_define_property_call(call_expr: &CallExpression) -> bool { - match call_expr.callee.as_member_expression() { + dbg!(call_expr.callee.without_parentheses()); + let callee = call_expr.callee.without_parentheses(); + + let member_expression = if let Expression::ChainExpression(chain_expr) = callee { + dbg!(chain_expr.expression.as_member_expression()) + } else { + callee.as_member_expression() + }; + match member_expression { Some(me) => { let prop_name = me.static_property_name(); me.object() .get_identifier_reference() - .map_or(false, |ident_ref| ident_ref.name == "Object") + .is_some_and(|ident_ref| ident_ref.name == "Object") && (prop_name == Some("defineProperty") || prop_name == Some("defineProperties")) - && !me.optional() } _ => false, } @@ -156,9 +164,12 @@ fn get_property_assignment<'a>( let mut parent = ctx.nodes().parent_node(node.id())?; loop { dbg!(parent); - if let AstKind::AssignmentExpression(_) | AstKind::SimpleAssignmentTarget(_) = parent.kind() - { + if let AstKind::AssignmentExpression(_) = parent.kind() { return Some(parent); + } else if let AstKind::AssignmentTarget(_) | AstKind::SimpleAssignmentTarget(_) = + parent.kind() + { + parent = ctx.nodes().parent_node(parent.id())?; } else if let AstKind::MemberExpression(member_expr) = parent.kind() { if let MemberExpression::ComputedMemberExpression(computed) = member_expr { if let AstKind::MemberExpression(node_expr) = node.kind() { diff --git a/crates/oxc_linter/src/snapshots/no_extend_native.snap b/crates/oxc_linter/src/snapshots/no_extend_native.snap new file mode 100644 index 0000000000000..d49cdb0c648b4 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_extend_native.snap @@ -0,0 +1,158 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint(no-extend-native): Object prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ Object.prototype.p = 0 + · ────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): BigInt prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ BigInt.prototype.p = 0 + · ────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): WeakRef prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ WeakRef.prototype.p = 0 + · ─────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): FinalizationRegistry prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ FinalizationRegistry.prototype.p = 0 + · ──────────────────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): AggregateError prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ AggregateError.prototype.p = 0 + · ────────────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Function prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ Function.prototype['p'] = 0 + · ─────────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): String prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ String['prototype'].p = 0 + · ───────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Number prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ Number['prototype']['p'] = 0 + · ──────────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Array prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ Object.defineProperty(Array.prototype, 'p', {value: 0}) + · ─────────────────────────────────────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Array prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ Object['defineProperty'](Array.prototype, 'p', {value: 0}) + · ────────────────────────────────────────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Array prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ Object['defineProperty'](Array['prototype'], 'p', {value: 0}) + · ───────────────────────────────────────────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Array prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ Object.defineProperties(Array.prototype, {p: {value: 0}}) + · ───────────────────────────────────────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Array prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ Object.defineProperties(Array.prototype, {p: {value: 0}, q: {value: 0}}) + · ──────────────────────────────────────────────────────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Number prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ Number['prototype']['p'] = 0 + · ──────────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Object prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ Object.prototype.p = 0; Object.prototype.q = 0 + · ────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Object prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:25] + 1 │ Object.prototype.p = 0; Object.prototype.q = 0 + · ────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Object prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:18] + 1 │ function foo() { Object.prototype.p = 0 } + · ────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Object prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ (Object?.prototype).p = 0 + · ───────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Object prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ (Object?.['prototype'])['p'] = 0 + · ──────────────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Object prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ Object.defineProperty(Object?.prototype, 'p', { value: 0 }) + · ─────────────────────────────────────────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Object prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ Object?.defineProperty(Object.prototype, 'p', { value: 0 }) + · ─────────────────────────────────────────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Object prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ Object?.['defineProperty'](Object?.['prototype'], 'p', {value: 0}) + · ────────────────────────────────────────────────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Object prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ (Object?.defineProperty)(Object.prototype, 'p', { value: 0 }) + · ───────────────────────────────────────────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Array prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ Array.prototype.p &&= 0 + · ─────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Array prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ Array.prototype.p ||= 0 + · ─────────────────────── + ╰──── + + ⚠ eslint(no-extend-native): Array prototype is read-only, properties should not be added. + ╭─[no_extend_native.tsx:1:1] + 1 │ Array.prototype.p ??= 0 + · ─────────────────────── + ╰──── From abab4c7fb02a44eb2dc7c007f6a074bab08bd922 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Wed, 18 Sep 2024 14:49:28 -0400 Subject: [PATCH 07/16] Remove dbg! --- .../src/rules/eslint/no_extend_native.rs | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs index f7da5407c9c33..a547b75871b8b 100644 --- a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs +++ b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs @@ -75,16 +75,12 @@ impl Rule for NoExtendNative { continue; } let node = ctx.nodes().get_node(reference.node_id()); - dbg!(name); - dbg!(node); // If this is not `*.prototype` access, skip it. let Some(prop_access) = get_prototype_property_accessed(ctx, node) else { continue; }; - dbg!(prop_access); // Check if being used like `String.prototype.xyz = 0` if let Some(prop_assign) = get_property_assignment(ctx, prop_access) { - dbg!(prop_assign); ctx.diagnostic( OxcDiagnostic::error(format!( "{} prototype is read-only, properties should not be added.", @@ -114,12 +110,10 @@ fn get_define_property_call<'a>( ctx: &'a LintContext, node: &AstNode<'a>, ) -> Option<&'a AstNode<'a>> { - dbg!("get_define_property_call1", node); let Some(mut ancestor) = ctx.nodes().parent_node(node.id()) else { return None; }; loop { - dbg!(ancestor); if let AstKind::CallExpression(call_expr) = ancestor.kind() { if !is_define_property_call(call_expr) { return None; @@ -134,11 +128,10 @@ fn get_define_property_call<'a>( } fn is_define_property_call(call_expr: &CallExpression) -> bool { - dbg!(call_expr.callee.without_parentheses()); let callee = call_expr.callee.without_parentheses(); let member_expression = if let Expression::ChainExpression(chain_expr) = callee { - dbg!(chain_expr.expression.as_member_expression()) + chain_expr.expression.as_member_expression() } else { callee.as_member_expression() }; @@ -160,10 +153,8 @@ fn get_property_assignment<'a>( ctx: &'a LintContext, node: &AstNode<'a>, ) -> Option<&'a AstNode<'a>> { - dbg!(node); let mut parent = ctx.nodes().parent_node(node.id())?; loop { - dbg!(parent); if let AstKind::AssignmentExpression(_) = parent.kind() { return Some(parent); } else if let AstKind::AssignmentTarget(_) | AstKind::SimpleAssignmentTarget(_) = @@ -204,7 +195,6 @@ fn get_prototype_property_accessed<'a>( let Some(parent) = ctx.nodes().parent_node(node.id()) else { return None; }; - dbg!(parent); let mut prototype_node = Some(parent); let AstKind::MemberExpression(prop_access_expr) = parent.kind() else { return None; @@ -218,11 +208,9 @@ fn get_prototype_property_accessed<'a>( let Some(grandparent_node) = ctx.nodes().parent_node(parent.id()) else { return None; }; - dbg!(grandparent_node); if let AstKind::ChainExpression(chain_expr) = grandparent_node.kind() { prototype_node = Some(grandparent_node); if let Some(grandparent_parent) = ctx.nodes().parent_node(grandparent_node.id()) { - dbg!(grandparent_parent); prototype_node = Some(grandparent_parent); }; if let ChainElement::ComputedMemberExpression(computed) = &chain_expr.expression { From f05419a0d2f3a6d663b36deb3592c669571b4b66 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Wed, 18 Sep 2024 15:05:33 -0400 Subject: [PATCH 08/16] Add docs and remove some duplication --- .../src/rules/eslint/no_extend_native.rs | 81 ++++++++++++++----- 1 file changed, 59 insertions(+), 22 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs index a547b75871b8b..d0455535be258 100644 --- a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs +++ b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs @@ -16,20 +16,42 @@ pub struct NoExtendNative { declare_oxc_lint!( /// ### What it does /// + /// Prevents extending native global objects such as `Object`, `String`, or `Array` with new + /// properties. /// /// ### Why is this bad? /// + /// Extending native objects can cause unexpected behavior and conflicts with other code. + /// + /// For example: + /// ```js + /// // Adding a new property, which might seem okay + /// Object.prototype.extra = 55; + /// + /// // Defining a user object + /// const users = { + /// "1": "user1", + /// "2": "user2", + /// }; + /// + /// for (const id in users) { + /// // This will print "extra" as well as "1" and "2": + /// console.log(id); + /// } + /// ``` /// /// ### Examples /// /// Examples of **incorrect** code for this rule: /// ```js - /// FIXME: Tests will fail if examples are missing or syntactically incorrect. + /// Object.prototype.p = 0 + /// Object.defineProperty(Array.prototype, 'p', {value: 0}) /// ``` /// /// Examples of **correct** code for this rule: /// ```js - /// FIXME: Tests will fail if examples are missing or syntactically incorrect. + /// x.prototype.p = 0 + /// Object.defineProperty(x.prototype, 'p', {value: 0}) /// ``` NoExtendNative, suspicious, @@ -106,6 +128,8 @@ impl Rule for NoExtendNative { } } +/// If this usage of `*.prototype` is a `Object.defineProperty` or `Object.defineProperties` call, +/// then this function returns the `CallExpression` node. fn get_define_property_call<'a>( ctx: &'a LintContext, node: &AstNode<'a>, @@ -127,6 +151,7 @@ fn get_define_property_call<'a>( } } +/// Checks if a given `CallExpression` is a call to `Object.defineProperty` or `Object.defineProperties`. fn is_define_property_call(call_expr: &CallExpression) -> bool { let callee = call_expr.callee.without_parentheses(); @@ -208,34 +233,45 @@ fn get_prototype_property_accessed<'a>( let Some(grandparent_node) = ctx.nodes().parent_node(parent.id()) else { return None; }; - if let AstKind::ChainExpression(chain_expr) = grandparent_node.kind() { + + if let AstKind::ChainExpression(_) = grandparent_node.kind() { prototype_node = Some(grandparent_node); if let Some(grandparent_parent) = ctx.nodes().parent_node(grandparent_node.id()) { prototype_node = Some(grandparent_parent); - }; - if let ChainElement::ComputedMemberExpression(computed) = &chain_expr.expression { - if computed - .object - .as_member_expression() - .is_some_and(|object| object.content_eq(prop_access_expr)) - { - prototype_node = Some(grandparent_node); - } } } - // Expand the search to include computed member expressions like `Object['prototype']` - else if let AstKind::MemberExpression(expr) = grandparent_node.kind() { - if let MemberExpression::ComputedMemberExpression(computed) = expr { - if computed - .object - .as_member_expression() - .is_some_and(|object| object.content_eq(prop_access_expr)) - { - prototype_node = Some(grandparent_node); + + if is_computed_member_expression_matching(grandparent_node, prop_access_expr) { + prototype_node = Some(grandparent_node); + } + + prototype_node +} + +fn is_computed_member_expression_matching( + node: &AstNode, + prop_access_expr: &MemberExpression, +) -> bool { + match node.kind() { + AstKind::ChainExpression(chain_expr) => { + if let ChainElement::ComputedMemberExpression(computed) = &chain_expr.expression { + return computed + .object + .as_member_expression() + .is_some_and(|object| object.content_eq(prop_access_expr)); + } + } + AstKind::MemberExpression(expr) => { + if let MemberExpression::ComputedMemberExpression(computed) = expr { + return computed + .object + .as_member_expression() + .is_some_and(|object| object.content_eq(prop_access_expr)); } } + _ => {} } - prototype_node + false } #[test] @@ -249,6 +285,7 @@ fn test() { ("Object.toString.bind = 0", None), ("Object['toString'].bind = 0", None), ("Object.defineProperty(x, 'p', {value: 0})", None), + ("Object.defineProperty(x.prototype, 'p', {value: 0})", None), ("Object.defineProperties(x, {p: {value: 0}})", None), ("global.Object.prototype.toString = 0", None), ("this.Object.prototype.toString = 0", None), From 815a5488807efb1abaa37ac066c3050e61a7c32c Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Wed, 18 Sep 2024 15:33:38 -0400 Subject: [PATCH 09/16] Simplify config --- crates/oxc_linter/src/rules/eslint/no_extend_native.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs index d0455535be258..337526f5106b9 100644 --- a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs +++ b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs @@ -67,10 +67,9 @@ impl Rule for NoExtendNative { .and_then(serde_json::Value::as_array) .unwrap_or(&vec![]) .iter() - .map(serde_json::Value::as_str) - .filter(Option::is_some) - .map(|x| x.unwrap().into()) - .collect::>(), + .filter_map(serde_json::Value::as_str) + .map(CompactStr::from) + .collect(), } } From 5e0c7a1feb90e7bc490d17146889eb48ce95bc2b Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Wed, 18 Sep 2024 16:18:29 -0400 Subject: [PATCH 10/16] perf: put exceptions vec in box --- .../src/rules/eslint/no_extend_native.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs index 337526f5106b9..0149535a51608 100644 --- a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs +++ b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs @@ -10,7 +10,7 @@ use crate::{context::LintContext, rule::Rule, AstNode}; #[derive(Debug, Default, Clone)] pub struct NoExtendNative { /// A list of objects which are allowed to be exceptions to the rule. - exceptions: Vec, + exceptions: Box>, } declare_oxc_lint!( @@ -62,14 +62,15 @@ impl Rule for NoExtendNative { let obj = value.get(0); Self { - exceptions: obj - .and_then(|v| v.get("exceptions")) - .and_then(serde_json::Value::as_array) - .unwrap_or(&vec![]) - .iter() - .filter_map(serde_json::Value::as_str) - .map(CompactStr::from) - .collect(), + exceptions: Box::new( + obj.and_then(|v| v.get("exceptions")) + .and_then(serde_json::Value::as_array) + .unwrap_or(&vec![]) + .iter() + .filter_map(serde_json::Value::as_str) + .map(CompactStr::from) + .collect(), + ), } } From 387772df008c28889366e9375611d2d9a53aa852 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Wed, 18 Sep 2024 16:46:49 -0400 Subject: [PATCH 11/16] Fix typo --- crates/oxc_linter/src/rules/eslint/no_extend_native.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs index 0149535a51608..1df459eabd57e 100644 --- a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs +++ b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs @@ -89,7 +89,7 @@ impl Rule for NoExtendNative { if self.exceptions.contains(&compact_name) { continue; } - // If the first letter is capital, like `Object`, we will assume it is a native obejct + // If the first letter is capital, like `Object`, we will assume it is a native object let Some(first_char) = name.chars().next() else { continue; }; From 51f97f41030ffa8e4e0006c6f15676af83b3125b Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Wed, 18 Sep 2024 16:47:00 -0400 Subject: [PATCH 12/16] Fix format call --- crates/oxc_linter/src/rules/eslint/no_extend_native.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs index 1df459eabd57e..b6cd248327af3 100644 --- a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs +++ b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs @@ -105,8 +105,7 @@ impl Rule for NoExtendNative { if let Some(prop_assign) = get_property_assignment(ctx, prop_access) { ctx.diagnostic( OxcDiagnostic::error(format!( - "{} prototype is read-only, properties should not be added.", - name + "{name} prototype is read-only, properties should not be added." )) .with_label(prop_assign.span()), ); @@ -117,8 +116,7 @@ impl Rule for NoExtendNative { { ctx.diagnostic( OxcDiagnostic::error(format!( - "{} prototype is read-only, properties should not be added.", - name + "{name} prototype is read-only, properties should not be added." )) .with_label(define_property_call.span()), ); From afe36d4a3046c8941c8585bb204df8728710c26d Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Wed, 18 Sep 2024 16:52:09 -0400 Subject: [PATCH 13/16] Fix Box issue --- .../src/rules/eslint/no_extend_native.rs | 36 ++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs index b6cd248327af3..fc80412865595 100644 --- a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs +++ b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs @@ -8,9 +8,20 @@ use oxc_span::{CompactStr, GetSpan}; use crate::{context::LintContext, rule::Rule, AstNode}; #[derive(Debug, Default, Clone)] -pub struct NoExtendNative { +pub struct NoExtendNative(Box); + +#[derive(Debug, Default, Clone)] +pub struct NoExtendNativeConfig { /// A list of objects which are allowed to be exceptions to the rule. - exceptions: Box>, + exceptions: Vec, +} + +impl std::ops::Deref for NoExtendNative { + type Target = NoExtendNativeConfig; + + fn deref(&self) -> &Self::Target { + &self.0 + } } declare_oxc_lint!( @@ -61,17 +72,16 @@ impl Rule for NoExtendNative { fn from_configuration(value: serde_json::Value) -> Self { let obj = value.get(0); - Self { - exceptions: Box::new( - obj.and_then(|v| v.get("exceptions")) - .and_then(serde_json::Value::as_array) - .unwrap_or(&vec![]) - .iter() - .filter_map(serde_json::Value::as_str) - .map(CompactStr::from) - .collect(), - ), - } + Self(Box::new(NoExtendNativeConfig { + exceptions: obj + .and_then(|v| v.get("exceptions")) + .and_then(serde_json::Value::as_array) + .unwrap_or(&vec![]) + .iter() + .filter_map(serde_json::Value::as_str) + .map(CompactStr::from) + .collect(), + })) } fn run_once(&self, ctx: &LintContext) { From 57af32c9a81b8d2f9aa1f34c501b8777ac52e610 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Wed, 18 Sep 2024 16:58:37 -0400 Subject: [PATCH 14/16] Fix clippy lints --- .../src/rules/eslint/no_extend_native.rs | 28 ++++++------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs index fc80412865595..f214eb5f5814d 100644 --- a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs +++ b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs @@ -142,9 +142,7 @@ fn get_define_property_call<'a>( ctx: &'a LintContext, node: &AstNode<'a>, ) -> Option<&'a AstNode<'a>> { - let Some(mut ancestor) = ctx.nodes().parent_node(node.id()) else { - return None; - }; + let mut ancestor = ctx.nodes().parent_node(node.id())?; loop { if let AstKind::CallExpression(call_expr) = ancestor.kind() { if !is_define_property_call(call_expr) { @@ -225,22 +223,16 @@ fn get_prototype_property_accessed<'a>( let AstKind::IdentifierReference(_) = node.kind() else { return None; }; - let Some(parent) = ctx.nodes().parent_node(node.id()) else { - return None; - }; + let parent = ctx.nodes().parent_node(node.id())?; let mut prototype_node = Some(parent); let AstKind::MemberExpression(prop_access_expr) = parent.kind() else { return None; }; - let Some(prop_name) = prop_access_expr.static_property_name() else { - return None; - }; + let prop_name = prop_access_expr.static_property_name()?; if prop_name != "prototype" { return None; } - let Some(grandparent_node) = ctx.nodes().parent_node(parent.id()) else { - return None; - }; + let grandparent_node = ctx.nodes().parent_node(parent.id())?; if let AstKind::ChainExpression(_) = grandparent_node.kind() { prototype_node = Some(grandparent_node); @@ -269,13 +261,11 @@ fn is_computed_member_expression_matching( .is_some_and(|object| object.content_eq(prop_access_expr)); } } - AstKind::MemberExpression(expr) => { - if let MemberExpression::ComputedMemberExpression(computed) = expr { - return computed - .object - .as_member_expression() - .is_some_and(|object| object.content_eq(prop_access_expr)); - } + AstKind::MemberExpression(MemberExpression::ComputedMemberExpression(computed)) => { + return computed + .object + .as_member_expression() + .is_some_and(|object| object.content_eq(prop_access_expr)); } _ => {} } From ea19b08aa3550d365544109d01a026471652e7b3 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Wed, 18 Sep 2024 19:46:31 -0400 Subject: [PATCH 15/16] Use .iter_parents() --- .../src/rules/eslint/no_extend_native.rs | 54 ++++++++----------- 1 file changed, 22 insertions(+), 32 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs index f214eb5f5814d..c0ce9744b1aff 100644 --- a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs +++ b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs @@ -142,19 +142,14 @@ fn get_define_property_call<'a>( ctx: &'a LintContext, node: &AstNode<'a>, ) -> Option<&'a AstNode<'a>> { - let mut ancestor = ctx.nodes().parent_node(node.id())?; - loop { - if let AstKind::CallExpression(call_expr) = ancestor.kind() { - if !is_define_property_call(call_expr) { - return None; + for parent in ctx.nodes().iter_parents(node.id()).skip(1) { + if let AstKind::CallExpression(call_expr) = parent.kind() { + if is_define_property_call(call_expr) { + return Some(parent); } - return Some(ancestor); - } else if let AstKind::ChainExpression(_) | AstKind::Argument(_) = ancestor.kind() { - ancestor = ctx.nodes().parent_node(ancestor.id())?; - } else { - return None; } } + None } /// Checks if a given `CallExpression` is a call to `Object.defineProperty` or `Object.defineProperties`. @@ -184,34 +179,29 @@ fn get_property_assignment<'a>( ctx: &'a LintContext, node: &AstNode<'a>, ) -> Option<&'a AstNode<'a>> { - let mut parent = ctx.nodes().parent_node(node.id())?; - loop { - if let AstKind::AssignmentExpression(_) = parent.kind() { - return Some(parent); - } else if let AstKind::AssignmentTarget(_) | AstKind::SimpleAssignmentTarget(_) = - parent.kind() - { - parent = ctx.nodes().parent_node(parent.id())?; - } else if let AstKind::MemberExpression(member_expr) = parent.kind() { - if let MemberExpression::ComputedMemberExpression(computed) = member_expr { - if let AstKind::MemberExpression(node_expr) = node.kind() { - // Ignore computed member expressions like `obj[Object.prototype] = 0` (i.e., the - // given node is the `expression` of the computed member expression) - if computed - .expression - .as_member_expression() - .is_some_and(|expression| expression.content_eq(node_expr)) - { + for parent in ctx.nodes().iter_parents(node.id()).skip(1) { + match parent.kind() { + AstKind::AssignmentExpression(_) => return Some(parent), + AstKind::MemberExpression(member_expr) => { + if let MemberExpression::ComputedMemberExpression(computed) = member_expr { + if let AstKind::MemberExpression(node_expr) = node.kind() { + // Ignore computed member expressions like `obj[Object.prototype] = 0` (i.e., the + // given node is the `expression` of the computed member expression) + if computed + .expression + .as_member_expression() + .is_some_and(|expression| expression.content_eq(node_expr)) + { + return None; + } return None; } - return None; } } - parent = ctx.nodes().parent_node(parent.id())?; - } else { - return None; + _ => {} } } + None } /// Returns the ASTNode that represents a prototype property access, such as From 445edb5442171b87b567321df55cb5303f4f68a2 Mon Sep 17 00:00:00 2001 From: Cameron McHenry Date: Wed, 18 Sep 2024 19:50:42 -0400 Subject: [PATCH 16/16] Move binding into pattern --- .../src/rules/eslint/no_extend_native.rs | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs index c0ce9744b1aff..ba178600d8f38 100644 --- a/crates/oxc_linter/src/rules/eslint/no_extend_native.rs +++ b/crates/oxc_linter/src/rules/eslint/no_extend_native.rs @@ -182,20 +182,18 @@ fn get_property_assignment<'a>( for parent in ctx.nodes().iter_parents(node.id()).skip(1) { match parent.kind() { AstKind::AssignmentExpression(_) => return Some(parent), - AstKind::MemberExpression(member_expr) => { - if let MemberExpression::ComputedMemberExpression(computed) = member_expr { - if let AstKind::MemberExpression(node_expr) = node.kind() { - // Ignore computed member expressions like `obj[Object.prototype] = 0` (i.e., the - // given node is the `expression` of the computed member expression) - if computed - .expression - .as_member_expression() - .is_some_and(|expression| expression.content_eq(node_expr)) - { - return None; - } + AstKind::MemberExpression(MemberExpression::ComputedMemberExpression(computed)) => { + if let AstKind::MemberExpression(node_expr) = node.kind() { + // Ignore computed member expressions like `obj[Object.prototype] = 0` (i.e., the + // given node is the `expression` of the computed member expression) + if computed + .expression + .as_member_expression() + .is_some_and(|expression| expression.content_eq(node_expr)) + { return None; } + return None; } } _ => {}