Skip to content
This repository was archived by the owner on Aug 31, 2023. It is now read-only.

Commit 2530562

Browse files
ConaclosConaclos
committed
feat(rome_js_analyze): add noGlobalIsFinite
1 parent e2ffdf6 commit 2530562

File tree

18 files changed

+693
-57
lines changed

18 files changed

+693
-57
lines changed

‎CHANGELOG.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,11 @@ multiple files:
8383

8484
#### New rules
8585

86-
- Add [`noGlobalThis`](https://docs.rome.tools/lint/rules/noglobalthis/)
86+
- Add [`noGlobalIsFinite`](https://docs.rome.tools/lint/rules/noglobalisfinite/)
87+
88+
This rule recommends using `Number.isFinite` instead of the global and unsafe `isFinite` that attempts a type coercion.
89+
90+
- Add [`noGlobalIsNan`](https://docs.rome.tools/lint/rules/noglobalisnan/)
8791

8892
This rule recommends using `Number.isNaN` instead of the global and unsafe `isNaN` that attempts a type coercion.
8993

‎crates/rome_diagnostics_categories/src/categories.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ define_categories! {
100100
"lint/nursery/noDuplicateJsonKeys": "https://docs.rome.tools/lint/rules/noDuplicateJsonKeys",
101101
"lint/nursery/useNamingConvention": "https://docs.rome.tools/lint/rules/useNamingConvention",
102102
"lint/nursery/noGlobalIsNan": "https://docs.rome.tools/lint/rules/noGlobalIsNan",
103+
"lint/nursery/noGlobalIsFinite": "https://docs.rome.tools/lint/rules/noGlobalIsFinite",
103104
// Insert new nursery rule here
104105

105106

‎crates/rome_js_analyze/src/semantic_analyzers/nursery.rs‎

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/rome_js_analyze/src/semantic_analyzers/nursery/no_global_is_finite.rs‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
use crate::{semantic_services::Semantic, JsRuleAction};
2+
use rome_analyze::{context::RuleContext, declare_rule, ActionCategory, Rule, RuleDiagnostic};
3+
use rome_console::markup;
4+
use rome_diagnostics::Applicability;
5+
use rome_js_factory::make;
6+
use rome_js_syntax::{AnyJsExpression, T};
7+
use rome_rowan::{AstNode, BatchMutationExt};
8+
9+
declare_rule! {
10+
/// Use `Number.isFinite` instead of global `isFinite`.
11+
///
12+
/// `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).
13+
/// When the argument to `isFinite()` is not a number, the value is first coerced to a number.
14+
/// `Number.isFinite()` does not perform this coercion.
15+
/// Therefore, it is a more reliable way to test whether a number is finite.
16+
///
17+
/// ## Examples
18+
///
19+
/// ### Invalid
20+
///
21+
/// ```js,expect_diagnostic
22+
/// isFinite(false); // true
23+
/// ```
24+
///
25+
/// ## Valid
26+
///
27+
/// ```js
28+
/// Number.isFinite(false); // false
29+
/// ```
30+
pub(crate) NoGlobalIsFinite {
31+
version: "next",
32+
name: "noGlobalIsFinite",
33+
recommended: true,
34+
}
35+
}
36+
37+
impl Rule for NoGlobalIsFinite {
38+
type Query = Semantic<AnyJsExpression>;
39+
type State = ();
40+
type Signals = Option<Self::State>;
41+
type Options = ();
42+
43+
fn run(ctx: &RuleContext<Self>) -> Self::Signals {
44+
let node = ctx.query();
45+
let model = ctx.model();
46+
match node {
47+
AnyJsExpression::JsIdentifierExpression(expression) => {
48+
let name = expression.name().ok()?;
49+
if name.has_name("isFinite") && model.binding(&name).is_none() {
50+
return Some(());
51+
}
52+
}
53+
AnyJsExpression::JsStaticMemberExpression(expression) => {
54+
let object_name = expression
55+
.object()
56+
.ok()?
57+
.omit_parentheses()
58+
.as_js_identifier_expression()?
59+
.name()
60+
.ok()?;
61+
let member = expression.member().ok()?;
62+
if object_name.is_global_this()
63+
&& model.binding(&object_name).is_none()
64+
&& member.as_js_name()?.value_token().ok()?.text_trimmed() == "isFinite"
65+
{
66+
return Some(());
67+
}
68+
}
69+
AnyJsExpression::JsComputedMemberExpression(expression) => {
70+
let object_name = expression
71+
.object()
72+
.ok()?
73+
.omit_parentheses()
74+
.as_js_identifier_expression()?
75+
.name()
76+
.ok()?;
77+
let member = expression.member().ok()?.omit_parentheses();
78+
let member = member
79+
.as_any_js_literal_expression()?
80+
.as_js_string_literal_expression()?;
81+
if object_name.is_global_this()
82+
&& model.binding(&object_name).is_none()
83+
&& member.inner_string_text().ok()?.text() == "isFinite"
84+
{
85+
return Some(());
86+
}
87+
}
88+
_ => (),
89+
}
90+
None
91+
}
92+
93+
fn diagnostic(ctx: &RuleContext<Self>, _: &Self::State) -> Option<RuleDiagnostic> {
94+
let node = ctx.query();
95+
Some(
96+
RuleDiagnostic::new(
97+
rule_category!(),
98+
node.range(),
99+
markup! {
100+
<Emphasis>"isFinite"</Emphasis>" is unsafe. It attempts a type coercion. Use "<Emphasis>"Number.isFinite"</Emphasis>" instead."
101+
},
102+
)
103+
.note(markup! {
104+
"See "<Hyperlink href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/isFinite#description">"the MDN documentation"</Hyperlink>" for more details."
105+
}),
106+
)
107+
}
108+
109+
fn action(ctx: &RuleContext<Self>, _: &Self::State) -> Option<JsRuleAction> {
110+
let node = ctx.query();
111+
let mut mutation = ctx.root().begin();
112+
let (old, new) = match node {
113+
AnyJsExpression::JsIdentifierExpression(expression) => (
114+
node.clone(),
115+
make::js_static_member_expression(
116+
make::js_identifier_expression(make::js_reference_identifier(make::ident(
117+
"Number",
118+
)))
119+
.into(),
120+
make::token(T![.]),
121+
make::js_name(expression.name().ok()?.value_token().ok()?).into(),
122+
),
123+
),
124+
AnyJsExpression::JsStaticMemberExpression(expression) => (
125+
node.clone(),
126+
make::js_static_member_expression(
127+
make::js_static_member_expression(
128+
expression.object().ok()?,
129+
make::token(T![.]),
130+
make::js_name(make::ident("Number")).into(),
131+
)
132+
.into(),
133+
expression.operator_token().ok()?,
134+
expression.member().ok()?,
135+
),
136+
),
137+
AnyJsExpression::JsComputedMemberExpression(expression) => {
138+
let object = expression.object().ok()?;
139+
(
140+
object.clone(),
141+
make::js_static_member_expression(
142+
object,
143+
make::token(T![.]),
144+
make::js_name(make::ident("Number")).into(),
145+
),
146+
)
147+
}
148+
_ => return None,
149+
};
150+
mutation.replace_node(old, new.into());
151+
Some(JsRuleAction {
152+
category: ActionCategory::QuickFix,
153+
applicability: Applicability::MaybeIncorrect,
154+
message: markup! {
155+
"Use "<Emphasis>"Number.isFinite"</Emphasis>" instead."
156+
}
157+
.to_owned(),
158+
mutation,
159+
})
160+
}
161+
}

‎crates/rome_js_analyze/src/semantic_analyzers/nursery/no_global_is_nan.rs‎

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ impl Rule for NoGlobalIsNan {
8181
.as_js_string_literal_expression()?;
8282
if object_name.is_global_this()
8383
&& model.binding(&object_name).is_none()
84-
&& member.value_token().ok()?.text_trimmed() == "isNaN"
84+
&& member.inner_string_text().ok()?.text() == "isNaN"
8585
{
8686
return Some(());
8787
}
@@ -110,15 +110,45 @@ impl Rule for NoGlobalIsNan {
110110
fn action(ctx: &RuleContext<Self>, _: &Self::State) -> Option<JsRuleAction> {
111111
let node = ctx.query();
112112
let mut mutation = ctx.root().begin();
113-
let number_constructor =
114-
make::js_identifier_expression(make::js_reference_identifier(make::ident("Number")));
115-
let is_nan_name = make::js_name(make::ident("isNaN"));
116-
let expression = make::js_static_member_expression(
117-
number_constructor.into(),
118-
make::token(T![.]),
119-
is_nan_name.into(),
120-
);
121-
mutation.replace_node(node.clone(), expression.into());
113+
let (old, new) = match node {
114+
AnyJsExpression::JsIdentifierExpression(expression) => (
115+
node.clone(),
116+
make::js_static_member_expression(
117+
make::js_identifier_expression(make::js_reference_identifier(make::ident(
118+
"Number",
119+
)))
120+
.into(),
121+
make::token(T![.]),
122+
make::js_name(expression.name().ok()?.value_token().ok()?).into(),
123+
),
124+
),
125+
AnyJsExpression::JsStaticMemberExpression(expression) => (
126+
node.clone(),
127+
make::js_static_member_expression(
128+
make::js_static_member_expression(
129+
expression.object().ok()?,
130+
make::token(T![.]),
131+
make::js_name(make::ident("Number")).into(),
132+
)
133+
.into(),
134+
expression.operator_token().ok()?,
135+
expression.member().ok()?,
136+
),
137+
),
138+
AnyJsExpression::JsComputedMemberExpression(expression) => {
139+
let object = expression.object().ok()?;
140+
(
141+
object.clone(),
142+
make::js_static_member_expression(
143+
object,
144+
make::token(T![.]),
145+
make::js_name(make::ident("Number")).into(),
146+
),
147+
)
148+
}
149+
_ => return None,
150+
};
151+
mutation.replace_node(old, new.into());
122152
Some(JsRuleAction {
123153
category: ActionCategory::QuickFix,
124154
applicability: Applicability::MaybeIncorrect,

‎crates/rome_js_analyze/tests/specs/nursery/noGlobalIsFinite/invalid.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
isFinite({});
2+
3+
(isFinite)({});
4+
5+
globalThis.isFinite({});
6+
7+
(globalThis).isFinite({});
8+
9+
globalThis["isFinite"]({});
10+
11+
(globalThis)[("isFinite")]({});
12+
13+
function localIsNaN(isFinite) {
14+
globalThis.isFinite({});
15+
}
16+
17+
localIsNaN(isFinite);

0 commit comments

Comments
 (0)