From 25305621d72d1bea40f197ecc418b031b7a210a9 Mon Sep 17 00:00:00 2001 From: Victorien Elvinger Date: Sat, 24 Jun 2023 23:12:09 +0200 Subject: [PATCH] feat(rome_js_analyze): add noGlobalIsFinite --- CHANGELOG.md | 6 +- .../src/categories.rs | 1 + .../src/semantic_analyzers/nursery.rs | 3 +- .../nursery/no_global_is_finite.rs | 161 +++++++++++++ .../nursery/no_global_is_nan.rs | 50 +++- .../specs/nursery/noGlobalIsFinite/invalid.js | 17 ++ .../nursery/noGlobalIsFinite/invalid.js.snap | 224 ++++++++++++++++++ .../specs/nursery/noGlobalIsFinite/valid.js | 14 ++ .../nursery/noGlobalIsFinite/valid.js.snap | 24 ++ .../nursery/noGlobalIsNan/invalid.js.snap | 53 ++++- .../src/configuration/linter/rules.rs | 101 ++++---- .../src/configuration/parse/json/rules.rs | 19 ++ editors/vscode/configuration_schema.json | 7 + npm/backend-jsonrpc/src/workspace.ts | 5 + npm/rome/configuration_schema.json | 7 + .../components/generated/NumberOfRules.astro | 2 +- website/src/pages/lint/rules/index.mdx | 6 + .../src/pages/lint/rules/noGlobalIsFinite.md | 50 ++++ 18 files changed, 693 insertions(+), 57 deletions(-) create mode 100644 crates/rome_js_analyze/src/semantic_analyzers/nursery/no_global_is_finite.rs create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noGlobalIsFinite/invalid.js create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noGlobalIsFinite/invalid.js.snap create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noGlobalIsFinite/valid.js create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noGlobalIsFinite/valid.js.snap create mode 100644 website/src/pages/lint/rules/noGlobalIsFinite.md diff --git a/CHANGELOG.md b/CHANGELOG.md index f61d35a767c..46d1f44f41b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -83,7 +83,11 @@ multiple files: #### New rules -- Add [`noGlobalThis`](https://docs.rome.tools/lint/rules/noglobalthis/) +- Add [`noGlobalIsFinite`](https://docs.rome.tools/lint/rules/noglobalisfinite/) + + This rule recommends using `Number.isFinite` instead of the global and unsafe `isFinite` that attempts a type coercion. + +- Add [`noGlobalIsNan`](https://docs.rome.tools/lint/rules/noglobalisnan/) This rule recommends using `Number.isNaN` instead of the global and unsafe `isNaN` that attempts a type coercion. diff --git a/crates/rome_diagnostics_categories/src/categories.rs b/crates/rome_diagnostics_categories/src/categories.rs index 511343e5973..fae7d2da247 100644 --- a/crates/rome_diagnostics_categories/src/categories.rs +++ b/crates/rome_diagnostics_categories/src/categories.rs @@ -100,6 +100,7 @@ define_categories! { "lint/nursery/noDuplicateJsonKeys": "https://docs.rome.tools/lint/rules/noDuplicateJsonKeys", "lint/nursery/useNamingConvention": "https://docs.rome.tools/lint/rules/useNamingConvention", "lint/nursery/noGlobalIsNan": "https://docs.rome.tools/lint/rules/noGlobalIsNan", +"lint/nursery/noGlobalIsFinite": "https://docs.rome.tools/lint/rules/noGlobalIsFinite", // Insert new nursery rule here diff --git a/crates/rome_js_analyze/src/semantic_analyzers/nursery.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery.rs index ddcbd441eb0..ad7d15d5be7 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/nursery.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/nursery.rs @@ -5,9 +5,10 @@ pub(crate) mod no_accumulating_spread; pub(crate) mod no_banned_types; pub(crate) mod no_console_log; pub(crate) mod no_constant_condition; +pub(crate) mod no_global_is_finite; pub(crate) mod no_global_is_nan; pub(crate) mod use_camel_case; pub(crate) mod use_exhaustive_dependencies; pub(crate) mod use_hook_at_top_level; pub(crate) mod use_naming_convention; -declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_accumulating_spread :: NoAccumulatingSpread , self :: no_banned_types :: NoBannedTypes , self :: no_console_log :: NoConsoleLog , self :: no_constant_condition :: NoConstantCondition , self :: no_global_is_nan :: NoGlobalIsNan , self :: use_camel_case :: UseCamelCase , self :: use_exhaustive_dependencies :: UseExhaustiveDependencies , self :: use_hook_at_top_level :: UseHookAtTopLevel , self :: use_naming_convention :: UseNamingConvention ,] } } +declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_accumulating_spread :: NoAccumulatingSpread , self :: no_banned_types :: NoBannedTypes , self :: no_console_log :: NoConsoleLog , self :: no_constant_condition :: NoConstantCondition , self :: no_global_is_finite :: NoGlobalIsFinite , self :: no_global_is_nan :: NoGlobalIsNan , self :: use_camel_case :: UseCamelCase , self :: use_exhaustive_dependencies :: UseExhaustiveDependencies , self :: use_hook_at_top_level :: UseHookAtTopLevel , self :: use_naming_convention :: UseNamingConvention ,] } } diff --git a/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_global_is_finite.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_global_is_finite.rs new file mode 100644 index 00000000000..bf1577d26e6 --- /dev/null +++ b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_global_is_finite.rs @@ -0,0 +1,161 @@ +use crate::{semantic_services::Semantic, JsRuleAction}; +use rome_analyze::{context::RuleContext, declare_rule, ActionCategory, Rule, RuleDiagnostic}; +use rome_console::markup; +use rome_diagnostics::Applicability; +use rome_js_factory::make; +use rome_js_syntax::{AnyJsExpression, T}; +use rome_rowan::{AstNode, BatchMutationExt}; + +declare_rule! { + /// Use `Number.isFinite` instead of global `isFinite`. + /// + /// `Number.isFinite()` and `isFinite()` [have not the same behavior](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isFinite#difference_between_number.isfinite_and_global_isfinite). + /// When the argument to `isFinite()` is not a number, the value is first coerced to a number. + /// `Number.isFinite()` does not perform this coercion. + /// Therefore, it is a more reliable way to test whether a number is finite. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```js,expect_diagnostic + /// isFinite(false); // true + /// ``` + /// + /// ## Valid + /// + /// ```js + /// Number.isFinite(false); // false + /// ``` + pub(crate) NoGlobalIsFinite { + version: "next", + name: "noGlobalIsFinite", + recommended: true, + } +} + +impl Rule for NoGlobalIsFinite { + type Query = Semantic; + type State = (); + type Signals = Option; + type Options = (); + + fn run(ctx: &RuleContext) -> Self::Signals { + let node = ctx.query(); + let model = ctx.model(); + match node { + AnyJsExpression::JsIdentifierExpression(expression) => { + let name = expression.name().ok()?; + if name.has_name("isFinite") && model.binding(&name).is_none() { + return Some(()); + } + } + AnyJsExpression::JsStaticMemberExpression(expression) => { + let object_name = expression + .object() + .ok()? + .omit_parentheses() + .as_js_identifier_expression()? + .name() + .ok()?; + let member = expression.member().ok()?; + if object_name.is_global_this() + && model.binding(&object_name).is_none() + && member.as_js_name()?.value_token().ok()?.text_trimmed() == "isFinite" + { + return Some(()); + } + } + AnyJsExpression::JsComputedMemberExpression(expression) => { + let object_name = expression + .object() + .ok()? + .omit_parentheses() + .as_js_identifier_expression()? + .name() + .ok()?; + let member = expression.member().ok()?.omit_parentheses(); + let member = member + .as_any_js_literal_expression()? + .as_js_string_literal_expression()?; + if object_name.is_global_this() + && model.binding(&object_name).is_none() + && member.inner_string_text().ok()?.text() == "isFinite" + { + return Some(()); + } + } + _ => (), + } + None + } + + fn diagnostic(ctx: &RuleContext, _: &Self::State) -> Option { + let node = ctx.query(); + Some( + RuleDiagnostic::new( + rule_category!(), + node.range(), + markup! { + "isFinite"" is unsafe. It attempts a type coercion. Use ""Number.isFinite"" instead." + }, + ) + .note(markup! { + "See ""the MDN documentation"" for more details." + }), + ) + } + + fn action(ctx: &RuleContext, _: &Self::State) -> Option { + let node = ctx.query(); + let mut mutation = ctx.root().begin(); + let (old, new) = match node { + AnyJsExpression::JsIdentifierExpression(expression) => ( + node.clone(), + make::js_static_member_expression( + make::js_identifier_expression(make::js_reference_identifier(make::ident( + "Number", + ))) + .into(), + make::token(T![.]), + make::js_name(expression.name().ok()?.value_token().ok()?).into(), + ), + ), + AnyJsExpression::JsStaticMemberExpression(expression) => ( + node.clone(), + make::js_static_member_expression( + make::js_static_member_expression( + expression.object().ok()?, + make::token(T![.]), + make::js_name(make::ident("Number")).into(), + ) + .into(), + expression.operator_token().ok()?, + expression.member().ok()?, + ), + ), + AnyJsExpression::JsComputedMemberExpression(expression) => { + let object = expression.object().ok()?; + ( + object.clone(), + make::js_static_member_expression( + object, + make::token(T![.]), + make::js_name(make::ident("Number")).into(), + ), + ) + } + _ => return None, + }; + mutation.replace_node(old, new.into()); + Some(JsRuleAction { + category: ActionCategory::QuickFix, + applicability: Applicability::MaybeIncorrect, + message: markup! { + "Use ""Number.isFinite"" instead." + } + .to_owned(), + mutation, + }) + } +} diff --git a/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_global_is_nan.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_global_is_nan.rs index 58eff0397ea..f34fecf98c4 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_global_is_nan.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_global_is_nan.rs @@ -81,7 +81,7 @@ impl Rule for NoGlobalIsNan { .as_js_string_literal_expression()?; if object_name.is_global_this() && model.binding(&object_name).is_none() - && member.value_token().ok()?.text_trimmed() == "isNaN" + && member.inner_string_text().ok()?.text() == "isNaN" { return Some(()); } @@ -110,15 +110,45 @@ impl Rule for NoGlobalIsNan { fn action(ctx: &RuleContext, _: &Self::State) -> Option { let node = ctx.query(); let mut mutation = ctx.root().begin(); - let number_constructor = - make::js_identifier_expression(make::js_reference_identifier(make::ident("Number"))); - let is_nan_name = make::js_name(make::ident("isNaN")); - let expression = make::js_static_member_expression( - number_constructor.into(), - make::token(T![.]), - is_nan_name.into(), - ); - mutation.replace_node(node.clone(), expression.into()); + let (old, new) = match node { + AnyJsExpression::JsIdentifierExpression(expression) => ( + node.clone(), + make::js_static_member_expression( + make::js_identifier_expression(make::js_reference_identifier(make::ident( + "Number", + ))) + .into(), + make::token(T![.]), + make::js_name(expression.name().ok()?.value_token().ok()?).into(), + ), + ), + AnyJsExpression::JsStaticMemberExpression(expression) => ( + node.clone(), + make::js_static_member_expression( + make::js_static_member_expression( + expression.object().ok()?, + make::token(T![.]), + make::js_name(make::ident("Number")).into(), + ) + .into(), + expression.operator_token().ok()?, + expression.member().ok()?, + ), + ), + AnyJsExpression::JsComputedMemberExpression(expression) => { + let object = expression.object().ok()?; + ( + object.clone(), + make::js_static_member_expression( + object, + make::token(T![.]), + make::js_name(make::ident("Number")).into(), + ), + ) + } + _ => return None, + }; + mutation.replace_node(old, new.into()); Some(JsRuleAction { category: ActionCategory::QuickFix, applicability: Applicability::MaybeIncorrect, diff --git a/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsFinite/invalid.js b/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsFinite/invalid.js new file mode 100644 index 00000000000..e5e5b1a285a --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsFinite/invalid.js @@ -0,0 +1,17 @@ +isFinite({}); + +(isFinite)({}); + +globalThis.isFinite({}); + +(globalThis).isFinite({}); + +globalThis["isFinite"]({}); + +(globalThis)[("isFinite")]({}); + +function localIsNaN(isFinite) { + globalThis.isFinite({}); +} + +localIsNaN(isFinite); diff --git a/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsFinite/invalid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsFinite/invalid.js.snap new file mode 100644 index 00000000000..0a6743c4072 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsFinite/invalid.js.snap @@ -0,0 +1,224 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +expression: invalid.js +--- +# Input +```js +isFinite({}); + +(isFinite)({}); + +globalThis.isFinite({}); + +(globalThis).isFinite({}); + +globalThis["isFinite"]({}); + +(globalThis)[("isFinite")]({}); + +function localIsNaN(isFinite) { + globalThis.isFinite({}); +} + +localIsNaN(isFinite); + +``` + +# Diagnostics +``` +invalid.js:1:1 lint/nursery/noGlobalIsFinite FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! isFinite is unsafe. It attempts a type coercion. Use Number.isFinite instead. + + > 1 │ isFinite({}); + │ ^^^^^^^^ + 2 │ + 3 │ (isFinite)({}); + + i See the MDN documentation for more details. + + i Suggested fix: Use Number.isFinite instead. + + 1 │ - isFinite({}); + 1 │ + Number.isFinite({}); + 2 2 │ + 3 3 │ (isFinite)({}); + + +``` + +``` +invalid.js:3:2 lint/nursery/noGlobalIsFinite FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! isFinite is unsafe. It attempts a type coercion. Use Number.isFinite instead. + + 1 │ isFinite({}); + 2 │ + > 3 │ (isFinite)({}); + │ ^^^^^^^^ + 4 │ + 5 │ globalThis.isFinite({}); + + i See the MDN documentation for more details. + + i Suggested fix: Use Number.isFinite instead. + + 1 1 │ isFinite({}); + 2 2 │ + 3 │ - (isFinite)({}); + 3 │ + (Number.isFinite)({}); + 4 4 │ + 5 5 │ globalThis.isFinite({}); + + +``` + +``` +invalid.js:5:1 lint/nursery/noGlobalIsFinite FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! isFinite is unsafe. It attempts a type coercion. Use Number.isFinite instead. + + 3 │ (isFinite)({}); + 4 │ + > 5 │ globalThis.isFinite({}); + │ ^^^^^^^^^^^^^^^^^^^ + 6 │ + 7 │ (globalThis).isFinite({}); + + i See the MDN documentation for more details. + + i Suggested fix: Use Number.isFinite instead. + + 3 3 │ (isFinite)({}); + 4 4 │ + 5 │ - globalThis.isFinite({}); + 5 │ + globalThis.Number.isFinite({}); + 6 6 │ + 7 7 │ (globalThis).isFinite({}); + + +``` + +``` +invalid.js:7:1 lint/nursery/noGlobalIsFinite FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! isFinite is unsafe. It attempts a type coercion. Use Number.isFinite instead. + + 5 │ globalThis.isFinite({}); + 6 │ + > 7 │ (globalThis).isFinite({}); + │ ^^^^^^^^^^^^^^^^^^^^^ + 8 │ + 9 │ globalThis["isFinite"]({}); + + i See the MDN documentation for more details. + + i Suggested fix: Use Number.isFinite instead. + + 5 5 │ globalThis.isFinite({}); + 6 6 │ + 7 │ - (globalThis).isFinite({}); + 7 │ + (globalThis).Number.isFinite({}); + 8 8 │ + 9 9 │ globalThis["isFinite"]({}); + + +``` + +``` +invalid.js:9:1 lint/nursery/noGlobalIsFinite FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! isFinite is unsafe. It attempts a type coercion. Use Number.isFinite instead. + + 7 │ (globalThis).isFinite({}); + 8 │ + > 9 │ globalThis["isFinite"]({}); + │ ^^^^^^^^^^^^^^^^^^^^^^ + 10 │ + 11 │ (globalThis)[("isFinite")]({}); + + i See the MDN documentation for more details. + + i Suggested fix: Use Number.isFinite instead. + + 7 7 │ (globalThis).isFinite({}); + 8 8 │ + 9 │ - globalThis["isFinite"]({}); + 9 │ + globalThis.Number["isFinite"]({}); + 10 10 │ + 11 11 │ (globalThis)[("isFinite")]({}); + + +``` + +``` +invalid.js:11:1 lint/nursery/noGlobalIsFinite FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! isFinite is unsafe. It attempts a type coercion. Use Number.isFinite instead. + + 9 │ globalThis["isFinite"]({}); + 10 │ + > 11 │ (globalThis)[("isFinite")]({}); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^ + 12 │ + 13 │ function localIsNaN(isFinite) { + + i See the MDN documentation for more details. + + i Suggested fix: Use Number.isFinite instead. + + 11 │ (globalThis).Number[("isFinite")]({}); + │ +++++++ + +``` + +``` +invalid.js:14:5 lint/nursery/noGlobalIsFinite FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! isFinite is unsafe. It attempts a type coercion. Use Number.isFinite instead. + + 13 │ function localIsNaN(isFinite) { + > 14 │ globalThis.isFinite({}); + │ ^^^^^^^^^^^^^^^^^^^ + 15 │ } + 16 │ + + i See the MDN documentation for more details. + + i Suggested fix: Use Number.isFinite instead. + + 12 12 │ + 13 13 │ function localIsNaN(isFinite) { + 14 │ - ····globalThis.isFinite({}); + 14 │ + ····globalThis.Number.isFinite({}); + 15 15 │ } + 16 16 │ + + +``` + +``` +invalid.js:17:12 lint/nursery/noGlobalIsFinite FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! isFinite is unsafe. It attempts a type coercion. Use Number.isFinite instead. + + 15 │ } + 16 │ + > 17 │ localIsNaN(isFinite); + │ ^^^^^^^^ + 18 │ + + i See the MDN documentation for more details. + + i Suggested fix: Use Number.isFinite instead. + + 15 15 │ } + 16 16 │ + 17 │ - localIsNaN(isFinite); + 17 │ + localIsNaN(Number.isFinite); + 18 18 │ + + +``` + + diff --git a/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsFinite/valid.js b/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsFinite/valid.js new file mode 100644 index 00000000000..186ac2daa1e --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsFinite/valid.js @@ -0,0 +1,14 @@ +Number.isFinite(Number.NaN); + +globalThis.Number.isFinite(Number.NaN); + +function localIsFinite(isFinite) { + isFinite({}); +} + +function localVar() { + var isFinite; + isFinite() +} + +localIsFinite(Number.isFinite); diff --git a/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsFinite/valid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsFinite/valid.js.snap new file mode 100644 index 00000000000..d96740c5503 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsFinite/valid.js.snap @@ -0,0 +1,24 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +expression: valid.js +--- +# Input +```js +Number.isFinite(Number.NaN); + +globalThis.Number.isFinite(Number.NaN); + +function localIsFinite(isFinite) { + isFinite({}); +} + +function localVar() { + var isFinite; + isFinite() +} + +localIsFinite(Number.isFinite); + +``` + + diff --git a/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/invalid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/invalid.js.snap index b234434b755..dfdcb4b98ab 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/invalid.js.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/invalid.js.snap @@ -92,7 +92,7 @@ invalid.js:5:1 lint/nursery/noGlobalIsNan FIXABLE ━━━━━━━━━ 3 3 │ (isNaN)({}); 4 4 │ 5 │ - globalThis.isNaN({}); - 5 │ + Number.isNaN({}); + 5 │ + globalThis.Number.isNaN({}); 6 6 │ 7 7 │ (globalThis).isNaN({}); @@ -118,13 +118,60 @@ invalid.js:7:1 lint/nursery/noGlobalIsNan FIXABLE ━━━━━━━━━ 5 5 │ globalThis.isNaN({}); 6 6 │ 7 │ - (globalThis).isNaN({}); - 7 │ + Number.isNaN({}); + 7 │ + (globalThis).Number.isNaN({}); 8 8 │ 9 9 │ globalThis["isNaN"]({}); ``` +``` +invalid.js:9:1 lint/nursery/noGlobalIsNan FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. + + 7 │ (globalThis).isNaN({}); + 8 │ + > 9 │ globalThis["isNaN"]({}); + │ ^^^^^^^^^^^^^^^^^^^ + 10 │ + 11 │ (globalThis)[("isNaN")]({}); + + i See the MDN documentation for more details. + + i Suggested fix: Use Number.isNaN instead. + + 7 7 │ (globalThis).isNaN({}); + 8 8 │ + 9 │ - globalThis["isNaN"]({}); + 9 │ + globalThis.Number["isNaN"]({}); + 10 10 │ + 11 11 │ (globalThis)[("isNaN")]({}); + + +``` + +``` +invalid.js:11:1 lint/nursery/noGlobalIsNan FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. + + 9 │ globalThis["isNaN"]({}); + 10 │ + > 11 │ (globalThis)[("isNaN")]({}); + │ ^^^^^^^^^^^^^^^^^^^^^^^ + 12 │ + 13 │ function localIsNaN(isNaN) { + + i See the MDN documentation for more details. + + i Suggested fix: Use Number.isNaN instead. + + 11 │ (globalThis).Number[("isNaN")]({}); + │ +++++++ + +``` + ``` invalid.js:14:5 lint/nursery/noGlobalIsNan FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ @@ -143,7 +190,7 @@ invalid.js:14:5 lint/nursery/noGlobalIsNan FIXABLE ━━━━━━━━━ 12 12 │ 13 13 │ function localIsNaN(isNaN) { 14 │ - ····globalThis.isNaN({}); - 14 │ + ····Number.isNaN({}); + 14 │ + ····globalThis.Number.isNaN({}); 15 15 │ } 16 16 │ diff --git a/crates/rome_service/src/configuration/linter/rules.rs b/crates/rome_service/src/configuration/linter/rules.rs index 27f15794d0b..4136069c601 100644 --- a/crates/rome_service/src/configuration/linter/rules.rs +++ b/crates/rome_service/src/configuration/linter/rules.rs @@ -1843,6 +1843,10 @@ pub struct Nursery { #[bpaf(long("no-for-each"), argument("on|off|warn"), optional, hide)] #[serde(skip_serializing_if = "Option::is_none")] pub no_for_each: Option, + #[doc = "Use Number.isFinite instead of global isFinite."] + #[bpaf(long("no-global-is-finite"), argument("on|off|warn"), optional, hide)] + #[serde(skip_serializing_if = "Option::is_none")] + pub no_global_is_finite: Option, #[doc = "Use Number.isNaN instead of global isNaN."] #[bpaf(long("no-global-is-nan"), argument("on|off|warn"), optional, hide)] #[serde(skip_serializing_if = "Option::is_none")] @@ -1935,7 +1939,7 @@ pub struct Nursery { } impl Nursery { const GROUP_NAME: &'static str = "nursery"; - pub(crate) const GROUP_RULES: [&'static str; 25] = [ + pub(crate) const GROUP_RULES: [&'static str; 26] = [ "noAccumulatingSpread", "noAriaUnsupportedElements", "noBannedTypes", @@ -1945,6 +1949,7 @@ impl Nursery { "noDuplicateJsonKeys", "noDuplicateJsxProps", "noForEach", + "noGlobalIsFinite", "noGlobalIsNan", "noNoninteractiveTabindex", "noRedundantRoles", @@ -1962,12 +1967,13 @@ impl Nursery { "useNamingConvention", "useSimpleNumberKeys", ]; - const RECOMMENDED_RULES: [&'static str; 14] = [ + const RECOMMENDED_RULES: [&'static str; 15] = [ "noAriaUnsupportedElements", "noBannedTypes", "noConstantCondition", "noDuplicateJsonKeys", "noDuplicateJsxProps", + "noGlobalIsFinite", "noGlobalIsNan", "noRedundantRoles", "noSelfAssign", @@ -1978,23 +1984,24 @@ impl Nursery { "useLiteralEnumMembers", "useLiteralKeys", ]; - const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 14] = [ + const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 15] = [ RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[1]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[2]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[5]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[6]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[7]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[9]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[11]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[10]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[12]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[13]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[16]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[14]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[17]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[20]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[21]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[22]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[23]), ]; - const ALL_RULES_AS_FILTERS: [RuleFilter<'static>; 25] = [ + const ALL_RULES_AS_FILTERS: [RuleFilter<'static>; 26] = [ RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[0]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[1]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[2]), @@ -2020,6 +2027,7 @@ impl Nursery { RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[22]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[23]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[24]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[25]), ]; #[doc = r" Retrieves the recommended rules"] pub(crate) fn is_recommended(&self) -> bool { matches!(self.recommended, Some(true)) } @@ -2075,86 +2083,91 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[8])); } } - if let Some(rule) = self.no_global_is_nan.as_ref() { + if let Some(rule) = self.no_global_is_finite.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[9])); } } - if let Some(rule) = self.no_noninteractive_tabindex.as_ref() { + if let Some(rule) = self.no_global_is_nan.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[10])); } } - if let Some(rule) = self.no_redundant_roles.as_ref() { + if let Some(rule) = self.no_noninteractive_tabindex.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[11])); } } - if let Some(rule) = self.no_self_assign.as_ref() { + if let Some(rule) = self.no_redundant_roles.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[12])); } } - if let Some(rule) = self.no_static_only_class.as_ref() { + if let Some(rule) = self.no_self_assign.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[13])); } } - if let Some(rule) = self.use_aria_prop_types.as_ref() { + if let Some(rule) = self.no_static_only_class.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[14])); } } - if let Some(rule) = self.use_camel_case.as_ref() { + if let Some(rule) = self.use_aria_prop_types.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[15])); } } - if let Some(rule) = self.use_exhaustive_dependencies.as_ref() { + if let Some(rule) = self.use_camel_case.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[16])); } } - if let Some(rule) = self.use_grouped_type_import.as_ref() { + if let Some(rule) = self.use_exhaustive_dependencies.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[17])); } } - if let Some(rule) = self.use_heading_content.as_ref() { + if let Some(rule) = self.use_grouped_type_import.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18])); } } - if let Some(rule) = self.use_hook_at_top_level.as_ref() { + if let Some(rule) = self.use_heading_content.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[19])); } } - if let Some(rule) = self.use_is_nan.as_ref() { + if let Some(rule) = self.use_hook_at_top_level.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[20])); } } - if let Some(rule) = self.use_literal_enum_members.as_ref() { + if let Some(rule) = self.use_is_nan.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[21])); } } - if let Some(rule) = self.use_literal_keys.as_ref() { + if let Some(rule) = self.use_literal_enum_members.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[22])); } } - if let Some(rule) = self.use_naming_convention.as_ref() { + if let Some(rule) = self.use_literal_keys.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[23])); } } - if let Some(rule) = self.use_simple_number_keys.as_ref() { + if let Some(rule) = self.use_naming_convention.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[24])); } } + if let Some(rule) = self.use_simple_number_keys.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[25])); + } + } index_set } pub(crate) fn get_disabled_rules(&self) -> IndexSet { @@ -2204,86 +2217,91 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[8])); } } - if let Some(rule) = self.no_global_is_nan.as_ref() { + if let Some(rule) = self.no_global_is_finite.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[9])); } } - if let Some(rule) = self.no_noninteractive_tabindex.as_ref() { + if let Some(rule) = self.no_global_is_nan.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[10])); } } - if let Some(rule) = self.no_redundant_roles.as_ref() { + if let Some(rule) = self.no_noninteractive_tabindex.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[11])); } } - if let Some(rule) = self.no_self_assign.as_ref() { + if let Some(rule) = self.no_redundant_roles.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[12])); } } - if let Some(rule) = self.no_static_only_class.as_ref() { + if let Some(rule) = self.no_self_assign.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[13])); } } - if let Some(rule) = self.use_aria_prop_types.as_ref() { + if let Some(rule) = self.no_static_only_class.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[14])); } } - if let Some(rule) = self.use_camel_case.as_ref() { + if let Some(rule) = self.use_aria_prop_types.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[15])); } } - if let Some(rule) = self.use_exhaustive_dependencies.as_ref() { + if let Some(rule) = self.use_camel_case.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[16])); } } - if let Some(rule) = self.use_grouped_type_import.as_ref() { + if let Some(rule) = self.use_exhaustive_dependencies.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[17])); } } - if let Some(rule) = self.use_heading_content.as_ref() { + if let Some(rule) = self.use_grouped_type_import.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18])); } } - if let Some(rule) = self.use_hook_at_top_level.as_ref() { + if let Some(rule) = self.use_heading_content.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[19])); } } - if let Some(rule) = self.use_is_nan.as_ref() { + if let Some(rule) = self.use_hook_at_top_level.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[20])); } } - if let Some(rule) = self.use_literal_enum_members.as_ref() { + if let Some(rule) = self.use_is_nan.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[21])); } } - if let Some(rule) = self.use_literal_keys.as_ref() { + if let Some(rule) = self.use_literal_enum_members.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[22])); } } - if let Some(rule) = self.use_naming_convention.as_ref() { + if let Some(rule) = self.use_literal_keys.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[23])); } } - if let Some(rule) = self.use_simple_number_keys.as_ref() { + if let Some(rule) = self.use_naming_convention.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[24])); } } + if let Some(rule) = self.use_simple_number_keys.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[25])); + } + } index_set } #[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"] @@ -2292,10 +2310,10 @@ impl Nursery { pub(crate) fn is_recommended_rule(rule_name: &str) -> bool { Self::RECOMMENDED_RULES.contains(&rule_name) } - pub(crate) fn recommended_rules_as_filters() -> [RuleFilter<'static>; 14] { + pub(crate) fn recommended_rules_as_filters() -> [RuleFilter<'static>; 15] { Self::RECOMMENDED_RULES_AS_FILTERS } - pub(crate) fn all_rules_as_filters() -> [RuleFilter<'static>; 25] { Self::ALL_RULES_AS_FILTERS } + pub(crate) fn all_rules_as_filters() -> [RuleFilter<'static>; 26] { Self::ALL_RULES_AS_FILTERS } #[doc = r" Select preset rules"] pub(crate) fn collect_preset_rules( &self, @@ -2325,6 +2343,7 @@ impl Nursery { "noDuplicateJsonKeys" => self.no_duplicate_json_keys.as_ref(), "noDuplicateJsxProps" => self.no_duplicate_jsx_props.as_ref(), "noForEach" => self.no_for_each.as_ref(), + "noGlobalIsFinite" => self.no_global_is_finite.as_ref(), "noGlobalIsNan" => self.no_global_is_nan.as_ref(), "noNoninteractiveTabindex" => self.no_noninteractive_tabindex.as_ref(), "noRedundantRoles" => self.no_redundant_roles.as_ref(), diff --git a/crates/rome_service/src/configuration/parse/json/rules.rs b/crates/rome_service/src/configuration/parse/json/rules.rs index b94825b0b39..7af4ee5c554 100644 --- a/crates/rome_service/src/configuration/parse/json/rules.rs +++ b/crates/rome_service/src/configuration/parse/json/rules.rs @@ -1357,6 +1357,7 @@ impl VisitNode for Nursery { "noDuplicateJsonKeys", "noDuplicateJsxProps", "noForEach", + "noGlobalIsFinite", "noGlobalIsNan", "noNoninteractiveTabindex", "noRedundantRoles", @@ -1554,6 +1555,24 @@ impl VisitNode for Nursery { )); } }, + "noGlobalIsFinite" => match value { + AnyJsonValue::JsonStringValue(_) => { + let mut configuration = RuleConfiguration::default(); + self.map_to_known_string(&value, name_text, &mut configuration, diagnostics)?; + self.no_global_is_finite = Some(configuration); + } + AnyJsonValue::JsonObjectValue(_) => { + let mut configuration = RuleConfiguration::default(); + self.map_to_object(&value, name_text, &mut configuration, diagnostics)?; + self.no_global_is_finite = Some(configuration); + } + _ => { + diagnostics.push(DeserializationDiagnostic::new_incorrect_type( + "object or string", + value.range(), + )); + } + }, "noGlobalIsNan" => match value { AnyJsonValue::JsonStringValue(_) => { let mut configuration = RuleConfiguration::default(); diff --git a/editors/vscode/configuration_schema.json b/editors/vscode/configuration_schema.json index 97c5debb6ee..958198e9822 100644 --- a/editors/vscode/configuration_schema.json +++ b/editors/vscode/configuration_schema.json @@ -803,6 +803,13 @@ { "type": "null" } ] }, + "noGlobalIsFinite": { + "description": "Use Number.isFinite instead of global isFinite.", + "anyOf": [ + { "$ref": "#/definitions/RuleConfiguration" }, + { "type": "null" } + ] + }, "noGlobalIsNan": { "description": "Use Number.isNaN instead of global isNaN.", "anyOf": [ diff --git a/npm/backend-jsonrpc/src/workspace.ts b/npm/backend-jsonrpc/src/workspace.ts index 27194594160..a827ef86a63 100644 --- a/npm/backend-jsonrpc/src/workspace.ts +++ b/npm/backend-jsonrpc/src/workspace.ts @@ -541,6 +541,10 @@ export interface Nursery { * Prefer for...of statement instead of Array.forEach. */ noForEach?: RuleConfiguration; + /** + * Use Number.isFinite instead of global isFinite. + */ + noGlobalIsFinite?: RuleConfiguration; /** * Use Number.isNaN instead of global isNaN. */ @@ -1116,6 +1120,7 @@ export type Category = | "lint/nursery/noDuplicateJsonKeys" | "lint/nursery/useNamingConvention" | "lint/nursery/noGlobalIsNan" + | "lint/nursery/noGlobalIsFinite" | "lint/performance/noDelete" | "lint/security/noDangerouslySetInnerHtml" | "lint/security/noDangerouslySetInnerHtmlWithChildren" diff --git a/npm/rome/configuration_schema.json b/npm/rome/configuration_schema.json index 97c5debb6ee..958198e9822 100644 --- a/npm/rome/configuration_schema.json +++ b/npm/rome/configuration_schema.json @@ -803,6 +803,13 @@ { "type": "null" } ] }, + "noGlobalIsFinite": { + "description": "Use Number.isFinite instead of global isFinite.", + "anyOf": [ + { "$ref": "#/definitions/RuleConfiguration" }, + { "type": "null" } + ] + }, "noGlobalIsNan": { "description": "Use Number.isNaN instead of global isNaN.", "anyOf": [ diff --git a/website/src/components/generated/NumberOfRules.astro b/website/src/components/generated/NumberOfRules.astro index 70853d24626..400483b48d7 100644 --- a/website/src/components/generated/NumberOfRules.astro +++ b/website/src/components/generated/NumberOfRules.astro @@ -1,2 +1,2 @@ -

