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

feat(rome_js_analyze): Implement prefer-numeric-literals lint #3558

Merged
merged 51 commits into from
Nov 14, 2022
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
07b03e7
Implement prefer-numeric-literals rule
95th Nov 3, 2022
6c460fe
reorder
95th Nov 3, 2022
8766225
lint
95th Nov 3, 2022
d49e7a9
bindings
95th Nov 3, 2022
b696b25
update
95th Nov 4, 2022
d9bc5e6
codegen schema
95th Nov 4, 2022
5e41c58
update
95th Nov 4, 2022
5af1d47
bug fix
95th Nov 4, 2022
11a7d83
fix
95th Nov 4, 2022
aa319ce
refactor
95th Nov 4, 2022
d692856
update test
95th Nov 4, 2022
1c74868
update
95th Nov 4, 2022
538d934
Merge remote-tracking branch 'origin/main' into prefer_numeric_literal
95th Nov 4, 2022
52b03dc
review
95th Nov 5, 2022
c8131f8
refactor
95th Nov 5, 2022
d1ea461
fix space issue
95th Nov 5, 2022
d8531af
remove parentheses
95th Nov 5, 2022
a5aab1a
fix space issue
95th Nov 5, 2022
8bf348c
refactor
95th Nov 5, 2022
02b5163
refactor to use util method
95th Nov 5, 2022
f97ee10
rename
95th Nov 5, 2022
5259cba
reuse code
95th Nov 5, 2022
01e8cdd
bad commit
95th Nov 5, 2022
4db633c
lint
95th Nov 5, 2022
6dd8ad3
peel parentheses from obj
95th Nov 6, 2022
b8553fd
refactor
95th Nov 6, 2022
65f814c
Merge remote-tracking branch 'origin/main' into prefer_numeric_literal
95th Nov 6, 2022
205e46b
Merge remote-tracking branch 'origin/main' into prefer_numeric_literal
95th Nov 6, 2022
9c16f2f
fix
95th Nov 6, 2022
cd12996
Merge remote-tracking branch 'origin/main' into prefer_numeric_literal
95th Nov 8, 2022
a0b7df6
fix from review
95th Nov 8, 2022
bb02336
Fix, Refactor, Comments
95th Nov 8, 2022
90bd738
maybe incorrect
95th Nov 8, 2022
aaa828c
update docs
95th Nov 8, 2022
f3efaae
Update
95th Nov 8, 2022
2c87fb9
fix
95th Nov 8, 2022
02216b6
refactor
95th Nov 8, 2022
bb0fee1
split tests
95th Nov 8, 2022
9949404
update
95th Nov 8, 2022
b5457ed
Merge remote-tracking branch 'origin/main' into prefer_numeric_literal
95th Nov 8, 2022
e9e94a5
clippy
95th Nov 9, 2022
953c1bb
refactor
95th Nov 9, 2022
aaf98db
Merge remote-tracking branch 'origin/main' into prefer_numeric_literal
95th Nov 9, 2022
7fd00e0
fix from main
95th Nov 9, 2022
dc5ade8
review
95th Nov 9, 2022
182b223
fix
95th Nov 10, 2022
f1ba75c
update
95th Nov 10, 2022
1843288
Merge remote-tracking branch 'origin/main' into prefer_numeric_literal
95th Nov 10, 2022
384541b
fix
95th Nov 10, 2022
ea3c47d
update
95th Nov 12, 2022
d302f51
Merge remote-tracking branch 'origin/main' into prefer_numeric_literal
95th Nov 14, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/rome_diagnostics_categories/src/categories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ define_dategories! {
"lint/nursery/useExhaustiveDependencies": "https://docs.rome.tools/lint/rules/useExhaustiveDependencies",
"lint/nursery/useCamelCase": "https://docs.rome.tools/lint/rules/useCamelCase",
"lint/nursery/noBannedTypes":"https://docs.rome.tools/lint/rules/noBannedTypes",
"lint/nursery/useNumericLiterals": "https://docs.rome.tools/lint/rules/useNumericLiterals",

;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,16 +103,10 @@ fn is_in_boolean_context(node: &JsSyntaxNode) -> Option<bool> {
/// The checking algorithm of [JsNewExpression] is a little different from [JsCallExpression] due to
/// two nodes have different shapes
fn is_boolean_constructor_call(node: &JsSyntaxNode) -> Option<bool> {
let callee = JsCallArgumentList::cast(node.clone())?
let expr = JsCallArgumentList::cast(node.clone())?
.parent::<JsCallArguments>()?
.parent::<JsNewExpression>()?
.callee()
.ok()?;
if let JsAnyExpression::JsIdentifierExpression(ident) = callee {
Some(ident.name().ok()?.syntax().text_trimmed() == "Boolean")
} else {
None
}
.parent::<JsNewExpression>()?;
Some(expr.has_callee(|it| it.has_name("Boolean")))
}

/// Check if the SyntaxNode is a `Boolean` Call Expression
Expand All @@ -121,12 +115,8 @@ fn is_boolean_constructor_call(node: &JsSyntaxNode) -> Option<bool> {
/// Boolean(x)
/// ```
fn is_boolean_call(node: &JsSyntaxNode) -> Option<bool> {
let callee = JsCallExpression::cast(node.clone())?.callee().ok()?;
if let JsAnyExpression::JsIdentifierExpression(ident) = callee {
Some(ident.name().ok()?.syntax().text_trimmed() == "Boolean")
} else {
None
}
let expr = JsCallExpression::cast(node.clone())?;
Some(expr.has_callee(|it| it.has_name("Boolean")))
}

/// Check if the SyntaxNode is a Negate Unary Expression
Expand Down Expand Up @@ -169,21 +159,18 @@ impl Rule for NoExtraBooleanCast {
// Convert `Boolean(x)` -> `x` if parent `SyntaxNode` in any boolean `Type Coercion` context
// Only if `Boolean` Call Expression have one `JsAnyExpression` argument
if let Some(expr) = JsCallExpression::cast(syntax.clone()) {
let callee = expr.callee().ok()?;
if let JsAnyExpression::JsIdentifierExpression(ident) = callee {
if ident.name().ok()?.syntax().text_trimmed() == "Boolean" {
let arguments = expr.arguments().ok()?;
let len = arguments.args().len();
if len == 1 {
return arguments
.args()
.into_iter()
.next()?
.ok()
.map(|item| item.into_syntax())
.and_then(JsAnyExpression::cast)
.map(|expr| (expr, ExtraBooleanCastType::BooleanCall));
}
if expr.has_callee(|it| it.has_name("Boolean")) {
let arguments = expr.arguments().ok()?;
let len = arguments.args().len();
if len == 1 {
return arguments
.args()
.into_iter()
.next()?
.ok()
.map(|item| item.into_syntax())
.and_then(JsAnyExpression::cast)
.map(|expr| (expr, ExtraBooleanCastType::BooleanCall));
}
}
return None;
Expand All @@ -192,21 +179,18 @@ impl Rule for NoExtraBooleanCast {
// Convert `new Boolean(x)` -> `x` if parent `SyntaxNode` in any boolean `Type Coercion` context
// Only if `Boolean` Call Expression have one `JsAnyExpression` argument
return JsNewExpression::cast(syntax).and_then(|expr| {
let callee = expr.callee().ok()?;
if let JsAnyExpression::JsIdentifierExpression(ident) = callee {
if ident.name().ok()?.syntax().text_trimmed() == "Boolean" {
let arguments = expr.arguments()?;
let len = arguments.args().len();
if len == 1 {
return arguments
.args()
.into_iter()
.next()?
.ok()
.map(|item| item.into_syntax())
.and_then(JsAnyExpression::cast)
.map(|expr| (expr, ExtraBooleanCastType::BooleanCall));
}
if expr.has_callee(|it| it.has_name("Boolean")) {
let arguments = expr.arguments()?;
let len = arguments.args().len();
if len == 1 {
return arguments
.args()
.into_iter()
.next()?
.ok()
.map(|item| item.into_syntax())
.and_then(JsAnyExpression::cast)
.map(|expr| (expr, ExtraBooleanCastType::BooleanCall));
}
}
None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,13 @@ impl Rule for NoAsyncPromiseExecutor {
/// ((((((async function () {}))))))
/// ```
fn get_async_function_expression_like(expr: &JsAnyExpression) -> Option<JsAnyFunction> {
match expr {
match expr.clone().omit_parentheses() {
JsAnyExpression::JsFunctionExpression(func) => func
.async_token()
.map(|_| JsAnyFunction::JsFunctionExpression(func.clone())),
JsAnyExpression::JsArrowFunctionExpression(func) => func
.async_token()
.map(|_| JsAnyFunction::JsArrowFunctionExpression(func.clone())),
JsAnyExpression::JsParenthesizedExpression(expr) => {
let inner_expression = expr.expression().ok()?;
get_async_function_expression_like(&inner_expression)
}
_ => None,
}
}
3 changes: 2 additions & 1 deletion crates/rome_js_analyze/src/analyzers/nursery.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

190 changes: 190 additions & 0 deletions crates/rome_js_analyze/src/analyzers/nursery/use_numeric_literals.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
use crate::semantic_services::Semantic;
use crate::{ast_utils, JsRuleAction};
use rome_analyze::context::RuleContext;
use rome_analyze::{declare_rule, ActionCategory, Rule, RuleDiagnostic};
use rome_console::markup;
use rome_diagnostics::Applicability;
use rome_js_factory::make;
use rome_js_semantic::SemanticModel;
use rome_js_syntax::{JsAnyExpression, JsAnyLiteralExpression, JsCallExpression, JsSyntaxToken};
use rome_rowan::{AstNode, AstSeparatedList, BatchMutationExt};

declare_rule! {
/// Disallow `parseInt()` and `Number.parseInt()` in favor of binary, octal, and hexadecimal literals
///
/// ## Examples
///
/// ### Invalid
///
/// ```js,expect_diagnostic
/// parseInt("111110111", 2) === 503;
/// ```
///
/// ```js,expect_diagnostic
/// Number.parseInt("767", 8) === 503;
/// ```
/// ### Valid
///
/// ```js
/// parseInt(1);
/// parseInt(1, 3);
/// Number.parseInt(1);
/// Number.parseInt(1, 3);
///
/// 0b111110111 === 503;
/// 0o767 === 503;
/// 0x1F7 === 503;
///
/// a[parseInt](1,2);
///
/// parseInt(foo);
/// parseInt(foo, 2);
/// Number.parseInt(foo);
/// Number.parseInt(foo, 2);
/// ```
pub(crate) UseNumericLiterals {
version: "11.0.0",
name: "useNumericLiterals",
recommended: false,
}
}

pub struct CallInfo {
callee: &'static str,
text: String,
radix: Radix,
}

impl Rule for UseNumericLiterals {
type Query = Semantic<JsCallExpression>;
type State = CallInfo;
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Option<Self::State> {
let expr = ctx.query();
let model = ctx.model();
CallInfo::try_from_expr(expr, model)
}

fn diagnostic(ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
let node = ctx.query();

Some(RuleDiagnostic::new(
rule_category!(),
node.range(),
markup! { "Use "{state.radix.description()}" literals instead of "{state.callee} }
.to_owned(),
))
}

fn action(ctx: &RuleContext<Self>, call: &Self::State) -> Option<JsRuleAction> {
let node = ctx.query();
let mut mutation = ctx.root().begin();

let number = call.to_numeric_literal()?;
let number = ast_utils::token_with_source_trivia(number, node);

mutation.replace_node_discard_trivia(
JsAnyExpression::JsCallExpression(node.clone()),
JsAnyExpression::JsAnyLiteralExpression(
JsAnyLiteralExpression::JsNumberLiteralExpression(
make::js_number_literal_expression(number),
),
),
);

Some(JsRuleAction {
category: ActionCategory::QuickFix,
applicability: Applicability::MaybeIncorrect,
message: markup! { "Replace with "{call.radix.description()}" literals" }.to_owned(),
mutation,
})
}
}

impl CallInfo {
fn try_from_expr(expr: &JsCallExpression, model: &SemanticModel) -> Option<CallInfo> {
let args = expr.arguments().ok()?.args();
if args.len() != 2 {
return None;
}
let mut args = args.into_iter();
let text = args
.next()?
.ok()?
.as_js_any_expression()?
.as_string_or_no_substitution_template()?;
let radix = args
.next()?
.ok()?
.as_js_any_expression()?
.as_js_any_literal_expression()?
.as_js_number_literal_expression()?
.as_number()?;
let callee = get_callee(expr, model)?;

Some(CallInfo {
callee,
text,
radix: Radix::from_f64(radix)?,
})
}

fn to_numeric_literal(&self) -> Option<JsSyntaxToken> {
i128::from_str_radix(&self.text, self.radix as u32).ok()?;
let number = format!("{}{}", self.radix.prefix(), self.text);
let number = make::js_number_literal(&number);
Some(number)
}
}

fn get_callee(expr: &JsCallExpression, model: &SemanticModel) -> Option<&'static str> {
if expr.has_callee(|it| it.has_name("parseInt") && model.declaration(&it).is_none()) {
return Some("parseInt()");
}
if expr.callee().ok()?.is_member_access(
|it| it.has_name("Number") && model.declaration(&it).is_none(),
"parseInt",
) {
return Some("Number.parseInt()");
}
None
}

#[derive(Copy, Clone)]
enum Radix {
Binary = 2,
Octal = 8,
Hexadecimal = 16,
}

impl Radix {
fn from_f64(v: f64) -> Option<Self> {
Some(if v == 2.0 {
Self::Binary
} else if v == 8.0 {
Self::Octal
} else if v == 16.0 {
Self::Hexadecimal
} else {
return None;
})
}

fn prefix(&self) -> &'static str {
match self {
Radix::Binary => "0b",
Radix::Octal => "0o",
Radix::Hexadecimal => "0x",
}
}

fn description(&self) -> &'static str {
match self {
Radix::Binary => "binary",
Radix::Octal => "octal",
Radix::Hexadecimal => "hexadecimal",
}
}
}
53 changes: 53 additions & 0 deletions crates/rome_js_analyze/src/ast_utils.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
use rome_js_syntax::{JsLanguage, JsSyntaxNode, JsSyntaxToken, TriviaPieceKind};
use rome_rowan::{AstNode, TriviaPiece};

/// Add any leading and trailing trivia from given source node to the token.
///
/// Adds whitespace trivia if needed for safe replacement of source node.
pub fn token_with_source_trivia<T>(token: JsSyntaxToken, source: &T) -> JsSyntaxToken
where
T: AstNode<Language = JsLanguage>,
{
let mut text = String::new();
let node = source.syntax();
let mut leading = vec![];
let mut trailing = vec![];

add_leading_trivia(&mut leading, &mut text, node);
text.push_str(token.text());
add_trailing_trivia(&mut trailing, &mut text, node);

JsSyntaxToken::new_detached(token.kind(), &text, leading, trailing)
}

fn add_leading_trivia(trivia: &mut Vec<TriviaPiece>, text: &mut String, node: &JsSyntaxNode) {
let Some(token) = node.first_token() else { return };
for t in token.leading_trivia().pieces() {
text.push_str(t.text());
trivia.push(TriviaPiece::new(t.kind(), t.text_len()));
}
if !trivia.is_empty() {
return;
}
let Some(token) = token.prev_token() else { return };
if !token.kind().is_punct() && token.trailing_trivia().text().is_empty() {
95th marked this conversation as resolved.
Show resolved Hide resolved
text.push(' ');
trivia.push(TriviaPiece::new(TriviaPieceKind::Whitespace, 1));
}
}

fn add_trailing_trivia(trivia: &mut Vec<TriviaPiece>, text: &mut String, node: &JsSyntaxNode) {
let Some(token) = node.last_token() else { return };
for t in token.trailing_trivia().pieces() {
text.push_str(t.text());
trivia.push(TriviaPiece::new(t.kind(), t.text_len()));
}
if !trivia.is_empty() {
return;
}
let Some(token) = token.next_token() else { return };
if !token.kind().is_punct() && token.leading_trivia().text().is_empty() {
text.push(' ');
trivia.push(TriviaPiece::new(TriviaPieceKind::Whitespace, 1));
}
}
1 change: 1 addition & 0 deletions crates/rome_js_analyze/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use std::{borrow::Cow, error::Error};

mod analyzers;
mod assists;
mod ast_utils;
mod control_flow;
pub mod globals;
mod react;
Expand Down
Loading