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

Commit

Permalink
feat(rome_js_analyze): Implement prefer-numeric-literals lint (#3558)
Browse files Browse the repository at this point in the history
  • Loading branch information
95th committed Nov 14, 2022
1 parent 745bdaa commit c4797d3
Show file tree
Hide file tree
Showing 22 changed files with 2,051 additions and 120 deletions.
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("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("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("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("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.

199 changes: 199 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,199 @@
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, JsAnyMemberExpression, 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_constant()?;
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> {
let callee = expr.callee().ok()?.omit_parentheses();
if let Some(id) = callee.as_reference_identifier() {
if id.has_name("parseInt") && model.declaration(&id).is_none() {
return Some("parseInt()");
}
}

let callee = JsAnyMemberExpression::cast_ref(callee.syntax())?;
let object = callee.get_object_reference_identifier()?;
if object.has_name("Number")
&& model.declaration(&object).is_none()
&& callee.has_member_name("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().pieces().next().is_none() {
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().pieces().next().is_none() {
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

0 comments on commit c4797d3

Please sign in to comment.