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

feat(rome_js_analyze): implements useSimpleNumberKeys #4108

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
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 @@ -91,6 +91,7 @@ define_dategories! {
"lint/nursery/noNoninteractiveElementToInteractiveRole": "https://docs.rome.tools/lint/rules/noNoninteractiveElementToInteractiveRole",
"lint/nursery/useValidForDirection": "https://docs.rome.tools/lint/rules/useValidForDirection",
"lint/nursery/useHookAtTopLevel": "https://docs.rome.tools/lint/rules/useHookAtTopLevel",
"lint/nursery/useSimpleNumberKeys": "https://docs.rome.tools/lint/rules/useSimpleNumberKeys",
// Insert new nursery rule here

// performance
Expand Down
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.

283 changes: 283 additions & 0 deletions crates/rome_js_analyze/src/analyzers/nursery/use_simple_number_keys.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,283 @@
use crate::JsRuleAction;
use rome_analyze::{context::RuleContext, declare_rule, ActionCategory, Ast, Rule, RuleDiagnostic};
use rome_console::markup;
use rome_diagnostics::Applicability;
use rome_js_factory::make;
use rome_js_syntax::{JsLiteralMemberName, JsSyntaxKind, JsSyntaxToken};
use rome_rowan::{AstNode, BatchMutationExt};
use std::str::FromStr;

declare_rule! {
/// Disallow number literal object member names which are not base10 or uses underscore as separator
///
/// ## Examples
///
/// ### Invalid
///
/// ```js,expect_diagnostic
/// ({ 0x1: 1 });
/// ```
/// ```js,expect_diagnostic
/// ({ 11_1.11: "ee" });
/// ```
/// ```js,expect_diagnostic
/// ({ 0o1: 1 });
/// ```
/// ```js,expect_diagnostic
/// ({ 1n: 1 });
/// ```
/// ```js,expect_diagnostic
/// ({ 11_1.11: "ee" });
/// ```
///
/// ## Valid
///
/// ```js
/// ({ 0: "zero" });
/// ({ 122: "integer" });
/// ({ 1.22: "floating point" });
/// ({ 3.1e12: "floating point with e" });
/// ```
///
pub(crate) UseSimpleNumberKeys {
version: "next",
name: "useSimpleNumberKeys",
recommended: false,
}
}

#[derive(Clone)]
pub enum NumberLiteral {
Binary {
value: String,
big_int: bool,
},
Decimal {
value: String,
big_int: bool,
underscore: bool,
},
Octal {
value: String,
big_int: bool,
},
Hexadecimal {
value: String,
big_int: bool,
},
FloatingPoint {
value: String,
exponent: bool,
underscore: bool,
},
}

pub struct NumberLiteralError;

impl TryFrom<JsSyntaxToken> for NumberLiteral {
type Error = NumberLiteralError;

fn try_from(token: JsSyntaxToken) -> Result<Self, Self::Error> {
match token.kind() {
JsSyntaxKind::JS_NUMBER_LITERAL | JsSyntaxKind::JS_BIG_INT_LITERAL => {
let chars: Vec<char> = token.to_string().chars().collect();
let mut value = String::new();

let mut is_first_char_zero: bool = false;
let mut is_second_char_a_letter: Option<char> = None;
let mut contains_dot: bool = false;
let mut exponent: bool = false;
let mut largest_digit: char = '0';
let mut underscore: bool = false;
let mut big_int: bool = false;

for i in 0..chars.len() {
if i == 0 && chars[i] == '0' && chars.len() > 1 {
is_first_char_zero = true;
continue;
}

if chars[i] == 'n' {
big_int = true;
break;
}

if chars[i] == 'e' || chars[i] == 'E' {
exponent = true;
}

if i == 1 && chars[i].is_alphabetic() && !exponent {
is_second_char_a_letter = Some(chars[i]);
continue;
}

if chars[i] == '_' {
underscore = true;
continue;
}

if chars[i] == '.' {
contains_dot = true;
}

if largest_digit < chars[i] {
largest_digit = chars[i];
}

value.push(chars[i])
}

if contains_dot {
return Ok(Self::FloatingPoint {
value,
exponent,
underscore,
});
};
if !is_first_char_zero {
return Ok(Self::Decimal {
value,
big_int,
underscore,
});
};

match is_second_char_a_letter {
Some('b' | 'B') => return Ok(Self::Binary { value, big_int }),
Some('o' | 'O') => return Ok(Self::Octal { value, big_int }),
Some('x' | 'X') => return Ok(Self::Hexadecimal { value, big_int }),
_ => (),
}

if largest_digit < '8' {
return Ok(Self::Octal { value, big_int });
}

Ok(Self::Decimal {
value,
big_int,
underscore,
})
}
_ => Err(NumberLiteralError),
}
}
}

impl NumberLiteral {
fn value(&self) -> &String {
match self {
Self::Decimal { value, .. } => value,
Self::Binary { value, .. } => value,
Self::FloatingPoint { value, .. } => value,
Self::Octal { value, .. } => value,
Self::Hexadecimal { value, .. } => value,
}
}
}

impl NumberLiteral {
fn to_base_ten(&self) -> Option<f64> {
match self {
Self::Binary { value, .. } => i64::from_str_radix(value, 2).map(|num| num as f64).ok(),
Self::Decimal { value, .. } | Self::FloatingPoint { value, .. } => {
f64::from_str(value).ok()
}
Self::Octal { value, .. } => i64::from_str_radix(value, 7).map(|num| num as f64).ok(),
Self::Hexadecimal { value, .. } => {
i64::from_str_radix(value, 16).map(|num| num as f64).ok()
}
}
}
}

impl Rule for UseSimpleNumberKeys {
type Query = Ast<JsLiteralMemberName>;
dhrjarun marked this conversation as resolved.
Show resolved Hide resolved
type State = NumberLiteral;
type Signals = Vec<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let mut signals: Self::Signals = Vec::new();
let node = ctx.query();

if let Ok(token) = node.value() {
let number_literal = NumberLiteral::try_from(token).ok();

if let Some(number_literal) = number_literal {
match number_literal {
NumberLiteral::Decimal { big_int: true, .. }
| NumberLiteral::Decimal {
underscore: true, ..
} => signals.push(number_literal),
NumberLiteral::FloatingPoint {
underscore: true, ..
} => signals.push(number_literal),
NumberLiteral::Binary { .. } => signals.push(number_literal),
NumberLiteral::Hexadecimal { .. } => signals.push(number_literal),
NumberLiteral::Octal { .. } => signals.push(number_literal),
_ => (),
}
}
}

signals
}

fn diagnostic(
_ctx: &RuleContext<Self>,
number_literal: &Self::State,
) -> Option<RuleDiagnostic> {
let title = match number_literal {
NumberLiteral::Decimal { big_int: true, .. } => "Bigint is not allowed here.",
NumberLiteral::Decimal {
underscore: true, ..
} => "Number literal with underscore is not allowed here.",
NumberLiteral::FloatingPoint {
underscore: true, ..
} => "Number literal with underscore is not allowed.",
NumberLiteral::Binary { .. } => "Binary number literal in is not allowed here.",
NumberLiteral::Hexadecimal { .. } => "Hexadecimal number literal is not allowed here.",
NumberLiteral::Octal { .. } => "Octal number literal is not allowed here.",
_ => "",
};
dhrjarun marked this conversation as resolved.
Show resolved Hide resolved

let diagnostic =
RuleDiagnostic::new(rule_category!(), _ctx.query().range(), title.to_string());

Some(diagnostic)
}

fn action(ctx: &RuleContext<Self>, number_literal: &Self::State) -> Option<JsRuleAction> {
let mut mutation = ctx.root().begin();
let node = ctx.query();
let message;
dhrjarun marked this conversation as resolved.
Show resolved Hide resolved

if let Ok(token) = node.value() {
dhrjarun marked this conversation as resolved.
Show resolved Hide resolved
match number_literal {
dhrjarun marked this conversation as resolved.
Show resolved Hide resolved
NumberLiteral::Binary { .. }
| NumberLiteral::Octal { .. }
| NumberLiteral::Hexadecimal { .. } => {
let text = number_literal.to_base_ten()?;
message = markup! ("Replace "{ node.to_string() } " with "{text.to_string()})
.to_owned();
mutation.replace_token(token, make::js_number_literal(text));
}
NumberLiteral::FloatingPoint { .. } | NumberLiteral::Decimal { .. } => {
let text = number_literal.value();
message = markup! ("Replace "{ node.to_string() } " with "{text}).to_owned();
mutation.replace_token(token, make::js_number_literal(text));
}
};

return Some(JsRuleAction {
category: ActionCategory::QuickFix,
applicability: Applicability::Always,
message,
mutation,
});
}

None
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,6 @@

// valid for now

// ESLint already catches properties keyed with different-formatted number literals, we haven't implemented it yet.
({ 0x1: 1, 1: 2 });
({ 012: 1, 10: 2 });
({ 0b1: 1, 1: 2 });
({ 0o1: 1, 1: 2 });
({ 1n: 1, 1: 2 });
({ 1_0: 1, 10: 2 });

// This particular simple computed property case with just a string literal would be easy to catch,
// but we don't want to open Pandora's static analysis box so we have to draw a line somewhere
({ a: 1, ["a"]: 1 });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,6 @@ expression: noDuplicateObjectKeys.js

// valid for now

// ESLint already catches properties keyed with different-formatted number literals, we haven't implemented it yet.
({ 0x1: 1, 1: 2 });
({ 012: 1, 10: 2 });
({ 0b1: 1, 1: 2 });
({ 0o1: 1, 1: 2 });
({ 1n: 1, 1: 2 });
({ 1_0: 1, 10: 2 });

// This particular simple computed property case with just a string literal would be easy to catch,
// but we don't want to open Pandora's static analysis box so we have to draw a line somewhere
({ a: 1, ["a"]: 1 });
Expand Down
16 changes: 16 additions & 0 deletions crates/rome_js_analyze/tests/specs/nursery/useSimpleNumberKeys.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// valid
({ 0: "zero" });
({ 1: "one" });
({ 1.2: "12" });
({ 3.1e12: "12" });
({ 0.1e12: "ee" });

// invalid
({ 1n: 1 });
({ 0x1: 1 });
({ 012: 1 });
({ 0b1: 1 });
({ 0o1: 1 });
({ 1_0: 1 });
({ 0.1e1_2: "ed" });
({ 11_1.11: "ee" });
Loading