From e2ffdf6991def890ba7de81b2c8beeb2d2387375 Mon Sep 17 00:00:00 2001 From: Victorien ELVINGER Date: Sat, 24 Jun 2023 22:59:39 +0200 Subject: [PATCH] feat(rome_js_analyze): noGlobalIsNan (#4612) --- CHANGELOG.md | 5 + .../src/categories.rs | 1 + .../src/semantic_analyzers/nursery.rs | 3 +- .../nursery/no_global_is_nan.rs | 132 +++++++++++++ .../specs/nursery/noGlobalIsNan/invalid.js | 17 ++ .../nursery/noGlobalIsNan/invalid.js.snap | 177 ++++++++++++++++++ .../specs/nursery/noGlobalIsNan/valid.js | 14 ++ .../specs/nursery/noGlobalIsNan/valid.js.snap | 24 +++ .../src/configuration/linter/rules.rs | 97 ++++++---- .../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 + website/src/pages/lint/rules/noGlobalIsNan.md | 50 +++++ 16 files changed, 525 insertions(+), 41 deletions(-) create mode 100644 crates/rome_js_analyze/src/semantic_analyzers/nursery/no_global_is_nan.rs create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/invalid.js create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/invalid.js.snap create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/valid.js create mode 100644 crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/valid.js.snap create mode 100644 website/src/pages/lint/rules/noGlobalIsNan.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 97ac3a9cfa1..f61d35a767c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,6 +81,11 @@ multiple files: The _Rome_ formatter takes care of removing extra semicolons. Thus, there is no need for this rule. +#### New rules + +- Add [`noGlobalThis`](https://docs.rome.tools/lint/rules/noglobalthis/) + + This rule recommends using `Number.isNaN` instead of the global and unsafe `isNaN` that attempts a type coercion. #### Other changes diff --git a/crates/rome_diagnostics_categories/src/categories.rs b/crates/rome_diagnostics_categories/src/categories.rs index 92cda8580a6..511343e5973 100644 --- a/crates/rome_diagnostics_categories/src/categories.rs +++ b/crates/rome_diagnostics_categories/src/categories.rs @@ -99,6 +99,7 @@ define_categories! { "lint/nursery/noStaticOnlyClass": "https://docs.rome.tools/lint/rules/noStaticOnlyClass", "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", // 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 3e0ca8e0d54..ddcbd441eb0 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/nursery.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/nursery.rs @@ -5,8 +5,9 @@ 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_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 :: 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_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_nan.rs b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_global_is_nan.rs new file mode 100644 index 00000000000..58eff0397ea --- /dev/null +++ b/crates/rome_js_analyze/src/semantic_analyzers/nursery/no_global_is_nan.rs @@ -0,0 +1,132 @@ +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.isNaN` instead of global `isNaN`. + /// + /// `Number.isNaN()` and `isNaN()` [have not the same behavior](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/isNaN#description). + /// When the argument to `isNaN()` is not a number, the value is first coerced to a number. + /// `Number.isNaN()` does not perform this coercion. + /// Therefore, it is a more reliable way to test whether a value is `NaN`. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```js,expect_diagnostic + /// isNaN({}); // true + /// ``` + /// + /// ## Valid + /// + /// ```js + /// Number.isNaN({}); // false + /// ``` + /// + pub(crate) NoGlobalIsNan { + version: "next", + name: "noGlobalIsNan", + recommended: true, + } +} + +impl Rule for NoGlobalIsNan { + 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("isNaN") && 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() == "isNaN" + { + 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.value_token().ok()?.text_trimmed() == "isNaN" + { + return Some(()); + } + } + _ => (), + } + None + } + + fn diagnostic(ctx: &RuleContext, _: &Self::State) -> Option { + let node = ctx.query(); + Some( + RuleDiagnostic::new( + rule_category!(), + node.range(), + markup! { + "isNaN"" is unsafe. It attempts a type coercion. Use ""Number.isNaN"" 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 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()); + Some(JsRuleAction { + category: ActionCategory::QuickFix, + applicability: Applicability::MaybeIncorrect, + message: markup! { + "Use ""Number.isNaN"" instead." + } + .to_owned(), + mutation, + }) + } +} diff --git a/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/invalid.js b/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/invalid.js new file mode 100644 index 00000000000..72cb6eda3e3 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/invalid.js @@ -0,0 +1,17 @@ +isNaN({}); + +(isNaN)({}); + +globalThis.isNaN({}); + +(globalThis).isNaN({}); + +globalThis["isNaN"]({}); + +(globalThis)[("isNaN")]({}); + +function localIsNaN(isNaN) { + globalThis.isNaN({}); +} + +localIsNaN(isNaN); 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 new file mode 100644 index 00000000000..b234434b755 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/invalid.js.snap @@ -0,0 +1,177 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +expression: invalid.js +--- +# Input +```js +isNaN({}); + +(isNaN)({}); + +globalThis.isNaN({}); + +(globalThis).isNaN({}); + +globalThis["isNaN"]({}); + +(globalThis)[("isNaN")]({}); + +function localIsNaN(isNaN) { + globalThis.isNaN({}); +} + +localIsNaN(isNaN); + +``` + +# Diagnostics +``` +invalid.js:1:1 lint/nursery/noGlobalIsNan FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. + + > 1 │ isNaN({}); + │ ^^^^^ + 2 │ + 3 │ (isNaN)({}); + + i See the MDN documentation for more details. + + i Suggested fix: Use Number.isNaN instead. + + 1 │ - isNaN({}); + 1 │ + Number.isNaN({}); + 2 2 │ + 3 3 │ (isNaN)({}); + + +``` + +``` +invalid.js:3:2 lint/nursery/noGlobalIsNan FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. + + 1 │ isNaN({}); + 2 │ + > 3 │ (isNaN)({}); + │ ^^^^^ + 4 │ + 5 │ globalThis.isNaN({}); + + i See the MDN documentation for more details. + + i Suggested fix: Use Number.isNaN instead. + + 1 1 │ isNaN({}); + 2 2 │ + 3 │ - (isNaN)({}); + 3 │ + (Number.isNaN)({}); + 4 4 │ + 5 5 │ globalThis.isNaN({}); + + +``` + +``` +invalid.js:5:1 lint/nursery/noGlobalIsNan FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. + + 3 │ (isNaN)({}); + 4 │ + > 5 │ globalThis.isNaN({}); + │ ^^^^^^^^^^^^^^^^ + 6 │ + 7 │ (globalThis).isNaN({}); + + i See the MDN documentation for more details. + + i Suggested fix: Use Number.isNaN instead. + + 3 3 │ (isNaN)({}); + 4 4 │ + 5 │ - globalThis.isNaN({}); + 5 │ + Number.isNaN({}); + 6 6 │ + 7 7 │ (globalThis).isNaN({}); + + +``` + +``` +invalid.js:7:1 lint/nursery/noGlobalIsNan FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. + + 5 │ globalThis.isNaN({}); + 6 │ + > 7 │ (globalThis).isNaN({}); + │ ^^^^^^^^^^^^^^^^^^ + 8 │ + 9 │ globalThis["isNaN"]({}); + + i See the MDN documentation for more details. + + i Suggested fix: Use Number.isNaN instead. + + 5 5 │ globalThis.isNaN({}); + 6 6 │ + 7 │ - (globalThis).isNaN({}); + 7 │ + Number.isNaN({}); + 8 8 │ + 9 9 │ globalThis["isNaN"]({}); + + +``` + +``` +invalid.js:14:5 lint/nursery/noGlobalIsNan FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. + + 13 │ function localIsNaN(isNaN) { + > 14 │ globalThis.isNaN({}); + │ ^^^^^^^^^^^^^^^^ + 15 │ } + 16 │ + + i See the MDN documentation for more details. + + i Suggested fix: Use Number.isNaN instead. + + 12 12 │ + 13 13 │ function localIsNaN(isNaN) { + 14 │ - ····globalThis.isNaN({}); + 14 │ + ····Number.isNaN({}); + 15 15 │ } + 16 16 │ + + +``` + +``` +invalid.js:17:12 lint/nursery/noGlobalIsNan FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. + + 15 │ } + 16 │ + > 17 │ localIsNaN(isNaN); + │ ^^^^^ + 18 │ + + i See the MDN documentation for more details. + + i Suggested fix: Use Number.isNaN instead. + + 15 15 │ } + 16 16 │ + 17 │ - localIsNaN(isNaN); + 17 │ + localIsNaN(Number.isNaN); + 18 18 │ + + +``` + + diff --git a/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/valid.js b/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/valid.js new file mode 100644 index 00000000000..854821c74c7 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/valid.js @@ -0,0 +1,14 @@ +Number.isNaN(Number.NaN); + +globalThis.Number.isNaN(Number.NaN); + +function localIsNaN(isNaN) { + isNaN({}); +} + +function localVar() { + var isNaN; + isNaN() +} + +localIsNaN(Number.isNaN); diff --git a/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/valid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/valid.js.snap new file mode 100644 index 00000000000..d773ffb940e --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/noGlobalIsNan/valid.js.snap @@ -0,0 +1,24 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +expression: valid.js +--- +# Input +```js +Number.isNaN(Number.NaN); + +globalThis.Number.isNaN(Number.NaN); + +function localIsNaN(isNaN) { + isNaN({}); +} + +function localVar() { + var isNaN; + isNaN() +} + +localIsNaN(Number.isNaN); + +``` + + diff --git a/crates/rome_service/src/configuration/linter/rules.rs b/crates/rome_service/src/configuration/linter/rules.rs index fe347f0801a..27f15794d0b 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.isNaN instead of global isNaN."] + #[bpaf(long("no-global-is-nan"), argument("on|off|warn"), optional, hide)] + #[serde(skip_serializing_if = "Option::is_none")] + pub no_global_is_nan: Option, #[doc = "Enforce that tabIndex is not assigned to non-interactive HTML elements."] #[bpaf( long("no-noninteractive-tabindex"), @@ -1931,7 +1935,7 @@ pub struct Nursery { } impl Nursery { const GROUP_NAME: &'static str = "nursery"; - pub(crate) const GROUP_RULES: [&'static str; 24] = [ + pub(crate) const GROUP_RULES: [&'static str; 25] = [ "noAccumulatingSpread", "noAriaUnsupportedElements", "noBannedTypes", @@ -1941,6 +1945,7 @@ impl Nursery { "noDuplicateJsonKeys", "noDuplicateJsxProps", "noForEach", + "noGlobalIsNan", "noNoninteractiveTabindex", "noRedundantRoles", "noSelfAssign", @@ -1957,12 +1962,13 @@ impl Nursery { "useNamingConvention", "useSimpleNumberKeys", ]; - const RECOMMENDED_RULES: [&'static str; 13] = [ + const RECOMMENDED_RULES: [&'static str; 14] = [ "noAriaUnsupportedElements", "noBannedTypes", "noConstantCondition", "noDuplicateJsonKeys", "noDuplicateJsxProps", + "noGlobalIsNan", "noRedundantRoles", "noSelfAssign", "noStaticOnlyClass", @@ -1972,22 +1978,23 @@ impl Nursery { "useLiteralEnumMembers", "useLiteralKeys", ]; - const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 13] = [ + const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 14] = [ 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[10]), + 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[12]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[15]), + 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[19]), + 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[21]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[22]), ]; - const ALL_RULES_AS_FILTERS: [RuleFilter<'static>; 24] = [ + const ALL_RULES_AS_FILTERS: [RuleFilter<'static>; 25] = [ 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]), @@ -2012,6 +2019,7 @@ impl Nursery { 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]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[24]), ]; #[doc = r" Retrieves the recommended rules"] pub(crate) fn is_recommended(&self) -> bool { matches!(self.recommended, Some(true)) } @@ -2067,81 +2075,86 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[8])); } } - 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[9])); } } - 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[10])); } } - 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[11])); } } - 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[12])); } } - 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[13])); } } - 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[14])); } } - 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[15])); } } - 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[16])); } } - 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[17])); } } - 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[18])); } } - 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[19])); } } - 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[20])); } } - 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[21])); } } - 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[22])); } } - 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[23])); } } + 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[24])); + } + } index_set } pub(crate) fn get_disabled_rules(&self) -> IndexSet { @@ -2191,81 +2204,86 @@ impl Nursery { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[8])); } } - 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[9])); } } - 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[10])); } } - 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[11])); } } - 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[12])); } } - 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[13])); } } - 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[14])); } } - 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[15])); } } - 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[16])); } } - 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[17])); } } - 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[18])); } } - 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[19])); } } - 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[20])); } } - 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[21])); } } - 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[22])); } } - 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[23])); } } + 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[24])); + } + } index_set } #[doc = r" Checks if, given a rule name, matches one of the rules contained in this category"] @@ -2274,10 +2292,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>; 13] { + pub(crate) fn recommended_rules_as_filters() -> [RuleFilter<'static>; 14] { Self::RECOMMENDED_RULES_AS_FILTERS } - pub(crate) fn all_rules_as_filters() -> [RuleFilter<'static>; 24] { Self::ALL_RULES_AS_FILTERS } + pub(crate) fn all_rules_as_filters() -> [RuleFilter<'static>; 25] { Self::ALL_RULES_AS_FILTERS } #[doc = r" Select preset rules"] pub(crate) fn collect_preset_rules( &self, @@ -2307,6 +2325,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(), + "noGlobalIsNan" => self.no_global_is_nan.as_ref(), "noNoninteractiveTabindex" => self.no_noninteractive_tabindex.as_ref(), "noRedundantRoles" => self.no_redundant_roles.as_ref(), "noSelfAssign" => self.no_self_assign.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 b78d96dfb94..b94825b0b39 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", + "noGlobalIsNan", "noNoninteractiveTabindex", "noRedundantRoles", "noSelfAssign", @@ -1553,6 +1554,24 @@ impl VisitNode for Nursery { )); } }, + "noGlobalIsNan" => match value { + AnyJsonValue::JsonStringValue(_) => { + let mut configuration = RuleConfiguration::default(); + self.map_to_known_string(&value, name_text, &mut configuration, diagnostics)?; + self.no_global_is_nan = Some(configuration); + } + AnyJsonValue::JsonObjectValue(_) => { + let mut configuration = RuleConfiguration::default(); + self.map_to_object(&value, name_text, &mut configuration, diagnostics)?; + self.no_global_is_nan = Some(configuration); + } + _ => { + diagnostics.push(DeserializationDiagnostic::new_incorrect_type( + "object or string", + value.range(), + )); + } + }, "noNoninteractiveTabindex" => match value { AnyJsonValue::JsonStringValue(_) => { let mut configuration = RuleConfiguration::default(); diff --git a/editors/vscode/configuration_schema.json b/editors/vscode/configuration_schema.json index 6ddef97007a..97c5debb6ee 100644 --- a/editors/vscode/configuration_schema.json +++ b/editors/vscode/configuration_schema.json @@ -803,6 +803,13 @@ { "type": "null" } ] }, + "noGlobalIsNan": { + "description": "Use Number.isNaN instead of global isNaN.", + "anyOf": [ + { "$ref": "#/definitions/RuleConfiguration" }, + { "type": "null" } + ] + }, "noNoninteractiveTabindex": { "description": "Enforce that tabIndex is not assigned to non-interactive HTML elements.", "anyOf": [ diff --git a/npm/backend-jsonrpc/src/workspace.ts b/npm/backend-jsonrpc/src/workspace.ts index 3394958b938..27194594160 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.isNaN instead of global isNaN. + */ + noGlobalIsNan?: RuleConfiguration; /** * Enforce that tabIndex is not assigned to non-interactive HTML elements. */ @@ -1111,6 +1115,7 @@ export type Category = | "lint/nursery/noStaticOnlyClass" | "lint/nursery/noDuplicateJsonKeys" | "lint/nursery/useNamingConvention" + | "lint/nursery/noGlobalIsNan" | "lint/performance/noDelete" | "lint/security/noDangerouslySetInnerHtml" | "lint/security/noDangerouslySetInnerHtmlWithChildren" diff --git a/npm/rome/configuration_schema.json b/npm/rome/configuration_schema.json index 6ddef97007a..97c5debb6ee 100644 --- a/npm/rome/configuration_schema.json +++ b/npm/rome/configuration_schema.json @@ -803,6 +803,13 @@ { "type": "null" } ] }, + "noGlobalIsNan": { + "description": "Use Number.isNaN instead of global isNaN.", + "anyOf": [ + { "$ref": "#/definitions/RuleConfiguration" }, + { "type": "null" } + ] + }, "noNoninteractiveTabindex": { "description": "Enforce that tabIndex is not assigned to non-interactive HTML elements.", "anyOf": [ diff --git a/website/src/components/generated/NumberOfRules.astro b/website/src/components/generated/NumberOfRules.astro index 096807cd16d..70853d24626 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 143 rules

\ No newline at end of file +

Rome's linter has a total of 144 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 b016819a836..031d2f7084f 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.

+

+ noGlobalIsNan +

+Use Number.isNaN instead of global isNaN. +
+

noNoninteractiveTabindex

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