From 846c15e564be64332e1ec2d7cdb4acde08b0aa07 Mon Sep 17 00:00:00 2001 From: Victorien ELVINGER Date: Thu, 9 Mar 2023 19:06:01 +0100 Subject: [PATCH] refactor(rome_js_analyze): noDelete (#4272) --- .../src/analyzers/performance/no_delete.rs | 235 +++++++----------- .../tests/specs/performance/noDelete.js | 12 - .../tests/specs/performance/noDelete.js.snap | 115 --------- .../specs/performance/noDelete/invalid.jsonc | 14 ++ .../performance/noDelete/invalid.jsonc.snap | 230 +++++++++++++++++ .../specs/performance/noDelete/valid.jsonc | 8 + .../performance/noDelete/valid.jsonc.snap | 31 +++ .../src/configuration/linter/rules.rs | 2 +- editors/vscode/configuration_schema.json | 2 +- npm/backend-jsonrpc/src/workspace.ts | 2 +- npm/rome/configuration_schema.json | 2 +- website/src/pages/lint/rules/index.mdx | 2 +- website/src/pages/lint/rules/noDelete.md | 48 +++- 13 files changed, 409 insertions(+), 294 deletions(-) delete mode 100644 crates/rome_js_analyze/tests/specs/performance/noDelete.js delete mode 100644 crates/rome_js_analyze/tests/specs/performance/noDelete.js.snap create mode 100644 crates/rome_js_analyze/tests/specs/performance/noDelete/invalid.jsonc create mode 100644 crates/rome_js_analyze/tests/specs/performance/noDelete/invalid.jsonc.snap create mode 100644 crates/rome_js_analyze/tests/specs/performance/noDelete/valid.jsonc create mode 100644 crates/rome_js_analyze/tests/specs/performance/noDelete/valid.jsonc.snap diff --git a/crates/rome_js_analyze/src/analyzers/performance/no_delete.rs b/crates/rome_js_analyze/src/analyzers/performance/no_delete.rs index 5a8cc8fc509..79099a4c967 100644 --- a/crates/rome_js_analyze/src/analyzers/performance/no_delete.rs +++ b/crates/rome_js_analyze/src/analyzers/performance/no_delete.rs @@ -3,24 +3,33 @@ use rome_console::markup; use rome_diagnostics::Applicability; use rome_js_factory::make; use rome_js_syntax::{ - AnyJsAssignment, AnyJsAssignmentPattern, AnyJsExpression, JsComputedMemberExpression, - JsComputedMemberExpressionFields, JsStaticMemberExpression, JsStaticMemberExpressionFields, - JsUnaryExpression, JsUnaryOperator, T, + AnyJsAssignment, AnyJsAssignmentPattern, AnyJsExpression, JsComputedMemberExpressionFields, + JsStaticMemberExpressionFields, JsUnaryExpression, JsUnaryOperator, T, }; use rome_rowan::{AstNode, BatchMutationExt}; use crate::JsRuleAction; declare_rule! { - /// Disallow the use of the `delete` operator + /// Disallow the use of the `delete` operator. + /// + /// The `delete` operator enables the removal of a property from an object. + /// + /// The `delete` operator should be avoided because it [can prevent some optimizations of _JavaScript_ engines](https://webkit.org/blog/10298/inline-caching-delete/). + /// Moreover, it can lead to unexpected results. + /// For instance, deleting an array element [does not change the length of the array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/delete#deleting_array_elements). + /// + /// The only legitimate use of `delete` is on an object that behaves like a _map_. + /// To allow this pattern, this rule does not report `delete` on computed properties that are not literal values. + /// Consider using [Map](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map) instead of an object. /// /// ## Examples /// /// ### Invalid /// /// ```js,expect_diagnostic - /// const arr = [['a','b','c'], [1, 2, 3]]; - /// delete arr[0][2]; + /// const arr = [1, 2, 3]; + /// delete arr[0]; /// ``` /// /// ```js,expect_diagnostic @@ -34,6 +43,19 @@ declare_rule! { /// const foo = new Set([1,2,3]); /// foo.delete(1); ///``` + /// + /// ```js + /// const map = Object.create(null); + /// const key = "key" + /// map[key] = "value" + /// delete map[key]; + ///``` + /// + /// ```js + /// let x = 5; + /// delete f(); // uncovered by this rule. + ///``` + /// pub(crate) NoDelete { version: "0.7.0", name: "noDelete", @@ -43,183 +65,98 @@ declare_rule! { impl Rule for NoDelete { type Query = Ast; - type State = MemberExpression; + type State = AnyJsExpression; type Signals = Option; type Options = (); - fn run(ctx: &RuleContext) -> Option { + fn run(ctx: &RuleContext) -> Self::Signals { let node = ctx.query(); - let op = node.operator().ok()?; if op != JsUnaryOperator::Delete { return None; } - let argument = node.argument().ok()?; - MemberExpression::try_from(argument).ok() + + let should_report = if let Some(computed) = argument.as_js_computed_member_expression() { + // `delete record[x]` is allowed, but if `x` is a literal value. + computed + .member() + .ok()? + .as_any_js_literal_expression() + .is_some() + } else { + // if `argument` is not a computed or static member, + // then `delete` has either no effect or an undefined behavior. + // This should be rejected by another rule. + argument.as_js_static_member_expression().is_some() + }; + should_report.then_some(argument) } - fn diagnostic(ctx: &RuleContext, _state: &Self::State) -> Option { + fn diagnostic(ctx: &RuleContext, _: &Self::State) -> Option { let node = ctx.query(); - - Some( - RuleDiagnostic::new(rule_category!(),node.range(), markup! { - "This is an unexpected use of the ""delete"" operator." - }) - .description("This is an unexpected use of the `delete` operator.\nReplace this expression with an `undefined` assignment") - ) + Some(RuleDiagnostic::new( + rule_category!(), + node.range(), + markup! { + "Avoid the ""delete"" operator which can impact performance." + }, + )) } - fn action(ctx: &RuleContext, state: &Self::State) -> Option { + fn action(ctx: &RuleContext, argument: &Self::State) -> Option { let node = ctx.query(); - + let assignment = to_assignment(argument).ok()?; let mut mutation = ctx.root().begin(); mutation.replace_node( AnyJsExpression::from(node.clone()), AnyJsExpression::from(make::js_assignment_expression( - state.clone().try_into().ok()?, + AnyJsAssignmentPattern::AnyJsAssignment(assignment), make::token_decorated_with_space(T![=]), AnyJsExpression::from(make::js_identifier_expression( make::js_reference_identifier(make::ident("undefined")), )), )), ); - Some(JsRuleAction { category: ActionCategory::QuickFix, applicability: Applicability::MaybeIncorrect, - message: markup! { "Replace with undefined assignment" }.to_owned(), + message: markup! { "Use an ""undefined"" assignment instead." } + .to_owned(), mutation, }) } } -/// Wrapper type for member expression nodes that can be safely converted to assignment patterns -#[derive(Clone)] -pub(crate) enum MemberExpression { - JsStaticMemberExpression(JsStaticMemberExpression), - JsComputedMemberExpression(JsComputedMemberExpression), -} - -impl TryFrom for MemberExpression { - type Error = (); - - fn try_from(value: AnyJsExpression) -> Result { - match value { - AnyJsExpression::JsStaticMemberExpression(expr) => { - let JsStaticMemberExpressionFields { - object, - operator_token, - member, - } = expr.as_fields(); - - match object { - Ok(expr) if has_optional(&expr)? => return Err(()), - Err(_) => return Err(()), - _ => {} - } - - match operator_token { - Ok(token) if token.kind() == T![?.] => return Err(()), - Err(_) => return Err(()), - _ => {} - } - - if member.is_err() { - return Err(()); - } - - Ok(Self::JsStaticMemberExpression(expr)) - } - AnyJsExpression::JsComputedMemberExpression(expr) => { - let JsComputedMemberExpressionFields { - object, - optional_chain_token, - l_brack_token, - member, - r_brack_token, - } = expr.as_fields(); - - match object { - Ok(expr) if has_optional(&expr)? => return Err(()), - Err(_) => return Err(()), - _ => {} - } - - if optional_chain_token.is_some() - || l_brack_token.is_err() - || member.is_err() - || r_brack_token.is_err() - { - return Err(()); - } - - Ok(Self::JsComputedMemberExpression(expr)) - } - _ => Err(()), +fn to_assignment(expr: &AnyJsExpression) -> Result { + match expr { + AnyJsExpression::JsStaticMemberExpression(expr) if !expr.is_optional_chain() => { + let JsStaticMemberExpressionFields { + object, + operator_token, + member, + } = expr.as_fields(); + Ok(AnyJsAssignment::from(make::js_static_member_assignment( + object.map_err(drop)?, + operator_token.map_err(drop)?, + member.map_err(drop)?, + ))) } - } -} - -impl TryFrom for AnyJsAssignmentPattern { - type Error = (); - - fn try_from(expr: MemberExpression) -> Result { - match expr { - MemberExpression::JsStaticMemberExpression(expr) => { - let JsStaticMemberExpressionFields { - object, - operator_token, - member, - } = expr.as_fields(); - - Ok(Self::AnyJsAssignment(AnyJsAssignment::from( - make::js_static_member_assignment( - object.map_err(drop)?, - operator_token.map_err(drop)?, - member.map_err(drop)?, - ), - ))) - } - MemberExpression::JsComputedMemberExpression(expr) => { - let JsComputedMemberExpressionFields { - object, - optional_chain_token: _, - l_brack_token, - member, - r_brack_token, - } = expr.as_fields(); - - Ok(Self::AnyJsAssignment(AnyJsAssignment::from( - make::js_computed_member_assignment( - object.map_err(drop)?, - l_brack_token.map_err(drop)?, - member.map_err(drop)?, - r_brack_token.map_err(drop)?, - ), - ))) - } + AnyJsExpression::JsComputedMemberExpression(expr) if !expr.is_optional_chain() => { + let JsComputedMemberExpressionFields { + object, + optional_chain_token: _, + l_brack_token, + member, + r_brack_token, + } = expr.as_fields(); + Ok(AnyJsAssignment::from(make::js_computed_member_assignment( + object.map_err(drop)?, + l_brack_token.map_err(drop)?, + member.map_err(drop)?, + r_brack_token.map_err(drop)?, + ))) } - } -} - -fn has_optional(expr: &AnyJsExpression) -> Result { - match expr { - AnyJsExpression::JsStaticMemberExpression(expr) => match expr.operator_token() { - Ok(token) if token.kind() == T![?.] => Ok(true), - Err(_) => Err(()), - _ => match expr.object() { - Ok(expr) => has_optional(&expr), - Err(_) => Err(()), - }, - }, - AnyJsExpression::JsComputedMemberExpression(expr) => match expr.optional_chain_token() { - Some(_) => Ok(true), - None => match expr.object() { - Ok(expr) => has_optional(&expr), - Err(_) => Err(()), - }, - }, - _ => Ok(false), + _ => Err(()), } } diff --git a/crates/rome_js_analyze/tests/specs/performance/noDelete.js b/crates/rome_js_analyze/tests/specs/performance/noDelete.js deleted file mode 100644 index b45eebfb0dc..00000000000 --- a/crates/rome_js_analyze/tests/specs/performance/noDelete.js +++ /dev/null @@ -1,12 +0,0 @@ -delete a.b; -delete a?.b; -delete a["b"]; -delete a?.["b"]; -delete a.b.c; -delete a.b?.c; -delete a.b["c"]; -delete a.b?.["c"]; -delete a?.b.c; -delete a?.b?.c; -delete a?.b["c"]; -delete a?.b?.["c"]; diff --git a/crates/rome_js_analyze/tests/specs/performance/noDelete.js.snap b/crates/rome_js_analyze/tests/specs/performance/noDelete.js.snap deleted file mode 100644 index 08eedd8dfe5..00000000000 --- a/crates/rome_js_analyze/tests/specs/performance/noDelete.js.snap +++ /dev/null @@ -1,115 +0,0 @@ ---- -source: crates/rome_js_analyze/tests/spec_tests.rs -expression: noDelete.js ---- -# Input -```js -delete a.b; -delete a?.b; -delete a["b"]; -delete a?.["b"]; -delete a.b.c; -delete a.b?.c; -delete a.b["c"]; -delete a.b?.["c"]; -delete a?.b.c; -delete a?.b?.c; -delete a?.b["c"]; -delete a?.b?.["c"]; - -``` - -# Diagnostics -``` -noDelete.js:1:1 lint/performance/noDelete FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! This is an unexpected use of the delete operator. - - > 1 │ delete a.b; - │ ^^^^^^^^^^ - 2 │ delete a?.b; - 3 │ delete a["b"]; - - i Suggested fix: Replace with undefined assignment - - 1 │ - delete·a.b; - 1 │ + a.b·=·undefined; - 2 2 │ delete a?.b; - 3 3 │ delete a["b"]; - - -``` - -``` -noDelete.js:3:1 lint/performance/noDelete FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! This is an unexpected use of the delete operator. - - 1 │ delete a.b; - 2 │ delete a?.b; - > 3 │ delete a["b"]; - │ ^^^^^^^^^^^^^ - 4 │ delete a?.["b"]; - 5 │ delete a.b.c; - - i Suggested fix: Replace with undefined assignment - - 1 1 │ delete a.b; - 2 2 │ delete a?.b; - 3 │ - delete·a["b"]; - 3 │ + a["b"]·=·undefined; - 4 4 │ delete a?.["b"]; - 5 5 │ delete a.b.c; - - -``` - -``` -noDelete.js:5:1 lint/performance/noDelete FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! This is an unexpected use of the delete operator. - - 3 │ delete a["b"]; - 4 │ delete a?.["b"]; - > 5 │ delete a.b.c; - │ ^^^^^^^^^^^^ - 6 │ delete a.b?.c; - 7 │ delete a.b["c"]; - - i Suggested fix: Replace with undefined assignment - - 3 3 │ delete a["b"]; - 4 4 │ delete a?.["b"]; - 5 │ - delete·a.b.c; - 5 │ + a.b.c·=·undefined; - 6 6 │ delete a.b?.c; - 7 7 │ delete a.b["c"]; - - -``` - -``` -noDelete.js:7:1 lint/performance/noDelete FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - ! This is an unexpected use of the delete operator. - - 5 │ delete a.b.c; - 6 │ delete a.b?.c; - > 7 │ delete a.b["c"]; - │ ^^^^^^^^^^^^^^^ - 8 │ delete a.b?.["c"]; - 9 │ delete a?.b.c; - - i Suggested fix: Replace with undefined assignment - - 5 5 │ delete a.b.c; - 6 6 │ delete a.b?.c; - 7 │ - delete·a.b["c"]; - 7 │ + a.b["c"]·=·undefined; - 8 8 │ delete a.b?.["c"]; - 9 9 │ delete a?.b.c; - - -``` - - diff --git a/crates/rome_js_analyze/tests/specs/performance/noDelete/invalid.jsonc b/crates/rome_js_analyze/tests/specs/performance/noDelete/invalid.jsonc new file mode 100644 index 00000000000..4514358bf25 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/performance/noDelete/invalid.jsonc @@ -0,0 +1,14 @@ +[ + "delete a.b;", + "delete a?.b;", + "delete a['b'];", + "delete a?.['b'];", + "delete a.b.c;", + "delete a.b?.c;", + "delete a.b['c'];", + "delete a.b?.['c'];", + "delete a?.b.c;", + "delete a?.b?.c;", + "delete a?.b['c'];", + "delete a?.b?.['c'];" +] diff --git a/crates/rome_js_analyze/tests/specs/performance/noDelete/invalid.jsonc.snap b/crates/rome_js_analyze/tests/specs/performance/noDelete/invalid.jsonc.snap new file mode 100644 index 00000000000..182d28327cc --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/performance/noDelete/invalid.jsonc.snap @@ -0,0 +1,230 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +assertion_line: 91 +expression: invalid.jsonc +--- +# Input +```js +delete a.b; +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/performance/noDelete FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Avoid the delete operator which can impact performance. + + > 1 │ delete a.b; + │ ^^^^^^^^^^ + + i Suggested fix: Use an undefined assignment instead. + + - delete·a.b; + + a.b·=·undefined; + + +``` + +# Input +```js +delete a?.b; +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/performance/noDelete ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Avoid the delete operator which can impact performance. + + > 1 │ delete a?.b; + │ ^^^^^^^^^^^ + + +``` + +# Input +```js +delete a['b']; +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/performance/noDelete FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Avoid the delete operator which can impact performance. + + > 1 │ delete a['b']; + │ ^^^^^^^^^^^^^ + + i Suggested fix: Use an undefined assignment instead. + + - delete·a['b']; + + a['b']·=·undefined; + + +``` + +# Input +```js +delete a?.['b']; +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/performance/noDelete ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Avoid the delete operator which can impact performance. + + > 1 │ delete a?.['b']; + │ ^^^^^^^^^^^^^^^ + + +``` + +# Input +```js +delete a.b.c; +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/performance/noDelete FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Avoid the delete operator which can impact performance. + + > 1 │ delete a.b.c; + │ ^^^^^^^^^^^^ + + i Suggested fix: Use an undefined assignment instead. + + - delete·a.b.c; + + a.b.c·=·undefined; + + +``` + +# Input +```js +delete a.b?.c; +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/performance/noDelete ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Avoid the delete operator which can impact performance. + + > 1 │ delete a.b?.c; + │ ^^^^^^^^^^^^^ + + +``` + +# Input +```js +delete a.b['c']; +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/performance/noDelete FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Avoid the delete operator which can impact performance. + + > 1 │ delete a.b['c']; + │ ^^^^^^^^^^^^^^^ + + i Suggested fix: Use an undefined assignment instead. + + - delete·a.b['c']; + + a.b['c']·=·undefined; + + +``` + +# Input +```js +delete a.b?.['c']; +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/performance/noDelete ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Avoid the delete operator which can impact performance. + + > 1 │ delete a.b?.['c']; + │ ^^^^^^^^^^^^^^^^^ + + +``` + +# Input +```js +delete a?.b.c; +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/performance/noDelete ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Avoid the delete operator which can impact performance. + + > 1 │ delete a?.b.c; + │ ^^^^^^^^^^^^^ + + +``` + +# Input +```js +delete a?.b?.c; +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/performance/noDelete ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Avoid the delete operator which can impact performance. + + > 1 │ delete a?.b?.c; + │ ^^^^^^^^^^^^^^ + + +``` + +# Input +```js +delete a?.b['c']; +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/performance/noDelete ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Avoid the delete operator which can impact performance. + + > 1 │ delete a?.b['c']; + │ ^^^^^^^^^^^^^^^^ + + +``` + +# Input +```js +delete a?.b?.['c']; +``` + +# Diagnostics +``` +invalid.jsonc:1:1 lint/performance/noDelete ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Avoid the delete operator which can impact performance. + + > 1 │ delete a?.b?.['c']; + │ ^^^^^^^^^^^^^^^^^^ + + +``` + + diff --git a/crates/rome_js_analyze/tests/specs/performance/noDelete/valid.jsonc b/crates/rome_js_analyze/tests/specs/performance/noDelete/valid.jsonc new file mode 100644 index 00000000000..1451aa79af0 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/performance/noDelete/valid.jsonc @@ -0,0 +1,8 @@ +[ + "delete a[x];", + "delete a['k' + 'ey'];", + "delete a.b[x];", + // uncovered by this rule + "delete x", + "delete f()" +] diff --git a/crates/rome_js_analyze/tests/specs/performance/noDelete/valid.jsonc.snap b/crates/rome_js_analyze/tests/specs/performance/noDelete/valid.jsonc.snap new file mode 100644 index 00000000000..b9b2234ba1e --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/performance/noDelete/valid.jsonc.snap @@ -0,0 +1,31 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +assertion_line: 91 +expression: valid.jsonc +--- +# Input +```js +delete a[x]; +``` + +# Input +```js +delete a['k' + 'ey']; +``` + +# Input +```js +delete a.b[x]; +``` + +# Input +```js +delete x +``` + +# Input +```js +delete f() +``` + + diff --git a/crates/rome_service/src/configuration/linter/rules.rs b/crates/rome_service/src/configuration/linter/rules.rs index dd702481eb9..9b03e46b28e 100644 --- a/crates/rome_service/src/configuration/linter/rules.rs +++ b/crates/rome_service/src/configuration/linter/rules.rs @@ -2016,7 +2016,7 @@ pub struct Performance { #[doc = r" It enables ALL rules for this group."] #[serde(skip_serializing_if = "Option::is_none")] pub all: Option, - #[doc = "Disallow the use of the delete operator"] + #[doc = "Disallow the use of the delete operator."] #[serde(skip_serializing_if = "Option::is_none")] pub no_delete: Option, } diff --git a/editors/vscode/configuration_schema.json b/editors/vscode/configuration_schema.json index c061a5bf11f..5b5af4c866c 100644 --- a/editors/vscode/configuration_schema.json +++ b/editors/vscode/configuration_schema.json @@ -816,7 +816,7 @@ "type": ["boolean", "null"] }, "noDelete": { - "description": "Disallow the use of the delete operator", + "description": "Disallow the use of the delete operator.", "anyOf": [ { "$ref": "#/definitions/RuleConfiguration" }, { "type": "null" } diff --git a/npm/backend-jsonrpc/src/workspace.ts b/npm/backend-jsonrpc/src/workspace.ts index beec47f6adf..e9c530114d0 100644 --- a/npm/backend-jsonrpc/src/workspace.ts +++ b/npm/backend-jsonrpc/src/workspace.ts @@ -542,7 +542,7 @@ export interface Performance { */ all?: boolean; /** - * Disallow the use of the delete operator + * Disallow the use of the delete operator. */ noDelete?: RuleConfiguration; /** diff --git a/npm/rome/configuration_schema.json b/npm/rome/configuration_schema.json index c061a5bf11f..5b5af4c866c 100644 --- a/npm/rome/configuration_schema.json +++ b/npm/rome/configuration_schema.json @@ -816,7 +816,7 @@ "type": ["boolean", "null"] }, "noDelete": { - "description": "Disallow the use of the delete operator", + "description": "Disallow the use of the delete operator.", "anyOf": [ { "$ref": "#/definitions/RuleConfiguration" }, { "type": "null" } diff --git a/website/src/pages/lint/rules/index.mdx b/website/src/pages/lint/rules/index.mdx index dc632daf45e..053f7d5a8c5 100644 --- a/website/src/pages/lint/rules/index.mdx +++ b/website/src/pages/lint/rules/index.mdx @@ -290,7 +290,7 @@ Rules catching ways your code could be written to run faster, or generally be mo noDelete recommended -Disallow the use of the delete operator +Disallow the use of the delete operator. diff --git a/website/src/pages/lint/rules/noDelete.md b/website/src/pages/lint/rules/noDelete.md index 3b47c838752..6efe7ce12e2 100644 --- a/website/src/pages/lint/rules/noDelete.md +++ b/website/src/pages/lint/rules/noDelete.md @@ -7,31 +7,41 @@ parent: lint/rules/index > This rule is recommended by Rome. -Disallow the use of the `delete` operator +Disallow the use of the `delete` operator. + +The `delete` operator enables the removal of a property from an object. + +The `delete` operator should be avoided because it [can prevent some optimizations of _JavaScript_ engines](https://webkit.org/blog/10298/inline-caching-delete/). +Moreover, it can lead to unexpected results. +For instance, deleting an array element [does not change the length of the array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/delete#deleting_array_elements). + +The only legitimate use of `delete` is on an object that behaves like a _map_. +To allow this pattern, this rule does not report `delete` on computed properties that are not literal values. +Consider using [Map](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map) instead of an object. ## Examples ### Invalid ```jsx -const arr = [['a','b','c'], [1, 2, 3]]; -delete arr[0][2]; +const arr = [1, 2, 3]; +delete arr[0]; ``` 
performance/noDelete.js:2:1 lint/performance/noDelete  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  ✖ This is an unexpected use of the delete operator.
+  ✖ Avoid the delete operator which can impact performance.
   
-    1 │ const arr = [['a','b','c'], [1, 2, 3]];
-  > 2 │ delete arr[0][2];
-      │ ^^^^^^^^^^^^^^^^
+    1 │ const arr = [1, 2, 3];
+  > 2 │ delete arr[0];
+      │ ^^^^^^^^^^^^^
     3 │ 
   
-  ℹ Suggested fix: Replace with undefined assignment
+  ℹ Suggested fix: Use an undefined assignment instead.
   
-    1 1 │   const arr = [['a','b','c'], [1, 2, 3]];
-    2   │ - delete·arr[0][2];
-      2 │ + arr[0][2]·=·undefined;
+    1 1 │   const arr = [1, 2, 3];
+    2   │ - delete·arr[0];
+      2 │ + arr[0]·=·undefined;
     3 3 │
@@ -43,14 +53,14 @@ delete obj.a.b.c; 
performance/noDelete.js:2:1 lint/performance/noDelete  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  ✖ This is an unexpected use of the delete operator.
+  ✖ Avoid the delete operator which can impact performance.
   
     1 │ const obj = {a: {b: {c: 123}}};
   > 2 │ delete obj.a.b.c;
       │ ^^^^^^^^^^^^^^^^
     3 │ 
   
-  ℹ Suggested fix: Replace with undefined assignment
+  ℹ Suggested fix: Use an undefined assignment instead.
   
     1 1 │   const obj = {a: {b: {c: 123}}};
     2   │ - delete·obj.a.b.c;
@@ -66,6 +76,18 @@ const foo = new Set([1,2,3]);
 foo.delete(1);
 ```
 
+```jsx
+const map = Object.create(null);
+const key = "key"
+map[key] = "value"
+delete map[key];
+```
+
+```jsx
+let x = 5;
+delete f(); // uncovered by this rule.
+```
+
 ## Related links
 
 - [Disable a rule](/linter/#disable-a-lint-rule)