Rome's linter has a total of 144 rules

\ No newline at end of file +

Rome's linter has a total of 145 rules

\ No newline at end of file diff --git a/website/src/pages/lint/rules/index.mdx b/website/src/pages/lint/rules/index.mdx index 031d2f7084f..f30d1050e9b 100644 --- a/website/src/pages/lint/rules/index.mdx +++ b/website/src/pages/lint/rules/index.mdx @@ -951,6 +951,12 @@ Prevents JSX properties to be assigned multiple times. Prefer for...of statement instead of Array.forEach.

+

+ noGlobalIsFinite +

+Use Number.isFinite instead of global isFinite. +
+

noGlobalIsNan

diff --git a/website/src/pages/lint/rules/noGlobalIsFinite.md b/website/src/pages/lint/rules/noGlobalIsFinite.md new file mode 100644 index 00000000000..223e71d599c --- /dev/null +++ b/website/src/pages/lint/rules/noGlobalIsFinite.md @@ -0,0 +1,50 @@ +--- +title: Lint Rule noGlobalIsFinite +parent: lint/rules/index +--- + +# noGlobalIsFinite (since vnext) + +Use `Number.isFinite` instead of global `isFinite`. + +`Number.isFinite()` and `isFinite()` [have not the same behavior](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isFinite#difference_between_number.isfinite_and_global_isfinite). +When the argument to `isFinite()` is not a number, the value is first coerced to a number. +`Number.isFinite()` does not perform this coercion. +Therefore, it is a more reliable way to test whether a number is finite. + +## Examples + +### Invalid + +```jsx +isFinite(false); // true +``` + +
nursery/noGlobalIsFinite.js:1:1 lint/nursery/noGlobalIsFinite  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ✖ isFinite is unsafe. It attempts a type coercion. Use Number.isFinite instead.
+  
+  > 1 │ isFinite(false); // true
+      │ ^^^^^^^^
+    2 │ 
+  
+  ℹ See the MDN documentation for more details.
+  
+  ℹ Suggested fix: Use Number.isFinite instead.
+  
+    1   │ - isFinite(false);·//·true
+      1 │ + Number.isFinite(false);·//·true
+    2 2 │   
+  
+
+ +## Valid + +```jsx +Number.isFinite(false); // false +``` + +## Related links + +- [Disable a rule](/linter/#disable-a-lint-rule) +- [Rule options](/linter/#rule-options)