From c1439bea0f9bdfaabb89ea4344f39949c239877e Mon Sep 17 00:00:00 2001 From: Sysix Date: Wed, 7 Aug 2024 22:32:59 +0200 Subject: [PATCH 01/23] feat(linter): implement eslint/no-magic-numbers --- crates/oxc_linter/src/rules.rs | 4 +- .../src/rules/eslint/no_magic_numbers.rs | 481 ++++++++++++++++++ 2 files changed, 484 insertions(+), 1 deletion(-) create mode 100644 crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 1bd2453bb2467..150db37cdd73a 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -79,6 +79,7 @@ mod eslint { pub mod no_iterator; pub mod no_label_var; pub mod no_loss_of_precision; + pub mod no_magic_numbers; pub mod no_multi_str; pub mod no_new; pub mod no_new_native_nonconstructor; @@ -474,7 +475,6 @@ oxc_macros::declare_all_lint_rules! { eslint::no_caller, eslint::no_case_declarations, eslint::no_class_assign, - eslint::no_multi_str, eslint::no_label_var, eslint::require_await, eslint::no_compare_neg_zero, @@ -509,6 +509,8 @@ oxc_macros::declare_all_lint_rules! { eslint::no_irregular_whitespace, eslint::no_iterator, eslint::no_loss_of_precision, + eslint::no_magic_numbers, + eslint::no_multi_str, eslint::no_new, eslint::no_new_wrappers, eslint::no_nonoctal_decimal_escape, diff --git a/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs b/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs new file mode 100644 index 0000000000000..dbc3164cf37c2 --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs @@ -0,0 +1,481 @@ +use oxc_ast::AstKind; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; +use oxc_syntax::operator::UnaryOperator; + +use crate::{ + context::LintContext, + fixer::{RuleFix, RuleFixer}, + rule::Rule, + AstNode, +}; + +#[derive(Debug, Clone)] +pub struct NoMagicNumbers { + ignore: Vec, + ignore_array_indexes: bool, + ignore_default_values: bool, + ignore_class_field_initial_values: bool, + enforce_const: bool, + detect_objects: bool, +} + +impl Default for NoMagicNumbers { + fn default() -> Self { + Self { + ignore: vec![], + ignore_array_indexes: false, + ignore_default_values: false, + ignore_class_field_initial_values: false, + enforce_const: false, + detect_objects: false + } + } +} + +declare_oxc_lint!( + /// ### What it does + /// + /// + /// ### Why is this bad? + /// + /// + /// ### Example + /// ```javascript + /// ``` + NoMagicNumbers, + nursery, // TODO: change category to `correctness`, `suspicious`, `pedantic`, `perf`, `restriction`, or `style` + // See for details + + pending // TODO: describe fix capabilities. Remove if no fix can be done, + // keep at 'pending' if you think one could be added but don't know how. + // Options are 'fix', 'fix-dangerous', 'suggestion', and 'suggestion-dangerous' +); + +struct InternConfig<'a> { + node: &'a AstNode<'a>, + value: f64, + raw: String +} + +impl Rule for NoMagicNumbers { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + + match node.kind() { + AstKind::NumericLiteral(literal) => { + let parent_node = ctx.nodes().parent_node(node.id()); + + let intern_config: InternConfig = match parent_node.unwrap().kind() { + AstKind::UnaryExpression(unary) if unary.operator == UnaryOperator::UnaryNegation => { + InternConfig { + node: parent_node.unwrap(), + value: 0.0 - literal.value, + raw: format!("-{}", literal.raw) + } + }, + _ => { + InternConfig { + node, + value: literal.value, + raw: literal.raw.to_string() + } + } + }; + + if self.is_ignore_value(&intern_config.value) { + return; + } + }, + _ => {} + } + } +} + +impl NoMagicNumbers { + + fn is_ignore_value(&self, number: &f64) -> bool { + self.ignore.contains(number) + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let ignore_array_indexes = Some(serde_json::json!([{"ignoreArrayIndexes": true}])); + let ignore_default_values = Some(serde_json::json!([{ "ignoreDefaultValues": true }])); + + let pass = vec![ + ("var x = parseInt(y, 10);", None), + ("var x = parseInt(y, -10);", None), + ("var x = Number.parseInt(y, 10);", None), + ("const foo = 42;", None), // { "ecmaVersion": 6 }, + ( + "var foo = 42;", + Some(serde_json::json!([{ "enforceConst": false }])), + ), // { "ecmaVersion": 6 }, + ("var foo = -42;", None), + ( + "var foo = 0 + 1 - 2 + -2;", + Some(serde_json::json!([{ "ignore": [0, 1, 2, -2] }])), + ), + ( + "var foo = 0 + 1 + 2 + 3 + 4;", + Some(serde_json::json!([{ "ignore": [0, 1, 2, 3, 4] }])), + ), + ("var foo = { bar:10 }", None), + ( + "setTimeout(function() {return 1;}, 0);", + Some(serde_json::json!([{ "ignore": [0, 1] }])), + ), + ( + "var data = ['foo', 'bar', 'baz']; var third = data[3];", + ignore_array_indexes.clone(), + ), + ( + "foo[0]", + ignore_array_indexes.clone(), + ), + ( + "foo[-0]", + ignore_array_indexes.clone(), + ), + ( + "foo[1]", + ignore_array_indexes.clone(), + ), + ( + "foo[100]", + ignore_array_indexes.clone(), + ), + ( + "foo[200.00]", + ignore_array_indexes.clone(), + ), + ( + "foo[3e4]", + ignore_array_indexes.clone(), + ), + ( + "foo[1.23e2]", + ignore_array_indexes.clone(), + ), + ( + "foo[230e-1]", + ignore_array_indexes.clone(), + ), + ( + "foo[0b110]", + ignore_array_indexes.clone(), + ), // { "ecmaVersion": 2015 }, + ( + "foo[0o71]", + ignore_array_indexes.clone(), + ), // { "ecmaVersion": 2015 }, + ( + "foo[0xABC]", + ignore_array_indexes.clone(), + ), + ( + "foo[0123]", + ignore_array_indexes.clone(), + ), // { "sourceType": "script" }, + ( + "foo[5.0000000000000001]", + ignore_array_indexes.clone(), + ), + ( + "foo[4294967294]", + ignore_array_indexes.clone(), + ), + ( + "foo[0n]", + ignore_array_indexes.clone(), + ), // { "ecmaVersion": 2020 }, + ( + "foo[-0n]", + ignore_array_indexes.clone(), + ), // { "ecmaVersion": 2020 }, + ( + "foo[1n]", + ignore_array_indexes.clone(), + ), // { "ecmaVersion": 2020 }, + ( + "foo[100n]", + ignore_array_indexes.clone(), + ), // { "ecmaVersion": 2020 }, + ( + "foo[0xABn]", + ignore_array_indexes.clone(), + ), // { "ecmaVersion": 2020 }, + ( + "foo[4294967294n]", + ignore_array_indexes.clone(), + ), // { "ecmaVersion": 2020 }, + ("var a = ;", None), // { "parserOptions": { "ecmaFeatures": { "jsx": true } } }, + ("var a =
;", None), // { "parserOptions": { "ecmaFeatures": { "jsx": true } } }, + ("f(100n)", Some(serde_json::json!([{ "ignore": ["100n"] }]))), // { "ecmaVersion": 2020 }, + ("f(-100n)", Some(serde_json::json!([{ "ignore": ["-100n"] }]))), // { "ecmaVersion": 2020 }, + ( + "const { param = 123 } = sourceObject;", + ignore_default_values.clone(), + ), // { "ecmaVersion": 6 }, + ( + "const func = (param = 123) => {}", + ignore_default_values.clone(), + ), // { "ecmaVersion": 6 }, + ( + "const func = ({ param = 123 }) => {}", + ignore_default_values.clone(), + ), // { "ecmaVersion": 6 }, + ( + "const [one = 1, two = 2] = []", + ignore_default_values.clone(), + ), // { "ecmaVersion": 6 }, + ( + "var one, two; [one = 1, two = 2] = []", + ignore_default_values.clone(), + ), // { "ecmaVersion": 6 }, + ("var x = parseInt?.(y, 10);", None), // { "ecmaVersion": 2020 }, + ("var x = Number?.parseInt(y, 10);", None), // { "ecmaVersion": 2020 }, + ("var x = (Number?.parseInt)(y, 10);", None), // { "ecmaVersion": 2020 }, + ("foo?.[777]", Some(serde_json::json!([{ "ignoreArrayIndexes": true }]))), // { "ecmaVersion": 2020 }, + ( + "class C { foo = 2; }", + Some(serde_json::json!([{ "ignoreClassFieldInitialValues": true }])), + ), // { "ecmaVersion": 2022 }, + ( + "class C { foo = -2; }", + Some(serde_json::json!([{ "ignoreClassFieldInitialValues": true }])), + ), // { "ecmaVersion": 2022 }, + ( + "class C { static foo = 2; }", + Some(serde_json::json!([{ "ignoreClassFieldInitialValues": true }])), + ), // { "ecmaVersion": 2022 }, + ( + "class C { #foo = 2; }", + Some(serde_json::json!([{ "ignoreClassFieldInitialValues": true }])), + ), // { "ecmaVersion": 2022 }, + ( + "class C { static #foo = 2; }", + Some(serde_json::json!([{ "ignoreClassFieldInitialValues": true }])), + ), // { "ecmaVersion": 2022 } + ]; + + let fail = vec![ + ( + "var foo = 42", + Some(serde_json::json!([{ "enforceConst": true }])), + ), // { "ecmaVersion": 6 }, + ("var foo = 0 + 1;", None), + ( + "var foo = 42n", + Some(serde_json::json!([{ "enforceConst": true }])), + ), // { "ecmaVersion": 2020 }, + ("var foo = 0n + 1n;", None), // { "ecmaVersion": 2020 }, + ("a = a + 5;", None), + ("a += 5;", None), + ("var foo = 0 + 1 + -2 + 2;", None), + ( + "var foo = 0 + 1 + 2;", + Some(serde_json::json!([{ "ignore": [0, 1] }])), + ), + ( + "var foo = { bar:10 }", + Some(serde_json::json!([{ "detectObjects": true }])), + ), + ("console.log(0x1A + 0x02); console.log(071);", None), // { "sourceType": "script" }, + ( + "var stats = {avg: 42};", + Some(serde_json::json!([{ "detectObjects": true }])), + ), + ("var colors = {}; colors.RED = 2; colors.YELLOW = 3; colors.BLUE = 4 + 5;", None), + ("function getSecondsInMinute() {return 60;}", None), + ("function getNegativeSecondsInMinute() {return -60;}", None), + ("var data = ['foo', 'bar', 'baz']; var third = data[3];", None), + ("var data = ['foo', 'bar', 'baz']; var third = data[3];", Some(serde_json::json!([{}]))), + ( + "var data = ['foo', 'bar', 'baz']; var third = data[3];", + Some(serde_json::json!([{ "ignoreArrayIndexes": false }])), + ), + ( + "foo[-100]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "foo[-1.5]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "foo[-1]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "foo[-0.1]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "foo[-0b110]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), // { "ecmaVersion": 2015 }, + ( + "foo[-0o71]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), // { "ecmaVersion": 2015 }, + ( + "foo[-0x12]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "foo[-012]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), // { "sourceType": "script" }, + ( + "foo[0.1]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "foo[0.12e1]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "foo[1.5]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "foo[1.678e2]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "foo[56e-1]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "foo[5.000000000000001]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "foo[100.9]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "foo[4294967295]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "foo[1e300]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "foo[1e310]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "foo[-1e310]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "foo[-1n]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), // { "ecmaVersion": 2020 }, + ( + "foo[-100n]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), // { "ecmaVersion": 2020 }, + ( + "foo[-0x12n]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), // { "ecmaVersion": 2020 }, + ( + "foo[4294967295n]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), // { "ecmaVersion": 2020 }, + ( + "foo[+0]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "foo[+1]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "foo[-(-1)]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "foo[+0n]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), // { "ecmaVersion": 2020 }, + ( + "foo[+1n]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), // { "ecmaVersion": 2020 }, + ( + "foo[- -1n]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), // { "ecmaVersion": 2020 }, + ( + "100 .toString()", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ( + "200[100]", + Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), + ), + ("var a =
;", None), // { "parserOptions": { "ecmaFeatures": { "jsx": true } } }, + ("var min, max, mean; min = 1; max = 10; mean = 4;", Some(serde_json::json!([{}]))), + ("f(100n)", Some(serde_json::json!([{ "ignore": [100] }]))), // { "ecmaVersion": 2020 }, + ("f(-100n)", Some(serde_json::json!([{ "ignore": ["100n"] }]))), // { "ecmaVersion": 2020 }, + ("f(100n)", Some(serde_json::json!([{ "ignore": ["-100n"] }]))), // { "ecmaVersion": 2020 }, + ("f(100)", Some(serde_json::json!([{ "ignore": ["100n"] }]))), + ( + "const func = (param = 123) => {}", + Some(serde_json::json!([{ "ignoreDefaultValues": false }])), + ), // { "ecmaVersion": 6 }, + ("const { param = 123 } = sourceObject;", Some(serde_json::json!([{}]))), // { "ecmaVersion": 6 }, + ("const { param = 123 } = sourceObject;", None), // { "ecmaVersion": 6 }, + ( + "const { param = 123 } = sourceObject;", + Some(serde_json::json!([{ "ignoreDefaultValues": false }])), + ), // { "ecmaVersion": 6 }, + ( + "const [one = 1, two = 2] = []", + Some(serde_json::json!([{ "ignoreDefaultValues": false }])), + ), // { "ecmaVersion": 6 }, + ( + "var one, two; [one = 1, two = 2] = []", + Some(serde_json::json!([{ "ignoreDefaultValues": false }])), + ), // { "ecmaVersion": 6 }, + ("class C { foo = 2; }", None), // { "ecmaVersion": 2022 }, + ("class C { foo = 2; }", Some(serde_json::json!([{}]))), // { "ecmaVersion": 2022 }, + ( + "class C { foo = 2; }", + Some(serde_json::json!([{ "ignoreClassFieldInitialValues": false }])), + ), // { "ecmaVersion": 2022 }, + ( + "class C { foo = -2; }", + Some(serde_json::json!([{ "ignoreClassFieldInitialValues": false }])), + ), // { "ecmaVersion": 2022 }, + ( + "class C { static foo = 2; }", + Some(serde_json::json!([{ "ignoreClassFieldInitialValues": false }])), + ), // { "ecmaVersion": 2022 }, + ( + "class C { #foo = 2; }", + Some(serde_json::json!([{ "ignoreClassFieldInitialValues": false }])), + ), // { "ecmaVersion": 2022 }, + ( + "class C { static #foo = 2; }", + Some(serde_json::json!([{ "ignoreClassFieldInitialValues": false }])), + ), // { "ecmaVersion": 2022 }, + ( + "class C { foo = 2 + 3; }", + Some(serde_json::json!([{ "ignoreClassFieldInitialValues": true }])), + ), // { "ecmaVersion": 2022 }, + ("class C { 2; }", Some(serde_json::json!([{ "ignoreClassFieldInitialValues": true }]))), // { "ecmaVersion": 2022 }, + ("class C { [2]; }", Some(serde_json::json!([{ "ignoreClassFieldInitialValues": true }]))), // { "ecmaVersion": 2022 } + ]; + + Tester::new(NoMagicNumbers::NAME, pass, fail).test_and_snapshot(); +} From af754c5e62d764c565384ea16a732712a2b91bfc Mon Sep 17 00:00:00 2001 From: Sysix Date: Thu, 8 Aug 2024 00:35:04 +0200 Subject: [PATCH 02/23] feat(linter): implement eslint/no-magic-numbers --- .../src/rules/eslint/no_magic_numbers.rs | 564 +++++++++--------- 1 file changed, 270 insertions(+), 294 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs b/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs index dbc3164cf37c2..f619667470b5e 100644 --- a/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs @@ -1,18 +1,38 @@ -use oxc_ast::AstKind; +use std::ops::Mul; + +use oxc_ast::{ + ast::{Argument, AssignmentTarget, Expression, VariableDeclarationKind}, + AstKind, +}; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; use oxc_span::Span; use oxc_syntax::operator::UnaryOperator; -use crate::{ - context::LintContext, - fixer::{RuleFix, RuleFixer}, - rule::Rule, - AstNode, -}; +use crate::{context::LintContext, rule::Rule, AstNode}; + +fn must_use_const_diagnostic(span: &Span) -> OxcDiagnostic { + OxcDiagnostic::warn("Number constants declarations must use 'const'.").with_label(*span) +} + +fn no_magic_number_diagnostic(span: &Span, raw: &str) -> OxcDiagnostic { + OxcDiagnostic::warn(format!("No magic number: {raw}")).with_label(*span) +} + +#[derive(Debug, Default, Clone)] + +pub struct NoMagicNumbers(Box); + +impl std::ops::Deref for NoMagicNumbers { + type Target = NoMagicNumbersConfig; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} #[derive(Debug, Clone)] -pub struct NoMagicNumbers { +pub struct NoMagicNumbersConfig { ignore: Vec, ignore_array_indexes: bool, ignore_default_values: bool, @@ -21,7 +41,7 @@ pub struct NoMagicNumbers { detect_objects: bool, } -impl Default for NoMagicNumbers { +impl Default for NoMagicNumbersConfig { fn default() -> Self { Self { ignore: vec![], @@ -29,11 +49,71 @@ impl Default for NoMagicNumbers { ignore_default_values: false, ignore_class_field_initial_values: false, enforce_const: false, - detect_objects: false + detect_objects: false, } } } +impl TryFrom<&serde_json::Value> for NoMagicNumbersConfig { + type Error = OxcDiagnostic; + + fn try_from(raw: &serde_json::Value) -> Result { + println!("raw: {raw:?}"); + + if raw.is_null() { + return Ok(NoMagicNumbersConfig::default()); + } + + raw.as_array().unwrap().get(0).map_or_else( + || { + Err(OxcDiagnostic::warn( + "Expecting object for eslint/no-magic-numbers configuration", + )) + }, + |object| { + println!("config: {object:?}"); + Ok(Self { + ignore: object + .get("ignore") + .and_then(serde_json::Value::as_array) + .map(|v| { + v.iter() + .filter_map(serde_json::Value::as_f64) + .map(|v| f64::try_from(v).unwrap()) + .collect() + }) + .unwrap_or_default(), + ignore_array_indexes: object + .get("ignoreArrayIndexes") + .unwrap_or_else(|| &serde_json::Value::Bool(false)) + .as_bool() + .unwrap(), + ignore_default_values: object + .get("ignoreDefaultValues") + .unwrap_or_else(|| &serde_json::Value::Bool(false)) + .as_bool() + .unwrap(), + ignore_class_field_initial_values: object + .get("ignoreClassFieldInitialValues") + .unwrap_or_else(|| &serde_json::Value::Bool(false)) + .as_bool() + .unwrap(), + enforce_const: object + .get("enforceConst") + .unwrap_or_else(|| &serde_json::Value::Bool(false)) + .as_bool() + .unwrap(), + detect_objects: object + .get("detectObjects") + .unwrap_or_else(|| &serde_json::Value::Bool(false)) + .as_bool() + .unwrap(), + }) + }, + ) + } +} + declare_oxc_lint!( /// ### What it does /// @@ -53,50 +133,141 @@ declare_oxc_lint!( // Options are 'fix', 'fix-dangerous', 'suggestion', and 'suggestion-dangerous' ); +#[derive(Debug)] struct InternConfig<'a> { node: &'a AstNode<'a>, value: f64, - raw: String + raw: String, } impl Rule for NoMagicNumbers { + fn from_configuration(value: serde_json::Value) -> Self { + return Self(Box::new(NoMagicNumbersConfig::try_from(&value).unwrap())); + } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let ok_ast_kinds: Vec = if self.detect_objects { + vec![] + } else { + vec![ + AstKind::ObjectExpression, + AstKind::PropertyDefinition, + AstKind::AssignmentExpression + ] + }; + match node.kind() { AstKind::NumericLiteral(literal) => { let parent_node = ctx.nodes().parent_node(node.id()); - let intern_config: InternConfig = match parent_node.unwrap().kind() { - AstKind::UnaryExpression(unary) if unary.operator == UnaryOperator::UnaryNegation => { + let config: InternConfig = match parent_node.unwrap().kind() { + AstKind::UnaryExpression(unary) + if unary.operator == UnaryOperator::UnaryNegation => + { InternConfig { node: parent_node.unwrap(), value: 0.0 - literal.value, - raw: format!("-{}", literal.raw) - } - }, - _ => { - InternConfig { - node, - value: literal.value, - raw: literal.raw.to_string() + raw: format!("-{}", literal.raw), } } + _ => InternConfig { node, value: literal.value, raw: literal.raw.to_string() }, }; - if self.is_ignore_value(&intern_config.value) { + let parent = ctx.nodes().parent_node(config.node.id()).unwrap(); + let parent_parent = ctx.nodes().parent_node(parent.id()).unwrap(); + + println!( + " + + config {config:?} + parent: {parent:?} + parent_parent: {parent_parent:?} + + " + ); + + if self.is_ignore_value(&config.value) + || (self.ignore_default_values && self.is_default_value(&config.value, &parent)) + || (self.ignore_class_field_initial_values + && self.is_class_field_initial_value(&config.value, &parent)) + || self.is_parse_int_radix(&config.value, &parent_parent) + { return; } - }, + + match parent.kind() { + AstKind::VariableDeclarator(declarator) + if self.enforce_const + && declarator.kind != VariableDeclarationKind::Const => + { + println!("declarator: {declarator:?}"); + ctx.diagnostic(must_use_const_diagnostic(&literal.span)); + } + AstKind::BinaryExpression(expression) => { + if expression.left.is_number(literal.value) { + ctx.diagnostic(no_magic_number_diagnostic(&literal.span, &config.raw)) + } else if expression.right.is_number(literal.value) { + ctx.diagnostic(no_magic_number_diagnostic(&literal.span, &config.raw)) + } + } + AstKind::AssignmentExpression(expression) if self.detect_objects => { + if let AssignmentTarget::AssignmentTargetIdentifier(_) = expression.left { + ctx.diagnostic(no_magic_number_diagnostic(&literal.span, &config.raw)) + } + } + _ => {} + } + } _ => {} } } } impl NoMagicNumbers { - fn is_ignore_value(&self, number: &f64) -> bool { self.ignore.contains(number) } + + fn is_default_value<'a>(&self, number: &f64, parent_node: &AstNode<'a>) -> bool { + if let AstKind::AssignmentPattern(pattern) = parent_node.kind() { + return pattern.right.is_number(*number); + } + + false + } + + fn is_class_field_initial_value<'a>(&self, number: &f64, parent_node: &AstNode<'a>) -> bool { + if let AstKind::PropertyDefinition(property) = parent_node.kind() { + return property.value.as_ref().unwrap().is_number(*number); + } + + false + } + + fn is_parse_int_radix<'a>(&self, number: &f64, parent_parent_node: &AstNode<'a>) -> bool { + if let AstKind::CallExpression(expression) = parent_parent_node.kind() { + if expression.arguments.get(1).is_none() { + return false; + } + + let argument = expression.arguments.get(1).unwrap(); + return match argument { + Argument::NumericLiteral(numeric) => numeric.value == *number, + Argument::UnaryExpression(unary) + if unary.operator == UnaryOperator::UnaryNegation => + { + if let Expression::NumericLiteral(numeric) = &unary.argument { + return numeric.value == number.mul(-1.0); + } + + return false; + } + _ => false, + }; + } + + false + } } #[test] @@ -104,8 +275,11 @@ fn test() { use crate::tester::Tester; let ignore_array_indexes = Some(serde_json::json!([{"ignoreArrayIndexes": true}])); - let ignore_default_values = Some(serde_json::json!([{ "ignoreDefaultValues": true }])); - + let ignore_default_values = Some(serde_json::json!([{"ignoreDefaultValues": true}])); + let enforce_const = Some(serde_json::json!([{ "enforceConst": true}])); + let ignore_class_field_initial_values = + Some(serde_json::json!([{ "ignoreClassFieldInitialValues": true }])); + let pass = vec![ ("var x = parseInt(y, 10);", None), ("var x = parseInt(y, -10);", None), @@ -129,151 +303,52 @@ fn test() { "setTimeout(function() {return 1;}, 0);", Some(serde_json::json!([{ "ignore": [0, 1] }])), ), - ( - "var data = ['foo', 'bar', 'baz']; var third = data[3];", - ignore_array_indexes.clone(), - ), - ( - "foo[0]", - ignore_array_indexes.clone(), - ), - ( - "foo[-0]", - ignore_array_indexes.clone(), - ), - ( - "foo[1]", - ignore_array_indexes.clone(), - ), - ( - "foo[100]", - ignore_array_indexes.clone(), - ), - ( - "foo[200.00]", - ignore_array_indexes.clone(), - ), - ( - "foo[3e4]", - ignore_array_indexes.clone(), - ), - ( - "foo[1.23e2]", - ignore_array_indexes.clone(), - ), - ( - "foo[230e-1]", - ignore_array_indexes.clone(), - ), - ( - "foo[0b110]", - ignore_array_indexes.clone(), - ), // { "ecmaVersion": 2015 }, - ( - "foo[0o71]", - ignore_array_indexes.clone(), - ), // { "ecmaVersion": 2015 }, - ( - "foo[0xABC]", - ignore_array_indexes.clone(), - ), - ( - "foo[0123]", - ignore_array_indexes.clone(), - ), // { "sourceType": "script" }, - ( - "foo[5.0000000000000001]", - ignore_array_indexes.clone(), - ), - ( - "foo[4294967294]", - ignore_array_indexes.clone(), - ), - ( - "foo[0n]", - ignore_array_indexes.clone(), - ), // { "ecmaVersion": 2020 }, - ( - "foo[-0n]", - ignore_array_indexes.clone(), - ), // { "ecmaVersion": 2020 }, - ( - "foo[1n]", - ignore_array_indexes.clone(), - ), // { "ecmaVersion": 2020 }, - ( - "foo[100n]", - ignore_array_indexes.clone(), - ), // { "ecmaVersion": 2020 }, - ( - "foo[0xABn]", - ignore_array_indexes.clone(), - ), // { "ecmaVersion": 2020 }, - ( - "foo[4294967294n]", - ignore_array_indexes.clone(), - ), // { "ecmaVersion": 2020 }, + ("var data = ['foo', 'bar', 'baz']; var third = data[3];", ignore_array_indexes.clone()), + ("foo[0]", ignore_array_indexes.clone()), + ("foo[-0]", ignore_array_indexes.clone()), + ("foo[1]", ignore_array_indexes.clone()), + ("foo[100]", ignore_array_indexes.clone()), + ("foo[200.00]", ignore_array_indexes.clone()), + ("foo[3e4]", ignore_array_indexes.clone()), + ("foo[1.23e2]", ignore_array_indexes.clone()), + ("foo[230e-1]", ignore_array_indexes.clone()), + ("foo[0b110]", ignore_array_indexes.clone()), // { "ecmaVersion": 2015 }, + ("foo[0o71]", ignore_array_indexes.clone()), // { "ecmaVersion": 2015 }, + ("foo[0xABC]", ignore_array_indexes.clone()), + ("foo[0123]", ignore_array_indexes.clone()), // { "sourceType": "script" }, + ("foo[5.0000000000000001]", ignore_array_indexes.clone()), + ("foo[4294967294]", ignore_array_indexes.clone()), + ("foo[0n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + ("foo[-0n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + ("foo[1n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + ("foo[100n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + ("foo[0xABn]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + ("foo[4294967294n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, ("var a = ;", None), // { "parserOptions": { "ecmaFeatures": { "jsx": true } } }, ("var a =
;", None), // { "parserOptions": { "ecmaFeatures": { "jsx": true } } }, ("f(100n)", Some(serde_json::json!([{ "ignore": ["100n"] }]))), // { "ecmaVersion": 2020 }, ("f(-100n)", Some(serde_json::json!([{ "ignore": ["-100n"] }]))), // { "ecmaVersion": 2020 }, - ( - "const { param = 123 } = sourceObject;", - ignore_default_values.clone(), - ), // { "ecmaVersion": 6 }, - ( - "const func = (param = 123) => {}", - ignore_default_values.clone(), - ), // { "ecmaVersion": 6 }, - ( - "const func = ({ param = 123 }) => {}", - ignore_default_values.clone(), - ), // { "ecmaVersion": 6 }, - ( - "const [one = 1, two = 2] = []", - ignore_default_values.clone(), - ), // { "ecmaVersion": 6 }, - ( - "var one, two; [one = 1, two = 2] = []", - ignore_default_values.clone(), - ), // { "ecmaVersion": 6 }, + ("const { param = 123 } = sourceObject;", ignore_default_values.clone()), // { "ecmaVersion": 6 }, + ("const func = (param = 123) => {}", ignore_default_values.clone()), // { "ecmaVersion": 6 }, + ("const func = ({ param = 123 }) => {}", ignore_default_values.clone()), // { "ecmaVersion": 6 }, + ("const [one = 1, two = 2] = []", ignore_default_values.clone()), // { "ecmaVersion": 6 }, + ("var one, two; [one = 1, two = 2] = []", ignore_default_values.clone()), // { "ecmaVersion": 6 }, ("var x = parseInt?.(y, 10);", None), // { "ecmaVersion": 2020 }, ("var x = Number?.parseInt(y, 10);", None), // { "ecmaVersion": 2020 }, ("var x = (Number?.parseInt)(y, 10);", None), // { "ecmaVersion": 2020 }, - ("foo?.[777]", Some(serde_json::json!([{ "ignoreArrayIndexes": true }]))), // { "ecmaVersion": 2020 }, - ( - "class C { foo = 2; }", - Some(serde_json::json!([{ "ignoreClassFieldInitialValues": true }])), - ), // { "ecmaVersion": 2022 }, - ( - "class C { foo = -2; }", - Some(serde_json::json!([{ "ignoreClassFieldInitialValues": true }])), - ), // { "ecmaVersion": 2022 }, - ( - "class C { static foo = 2; }", - Some(serde_json::json!([{ "ignoreClassFieldInitialValues": true }])), - ), // { "ecmaVersion": 2022 }, - ( - "class C { #foo = 2; }", - Some(serde_json::json!([{ "ignoreClassFieldInitialValues": true }])), - ), // { "ecmaVersion": 2022 }, - ( - "class C { static #foo = 2; }", - Some(serde_json::json!([{ "ignoreClassFieldInitialValues": true }])), - ), // { "ecmaVersion": 2022 } + ("foo?.[777]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + ("class C { foo = 2; }", ignore_class_field_initial_values.clone()), // { "ecmaVersion": 2022 }, + ("class C { foo = -2; }", ignore_class_field_initial_values.clone()), // { "ecmaVersion": 2022 }, + ("class C { static foo = 2; }", ignore_class_field_initial_values.clone()), // { "ecmaVersion": 2022 }, + ("class C { #foo = 2; }", ignore_class_field_initial_values.clone()), // { "ecmaVersion": 2022 }, + ("class C { static #foo = 2; }", ignore_class_field_initial_values.clone()), // { "ecmaVersion": 2022 } ]; let fail = vec![ - ( - "var foo = 42", - Some(serde_json::json!([{ "enforceConst": true }])), - ), // { "ecmaVersion": 6 }, + ("var foo = 42", enforce_const.clone()), // { "ecmaVersion": 6 }, ("var foo = 0 + 1;", None), - ( - "var foo = 42n", - Some(serde_json::json!([{ "enforceConst": true }])), - ), // { "ecmaVersion": 2020 }, - ("var foo = 0n + 1n;", None), // { "ecmaVersion": 2020 }, + // ("var foo = 42n", enforce_const.clone()), // { "ecmaVersion": 2020 }, + // ("var foo = 0n + 1n;", None), // { "ecmaVersion": 2020 }, ("a = a + 5;", None), ("a += 5;", None), ("var foo = 0 + 1 + -2 + 2;", None), @@ -295,134 +370,38 @@ fn test() { ("function getNegativeSecondsInMinute() {return -60;}", None), ("var data = ['foo', 'bar', 'baz']; var third = data[3];", None), ("var data = ['foo', 'bar', 'baz']; var third = data[3];", Some(serde_json::json!([{}]))), - ( - "var data = ['foo', 'bar', 'baz']; var third = data[3];", - Some(serde_json::json!([{ "ignoreArrayIndexes": false }])), - ), - ( - "foo[-100]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "foo[-1.5]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "foo[-1]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "foo[-0.1]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "foo[-0b110]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), // { "ecmaVersion": 2015 }, - ( - "foo[-0o71]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), // { "ecmaVersion": 2015 }, - ( - "foo[-0x12]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "foo[-012]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), // { "sourceType": "script" }, - ( - "foo[0.1]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "foo[0.12e1]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "foo[1.5]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "foo[1.678e2]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "foo[56e-1]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "foo[5.000000000000001]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "foo[100.9]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "foo[4294967295]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "foo[1e300]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "foo[1e310]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "foo[-1e310]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "foo[-1n]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), // { "ecmaVersion": 2020 }, - ( - "foo[-100n]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), // { "ecmaVersion": 2020 }, - ( - "foo[-0x12n]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), // { "ecmaVersion": 2020 }, - ( - "foo[4294967295n]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), // { "ecmaVersion": 2020 }, - ( - "foo[+0]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "foo[+1]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "foo[-(-1)]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "foo[+0n]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), // { "ecmaVersion": 2020 }, - ( - "foo[+1n]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), // { "ecmaVersion": 2020 }, - ( - "foo[- -1n]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), // { "ecmaVersion": 2020 }, - ( - "100 .toString()", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), - ( - "200[100]", - Some(serde_json::json!([{ "ignoreArrayIndexes": true }])), - ), + ("var data = ['foo', 'bar', 'baz']; var third = data[3];", ignore_array_indexes.clone()), + ("foo[-100]", ignore_array_indexes.clone()), + ("foo[-1.5]", ignore_array_indexes.clone()), + ("foo[-1]", ignore_array_indexes.clone()), + ("foo[-0.1]", ignore_array_indexes.clone()), + ("foo[-0b110]", ignore_array_indexes.clone()), // { "ecmaVersion": 2015 }, + ("foo[-0o71]", ignore_array_indexes.clone()), // { "ecmaVersion": 2015 }, + ("foo[-0x12]", ignore_array_indexes.clone()), + ("foo[-012]", ignore_array_indexes.clone()), // { "sourceType": "script" }, + ("foo[0.1]", ignore_array_indexes.clone()), + ("foo[0.12e1]", ignore_array_indexes.clone()), + ("foo[1.5]", ignore_array_indexes.clone()), + ("foo[1.678e2]", ignore_array_indexes.clone()), + ("foo[56e-1]", ignore_array_indexes.clone()), + ("foo[5.000000000000001]", ignore_array_indexes.clone()), + ("foo[100.9]", ignore_array_indexes.clone()), + ("foo[4294967295]", ignore_array_indexes.clone()), + ("foo[1e300]", ignore_array_indexes.clone()), + ("foo[1e310]", ignore_array_indexes.clone()), + ("foo[-1e310]", ignore_array_indexes.clone()), + ("foo[-1n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + ("foo[-100n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + ("foo[-0x12n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + ("foo[4294967295n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + ("foo[+0]", ignore_array_indexes.clone()), + ("foo[+1]", ignore_array_indexes.clone()), + ("foo[-(-1)]", ignore_array_indexes.clone()), + ("foo[+0n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + ("foo[+1n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + ("foo[- -1n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + ("100 .toString()", ignore_array_indexes.clone()), + ("200[100]", ignore_array_indexes.clone()), ("var a =
;", None), // { "parserOptions": { "ecmaFeatures": { "jsx": true } } }, ("var min, max, mean; min = 1; max = 10; mean = 4;", Some(serde_json::json!([{}]))), ("f(100n)", Some(serde_json::json!([{ "ignore": [100] }]))), // { "ecmaVersion": 2020 }, @@ -469,12 +448,9 @@ fn test() { "class C { static #foo = 2; }", Some(serde_json::json!([{ "ignoreClassFieldInitialValues": false }])), ), // { "ecmaVersion": 2022 }, - ( - "class C { foo = 2 + 3; }", - Some(serde_json::json!([{ "ignoreClassFieldInitialValues": true }])), - ), // { "ecmaVersion": 2022 }, - ("class C { 2; }", Some(serde_json::json!([{ "ignoreClassFieldInitialValues": true }]))), // { "ecmaVersion": 2022 }, - ("class C { [2]; }", Some(serde_json::json!([{ "ignoreClassFieldInitialValues": true }]))), // { "ecmaVersion": 2022 } + ("class C { foo = 2 + 3; }", ignore_class_field_initial_values.clone()), // { "ecmaVersion": 2022 }, + ("class C { 2; }", ignore_class_field_initial_values.clone()), // { "ecmaVersion": 2022 }, + ("class C { [2]; }", ignore_class_field_initial_values.clone()), // { "ecmaVersion": 2022 } ]; Tester::new(NoMagicNumbers::NAME, pass, fail).test_and_snapshot(); From 60927d14e94e6c3077a1b5d94e5211f4af7b97d4 Mon Sep 17 00:00:00 2001 From: Sysix Date: Sat, 10 Aug 2024 19:23:29 +0200 Subject: [PATCH 03/23] feat(linter): implement eslint/no-magic-numbers --- .../src/rules/eslint/no_magic_numbers.rs | 273 ++++++++++++++---- 1 file changed, 209 insertions(+), 64 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs b/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs index f619667470b5e..106abbf5a8317 100644 --- a/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs @@ -1,7 +1,7 @@ use std::ops::Mul; use oxc_ast::{ - ast::{Argument, AssignmentTarget, Expression, VariableDeclarationKind}, + ast::{Argument, AssignmentTarget, Expression, MemberExpression, VariableDeclarationKind}, AstKind, }; use oxc_diagnostics::OxcDiagnostic; @@ -11,6 +11,11 @@ use oxc_syntax::operator::UnaryOperator; use crate::{context::LintContext, rule::Rule, AstNode}; +enum NoMagicNumberReportReason<'a> { + MustUseConst(&'a Span), + NoMagicNumber(&'a Span, &'a str), +} + fn must_use_const_diagnostic(span: &Span) -> OxcDiagnostic { OxcDiagnostic::warn("Number constants declarations must use 'const'.").with_label(*span) } @@ -140,82 +145,91 @@ struct InternConfig<'a> { raw: String, } +impl InternConfig<'_> { + fn try_node<'a>( + node: &'a AstNode<'a>, + parent_node: &'a AstNode<'a>, + ) -> Result, OxcDiagnostic> { + let is_negative = matches!(parent_node.kind(), AstKind::UnaryExpression(unary) if unary.operator == UnaryOperator::UnaryNegation); + + if let AstKind::BigIntLiteral(bigint) = node.kind() { + let mut chars = bigint.raw.chars(); + + chars.next_back(); + + if is_negative { + return Ok(InternConfig { + node: parent_node, + value: 0.0 - chars.as_str().parse::().unwrap(), + raw: format!("-{}", bigint.raw), + }); + } else { + return Ok(InternConfig { + node, + value: chars.as_str().parse::().unwrap(), + raw: bigint.raw.clone().into(), + }); + } + } else if let AstKind::NumericLiteral(numeric) = node.kind() { + if is_negative { + return Ok(InternConfig { + node: parent_node, + value: 0.0 - numeric.value, + raw: format!("-{}", numeric.raw), + }); + } else { + return Ok(InternConfig { node, value: numeric.value, raw: numeric.raw.into() }); + } + } else { + return Err(OxcDiagnostic::warn(format!( + "expected AstKind BingIntLiteral or NumericLiteral, got {:?}", + node.kind().debug_name() + ))); + } + } + + pub fn from<'a>(node: &'a AstNode<'a>, parent: &'a AstNode<'a>) -> InternConfig<'a> { + return InternConfig::try_node(node, parent).unwrap(); + } +} impl Rule for NoMagicNumbers { fn from_configuration(value: serde_json::Value) -> Self { return Self(Box::new(NoMagicNumbersConfig::try_from(&value).unwrap())); } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + match node.kind() { + AstKind::BigIntLiteral(literal) => { + let config = InternConfig::from(node, ctx.nodes().parent_node(node.id()).unwrap()); - let ok_ast_kinds: Vec = if self.detect_objects { - vec![] - } else { - vec![ - AstKind::ObjectExpression, - AstKind::PropertyDefinition, - AstKind::AssignmentExpression - ] - }; + let parent = ctx.nodes().parent_node(config.node.id()).unwrap(); + let parent_parent = ctx.nodes().parent_node(parent.id()).unwrap(); - match node.kind() { - AstKind::NumericLiteral(literal) => { - let parent_node = ctx.nodes().parent_node(node.id()); + if self.is_skipable(&config, parent, parent_parent) { + return; + } - let config: InternConfig = match parent_node.unwrap().kind() { - AstKind::UnaryExpression(unary) - if unary.operator == UnaryOperator::UnaryNegation => - { - InternConfig { - node: parent_node.unwrap(), - value: 0.0 - literal.value, - raw: format!("-{}", literal.raw), - } - } - _ => InternConfig { node, value: literal.value, raw: literal.raw.to_string() }, - }; + if let Some(reason) = self.get_report_reason(&parent, &config, &literal.span) { + ctx.diagnostic(match reason { + NoMagicNumberReportReason::MustUseConst(span) => must_use_const_diagnostic(span), + NoMagicNumberReportReason::NoMagicNumber(span, raw) => no_magic_number_diagnostic(span, raw) + }); + } + } + AstKind::NumericLiteral(literal) => { + let config = InternConfig::from(node, ctx.nodes().parent_node(node.id()).unwrap()); let parent = ctx.nodes().parent_node(config.node.id()).unwrap(); let parent_parent = ctx.nodes().parent_node(parent.id()).unwrap(); - println!( - " - - config {config:?} - parent: {parent:?} - parent_parent: {parent_parent:?} - - " - ); - - if self.is_ignore_value(&config.value) - || (self.ignore_default_values && self.is_default_value(&config.value, &parent)) - || (self.ignore_class_field_initial_values - && self.is_class_field_initial_value(&config.value, &parent)) - || self.is_parse_int_radix(&config.value, &parent_parent) - { + if self.is_skipable(&config, parent, parent_parent) { return; } - match parent.kind() { - AstKind::VariableDeclarator(declarator) - if self.enforce_const - && declarator.kind != VariableDeclarationKind::Const => - { - println!("declarator: {declarator:?}"); - ctx.diagnostic(must_use_const_diagnostic(&literal.span)); - } - AstKind::BinaryExpression(expression) => { - if expression.left.is_number(literal.value) { - ctx.diagnostic(no_magic_number_diagnostic(&literal.span, &config.raw)) - } else if expression.right.is_number(literal.value) { - ctx.diagnostic(no_magic_number_diagnostic(&literal.span, &config.raw)) - } - } - AstKind::AssignmentExpression(expression) if self.detect_objects => { - if let AssignmentTarget::AssignmentTargetIdentifier(_) = expression.left { - ctx.diagnostic(no_magic_number_diagnostic(&literal.span, &config.raw)) - } - } - _ => {} + if let Some(reason) = self.get_report_reason(&parent, &config, &literal.span) { + ctx.diagnostic(match reason { + NoMagicNumberReportReason::MustUseConst(span) => must_use_const_diagnostic(span), + NoMagicNumberReportReason::NoMagicNumber(span, raw) => no_magic_number_diagnostic(span, raw) + }); } } _ => {} @@ -268,6 +282,134 @@ impl NoMagicNumbers { false } + + /// Returns whether the given node is used as an array index. + /// Value must coerce to a valid array index name: "0", "1", "2" ... "4294967294". + /// + /// All other values, like "-1", "2.5", or "4294967295", are just "normal" object properties, + /// which can be created and accessed on an array in addition to the array index properties, + /// but they don't affect array's length and are not considered by methods such as .map(), .forEach() etc. + /// + /// The maximum array length by the specification is 2 ** 32 - 1 = 4294967295, + /// thus the maximum valid index is 2 ** 32 - 2 = 4294967294. + /// + /// All notations are allowed, as long as the value coerces to one of "0", "1", "2" ... "4294967294". + /// + /// Valid examples: + /// a[0], a[1], a[1.2e1], a[0xAB], a[0n], a[1n] + /// a[-0] (same as a[0] because -0 coerces to "0") + /// a[-0n] (-0n evaluates to 0n) + /// + /// Invalid examples: + /// a[-1], a[-0xAB], a[-1n], a[2.5], a[1.23e1], a[12e-1] + /// a[4294967295] (above the max index, it's an access to a regular property a["4294967295"]) + /// a[999999999999999999999] (even if it wasn't above the max index, it would be a["1e+21"]) + /// a[1e310] (same as a["Infinity"]) + fn is_array_index<'a>(&self, node: &AstNode<'a>, parent_node: &AstNode<'a>) -> bool { + match node.kind() { + AstKind::UnaryExpression(unary) => { + if unary.operator == UnaryOperator::UnaryNegation && !unary.argument.is_number_0() { + return false; + } + + match &unary.argument { + Expression::NumericLiteral(numeric) => { + return numeric.value >= 0.0 + && numeric.value.fract() == 0.0 + && numeric.value < u32::MAX as f64; + } + _ => {} + } + + false + } + AstKind::BigIntLiteral(bigint) => { + if let AstKind::MemberExpression(expression) = parent_node.kind() { + if let MemberExpression::ComputedMemberExpression(computed_expression) = + expression + { + return computed_expression.expression.is_number(bigint.value) + && bigint.value >= 0.0 + && bigint.value.fract() == 0.0 + && bigint.value < u32::MAX as f64; + } + } + + false + } + AstKind::NumericLiteral(numeric) => { + if let AstKind::MemberExpression(expression) = parent_node.kind() { + if let MemberExpression::ComputedMemberExpression(computed_expression) = + expression + { + return computed_expression.expression.is_number(numeric.value) + && numeric.value >= 0.0 + && numeric.value.fract() == 0.0 + && numeric.value < u32::MAX as f64; + } + } + + false + } + _ => false, + } + } + + fn is_skipable<'a>( + &self, + config: &InternConfig<'a>, + parent: &AstNode<'a>, + parent_parent: &AstNode<'a>, + ) -> bool { + if self.is_ignore_value(&config.value) + || self.is_parse_int_radix(&config.value, &parent_parent) + || (self.ignore_default_values && self.is_default_value(&config.value, &parent)) + || (self.ignore_class_field_initial_values + && self.is_class_field_initial_value(&config.value, &parent)) + || (self.ignore_array_indexes && self.is_array_index(&config.node, &parent)) + { + return true; + } + + if !self.detect_objects + && (matches!(parent.kind(), AstKind::ObjectExpression(_)) + || matches!(parent.kind(), AstKind::PropertyDefinition(_)) + || matches!(parent.kind(), AstKind::ObjectProperty(_))) + { + return true; + } + + false + } + + fn get_report_reason<'a>( + &self, + parent: &'a AstNode<'a>, + config: &'a InternConfig<'a>, + span: &'a Span, + ) -> Option> { + match parent.kind() { + AstKind::VariableDeclarator(declarator) + if self.enforce_const && declarator.kind != VariableDeclarationKind::Const => + { + return Some(NoMagicNumberReportReason::MustUseConst(span)); + } + AstKind::AssignmentExpression(expression) => { + if let AssignmentTarget::AssignmentTargetIdentifier(_) = expression.left { + return Some(NoMagicNumberReportReason::NoMagicNumber(span, &config.raw)); + } + } + AstKind::BinaryExpression(_) + | AstKind::MemberExpression(_) + | AstKind::ObjectProperty(_) + | AstKind::ReturnStatement(_) => { + return Some(NoMagicNumberReportReason::NoMagicNumber(span, &config.raw)); + } + _ => {} + } + + None + } } #[test] @@ -370,7 +512,10 @@ fn test() { ("function getNegativeSecondsInMinute() {return -60;}", None), ("var data = ['foo', 'bar', 'baz']; var third = data[3];", None), ("var data = ['foo', 'bar', 'baz']; var third = data[3];", Some(serde_json::json!([{}]))), - ("var data = ['foo', 'bar', 'baz']; var third = data[3];", ignore_array_indexes.clone()), + ( + "var data = ['foo', 'bar', 'baz']; var third = data[3];", + Some(serde_json::json!([{"ignoreArrayIndexes": false}])), + ), ("foo[-100]", ignore_array_indexes.clone()), ("foo[-1.5]", ignore_array_indexes.clone()), ("foo[-1]", ignore_array_indexes.clone()), @@ -402,7 +547,7 @@ fn test() { ("foo[- -1n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, ("100 .toString()", ignore_array_indexes.clone()), ("200[100]", ignore_array_indexes.clone()), - ("var a =
;", None), // { "parserOptions": { "ecmaFeatures": { "jsx": true } } }, + // ("var a =
;", None), // { "parserOptions": { "ecmaFeatures": { "jsx": true } } }, ("var min, max, mean; min = 1; max = 10; mean = 4;", Some(serde_json::json!([{}]))), ("f(100n)", Some(serde_json::json!([{ "ignore": [100] }]))), // { "ecmaVersion": 2020 }, ("f(-100n)", Some(serde_json::json!([{ "ignore": ["100n"] }]))), // { "ecmaVersion": 2020 }, From a921faa097f46269ee3d8a02f6f884303e988120 Mon Sep 17 00:00:00 2001 From: Sysix Date: Sun, 1 Sep 2024 16:55:49 +0200 Subject: [PATCH 04/23] feat(linter): implement eslint/no-magic-numbers --- .../src/rules/eslint/no_magic_numbers.rs | 215 +++++---- .../src/snapshots/no_magic_numbers.snap | 410 ++++++++++++++++++ 2 files changed, 514 insertions(+), 111 deletions(-) create mode 100644 crates/oxc_linter/src/snapshots/no_magic_numbers.snap diff --git a/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs b/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs index 106abbf5a8317..b8b9f25991ca1 100644 --- a/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs @@ -1,7 +1,10 @@ use std::ops::Mul; use oxc_ast::{ - ast::{Argument, AssignmentTarget, Expression, MemberExpression, VariableDeclarationKind}, + ast::{ + Argument, AssignmentTarget, Expression, MemberExpression, UnaryExpression, + VariableDeclarationKind, + }, AstKind, }; use oxc_diagnostics::OxcDiagnostic; @@ -63,8 +66,6 @@ impl TryFrom<&serde_json::Value> for NoMagicNumbersConfig { type Error = OxcDiagnostic; fn try_from(raw: &serde_json::Value) -> Result { - println!("raw: {raw:?}"); - if raw.is_null() { return Ok(NoMagicNumbersConfig::default()); } @@ -152,25 +153,7 @@ impl InternConfig<'_> { ) -> Result, OxcDiagnostic> { let is_negative = matches!(parent_node.kind(), AstKind::UnaryExpression(unary) if unary.operator == UnaryOperator::UnaryNegation); - if let AstKind::BigIntLiteral(bigint) = node.kind() { - let mut chars = bigint.raw.chars(); - - chars.next_back(); - - if is_negative { - return Ok(InternConfig { - node: parent_node, - value: 0.0 - chars.as_str().parse::().unwrap(), - raw: format!("-{}", bigint.raw), - }); - } else { - return Ok(InternConfig { - node, - value: chars.as_str().parse::().unwrap(), - raw: bigint.raw.clone().into(), - }); - } - } else if let AstKind::NumericLiteral(numeric) = node.kind() { + if let AstKind::NumericLiteral(numeric) = node.kind() { if is_negative { return Ok(InternConfig { node: parent_node, @@ -198,7 +181,7 @@ impl Rule for NoMagicNumbers { } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { match node.kind() { - AstKind::BigIntLiteral(literal) => { + AstKind::NumericLiteral(literal) => { let config = InternConfig::from(node, ctx.nodes().parent_node(node.id()).unwrap()); let parent = ctx.nodes().parent_node(config.node.id()).unwrap(); @@ -210,41 +193,52 @@ impl Rule for NoMagicNumbers { if let Some(reason) = self.get_report_reason(&parent, &config, &literal.span) { ctx.diagnostic(match reason { - NoMagicNumberReportReason::MustUseConst(span) => must_use_const_diagnostic(span), - NoMagicNumberReportReason::NoMagicNumber(span, raw) => no_magic_number_diagnostic(span, raw) + NoMagicNumberReportReason::MustUseConst(span) => { + must_use_const_diagnostic(span) + } + NoMagicNumberReportReason::NoMagicNumber(span, raw) => { + no_magic_number_diagnostic(span, raw) + } }); } } - AstKind::NumericLiteral(literal) => { - let config = InternConfig::from(node, ctx.nodes().parent_node(node.id()).unwrap()); + _ => {} + } + } +} - let parent = ctx.nodes().parent_node(config.node.id()).unwrap(); - let parent_parent = ctx.nodes().parent_node(parent.id()).unwrap(); +impl NoMagicNumbers { + fn is_numeric_value<'a>(expression: &Expression<'a>, number: &f64) -> bool { + // check for Expression::NumericLiteral + if expression.is_number(*number) { + return true; + } - if self.is_skipable(&config, parent, parent_parent) { - return; + if let Expression::UnaryExpression(unary) = expression { + if let Expression::NumericLiteral(_) = &unary.argument { + if unary.operator == UnaryOperator::UnaryPlus { + return unary.argument.is_number(*number); } - if let Some(reason) = self.get_report_reason(&parent, &config, &literal.span) { - ctx.diagnostic(match reason { - NoMagicNumberReportReason::MustUseConst(span) => must_use_const_diagnostic(span), - NoMagicNumberReportReason::NoMagicNumber(span, raw) => no_magic_number_diagnostic(span, raw) - }); + if unary.operator == UnaryOperator::UnaryNegation { + return unary.argument.is_number(number * -1.0); } } - _ => {} } - } -} -impl NoMagicNumbers { + false + } fn is_ignore_value(&self, number: &f64) -> bool { self.ignore.contains(number) } fn is_default_value<'a>(&self, number: &f64, parent_node: &AstNode<'a>) -> bool { + if let AstKind::AssignmentTargetWithDefault(assignment) = parent_node.kind() { + return NoMagicNumbers::is_numeric_value(&assignment.init, number); + } + if let AstKind::AssignmentPattern(pattern) = parent_node.kind() { - return pattern.right.is_number(*number); + return NoMagicNumbers::is_numeric_value(&pattern.right, number); } false @@ -252,7 +246,7 @@ impl NoMagicNumbers { fn is_class_field_initial_value<'a>(&self, number: &f64, parent_node: &AstNode<'a>) -> bool { if let AstKind::PropertyDefinition(property) = parent_node.kind() { - return property.value.as_ref().unwrap().is_number(*number); + return NoMagicNumbers::is_numeric_value(property.value.as_ref().unwrap(), number); } false @@ -306,50 +300,53 @@ impl NoMagicNumbers { /// a[999999999999999999999] (even if it wasn't above the max index, it would be a["1e+21"]) /// a[1e310] (same as a["Infinity"]) fn is_array_index<'a>(&self, node: &AstNode<'a>, parent_node: &AstNode<'a>) -> bool { - match node.kind() { - AstKind::UnaryExpression(unary) => { - if unary.operator == UnaryOperator::UnaryNegation && !unary.argument.is_number_0() { + println!( + " + + node: {node:?} + + " + ); + + fn is_unanary_index(unary: &UnaryExpression) -> bool { + match &unary.argument { + Expression::NumericLiteral(numeric) => { + if unary.operator == UnaryOperator::UnaryNegation { + return numeric.value == 0.0; + } + // ToDo: check why ("foo[+0]", ignore_array_indexes.clone()), should fail return false; - } - match &unary.argument { - Expression::NumericLiteral(numeric) => { - return numeric.value >= 0.0 - && numeric.value.fract() == 0.0 - && numeric.value < u32::MAX as f64; - } - _ => {} + // return numeric.value >= 0.0 + // && numeric.value.fract() == 0.0 + // && numeric.value < u32::MAX as f64; } - - false + _ => false, } - AstKind::BigIntLiteral(bigint) => { - if let AstKind::MemberExpression(expression) = parent_node.kind() { - if let MemberExpression::ComputedMemberExpression(computed_expression) = - expression - { - return computed_expression.expression.is_number(bigint.value) - && bigint.value >= 0.0 - && bigint.value.fract() == 0.0 - && bigint.value < u32::MAX as f64; - } - } - - false + } + match node.kind() { + AstKind::UnaryExpression(unary) => { + return is_unanary_index(&unary); } AstKind::NumericLiteral(numeric) => { - if let AstKind::MemberExpression(expression) = parent_node.kind() { - if let MemberExpression::ComputedMemberExpression(computed_expression) = - expression - { - return computed_expression.expression.is_number(numeric.value) - && numeric.value >= 0.0 - && numeric.value.fract() == 0.0 - && numeric.value < u32::MAX as f64; + match parent_node.kind() { + AstKind::MemberExpression(expression) => { + if let MemberExpression::ComputedMemberExpression(computed_expression) = + expression + { + return computed_expression.expression.is_number(numeric.value) + && numeric.value >= 0.0 + && numeric.value.fract() == 0.0 + && numeric.value < u32::MAX as f64; + } + + false + } + AstKind::UnaryExpression(unary) => { + return is_unanary_index(&unary); } + _ => false, } - - false } _ => false, } @@ -373,7 +370,6 @@ impl NoMagicNumbers { if !self.detect_objects && (matches!(parent.kind(), AstKind::ObjectExpression(_)) - || matches!(parent.kind(), AstKind::PropertyDefinition(_)) || matches!(parent.kind(), AstKind::ObjectProperty(_))) { return true; @@ -389,26 +385,23 @@ impl NoMagicNumbers { span: &'a Span, ) -> Option> { match parent.kind() { - AstKind::VariableDeclarator(declarator) - if self.enforce_const && declarator.kind != VariableDeclarationKind::Const => - { - return Some(NoMagicNumberReportReason::MustUseConst(span)); + AstKind::VariableDeclarator(declarator) => { + if self.enforce_const && declarator.kind != VariableDeclarationKind::Const { + return Some(NoMagicNumberReportReason::MustUseConst(span)); + } + + None } AstKind::AssignmentExpression(expression) => { if let AssignmentTarget::AssignmentTargetIdentifier(_) = expression.left { return Some(NoMagicNumberReportReason::NoMagicNumber(span, &config.raw)); } + + None } - AstKind::BinaryExpression(_) - | AstKind::MemberExpression(_) - | AstKind::ObjectProperty(_) - | AstKind::ReturnStatement(_) => { - return Some(NoMagicNumberReportReason::NoMagicNumber(span, &config.raw)); - } - _ => {} + AstKind::JSXExpressionContainer(_) => None, + _ => return Some(NoMagicNumberReportReason::NoMagicNumber(span, &config.raw)), } - - None } } @@ -460,16 +453,16 @@ fn test() { ("foo[0123]", ignore_array_indexes.clone()), // { "sourceType": "script" }, ("foo[5.0000000000000001]", ignore_array_indexes.clone()), ("foo[4294967294]", ignore_array_indexes.clone()), - ("foo[0n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, - ("foo[-0n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, - ("foo[1n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, - ("foo[100n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, - ("foo[0xABn]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, - ("foo[4294967294n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + // ("foo[0n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + // ("foo[-0n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + // ("foo[1n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + // ("foo[100n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + // ("foo[0xABn]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + // ("foo[4294967294n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, ("var a = ;", None), // { "parserOptions": { "ecmaFeatures": { "jsx": true } } }, ("var a =
;", None), // { "parserOptions": { "ecmaFeatures": { "jsx": true } } }, - ("f(100n)", Some(serde_json::json!([{ "ignore": ["100n"] }]))), // { "ecmaVersion": 2020 }, - ("f(-100n)", Some(serde_json::json!([{ "ignore": ["-100n"] }]))), // { "ecmaVersion": 2020 }, + // ("f(100n)", Some(serde_json::json!([{ "ignore": ["100n"] }]))), // { "ecmaVersion": 2020 }, + // ("f(-100n)", Some(serde_json::json!([{ "ignore": ["-100n"] }]))), // { "ecmaVersion": 2020 }, ("const { param = 123 } = sourceObject;", ignore_default_values.clone()), // { "ecmaVersion": 6 }, ("const func = (param = 123) => {}", ignore_default_values.clone()), // { "ecmaVersion": 6 }, ("const func = ({ param = 123 }) => {}", ignore_default_values.clone()), // { "ecmaVersion": 6 }, @@ -535,24 +528,24 @@ fn test() { ("foo[1e300]", ignore_array_indexes.clone()), ("foo[1e310]", ignore_array_indexes.clone()), ("foo[-1e310]", ignore_array_indexes.clone()), - ("foo[-1n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, - ("foo[-100n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, - ("foo[-0x12n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, - ("foo[4294967295n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + // ("foo[-1n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + // ("foo[-100n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + // ("foo[-0x12n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + // ("foo[4294967295n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, ("foo[+0]", ignore_array_indexes.clone()), ("foo[+1]", ignore_array_indexes.clone()), ("foo[-(-1)]", ignore_array_indexes.clone()), - ("foo[+0n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, - ("foo[+1n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, - ("foo[- -1n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + // ("foo[+0n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + // ("foo[+1n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + // ("foo[- -1n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, ("100 .toString()", ignore_array_indexes.clone()), ("200[100]", ignore_array_indexes.clone()), // ("var a =
;", None), // { "parserOptions": { "ecmaFeatures": { "jsx": true } } }, ("var min, max, mean; min = 1; max = 10; mean = 4;", Some(serde_json::json!([{}]))), - ("f(100n)", Some(serde_json::json!([{ "ignore": [100] }]))), // { "ecmaVersion": 2020 }, - ("f(-100n)", Some(serde_json::json!([{ "ignore": ["100n"] }]))), // { "ecmaVersion": 2020 }, - ("f(100n)", Some(serde_json::json!([{ "ignore": ["-100n"] }]))), // { "ecmaVersion": 2020 }, - ("f(100)", Some(serde_json::json!([{ "ignore": ["100n"] }]))), + // ("f(100n)", Some(serde_json::json!([{ "ignore": [100] }]))), // { "ecmaVersion": 2020 }, + // ("f(-100n)", Some(serde_json::json!([{ "ignore": ["100n"] }]))), // { "ecmaVersion": 2020 }, + // ("f(100n)", Some(serde_json::json!([{ "ignore": ["-100n"] }]))), // { "ecmaVersion": 2020 }, + // ("f(100)", Some(serde_json::json!([{ "ignore": ["100n"] }]))), ( "const func = (param = 123) => {}", Some(serde_json::json!([{ "ignoreDefaultValues": false }])), diff --git a/crates/oxc_linter/src/snapshots/no_magic_numbers.snap b/crates/oxc_linter/src/snapshots/no_magic_numbers.snap new file mode 100644 index 0000000000000..d3bb77a8b3561 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_magic_numbers.snap @@ -0,0 +1,410 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint(no-magic-numbers): Number constants declarations must use 'const'. + ╭─[no_magic_numbers.tsx:1:11] + 1 │ var foo = 42 + · ── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 0 + ╭─[no_magic_numbers.tsx:1:11] + 1 │ var foo = 0 + 1; + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 1 + ╭─[no_magic_numbers.tsx:1:15] + 1 │ var foo = 0 + 1; + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 5 + ╭─[no_magic_numbers.tsx:1:9] + 1 │ a = a + 5; + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 5 + ╭─[no_magic_numbers.tsx:1:6] + 1 │ a += 5; + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 0 + ╭─[no_magic_numbers.tsx:1:11] + 1 │ var foo = 0 + 1 + -2 + 2; + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 1 + ╭─[no_magic_numbers.tsx:1:15] + 1 │ var foo = 0 + 1 + -2 + 2; + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: -2 + ╭─[no_magic_numbers.tsx:1:20] + 1 │ var foo = 0 + 1 + -2 + 2; + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 2 + ╭─[no_magic_numbers.tsx:1:24] + 1 │ var foo = 0 + 1 + -2 + 2; + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 2 + ╭─[no_magic_numbers.tsx:1:19] + 1 │ var foo = 0 + 1 + 2; + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 10 + ╭─[no_magic_numbers.tsx:1:17] + 1 │ var foo = { bar:10 } + · ── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 0x1A + ╭─[no_magic_numbers.tsx:1:13] + 1 │ console.log(0x1A + 0x02); console.log(071); + · ──── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 0x02 + ╭─[no_magic_numbers.tsx:1:20] + 1 │ console.log(0x1A + 0x02); console.log(071); + · ──── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 071 + ╭─[no_magic_numbers.tsx:1:39] + 1 │ console.log(0x1A + 0x02); console.log(071); + · ─── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 42 + ╭─[no_magic_numbers.tsx:1:19] + 1 │ var stats = {avg: 42}; + · ── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 4 + ╭─[no_magic_numbers.tsx:1:67] + 1 │ var colors = {}; colors.RED = 2; colors.YELLOW = 3; colors.BLUE = 4 + 5; + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 5 + ╭─[no_magic_numbers.tsx:1:71] + 1 │ var colors = {}; colors.RED = 2; colors.YELLOW = 3; colors.BLUE = 4 + 5; + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 60 + ╭─[no_magic_numbers.tsx:1:39] + 1 │ function getSecondsInMinute() {return 60;} + · ── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: -60 + ╭─[no_magic_numbers.tsx:1:48] + 1 │ function getNegativeSecondsInMinute() {return -60;} + · ── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 3 + ╭─[no_magic_numbers.tsx:1:52] + 1 │ var data = ['foo', 'bar', 'baz']; var third = data[3]; + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 3 + ╭─[no_magic_numbers.tsx:1:52] + 1 │ var data = ['foo', 'bar', 'baz']; var third = data[3]; + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 3 + ╭─[no_magic_numbers.tsx:1:52] + 1 │ var data = ['foo', 'bar', 'baz']; var third = data[3]; + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: -100 + ╭─[no_magic_numbers.tsx:1:6] + 1 │ foo[-100] + · ─── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: -1.5 + ╭─[no_magic_numbers.tsx:1:6] + 1 │ foo[-1.5] + · ─── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: -1 + ╭─[no_magic_numbers.tsx:1:6] + 1 │ foo[-1] + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: -0.1 + ╭─[no_magic_numbers.tsx:1:6] + 1 │ foo[-0.1] + · ─── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: -0b110 + ╭─[no_magic_numbers.tsx:1:6] + 1 │ foo[-0b110] + · ───── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: -0o71 + ╭─[no_magic_numbers.tsx:1:6] + 1 │ foo[-0o71] + · ──── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: -0x12 + ╭─[no_magic_numbers.tsx:1:6] + 1 │ foo[-0x12] + · ──── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: -012 + ╭─[no_magic_numbers.tsx:1:6] + 1 │ foo[-012] + · ─── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 0.1 + ╭─[no_magic_numbers.tsx:1:5] + 1 │ foo[0.1] + · ─── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 0.12e1 + ╭─[no_magic_numbers.tsx:1:5] + 1 │ foo[0.12e1] + · ────── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 1.5 + ╭─[no_magic_numbers.tsx:1:5] + 1 │ foo[1.5] + · ─── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 1.678e2 + ╭─[no_magic_numbers.tsx:1:5] + 1 │ foo[1.678e2] + · ─────── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 56e-1 + ╭─[no_magic_numbers.tsx:1:5] + 1 │ foo[56e-1] + · ───── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 5.000000000000001 + ╭─[no_magic_numbers.tsx:1:5] + 1 │ foo[5.000000000000001] + · ───────────────── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 100.9 + ╭─[no_magic_numbers.tsx:1:5] + 1 │ foo[100.9] + · ───── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 4294967295 + ╭─[no_magic_numbers.tsx:1:5] + 1 │ foo[4294967295] + · ────────── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 1e300 + ╭─[no_magic_numbers.tsx:1:5] + 1 │ foo[1e300] + · ───── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 1e310 + ╭─[no_magic_numbers.tsx:1:5] + 1 │ foo[1e310] + · ───── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: -1e310 + ╭─[no_magic_numbers.tsx:1:6] + 1 │ foo[-1e310] + · ───── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 0 + ╭─[no_magic_numbers.tsx:1:6] + 1 │ foo[+0] + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 1 + ╭─[no_magic_numbers.tsx:1:6] + 1 │ foo[+1] + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: -1 + ╭─[no_magic_numbers.tsx:1:8] + 1 │ foo[-(-1)] + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 100 + ╭─[no_magic_numbers.tsx:1:1] + 1 │ 100 .toString() + · ─── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 200 + ╭─[no_magic_numbers.tsx:1:1] + 1 │ 200[100] + · ─── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 1 + ╭─[no_magic_numbers.tsx:1:27] + 1 │ var min, max, mean; min = 1; max = 10; mean = 4; + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 10 + ╭─[no_magic_numbers.tsx:1:36] + 1 │ var min, max, mean; min = 1; max = 10; mean = 4; + · ── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 4 + ╭─[no_magic_numbers.tsx:1:47] + 1 │ var min, max, mean; min = 1; max = 10; mean = 4; + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 123 + ╭─[no_magic_numbers.tsx:1:23] + 1 │ const func = (param = 123) => {} + · ─── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 123 + ╭─[no_magic_numbers.tsx:1:17] + 1 │ const { param = 123 } = sourceObject; + · ─── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 123 + ╭─[no_magic_numbers.tsx:1:17] + 1 │ const { param = 123 } = sourceObject; + · ─── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 123 + ╭─[no_magic_numbers.tsx:1:17] + 1 │ const { param = 123 } = sourceObject; + · ─── + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 1 + ╭─[no_magic_numbers.tsx:1:14] + 1 │ const [one = 1, two = 2] = [] + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 2 + ╭─[no_magic_numbers.tsx:1:23] + 1 │ const [one = 1, two = 2] = [] + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 1 + ╭─[no_magic_numbers.tsx:1:22] + 1 │ var one, two; [one = 1, two = 2] = [] + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 2 + ╭─[no_magic_numbers.tsx:1:31] + 1 │ var one, two; [one = 1, two = 2] = [] + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 2 + ╭─[no_magic_numbers.tsx:1:17] + 1 │ class C { foo = 2; } + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 2 + ╭─[no_magic_numbers.tsx:1:17] + 1 │ class C { foo = 2; } + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 2 + ╭─[no_magic_numbers.tsx:1:17] + 1 │ class C { foo = 2; } + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: -2 + ╭─[no_magic_numbers.tsx:1:18] + 1 │ class C { foo = -2; } + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 2 + ╭─[no_magic_numbers.tsx:1:24] + 1 │ class C { static foo = 2; } + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 2 + ╭─[no_magic_numbers.tsx:1:18] + 1 │ class C { #foo = 2; } + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 2 + ╭─[no_magic_numbers.tsx:1:25] + 1 │ class C { static #foo = 2; } + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 2 + ╭─[no_magic_numbers.tsx:1:17] + 1 │ class C { foo = 2 + 3; } + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 3 + ╭─[no_magic_numbers.tsx:1:21] + 1 │ class C { foo = 2 + 3; } + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 2 + ╭─[no_magic_numbers.tsx:1:11] + 1 │ class C { 2; } + · ─ + ╰──── + + ⚠ eslint(no-magic-numbers): No magic number: 2 + ╭─[no_magic_numbers.tsx:1:12] + 1 │ class C { [2]; } + · ─ + ╰──── From c99f8561f49bbb741489ea14145b3caad4b6cc0d Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sun, 1 Sep 2024 14:56:41 +0000 Subject: [PATCH 05/23] [autofix.ci] apply automated fixes --- .../src/rules/eslint/no_magic_numbers.rs | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs b/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs index b8b9f25991ca1..e44511bcbbada 100644 --- a/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs @@ -328,26 +328,24 @@ impl NoMagicNumbers { AstKind::UnaryExpression(unary) => { return is_unanary_index(&unary); } - AstKind::NumericLiteral(numeric) => { - match parent_node.kind() { - AstKind::MemberExpression(expression) => { - if let MemberExpression::ComputedMemberExpression(computed_expression) = - expression - { - return computed_expression.expression.is_number(numeric.value) - && numeric.value >= 0.0 - && numeric.value.fract() == 0.0 - && numeric.value < u32::MAX as f64; - } - - false + AstKind::NumericLiteral(numeric) => match parent_node.kind() { + AstKind::MemberExpression(expression) => { + if let MemberExpression::ComputedMemberExpression(computed_expression) = + expression + { + return computed_expression.expression.is_number(numeric.value) + && numeric.value >= 0.0 + && numeric.value.fract() == 0.0 + && numeric.value < u32::MAX as f64; } - AstKind::UnaryExpression(unary) => { - return is_unanary_index(&unary); - } - _ => false, + + false } - } + AstKind::UnaryExpression(unary) => { + return is_unanary_index(&unary); + } + _ => false, + }, _ => false, } } From a223b1d657085cd300af7b0fea4fd56aaf4ba70d Mon Sep 17 00:00:00 2001 From: Sysix Date: Sun, 1 Sep 2024 19:16:57 +0200 Subject: [PATCH 06/23] feat(linter): implement typescript/no-magic-numbers --- crates/oxc_linter/src/rules.rs | 4 +- .../no_magic_numbers.rs | 429 +++++++++++++--- .../src/snapshots/no_magic_numbers.snap | 482 +++++++++++++++--- 3 files changed, 785 insertions(+), 130 deletions(-) rename crates/oxc_linter/src/rules/{eslint => typescript}/no_magic_numbers.rs (60%) diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 150db37cdd73a..db2674593df1c 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -79,7 +79,6 @@ mod eslint { pub mod no_iterator; pub mod no_label_var; pub mod no_loss_of_precision; - pub mod no_magic_numbers; pub mod no_multi_str; pub mod no_new; pub mod no_new_native_nonconstructor; @@ -147,6 +146,7 @@ mod typescript { pub mod no_extra_non_null_assertion; pub mod no_extraneous_class; pub mod no_import_type_side_effects; + pub mod no_magic_numbers; pub mod no_misused_new; pub mod no_namespace; pub mod no_non_null_asserted_nullish_coalescing; @@ -509,7 +509,6 @@ oxc_macros::declare_all_lint_rules! { eslint::no_irregular_whitespace, eslint::no_iterator, eslint::no_loss_of_precision, - eslint::no_magic_numbers, eslint::no_multi_str, eslint::no_new, eslint::no_new_wrappers, @@ -569,6 +568,7 @@ oxc_macros::declare_all_lint_rules! { typescript::no_explicit_any, typescript::no_extra_non_null_assertion, typescript::no_import_type_side_effects, + typescript::no_magic_numbers, typescript::no_misused_new, typescript::no_namespace, typescript::no_non_null_asserted_optional_chain, diff --git a/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs similarity index 60% rename from crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs rename to crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs index e44511bcbbada..88913f3e84de1 100644 --- a/crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs @@ -2,13 +2,14 @@ use std::ops::Mul; use oxc_ast::{ ast::{ - Argument, AssignmentTarget, Expression, MemberExpression, UnaryExpression, + Argument, AssignmentTarget, Expression, MemberExpression, TSLiteral, UnaryExpression, VariableDeclarationKind, }, AstKind, }; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; +use oxc_semantic::AstNodes; use oxc_span::Span; use oxc_syntax::operator::UnaryOperator; @@ -47,6 +48,10 @@ pub struct NoMagicNumbersConfig { ignore_class_field_initial_values: bool, enforce_const: bool, detect_objects: bool, + ignore_enums: bool, + ignore_numeric_literal_types: bool, + ignore_readonly_class_properties: bool, + ignore_type_indexes: bool, } impl Default for NoMagicNumbersConfig { @@ -58,6 +63,10 @@ impl Default for NoMagicNumbersConfig { ignore_class_field_initial_values: false, enforce_const: false, detect_objects: false, + ignore_enums: false, + ignore_numeric_literal_types: false, + ignore_readonly_class_properties: false, + ignore_type_indexes: false, } } } @@ -77,7 +86,13 @@ impl TryFrom<&serde_json::Value> for NoMagicNumbersConfig { )) }, |object| { - println!("config: {object:?}"); + fn get_bool_property(object: &serde_json::Value, index: &str) -> bool { + object + .get(index) + .unwrap_or_else(|| &serde_json::Value::Bool(false)) + .as_bool() + .unwrap() + } Ok(Self { ignore: object .get("ignore") @@ -89,31 +104,24 @@ impl TryFrom<&serde_json::Value> for NoMagicNumbersConfig { .collect() }) .unwrap_or_default(), - ignore_array_indexes: object - .get("ignoreArrayIndexes") - .unwrap_or_else(|| &serde_json::Value::Bool(false)) - .as_bool() - .unwrap(), - ignore_default_values: object - .get("ignoreDefaultValues") - .unwrap_or_else(|| &serde_json::Value::Bool(false)) - .as_bool() - .unwrap(), - ignore_class_field_initial_values: object - .get("ignoreClassFieldInitialValues") - .unwrap_or_else(|| &serde_json::Value::Bool(false)) - .as_bool() - .unwrap(), - enforce_const: object - .get("enforceConst") - .unwrap_or_else(|| &serde_json::Value::Bool(false)) - .as_bool() - .unwrap(), - detect_objects: object - .get("detectObjects") - .unwrap_or_else(|| &serde_json::Value::Bool(false)) - .as_bool() - .unwrap(), + ignore_array_indexes: get_bool_property(object, "ignoreArrayIndexes"), + ignore_default_values: get_bool_property(object, "ignoreDefaultValues"), + ignore_class_field_initial_values: get_bool_property( + object, + "ignoreClassFieldInitialValues", + ), + enforce_const: get_bool_property(object, "enforceConst"), + detect_objects: get_bool_property(object, "detectObjects"), + ignore_enums: get_bool_property(object, "ignoreEnums"), + ignore_numeric_literal_types: get_bool_property( + object, + "ignoreNumericLiteralTypes", + ), + ignore_readonly_class_properties: get_bool_property( + object, + "ignoreReadonlyClassProperties", + ), + ignore_type_indexes: get_bool_property(object, "ignoreTypeIndexes"), }) }, ) @@ -151,6 +159,7 @@ impl InternConfig<'_> { node: &'a AstNode<'a>, parent_node: &'a AstNode<'a>, ) -> Result, OxcDiagnostic> { + let is_unary = matches!(parent_node.kind(), AstKind::UnaryExpression(_)); let is_negative = matches!(parent_node.kind(), AstKind::UnaryExpression(unary) if unary.operator == UnaryOperator::UnaryNegation); if let AstKind::NumericLiteral(numeric) = node.kind() { @@ -161,7 +170,11 @@ impl InternConfig<'_> { raw: format!("-{}", numeric.raw), }); } else { - return Ok(InternConfig { node, value: numeric.value, raw: numeric.raw.into() }); + return Ok(InternConfig { + node: if !is_unary { node } else { parent_node }, + value: numeric.value, + raw: numeric.raw.into(), + }); } } else { return Err(OxcDiagnostic::warn(format!( @@ -182,15 +195,15 @@ impl Rule for NoMagicNumbers { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { match node.kind() { AstKind::NumericLiteral(literal) => { - let config = InternConfig::from(node, ctx.nodes().parent_node(node.id()).unwrap()); + let nodes = ctx.nodes(); + let config = InternConfig::from(node, nodes.parent_node(node.id()).unwrap()); - let parent = ctx.nodes().parent_node(config.node.id()).unwrap(); - let parent_parent = ctx.nodes().parent_node(parent.id()).unwrap(); - - if self.is_skipable(&config, parent, parent_parent) { + if self.is_skipable(&config, &nodes) { return; } + let parent = nodes.parent_node(config.node.id()).unwrap(); + if let Some(reason) = self.get_report_reason(&parent, &config, &literal.span) { ctx.diagnostic(match reason { NoMagicNumberReportReason::MustUseConst(span) => { @@ -209,7 +222,6 @@ impl Rule for NoMagicNumbers { impl NoMagicNumbers { fn is_numeric_value<'a>(expression: &Expression<'a>, number: &f64) -> bool { - // check for Expression::NumericLiteral if expression.is_number(*number) { return true; } @@ -300,14 +312,6 @@ impl NoMagicNumbers { /// a[999999999999999999999] (even if it wasn't above the max index, it would be a["1e+21"]) /// a[1e310] (same as a["Infinity"]) fn is_array_index<'a>(&self, node: &AstNode<'a>, parent_node: &AstNode<'a>) -> bool { - println!( - " - - node: {node:?} - - " - ); - fn is_unanary_index(unary: &UnaryExpression) -> bool { match &unary.argument { Expression::NumericLiteral(numeric) => { @@ -350,22 +354,100 @@ impl NoMagicNumbers { } } - fn is_skipable<'a>( + fn is_ts_enum<'a>(&self, parent_node: &AstNode<'a>) -> bool { + return matches!(parent_node.kind(), AstKind::TSEnumMember(_)); + } + + fn is_ts_numeric_literal<'a>(&self, parent_node: &AstNode<'a>, parent_parent_node: &AstNode<'a>) -> bool { + if let AstKind::TSLiteralType(literal) = parent_node.kind() { + if !matches!( + literal.literal, + TSLiteral::NumericLiteral(_) | TSLiteral::UnaryExpression(_) + ) { + return false + } + + if matches!(parent_parent_node.kind(), AstKind::TSTypeAliasDeclaration(_) | AstKind::TSUnionType(_)) { + return true + } + + // https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/src/rules/no-magic-numbers.ts#L209 + } + + false + } + + fn is_ts_readonly_property<'a>(&self, parent_node: &AstNode<'a>) -> bool { + if let AstKind::PropertyDefinition(property) = parent_node.kind() { + return property.readonly; + } + return false; + } + + fn is_ts_indexed_access_type<'a>( &self, - config: &InternConfig<'a>, - parent: &AstNode<'a>, - parent_parent: &AstNode<'a>, + parent_parent_node: &AstNode<'a>, + ctx: &AstNodes<'a>, ) -> bool { - if self.is_ignore_value(&config.value) - || self.is_parse_int_radix(&config.value, &parent_parent) - || (self.ignore_default_values && self.is_default_value(&config.value, &parent)) - || (self.ignore_class_field_initial_values - && self.is_class_field_initial_value(&config.value, &parent)) - || (self.ignore_array_indexes && self.is_array_index(&config.node, &parent)) + if matches!(parent_parent_node.kind(), AstKind::TSIndexedAccessType(_)) { + return true; + } + + let mut node = parent_parent_node; + + while matches!( + node.kind(), + AstKind::TSUnionType(_) + | AstKind::TSIntersectionType(_) + | AstKind::TSParenthesizedType(_) + ) { + node = ctx.parent_node(node.id()).unwrap(); + } + + return matches!(node.kind(), AstKind::TSIndexedAccessType(_)); + } + + fn is_skipable<'a>(&self, config: &InternConfig<'a>, ctx: &AstNodes<'a>) -> bool { + let parent: &AstNode<'a> = ctx.parent_node(config.node.id()).unwrap(); + let parent_parent = ctx.parent_node(parent.id()).unwrap(); + + if self.is_ignore_value(&config.value) { + return true; + } + if self.is_parse_int_radix(&config.value, &parent_parent) { + return true; + } + + if self.ignore_numeric_literal_types && self.is_ts_numeric_literal(&parent, &parent_parent) { + return true; + } + + if self.ignore_enums && self.is_ts_enum(&parent) { + return true; + } + + if self.ignore_readonly_class_properties && self.is_ts_readonly_property(&parent) { + return true; + } + + if self.ignore_type_indexes && self.is_ts_indexed_access_type(&parent_parent, &ctx) { + return true; + } + + if self.ignore_default_values && self.is_default_value(&config.value, &parent) { + return true; + } + + if self.ignore_class_field_initial_values + && self.is_class_field_initial_value(&config.value, &parent) { return true; } + if self.ignore_array_indexes && self.is_array_index(&config.node, &parent) { + return true; + } + if !self.detect_objects && (matches!(parent.kind(), AstKind::ObjectExpression(_)) || matches!(parent.kind(), AstKind::ObjectProperty(_))) @@ -412,6 +494,9 @@ fn test() { let enforce_const = Some(serde_json::json!([{ "enforceConst": true}])); let ignore_class_field_initial_values = Some(serde_json::json!([{ "ignoreClassFieldInitialValues": true }])); + let ignore_numeric_literal_types = + Some(serde_json::json!([{ "ignoreNumericLiteralTypes": true }])); + let ignore_typed_index_arrays = Some(serde_json::json!([{ "ignoreTypeIndexes": true }])); let pass = vec![ ("var x = parseInt(y, 10);", None), @@ -451,16 +536,16 @@ fn test() { ("foo[0123]", ignore_array_indexes.clone()), // { "sourceType": "script" }, ("foo[5.0000000000000001]", ignore_array_indexes.clone()), ("foo[4294967294]", ignore_array_indexes.clone()), - // ("foo[0n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, - // ("foo[-0n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, - // ("foo[1n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, - // ("foo[100n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, - // ("foo[0xABn]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, - // ("foo[4294967294n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + ("foo[0n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + ("foo[-0n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + ("foo[1n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + ("foo[100n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + ("foo[0xABn]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, + ("foo[4294967294n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, ("var a = ;", None), // { "parserOptions": { "ecmaFeatures": { "jsx": true } } }, ("var a =
;", None), // { "parserOptions": { "ecmaFeatures": { "jsx": true } } }, - // ("f(100n)", Some(serde_json::json!([{ "ignore": ["100n"] }]))), // { "ecmaVersion": 2020 }, - // ("f(-100n)", Some(serde_json::json!([{ "ignore": ["-100n"] }]))), // { "ecmaVersion": 2020 }, + ("f(100n)", Some(serde_json::json!([{ "ignore": ["100n"] }]))), // { "ecmaVersion": 2020 }, + ("f(-100n)", Some(serde_json::json!([{ "ignore": ["-100n"] }]))), // { "ecmaVersion": 2020 }, ("const { param = 123 } = sourceObject;", ignore_default_values.clone()), // { "ecmaVersion": 6 }, ("const func = (param = 123) => {}", ignore_default_values.clone()), // { "ecmaVersion": 6 }, ("const func = ({ param = 123 }) => {}", ignore_default_values.clone()), // { "ecmaVersion": 6 }, @@ -475,6 +560,128 @@ fn test() { ("class C { static foo = 2; }", ignore_class_field_initial_values.clone()), // { "ecmaVersion": 2022 }, ("class C { #foo = 2; }", ignore_class_field_initial_values.clone()), // { "ecmaVersion": 2022 }, ("class C { static #foo = 2; }", ignore_class_field_initial_values.clone()), // { "ecmaVersion": 2022 } + ("const FOO = 10;", ignore_numeric_literal_types.clone()), + ("type Foo = 'bar';", None), + ("type Foo = true;", None), + ("type Foo = 1;", ignore_numeric_literal_types.clone()), + ("type Foo = -1;", ignore_numeric_literal_types.clone()), + ("type Foo = 1 | 2 | 3;", ignore_numeric_literal_types.clone()), + ("type Foo = 1 | -1;", ignore_numeric_literal_types.clone()), + ( + " + enum foo { + SECOND = 1000, + NUM = '0123456789', + NEG = -1, + POS = +1, + } + ", + Some(serde_json::json!([{ "ignoreEnums": true }])), + ), + ( + " + class Foo { + readonly A = 1; + readonly B = 2; + public static readonly C = 1; + static readonly D = 1; + readonly E = -1; + readonly F = +1; + private readonly G = 100n; + } + ", + Some(serde_json::json!([{ "ignoreReadonlyClassProperties": true }])), + ), + ("type Foo = Bar[0];", ignore_typed_index_arrays.clone()), + ("type Foo = Bar[-1];", ignore_typed_index_arrays.clone()), + ("type Foo = Bar[0xab];", ignore_typed_index_arrays.clone()), + ("type Foo = Bar[5.6e1];", ignore_typed_index_arrays.clone()), + ("type Foo = Bar[10n];", ignore_typed_index_arrays.clone()), + ("type Foo = Bar[1 | -2];", ignore_typed_index_arrays.clone()), + ("type Foo = Bar[1 & -2];", ignore_typed_index_arrays.clone()), + ("type Foo = Bar[1 & number];", ignore_typed_index_arrays.clone()), + ("type Foo = Bar[((1 & -2) | 3) | 4];", ignore_typed_index_arrays.clone()), + ("type Foo = Parameters[2];", ignore_typed_index_arrays.clone()), + ("type Foo = Bar['baz'];", ignore_typed_index_arrays.clone()), + ("type Foo = Bar['baz'];", Some(serde_json::json!([{ "ignoreTypeIndexes": false }]))), + ( + " + type Others = [['a'], ['b']]; + + type Foo = { + [K in keyof Others[0]]: Others[K]; + }; + ", + Some(serde_json::json!([{ "ignoreTypeIndexes": true }])), + ), + ("type Foo = 1;", Some(serde_json::json!([{ "ignore": [1] }]))), + ("type Foo = -2;", Some(serde_json::json!([{ "ignore": [-2] }]))), + ("type Foo = 3n;", Some(serde_json::json!([{ "ignore": ["3n"] }]))), + ("type Foo = -4n;", Some(serde_json::json!([{ "ignore": ["-4n"] }]))), + ("type Foo = 5.6;", Some(serde_json::json!([{ "ignore": [5.6] }]))), + ("type Foo = -7.8;", Some(serde_json::json!([{ "ignore": [-7.8] }]))), + ("type Foo = 0x0a;", Some(serde_json::json!([{ "ignore": [0x0a] }]))), + ("type Foo = -0xbc;", Some(serde_json::json!([{ "ignore": [-0xbc] }]))), + ("type Foo = 1e2;", Some(serde_json::json!([{ "ignore": [1e2] }]))), + ("type Foo = -3e4;", Some(serde_json::json!([{ "ignore": [-3e4] }]))), + ("type Foo = 5e-6;", Some(serde_json::json!([{ "ignore": [5e-6] }]))), + ("type Foo = -7e-8;", Some(serde_json::json!([{ "ignore": [-7e-8] }]))), + ("type Foo = 1.1e2;", Some(serde_json::json!([{ "ignore": [1.1e2] }]))), + ("type Foo = -3.1e4;", Some(serde_json::json!([{ "ignore": [-3.1e4] }]))), + ("type Foo = 5.1e-6;", Some(serde_json::json!([{ "ignore": [5.1e-6] }]))), + ("type Foo = -7.1e-8;", Some(serde_json::json!([{ "ignore": [-7.1e-8] }]))), + ( + " + interface Foo { + bar: 1; + } + ", + Some(serde_json::json!([{ "ignoreNumericLiteralTypes": true, "ignore": [1] }])), + ), + ( + " + enum foo { + SECOND = 1000, + NUM = '0123456789', + NEG = -1, + POS = +2, + } + ", + Some(serde_json::json!([{ "ignoreEnums": false, "ignore": [1000, -1, 2] }])), + ), + ( + " + class Foo { + readonly A = 1; + readonly B = 2; + public static readonly C = 3; + static readonly D = 4; + readonly E = -5; + readonly F = +6; + private readonly G = 100n; + private static readonly H = -2000n; + } + ", + Some( + serde_json::json!([ { "ignoreReadonlyClassProperties": false, "ignore": [1, 2, 3, 4, -5, 6, "100n", "-2000n"], }, ]), + ), + ), + ( + "type Foo = Bar[0];", + Some(serde_json::json!([{ "ignoreTypeIndexes": false, "ignore": [0] }])), + ), + ( + " + type Other = { + [0]: 3; + }; + + type Foo = { + [K in keyof Other]: `${K & number}`; + }; + ", + Some(serde_json::json!([{ "ignoreTypeIndexes": true, "ignore": [0, 3] }])), + ), ]; let fail = vec![ @@ -587,6 +794,108 @@ fn test() { ("class C { foo = 2 + 3; }", ignore_class_field_initial_values.clone()), // { "ecmaVersion": 2022 }, ("class C { 2; }", ignore_class_field_initial_values.clone()), // { "ecmaVersion": 2022 }, ("class C { [2]; }", ignore_class_field_initial_values.clone()), // { "ecmaVersion": 2022 } + ("type Foo = 1;", Some(serde_json::json!([{ "ignoreNumericLiteralTypes": false }]))), + ("type Foo = -1;", Some(serde_json::json!([{ "ignoreNumericLiteralTypes": false }]))), + ( + "type Foo = 1 | 2 | 3;", + Some(serde_json::json!([{ "ignoreNumericLiteralTypes": false }])), + ), + ("type Foo = 1 | -1;", Some(serde_json::json!([{ "ignoreNumericLiteralTypes": false }]))), + ( + " + interface Foo { + bar: 1; + } + ", + Some(serde_json::json!([{ "ignoreNumericLiteralTypes": true }])), + ), + ( + " + enum foo { + SECOND = 1000, + NUM = '0123456789', + NEG = -1, + POS = +1, + } + ", + Some(serde_json::json!([{ "ignoreEnums": false }])), + ), + ( + " + class Foo { + readonly A = 1; + readonly B = 2; + public static readonly C = 3; + static readonly D = 4; + readonly E = -5; + readonly F = +6; + private readonly G = 100n; + } + ", + Some(serde_json::json!([{ "ignoreReadonlyClassProperties": false }])), + ), + ("type Foo = Bar[0];", Some(serde_json::json!([{ "ignoreTypeIndexes": false }]))), + ("type Foo = Bar[-1];", Some(serde_json::json!([{ "ignoreTypeIndexes": false }]))), + ("type Foo = Bar[0xab];", Some(serde_json::json!([{ "ignoreTypeIndexes": false }]))), + ("type Foo = Bar[5.6e1];", Some(serde_json::json!([{ "ignoreTypeIndexes": false }]))), + // ("type Foo = Bar[10n];", Some(serde_json::json!([{ "ignoreTypeIndexes": false }]))), + ("type Foo = Bar[1 | -2];", Some(serde_json::json!([{ "ignoreTypeIndexes": false }]))), + ("type Foo = Bar[1 & -2];", Some(serde_json::json!([{ "ignoreTypeIndexes": false }]))), + ("type Foo = Bar[1 & number];", Some(serde_json::json!([{ "ignoreTypeIndexes": false }]))), + ( + "type Foo = Bar[((1 & -2) | 3) | 4];", + Some(serde_json::json!([{ "ignoreTypeIndexes": false }])), + ), + ( + "type Foo = Parameters[2];", + Some(serde_json::json!([{ "ignoreTypeIndexes": false }])), + ), + ( + " + type Others = [['a'], ['b']]; + + type Foo = { + [K in keyof Others[0]]: Others[K]; + }; + ", + Some(serde_json::json!([{ "ignoreTypeIndexes": false }])), + ), + ( + " + type Other = { + [0]: 3; + }; + + type Foo = { + [K in keyof Other]: `${K & number}`; + }; + ", + Some(serde_json::json!([{ "ignoreTypeIndexes": true }])), + ), + ( + " + type Foo = { + [K in 0 | 1 | 2]: 0; + }; + ", + Some(serde_json::json!([{ "ignoreTypeIndexes": true }])), + ), + ("type Foo = 1;", Some(serde_json::json!([{ "ignore": [-1] }]))), + ("type Foo = -2;", Some(serde_json::json!([{ "ignore": [2] }]))), + // ("type Foo = 3n;", Some(serde_json::json!([{ "ignore": ["-3n"] }]))), + // ("type Foo = -4n;", Some(serde_json::json!([{ "ignore": ["4n"] }]))), + ("type Foo = 5.6;", Some(serde_json::json!([{ "ignore": [-5.6] }]))), + ("type Foo = -7.8;", Some(serde_json::json!([{ "ignore": [7.8] }]))), + ("type Foo = 0x0a;", Some(serde_json::json!([{ "ignore": [-0x0a] }]))), + ("type Foo = -0xbc;", Some(serde_json::json!([{ "ignore": [0xbc] }]))), + ("type Foo = 1e2;", Some(serde_json::json!([{ "ignore": [-1e2] }]))), + ("type Foo = -3e4;", Some(serde_json::json!([{ "ignore": [3e4] }]))), + ("type Foo = 5e-6;", Some(serde_json::json!([{ "ignore": [-5e-6] }]))), + ("type Foo = -7e-8;", Some(serde_json::json!([{ "ignore": [7e-8] }]))), + ("type Foo = 1.1e2;", Some(serde_json::json!([{ "ignore": [-1.1e2] }]))), + ("type Foo = -3.1e4;", Some(serde_json::json!([{ "ignore": [3.1e4] }]))), + ("type Foo = 5.1e-6;", Some(serde_json::json!([{ "ignore": [-5.1e-6] }]))), + ("type Foo = -7.1e-8;", Some(serde_json::json!([{ "ignore": [7.1e-8] }]))), ]; Tester::new(NoMagicNumbers::NAME, pass, fail).test_and_snapshot(); diff --git a/crates/oxc_linter/src/snapshots/no_magic_numbers.snap b/crates/oxc_linter/src/snapshots/no_magic_numbers.snap index d3bb77a8b3561..892a4449a7152 100644 --- a/crates/oxc_linter/src/snapshots/no_magic_numbers.snap +++ b/crates/oxc_linter/src/snapshots/no_magic_numbers.snap @@ -1,410 +1,756 @@ --- source: crates/oxc_linter/src/tester.rs --- - ⚠ eslint(no-magic-numbers): Number constants declarations must use 'const'. + ⚠ typescript-eslint(no-magic-numbers): Number constants declarations must use 'const'. ╭─[no_magic_numbers.tsx:1:11] 1 │ var foo = 42 · ── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 0 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 0 ╭─[no_magic_numbers.tsx:1:11] 1 │ var foo = 0 + 1; · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 1 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 ╭─[no_magic_numbers.tsx:1:15] 1 │ var foo = 0 + 1; · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 5 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 5 ╭─[no_magic_numbers.tsx:1:9] 1 │ a = a + 5; · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 5 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 5 ╭─[no_magic_numbers.tsx:1:6] 1 │ a += 5; · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 0 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 0 ╭─[no_magic_numbers.tsx:1:11] 1 │ var foo = 0 + 1 + -2 + 2; · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 1 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 ╭─[no_magic_numbers.tsx:1:15] 1 │ var foo = 0 + 1 + -2 + 2; · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: -2 + ⚠ typescript-eslint(no-magic-numbers): No magic number: -2 ╭─[no_magic_numbers.tsx:1:20] 1 │ var foo = 0 + 1 + -2 + 2; · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 2 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 ╭─[no_magic_numbers.tsx:1:24] 1 │ var foo = 0 + 1 + -2 + 2; · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 2 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 ╭─[no_magic_numbers.tsx:1:19] 1 │ var foo = 0 + 1 + 2; · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 10 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 10 ╭─[no_magic_numbers.tsx:1:17] 1 │ var foo = { bar:10 } · ── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 0x1A + ⚠ typescript-eslint(no-magic-numbers): No magic number: 0x1A ╭─[no_magic_numbers.tsx:1:13] 1 │ console.log(0x1A + 0x02); console.log(071); · ──── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 0x02 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 0x02 ╭─[no_magic_numbers.tsx:1:20] 1 │ console.log(0x1A + 0x02); console.log(071); · ──── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 071 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 071 ╭─[no_magic_numbers.tsx:1:39] 1 │ console.log(0x1A + 0x02); console.log(071); · ─── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 42 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 42 ╭─[no_magic_numbers.tsx:1:19] 1 │ var stats = {avg: 42}; · ── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 4 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 4 ╭─[no_magic_numbers.tsx:1:67] 1 │ var colors = {}; colors.RED = 2; colors.YELLOW = 3; colors.BLUE = 4 + 5; · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 5 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 5 ╭─[no_magic_numbers.tsx:1:71] 1 │ var colors = {}; colors.RED = 2; colors.YELLOW = 3; colors.BLUE = 4 + 5; · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 60 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 60 ╭─[no_magic_numbers.tsx:1:39] 1 │ function getSecondsInMinute() {return 60;} · ── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: -60 + ⚠ typescript-eslint(no-magic-numbers): No magic number: -60 ╭─[no_magic_numbers.tsx:1:48] 1 │ function getNegativeSecondsInMinute() {return -60;} · ── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 3 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 3 ╭─[no_magic_numbers.tsx:1:52] 1 │ var data = ['foo', 'bar', 'baz']; var third = data[3]; · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 3 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 3 ╭─[no_magic_numbers.tsx:1:52] 1 │ var data = ['foo', 'bar', 'baz']; var third = data[3]; · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 3 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 3 ╭─[no_magic_numbers.tsx:1:52] 1 │ var data = ['foo', 'bar', 'baz']; var third = data[3]; · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: -100 + ⚠ typescript-eslint(no-magic-numbers): No magic number: -100 ╭─[no_magic_numbers.tsx:1:6] 1 │ foo[-100] · ─── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: -1.5 + ⚠ typescript-eslint(no-magic-numbers): No magic number: -1.5 ╭─[no_magic_numbers.tsx:1:6] 1 │ foo[-1.5] · ─── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: -1 + ⚠ typescript-eslint(no-magic-numbers): No magic number: -1 ╭─[no_magic_numbers.tsx:1:6] 1 │ foo[-1] · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: -0.1 + ⚠ typescript-eslint(no-magic-numbers): No magic number: -0.1 ╭─[no_magic_numbers.tsx:1:6] 1 │ foo[-0.1] · ─── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: -0b110 + ⚠ typescript-eslint(no-magic-numbers): No magic number: -0b110 ╭─[no_magic_numbers.tsx:1:6] 1 │ foo[-0b110] · ───── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: -0o71 + ⚠ typescript-eslint(no-magic-numbers): No magic number: -0o71 ╭─[no_magic_numbers.tsx:1:6] 1 │ foo[-0o71] · ──── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: -0x12 + ⚠ typescript-eslint(no-magic-numbers): No magic number: -0x12 ╭─[no_magic_numbers.tsx:1:6] 1 │ foo[-0x12] · ──── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: -012 + ⚠ typescript-eslint(no-magic-numbers): No magic number: -012 ╭─[no_magic_numbers.tsx:1:6] 1 │ foo[-012] · ─── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 0.1 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 0.1 ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[0.1] · ─── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 0.12e1 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 0.12e1 ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[0.12e1] · ────── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 1.5 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1.5 ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[1.5] · ─── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 1.678e2 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1.678e2 ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[1.678e2] · ─────── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 56e-1 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 56e-1 ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[56e-1] · ───── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 5.000000000000001 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 5.000000000000001 ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[5.000000000000001] · ───────────────── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 100.9 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 100.9 ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[100.9] · ───── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 4294967295 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 4294967295 ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[4294967295] · ────────── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 1e300 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1e300 ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[1e300] · ───── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 1e310 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1e310 ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[1e310] · ───── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: -1e310 + ⚠ typescript-eslint(no-magic-numbers): No magic number: -1e310 ╭─[no_magic_numbers.tsx:1:6] 1 │ foo[-1e310] · ───── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 0 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 0 ╭─[no_magic_numbers.tsx:1:6] 1 │ foo[+0] · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 1 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 ╭─[no_magic_numbers.tsx:1:6] 1 │ foo[+1] · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: -1 + ⚠ typescript-eslint(no-magic-numbers): No magic number: -1 ╭─[no_magic_numbers.tsx:1:8] 1 │ foo[-(-1)] · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 100 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 100 ╭─[no_magic_numbers.tsx:1:1] 1 │ 100 .toString() · ─── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 200 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 200 ╭─[no_magic_numbers.tsx:1:1] 1 │ 200[100] · ─── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 1 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 ╭─[no_magic_numbers.tsx:1:27] 1 │ var min, max, mean; min = 1; max = 10; mean = 4; · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 10 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 10 ╭─[no_magic_numbers.tsx:1:36] 1 │ var min, max, mean; min = 1; max = 10; mean = 4; · ── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 4 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 4 ╭─[no_magic_numbers.tsx:1:47] 1 │ var min, max, mean; min = 1; max = 10; mean = 4; · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 123 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 123 ╭─[no_magic_numbers.tsx:1:23] 1 │ const func = (param = 123) => {} · ─── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 123 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 123 ╭─[no_magic_numbers.tsx:1:17] 1 │ const { param = 123 } = sourceObject; · ─── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 123 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 123 ╭─[no_magic_numbers.tsx:1:17] 1 │ const { param = 123 } = sourceObject; · ─── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 123 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 123 ╭─[no_magic_numbers.tsx:1:17] 1 │ const { param = 123 } = sourceObject; · ─── ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 1 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 ╭─[no_magic_numbers.tsx:1:14] 1 │ const [one = 1, two = 2] = [] · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 2 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 ╭─[no_magic_numbers.tsx:1:23] 1 │ const [one = 1, two = 2] = [] · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 1 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 ╭─[no_magic_numbers.tsx:1:22] 1 │ var one, two; [one = 1, two = 2] = [] · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 2 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 ╭─[no_magic_numbers.tsx:1:31] 1 │ var one, two; [one = 1, two = 2] = [] · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 2 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 ╭─[no_magic_numbers.tsx:1:17] 1 │ class C { foo = 2; } · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 2 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 ╭─[no_magic_numbers.tsx:1:17] 1 │ class C { foo = 2; } · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 2 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 ╭─[no_magic_numbers.tsx:1:17] 1 │ class C { foo = 2; } · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: -2 + ⚠ typescript-eslint(no-magic-numbers): No magic number: -2 ╭─[no_magic_numbers.tsx:1:18] 1 │ class C { foo = -2; } · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 2 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 ╭─[no_magic_numbers.tsx:1:24] 1 │ class C { static foo = 2; } · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 2 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 ╭─[no_magic_numbers.tsx:1:18] 1 │ class C { #foo = 2; } · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 2 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 ╭─[no_magic_numbers.tsx:1:25] 1 │ class C { static #foo = 2; } · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 2 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 ╭─[no_magic_numbers.tsx:1:17] 1 │ class C { foo = 2 + 3; } · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 3 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 3 ╭─[no_magic_numbers.tsx:1:21] 1 │ class C { foo = 2 + 3; } · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 2 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 ╭─[no_magic_numbers.tsx:1:11] 1 │ class C { 2; } · ─ ╰──── - ⚠ eslint(no-magic-numbers): No magic number: 2 + ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 ╭─[no_magic_numbers.tsx:1:12] 1 │ class C { [2]; } · ─ ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 + ╭─[no_magic_numbers.tsx:1:12] + 1 │ type Foo = 1; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: -1 + ╭─[no_magic_numbers.tsx:1:13] + 1 │ type Foo = -1; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 + ╭─[no_magic_numbers.tsx:1:12] + 1 │ type Foo = 1 | 2 | 3; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 + ╭─[no_magic_numbers.tsx:1:16] + 1 │ type Foo = 1 | 2 | 3; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 3 + ╭─[no_magic_numbers.tsx:1:20] + 1 │ type Foo = 1 | 2 | 3; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 + ╭─[no_magic_numbers.tsx:1:12] + 1 │ type Foo = 1 | -1; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: -1 + ╭─[no_magic_numbers.tsx:1:17] + 1 │ type Foo = 1 | -1; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 + ╭─[no_magic_numbers.tsx:3:11] + 2 │ interface Foo { + 3 │ bar: 1; + · ─ + 4 │ } + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1000 + ╭─[no_magic_numbers.tsx:3:15] + 2 │ enum foo { + 3 │ SECOND = 1000, + · ──── + 4 │ NUM = '0123456789', + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: -1 + ╭─[no_magic_numbers.tsx:5:13] + 4 │ NUM = '0123456789', + 5 │ NEG = -1, + · ─ + 6 │ POS = +1, + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 + ╭─[no_magic_numbers.tsx:6:13] + 5 │ NEG = -1, + 6 │ POS = +1, + · ─ + 7 │ } + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 + ╭─[no_magic_numbers.tsx:3:19] + 2 │ class Foo { + 3 │ readonly A = 1; + · ─ + 4 │ readonly B = 2; + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 + ╭─[no_magic_numbers.tsx:4:19] + 3 │ readonly A = 1; + 4 │ readonly B = 2; + · ─ + 5 │ public static readonly C = 3; + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 3 + ╭─[no_magic_numbers.tsx:5:33] + 4 │ readonly B = 2; + 5 │ public static readonly C = 3; + · ─ + 6 │ static readonly D = 4; + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 4 + ╭─[no_magic_numbers.tsx:6:26] + 5 │ public static readonly C = 3; + 6 │ static readonly D = 4; + · ─ + 7 │ readonly E = -5; + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: -5 + ╭─[no_magic_numbers.tsx:7:20] + 6 │ static readonly D = 4; + 7 │ readonly E = -5; + · ─ + 8 │ readonly F = +6; + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 6 + ╭─[no_magic_numbers.tsx:8:20] + 7 │ readonly E = -5; + 8 │ readonly F = +6; + · ─ + 9 │ private readonly G = 100n; + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 0 + ╭─[no_magic_numbers.tsx:1:16] + 1 │ type Foo = Bar[0]; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: -1 + ╭─[no_magic_numbers.tsx:1:17] + 1 │ type Foo = Bar[-1]; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 0xab + ╭─[no_magic_numbers.tsx:1:16] + 1 │ type Foo = Bar[0xab]; + · ──── + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 5.6e1 + ╭─[no_magic_numbers.tsx:1:16] + 1 │ type Foo = Bar[5.6e1]; + · ───── + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 + ╭─[no_magic_numbers.tsx:1:16] + 1 │ type Foo = Bar[1 | -2]; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: -2 + ╭─[no_magic_numbers.tsx:1:21] + 1 │ type Foo = Bar[1 | -2]; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 + ╭─[no_magic_numbers.tsx:1:16] + 1 │ type Foo = Bar[1 & -2]; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: -2 + ╭─[no_magic_numbers.tsx:1:21] + 1 │ type Foo = Bar[1 & -2]; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 + ╭─[no_magic_numbers.tsx:1:16] + 1 │ type Foo = Bar[1 & number]; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 + ╭─[no_magic_numbers.tsx:1:18] + 1 │ type Foo = Bar[((1 & -2) | 3) | 4]; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: -2 + ╭─[no_magic_numbers.tsx:1:23] + 1 │ type Foo = Bar[((1 & -2) | 3) | 4]; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 3 + ╭─[no_magic_numbers.tsx:1:28] + 1 │ type Foo = Bar[((1 & -2) | 3) | 4]; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 4 + ╭─[no_magic_numbers.tsx:1:33] + 1 │ type Foo = Bar[((1 & -2) | 3) | 4]; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 + ╭─[no_magic_numbers.tsx:1:28] + 1 │ type Foo = Parameters[2]; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 0 + ╭─[no_magic_numbers.tsx:5:25] + 4 │ type Foo = { + 5 │ [K in keyof Others[0]]: Others[K]; + · ─ + 6 │ }; + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 0 + ╭─[no_magic_numbers.tsx:3:7] + 2 │ type Other = { + 3 │ [0]: 3; + · ─ + 4 │ }; + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 3 + ╭─[no_magic_numbers.tsx:3:11] + 2 │ type Other = { + 3 │ [0]: 3; + · ─ + 4 │ }; + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 0 + ╭─[no_magic_numbers.tsx:3:12] + 2 │ type Foo = { + 3 │ [K in 0 | 1 | 2]: 0; + · ─ + 4 │ }; + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 + ╭─[no_magic_numbers.tsx:3:16] + 2 │ type Foo = { + 3 │ [K in 0 | 1 | 2]: 0; + · ─ + 4 │ }; + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 + ╭─[no_magic_numbers.tsx:3:20] + 2 │ type Foo = { + 3 │ [K in 0 | 1 | 2]: 0; + · ─ + 4 │ }; + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 0 + ╭─[no_magic_numbers.tsx:3:24] + 2 │ type Foo = { + 3 │ [K in 0 | 1 | 2]: 0; + · ─ + 4 │ }; + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 + ╭─[no_magic_numbers.tsx:1:12] + 1 │ type Foo = 1; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: -2 + ╭─[no_magic_numbers.tsx:1:13] + 1 │ type Foo = -2; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 5.6 + ╭─[no_magic_numbers.tsx:1:12] + 1 │ type Foo = 5.6; + · ─── + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: -7.8 + ╭─[no_magic_numbers.tsx:1:13] + 1 │ type Foo = -7.8; + · ─── + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 0x0a + ╭─[no_magic_numbers.tsx:1:12] + 1 │ type Foo = 0x0a; + · ──── + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: -0xbc + ╭─[no_magic_numbers.tsx:1:13] + 1 │ type Foo = -0xbc; + · ──── + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1e2 + ╭─[no_magic_numbers.tsx:1:12] + 1 │ type Foo = 1e2; + · ─── + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: -3e4 + ╭─[no_magic_numbers.tsx:1:13] + 1 │ type Foo = -3e4; + · ─── + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 5e-6 + ╭─[no_magic_numbers.tsx:1:12] + 1 │ type Foo = 5e-6; + · ──── + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: -7e-8 + ╭─[no_magic_numbers.tsx:1:13] + 1 │ type Foo = -7e-8; + · ──── + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1.1e2 + ╭─[no_magic_numbers.tsx:1:12] + 1 │ type Foo = 1.1e2; + · ───── + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: -3.1e4 + ╭─[no_magic_numbers.tsx:1:13] + 1 │ type Foo = -3.1e4; + · ───── + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 5.1e-6 + ╭─[no_magic_numbers.tsx:1:12] + 1 │ type Foo = 5.1e-6; + · ────── + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: -7.1e-8 + ╭─[no_magic_numbers.tsx:1:13] + 1 │ type Foo = -7.1e-8; + · ────── + ╰──── From 41d0dc439fc8924075a961630fa31c7ad5351dc4 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sun, 1 Sep 2024 17:17:46 +0000 Subject: [PATCH 07/23] [autofix.ci] apply automated fixes --- .../src/rules/typescript/no_magic_numbers.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs index 88913f3e84de1..c085709ef5685 100644 --- a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs @@ -358,17 +358,24 @@ impl NoMagicNumbers { return matches!(parent_node.kind(), AstKind::TSEnumMember(_)); } - fn is_ts_numeric_literal<'a>(&self, parent_node: &AstNode<'a>, parent_parent_node: &AstNode<'a>) -> bool { + fn is_ts_numeric_literal<'a>( + &self, + parent_node: &AstNode<'a>, + parent_parent_node: &AstNode<'a>, + ) -> bool { if let AstKind::TSLiteralType(literal) = parent_node.kind() { if !matches!( literal.literal, TSLiteral::NumericLiteral(_) | TSLiteral::UnaryExpression(_) ) { - return false + return false; } - if matches!(parent_parent_node.kind(), AstKind::TSTypeAliasDeclaration(_) | AstKind::TSUnionType(_)) { - return true + if matches!( + parent_parent_node.kind(), + AstKind::TSTypeAliasDeclaration(_) | AstKind::TSUnionType(_) + ) { + return true; } // https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/src/rules/no-magic-numbers.ts#L209 @@ -418,7 +425,8 @@ impl NoMagicNumbers { return true; } - if self.ignore_numeric_literal_types && self.is_ts_numeric_literal(&parent, &parent_parent) { + if self.ignore_numeric_literal_types && self.is_ts_numeric_literal(&parent, &parent_parent) + { return true; } From 0de0e77ee0e4818aa00b1b8bc96044bcc5689398 Mon Sep 17 00:00:00 2001 From: Sysix Date: Sun, 1 Sep 2024 21:05:35 +0200 Subject: [PATCH 08/23] feat(linter): implement typescript/no-magic-numbers --- .../src/rules/typescript/no_magic_numbers.rs | 320 +++++++++++------- 1 file changed, 197 insertions(+), 123 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs index 88913f3e84de1..12dff91b8960e 100644 --- a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs @@ -20,12 +20,12 @@ enum NoMagicNumberReportReason<'a> { NoMagicNumber(&'a Span, &'a str), } -fn must_use_const_diagnostic(span: &Span) -> OxcDiagnostic { - OxcDiagnostic::warn("Number constants declarations must use 'const'.").with_label(*span) +fn must_use_const_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("Number constants declarations must use 'const'.").with_label(span) } -fn no_magic_number_diagnostic(span: &Span, raw: &str) -> OxcDiagnostic { - OxcDiagnostic::warn(format!("No magic number: {raw}")).with_label(*span) +fn no_magic_number_diagnostic(span: Span, raw: &str) -> OxcDiagnostic { + OxcDiagnostic::warn(format!("No magic number: {raw}")).with_label(span) } #[derive(Debug, Default, Clone)] @@ -40,7 +40,7 @@ impl std::ops::Deref for NoMagicNumbers { } } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Default)] pub struct NoMagicNumbersConfig { ignore: Vec, ignore_array_indexes: bool, @@ -54,23 +54,6 @@ pub struct NoMagicNumbersConfig { ignore_type_indexes: bool, } -impl Default for NoMagicNumbersConfig { - fn default() -> Self { - Self { - ignore: vec![], - ignore_array_indexes: false, - ignore_default_values: false, - ignore_class_field_initial_values: false, - enforce_const: false, - detect_objects: false, - ignore_enums: false, - ignore_numeric_literal_types: false, - ignore_readonly_class_properties: false, - ignore_type_indexes: false, - } - } -} - impl TryFrom<&serde_json::Value> for NoMagicNumbersConfig { type Error = OxcDiagnostic; @@ -79,10 +62,10 @@ impl TryFrom<&serde_json::Value> for NoMagicNumbersConfig { return Ok(NoMagicNumbersConfig::default()); } - raw.as_array().unwrap().get(0).map_or_else( + raw.as_array().unwrap().first().map_or_else( || { Err(OxcDiagnostic::warn( - "Expecting object for eslint/no-magic-numbers configuration", + "Expecting object for typescript/no-magic-numbers configuration", )) }, |object| { @@ -100,7 +83,6 @@ impl TryFrom<&serde_json::Value> for NoMagicNumbersConfig { .map(|v| { v.iter() .filter_map(serde_json::Value::as_f64) - .map(|v| f64::try_from(v).unwrap()) .collect() }) .unwrap_or_default(), @@ -131,20 +113,114 @@ impl TryFrom<&serde_json::Value> for NoMagicNumbersConfig { declare_oxc_lint!( /// ### What it does /// + /// The no-magic-numbers rule aims to make code more readable and refactoring easier by ensuring that special numbers are declared as constants to make their meaning explicit. + /// The current implementation does not support BigInt numbers. /// /// ### Why is this bad? /// - /// - /// ### Example + /// ‘Magic numbers’ are numbers that occur multiple times in code without an explicit meaning. They should preferably be replaced by named constants. + /// + /// ### Example bad + /// ```javascript + /// + /// var dutyFreePrice = 100; + /// var finalPrice = dutyFreePrice + (dutyFreePrice * 0.25); + /// ``` + /// + /// ### Example good with "ignore" + /// ```javascript + /// /*typescript no-magic-numbers: ["error", { "ignore": [1] }]*/ + /// var data = ['foo', 'bar', 'baz']; + /// var dataLast = data.length && data[data.length - 1]; + /// ``` + /// + /// ### Example good with "ignoreArrayIndexes" + /// ```javascript + /// /*typescript no-magic-numbers: ["error", { "ignoreArrayIndexes": true }]*/ + /// var item = data[2]; + /// data[100] = a; + /// f(data[0]); + /// a = data[-0]; // same as data[0], -0 will be coerced to "0" + /// a = data[0xAB]; + /// a = data[5.6e1]; + /// a = data[4294967294]; // max array index + /// ``` + /// + /// ### Example good with "ignoreDefaultValues" + /// ```javascript + /// /*typescript no-magic-numbers: ["error", { "ignoreDefaultValues": true }]*/ + /// const { tax = 0.25 } = accountancy; + /// function mapParallel(concurrency = 3) { /***/ } + /// ``` + /// + /// ### Example good with "ignoreClassFieldInitialValues" + /// ```javascript + /// /*typescript no-magic-numbers: ["error", { "ignoreClassFieldInitialValues": true }]*/ + /// class C { + /// foo = 2; + /// bar = -3; + /// #baz = 4; + /// static qux = 5; + /// } + /// ``` + /// + /// ### Example bad with "enforceConst" /// ```javascript + /// /*typescript no-magic-numbers: ["error", { "enforceConst": true }]*/ + /// var TAX = 0.25; + /// ``` + /// + /// ### Example bad with "detectObjects" + /// ```javascript + /// /*typescript no-magic-numbers: ["error", { "detectObjects": true }]*/ + /// var magic = { + /// tax: 0.25 + /// }; + /// ``` + /// + /// ### Example good with "detectObjects" + /// ```javascript + /// /*typescript no-magic-numbers: ["error", { "detectObjects": true }]*/ + /// var TAX = 0.25; + /// + /// var magic = { + /// tax: TAX + /// }; + /// ``` + /// + /// ### Example good with "ignoreEnums" + /// ```typescript + /// /*typescript no-magic-numbers: ["error", { "ignoreEnums": true }]*/ + /// enum foo { + /// SECOND = 1000, + /// } + /// ``` + /// + /// ### Example good with "ignoreNumericLiteralTypes" + /// ```typescript + /// /*typescript no-magic-numbers: ["error", { "ignoreNumericLiteralTypes": true }]*/ + /// type SmallPrimes = 2 | 3 | 5 | 7 | 11; + /// ``` + /// + /// ### Example good with "ignoreReadonlyClassProperties" + /// ```typescript + /// /*typescript no-magic-numbers: ["error", { "ignoreReadonlyClassProperties": true }]*/ + /// class Foo { + /// readonly A = 1; + /// readonly B = 2; + /// public static readonly C = 1; + /// static readonly D = 1; + /// } + /// ``` + /// + /// ### Example good with "ignoreTypeIndexes" + /// ```typescript + /// /*typescript no-magic-numbers: ["error", { "ignoreReadonlyClassProperties": true }]*/ + /// type Foo = Bar[0]; + /// type Baz = Parameters[2]; /// ``` NoMagicNumbers, - nursery, // TODO: change category to `correctness`, `suspicious`, `pedantic`, `perf`, `restriction`, or `style` - // See for details - - pending // TODO: describe fix capabilities. Remove if no fix can be done, - // keep at 'pending' if you think one could be added but don't know how. - // Options are 'fix', 'fix-dangerous', 'suggestion', and 'suggestion-dangerous' + style, ); #[derive(Debug)] @@ -164,23 +240,23 @@ impl InternConfig<'_> { if let AstKind::NumericLiteral(numeric) = node.kind() { if is_negative { - return Ok(InternConfig { + Ok(InternConfig { node: parent_node, value: 0.0 - numeric.value, raw: format!("-{}", numeric.raw), - }); + }) } else { - return Ok(InternConfig { - node: if !is_unary { node } else { parent_node }, + Ok(InternConfig { + node: if is_unary { parent_node } else { node }, value: numeric.value, raw: numeric.raw.into(), - }); + }) } } else { - return Err(OxcDiagnostic::warn(format!( + Err(OxcDiagnostic::warn(format!( "expected AstKind BingIntLiteral or NumericLiteral, got {:?}", node.kind().debug_name() - ))); + ))) } } @@ -190,46 +266,43 @@ impl InternConfig<'_> { } impl Rule for NoMagicNumbers { fn from_configuration(value: serde_json::Value) -> Self { - return Self(Box::new(NoMagicNumbersConfig::try_from(&value).unwrap())); + Self(Box::new(NoMagicNumbersConfig::try_from(&value).unwrap())) } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { - match node.kind() { - AstKind::NumericLiteral(literal) => { - let nodes = ctx.nodes(); - let config = InternConfig::from(node, nodes.parent_node(node.id()).unwrap()); + if let AstKind::NumericLiteral(literal) = node.kind() { + let nodes = ctx.nodes(); + let config = InternConfig::from(node, nodes.parent_node(node.id()).unwrap()); - if self.is_skipable(&config, &nodes) { - return; - } + if self.is_skipable(&config, nodes) { + return; + } - let parent = nodes.parent_node(config.node.id()).unwrap(); - - if let Some(reason) = self.get_report_reason(&parent, &config, &literal.span) { - ctx.diagnostic(match reason { - NoMagicNumberReportReason::MustUseConst(span) => { - must_use_const_diagnostic(span) - } - NoMagicNumberReportReason::NoMagicNumber(span, raw) => { - no_magic_number_diagnostic(span, raw) - } - }); - } + let parent = nodes.parent_node(config.node.id()).unwrap(); + + if let Some(reason) = self.get_report_reason(parent, &config, &literal.span) { + ctx.diagnostic(match reason { + NoMagicNumberReportReason::MustUseConst(span) => { + must_use_const_diagnostic(*span) + } + NoMagicNumberReportReason::NoMagicNumber(span, raw) => { + no_magic_number_diagnostic(*span, raw) + } + }); } - _ => {} } } } impl NoMagicNumbers { - fn is_numeric_value<'a>(expression: &Expression<'a>, number: &f64) -> bool { - if expression.is_number(*number) { + fn is_numeric_value(expression: &Expression<'_>, number: f64) -> bool { + if expression.is_number(number) { return true; } if let Expression::UnaryExpression(unary) = expression { if let Expression::NumericLiteral(_) = &unary.argument { if unary.operator == UnaryOperator::UnaryPlus { - return unary.argument.is_number(*number); + return unary.argument.is_number(number); } if unary.operator == UnaryOperator::UnaryNegation { @@ -240,11 +313,11 @@ impl NoMagicNumbers { false } - fn is_ignore_value(&self, number: &f64) -> bool { - self.ignore.contains(number) + fn is_ignore_value(&self, number: f64) -> bool { + self.ignore.contains(&number) } - fn is_default_value<'a>(&self, number: &f64, parent_node: &AstNode<'a>) -> bool { + fn is_default_value(number: f64, parent_node: &AstNode<'_>) -> bool { if let AstKind::AssignmentTargetWithDefault(assignment) = parent_node.kind() { return NoMagicNumbers::is_numeric_value(&assignment.init, number); } @@ -256,7 +329,7 @@ impl NoMagicNumbers { false } - fn is_class_field_initial_value<'a>(&self, number: &f64, parent_node: &AstNode<'a>) -> bool { + fn is_class_field_initial_value(number: f64, parent_node: &AstNode<'_>) -> bool { if let AstKind::PropertyDefinition(property) = parent_node.kind() { return NoMagicNumbers::is_numeric_value(property.value.as_ref().unwrap(), number); } @@ -264,7 +337,7 @@ impl NoMagicNumbers { false } - fn is_parse_int_radix<'a>(&self, number: &f64, parent_parent_node: &AstNode<'a>) -> bool { + fn is_parse_int_radix(number: f64, parent_parent_node: &AstNode<'_>) -> bool { if let AstKind::CallExpression(expression) = parent_parent_node.kind() { if expression.arguments.get(1).is_none() { return false; @@ -272,7 +345,7 @@ impl NoMagicNumbers { let argument = expression.arguments.get(1).unwrap(); return match argument { - Argument::NumericLiteral(numeric) => numeric.value == *number, + Argument::NumericLiteral(numeric) => numeric.value == number, Argument::UnaryExpression(unary) if unary.operator == UnaryOperator::UnaryNegation => { @@ -289,48 +362,47 @@ impl NoMagicNumbers { false } - /// Returns whether the given node is used as an array index. - /// Value must coerce to a valid array index name: "0", "1", "2" ... "4294967294". - /// - /// All other values, like "-1", "2.5", or "4294967295", are just "normal" object properties, - /// which can be created and accessed on an array in addition to the array index properties, - /// but they don't affect array's length and are not considered by methods such as .map(), .forEach() etc. + /// Returns whether the given node is used as an array index. + /// Value must coerce to a valid array index name: "0", "1", "2" ... "4294967294". /// - /// The maximum array length by the specification is 2 ** 32 - 1 = 4294967295, - /// thus the maximum valid index is 2 ** 32 - 2 = 4294967294. + /// All other values, like "-1", "2.5", or "4294967295", are just "normal" object properties, + /// which can be created and accessed on an array in addition to the array index properties, + /// but they don't affect array's length and are not considered by methods such as .map(), .forEach() etc. /// - /// All notations are allowed, as long as the value coerces to one of "0", "1", "2" ... "4294967294". + /// The maximum array length by the specification is 2 ** 32 - 1 = 4294967295, + /// thus the maximum valid index is 2 ** 32 - 2 = 4294967294. /// - /// Valid examples: - /// a[0], a[1], a[1.2e1], a[0xAB], a[0n], a[1n] - /// a[-0] (same as a[0] because -0 coerces to "0") - /// a[-0n] (-0n evaluates to 0n) + /// All notations are allowed, as long as the value coerces to one of "0", "1", "2" ... "4294967294". /// - /// Invalid examples: - /// a[-1], a[-0xAB], a[-1n], a[2.5], a[1.23e1], a[12e-1] - /// a[4294967295] (above the max index, it's an access to a regular property a["4294967295"]) - /// a[999999999999999999999] (even if it wasn't above the max index, it would be a["1e+21"]) - /// a[1e310] (same as a["Infinity"]) - fn is_array_index<'a>(&self, node: &AstNode<'a>, parent_node: &AstNode<'a>) -> bool { + /// Valid examples: + /// ```javascript + /// a[0], a[1], a[1.2e1], a[0xAB], a[0n], a[1n] + /// a[-0] // (same as a[0] because -0 coerces to "0") + /// a[-0n] // (-0n evaluates to 0n) + /// ``` + /// + /// Invalid examples: + /// ```javascript + /// a[-1], a[-0xAB], a[-1n], a[2.5], a[1.23e1], a[12e-1] + /// a[4294967295] // (above the max index, it's an access to a regular property a["4294967295"]) + /// a[999999999999999999999] // (even if it wasn't above the max index, it would be a["1e+21"]) + /// a[1e310] // (same as a["Infinity"]) + /// ``` + fn is_array_index<'a>(node: &AstNode<'a>, parent_node: &AstNode<'a>) -> bool { fn is_unanary_index(unary: &UnaryExpression) -> bool { match &unary.argument { Expression::NumericLiteral(numeric) => { if unary.operator == UnaryOperator::UnaryNegation { return numeric.value == 0.0; } - // ToDo: check why ("foo[+0]", ignore_array_indexes.clone()), should fail - return false; - - // return numeric.value >= 0.0 - // && numeric.value.fract() == 0.0 - // && numeric.value < u32::MAX as f64; + false } _ => false, } } match node.kind() { AstKind::UnaryExpression(unary) => { - return is_unanary_index(&unary); + is_unanary_index(unary) } AstKind::NumericLiteral(numeric) => match parent_node.kind() { AstKind::MemberExpression(expression) => { @@ -340,13 +412,13 @@ impl NoMagicNumbers { return computed_expression.expression.is_number(numeric.value) && numeric.value >= 0.0 && numeric.value.fract() == 0.0 - && numeric.value < u32::MAX as f64; + && numeric.value < f64::from(u32::MAX); } false } AstKind::UnaryExpression(unary) => { - return is_unanary_index(&unary); + is_unanary_index(unary) } _ => false, }, @@ -354,11 +426,11 @@ impl NoMagicNumbers { } } - fn is_ts_enum<'a>(&self, parent_node: &AstNode<'a>) -> bool { - return matches!(parent_node.kind(), AstKind::TSEnumMember(_)); + fn is_ts_enum(parent_node: &AstNode<'_>) -> bool { + matches!(parent_node.kind(), AstKind::TSEnumMember(_)) } - fn is_ts_numeric_literal<'a>(&self, parent_node: &AstNode<'a>, parent_parent_node: &AstNode<'a>) -> bool { + fn is_ts_numeric_literal<'a>(parent_node: &AstNode<'a>, parent_parent_node: &AstNode<'a>) -> bool { if let AstKind::TSLiteralType(literal) = parent_node.kind() { if !matches!( literal.literal, @@ -377,15 +449,15 @@ impl NoMagicNumbers { false } - fn is_ts_readonly_property<'a>(&self, parent_node: &AstNode<'a>) -> bool { + fn is_ts_readonly_property(parent_node: &AstNode<'_>) -> bool { if let AstKind::PropertyDefinition(property) = parent_node.kind() { return property.readonly; } - return false; + + false } fn is_ts_indexed_access_type<'a>( - &self, parent_parent_node: &AstNode<'a>, ctx: &AstNodes<'a>, ) -> bool { @@ -404,54 +476,56 @@ impl NoMagicNumbers { node = ctx.parent_node(node.id()).unwrap(); } - return matches!(node.kind(), AstKind::TSIndexedAccessType(_)); + matches!(node.kind(), AstKind::TSIndexedAccessType(_)) } fn is_skipable<'a>(&self, config: &InternConfig<'a>, ctx: &AstNodes<'a>) -> bool { - let parent: &AstNode<'a> = ctx.parent_node(config.node.id()).unwrap(); - let parent_parent = ctx.parent_node(parent.id()).unwrap(); - - if self.is_ignore_value(&config.value) { + if self.is_ignore_value(config.value) { return true; } - if self.is_parse_int_radix(&config.value, &parent_parent) { + + let parent = ctx.parent_node(config.node.id()).unwrap(); + + if self.ignore_enums && NoMagicNumbers::is_ts_enum(parent) { return true; } - if self.ignore_numeric_literal_types && self.is_ts_numeric_literal(&parent, &parent_parent) { + if self.ignore_readonly_class_properties && NoMagicNumbers::is_ts_readonly_property(parent) { return true; } - if self.ignore_enums && self.is_ts_enum(&parent) { + if self.ignore_default_values && NoMagicNumbers::is_default_value(config.value, parent) { return true; } - if self.ignore_readonly_class_properties && self.is_ts_readonly_property(&parent) { + if self.ignore_class_field_initial_values + && NoMagicNumbers::is_class_field_initial_value(config.value, parent) + { return true; } - if self.ignore_type_indexes && self.is_ts_indexed_access_type(&parent_parent, &ctx) { + if self.ignore_array_indexes && NoMagicNumbers::is_array_index(config.node, parent) { return true; } - if self.ignore_default_values && self.is_default_value(&config.value, &parent) { + if !self.detect_objects + && (matches!(parent.kind(), AstKind::ObjectExpression(_)) + || matches!(parent.kind(), AstKind::ObjectProperty(_))) + { return true; } - if self.ignore_class_field_initial_values - && self.is_class_field_initial_value(&config.value, &parent) - { + let parent_parent = ctx.parent_node(parent.id()).unwrap(); + + if NoMagicNumbers::is_parse_int_radix(config.value, parent_parent) { return true; } - if self.ignore_array_indexes && self.is_array_index(&config.node, &parent) { + if self.ignore_numeric_literal_types && NoMagicNumbers::is_ts_numeric_literal(parent, parent_parent) { return true; } - if !self.detect_objects - && (matches!(parent.kind(), AstKind::ObjectExpression(_)) - || matches!(parent.kind(), AstKind::ObjectProperty(_))) - { + if self.ignore_type_indexes && NoMagicNumbers::is_ts_indexed_access_type(parent_parent, ctx) { return true; } From 04c4d3c28a4e9fc688f5ac221602ab5bf4349337 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sun, 1 Sep 2024 19:10:09 +0000 Subject: [PATCH 09/23] [autofix.ci] apply automated fixes --- .../src/rules/typescript/no_magic_numbers.rs | 70 +++++++++---------- 1 file changed, 33 insertions(+), 37 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs index a488b5807e08d..95b1c24449ebd 100644 --- a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs @@ -80,11 +80,7 @@ impl TryFrom<&serde_json::Value> for NoMagicNumbersConfig { ignore: object .get("ignore") .and_then(serde_json::Value::as_array) - .map(|v| { - v.iter() - .filter_map(serde_json::Value::as_f64) - .collect() - }) + .map(|v| v.iter().filter_map(serde_json::Value::as_f64).collect()) .unwrap_or_default(), ignore_array_indexes: get_bool_property(object, "ignoreArrayIndexes"), ignore_default_values: get_bool_property(object, "ignoreDefaultValues"), @@ -119,21 +115,21 @@ declare_oxc_lint!( /// ### Why is this bad? /// /// ‘Magic numbers’ are numbers that occur multiple times in code without an explicit meaning. They should preferably be replaced by named constants. - /// + /// /// ### Example bad /// ```javascript - /// + /// /// var dutyFreePrice = 100; /// var finalPrice = dutyFreePrice + (dutyFreePrice * 0.25); /// ``` - /// + /// /// ### Example good with "ignore" /// ```javascript /// /*typescript no-magic-numbers: ["error", { "ignore": [1] }]*/ /// var data = ['foo', 'bar', 'baz']; /// var dataLast = data.length && data[data.length - 1]; /// ``` - /// + /// /// ### Example good with "ignoreArrayIndexes" /// ```javascript /// /*typescript no-magic-numbers: ["error", { "ignoreArrayIndexes": true }]*/ @@ -145,14 +141,14 @@ declare_oxc_lint!( /// a = data[5.6e1]; /// a = data[4294967294]; // max array index /// ``` - /// + /// /// ### Example good with "ignoreDefaultValues" /// ```javascript /// /*typescript no-magic-numbers: ["error", { "ignoreDefaultValues": true }]*/ /// const { tax = 0.25 } = accountancy; /// function mapParallel(concurrency = 3) { /***/ } /// ``` - /// + /// /// ### Example good with "ignoreClassFieldInitialValues" /// ```javascript /// /*typescript no-magic-numbers: ["error", { "ignoreClassFieldInitialValues": true }]*/ @@ -163,13 +159,13 @@ declare_oxc_lint!( /// static qux = 5; /// } /// ``` - /// + /// /// ### Example bad with "enforceConst" /// ```javascript /// /*typescript no-magic-numbers: ["error", { "enforceConst": true }]*/ /// var TAX = 0.25; /// ``` - /// + /// /// ### Example bad with "detectObjects" /// ```javascript /// /*typescript no-magic-numbers: ["error", { "detectObjects": true }]*/ @@ -177,17 +173,17 @@ declare_oxc_lint!( /// tax: 0.25 /// }; /// ``` - /// + /// /// ### Example good with "detectObjects" /// ```javascript /// /*typescript no-magic-numbers: ["error", { "detectObjects": true }]*/ /// var TAX = 0.25; - /// + /// /// var magic = { /// tax: TAX /// }; /// ``` - /// + /// /// ### Example good with "ignoreEnums" /// ```typescript /// /*typescript no-magic-numbers: ["error", { "ignoreEnums": true }]*/ @@ -195,13 +191,13 @@ declare_oxc_lint!( /// SECOND = 1000, /// } /// ``` - /// + /// /// ### Example good with "ignoreNumericLiteralTypes" /// ```typescript /// /*typescript no-magic-numbers: ["error", { "ignoreNumericLiteralTypes": true }]*/ /// type SmallPrimes = 2 | 3 | 5 | 7 | 11; /// ``` - /// + /// /// ### Example good with "ignoreReadonlyClassProperties" /// ```typescript /// /*typescript no-magic-numbers: ["error", { "ignoreReadonlyClassProperties": true }]*/ @@ -212,7 +208,7 @@ declare_oxc_lint!( /// static readonly D = 1; /// } /// ``` - /// + /// /// ### Example good with "ignoreTypeIndexes" /// ```typescript /// /*typescript no-magic-numbers: ["error", { "ignoreReadonlyClassProperties": true }]*/ @@ -379,15 +375,15 @@ impl NoMagicNumbers { /// a[0], a[1], a[1.2e1], a[0xAB], a[0n], a[1n] /// a[-0] // (same as a[0] because -0 coerces to "0") /// a[-0n] // (-0n evaluates to 0n) - /// ``` - /// + /// ``` + /// /// Invalid examples: /// ```javascript /// a[-1], a[-0xAB], a[-1n], a[2.5], a[1.23e1], a[12e-1] /// a[4294967295] // (above the max index, it's an access to a regular property a["4294967295"]) /// a[999999999999999999999] // (even if it wasn't above the max index, it would be a["1e+21"]) /// a[1e310] // (same as a["Infinity"]) - /// ``` + /// ``` fn is_array_index<'a>(node: &AstNode<'a>, parent_node: &AstNode<'a>) -> bool { fn is_unanary_index(unary: &UnaryExpression) -> bool { match &unary.argument { @@ -401,9 +397,7 @@ impl NoMagicNumbers { } } match node.kind() { - AstKind::UnaryExpression(unary) => { - is_unanary_index(unary) - } + AstKind::UnaryExpression(unary) => is_unanary_index(unary), AstKind::NumericLiteral(numeric) => match parent_node.kind() { AstKind::MemberExpression(expression) => { if let MemberExpression::ComputedMemberExpression(computed_expression) = @@ -417,9 +411,7 @@ impl NoMagicNumbers { false } - AstKind::UnaryExpression(unary) => { - is_unanary_index(unary) - } + AstKind::UnaryExpression(unary) => is_unanary_index(unary), _ => false, }, _ => false, @@ -430,7 +422,10 @@ impl NoMagicNumbers { matches!(parent_node.kind(), AstKind::TSEnumMember(_)) } - fn is_ts_numeric_literal<'a>(parent_node: &AstNode<'a>, parent_parent_node: &AstNode<'a>) -> bool { + fn is_ts_numeric_literal<'a>( + parent_node: &AstNode<'a>, + parent_parent_node: &AstNode<'a>, + ) -> bool { if let AstKind::TSLiteralType(literal) = parent_node.kind() { if !matches!( literal.literal, @@ -460,10 +455,7 @@ impl NoMagicNumbers { false } - fn is_ts_indexed_access_type<'a>( - parent_parent_node: &AstNode<'a>, - ctx: &AstNodes<'a>, - ) -> bool { + fn is_ts_indexed_access_type<'a>(parent_parent_node: &AstNode<'a>, ctx: &AstNodes<'a>) -> bool { if matches!(parent_parent_node.kind(), AstKind::TSIndexedAccessType(_)) { return true; } @@ -486,14 +478,15 @@ impl NoMagicNumbers { if self.is_ignore_value(config.value) { return true; } - + let parent = ctx.parent_node(config.node.id()).unwrap(); if self.ignore_enums && NoMagicNumbers::is_ts_enum(parent) { return true; } - if self.ignore_readonly_class_properties && NoMagicNumbers::is_ts_readonly_property(parent) { + if self.ignore_readonly_class_properties && NoMagicNumbers::is_ts_readonly_property(parent) + { return true; } @@ -524,11 +517,14 @@ impl NoMagicNumbers { return true; } - if self.ignore_numeric_literal_types && NoMagicNumbers::is_ts_numeric_literal(parent, parent_parent) { + if self.ignore_numeric_literal_types + && NoMagicNumbers::is_ts_numeric_literal(parent, parent_parent) + { return true; } - if self.ignore_type_indexes && NoMagicNumbers::is_ts_indexed_access_type(parent_parent, ctx) { + if self.ignore_type_indexes && NoMagicNumbers::is_ts_indexed_access_type(parent_parent, ctx) + { return true; } From 54bff0a106fdf6dc47df2187f13a849cc5c3cce9 Mon Sep 17 00:00:00 2001 From: Sysix Date: Sun, 1 Sep 2024 21:53:16 +0200 Subject: [PATCH 10/23] feat(linter): implement typescript/no-magic-numbers --- .../src/rules/typescript/no_magic_numbers.rs | 87 ++++++++++--------- 1 file changed, 44 insertions(+), 43 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs index a488b5807e08d..87cb4cf922c20 100644 --- a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs @@ -80,11 +80,7 @@ impl TryFrom<&serde_json::Value> for NoMagicNumbersConfig { ignore: object .get("ignore") .and_then(serde_json::Value::as_array) - .map(|v| { - v.iter() - .filter_map(serde_json::Value::as_f64) - .collect() - }) + .map(|v| v.iter().filter_map(serde_json::Value::as_f64).collect()) .unwrap_or_default(), ignore_array_indexes: get_bool_property(object, "ignoreArrayIndexes"), ignore_default_values: get_bool_property(object, "ignoreDefaultValues"), @@ -119,21 +115,21 @@ declare_oxc_lint!( /// ### Why is this bad? /// /// ‘Magic numbers’ are numbers that occur multiple times in code without an explicit meaning. They should preferably be replaced by named constants. - /// + /// /// ### Example bad /// ```javascript - /// + /// /// var dutyFreePrice = 100; /// var finalPrice = dutyFreePrice + (dutyFreePrice * 0.25); /// ``` - /// + /// /// ### Example good with "ignore" /// ```javascript /// /*typescript no-magic-numbers: ["error", { "ignore": [1] }]*/ /// var data = ['foo', 'bar', 'baz']; /// var dataLast = data.length && data[data.length - 1]; /// ``` - /// + /// /// ### Example good with "ignoreArrayIndexes" /// ```javascript /// /*typescript no-magic-numbers: ["error", { "ignoreArrayIndexes": true }]*/ @@ -145,14 +141,14 @@ declare_oxc_lint!( /// a = data[5.6e1]; /// a = data[4294967294]; // max array index /// ``` - /// + /// /// ### Example good with "ignoreDefaultValues" /// ```javascript /// /*typescript no-magic-numbers: ["error", { "ignoreDefaultValues": true }]*/ /// const { tax = 0.25 } = accountancy; /// function mapParallel(concurrency = 3) { /***/ } /// ``` - /// + /// /// ### Example good with "ignoreClassFieldInitialValues" /// ```javascript /// /*typescript no-magic-numbers: ["error", { "ignoreClassFieldInitialValues": true }]*/ @@ -163,13 +159,13 @@ declare_oxc_lint!( /// static qux = 5; /// } /// ``` - /// + /// /// ### Example bad with "enforceConst" /// ```javascript /// /*typescript no-magic-numbers: ["error", { "enforceConst": true }]*/ /// var TAX = 0.25; /// ``` - /// + /// /// ### Example bad with "detectObjects" /// ```javascript /// /*typescript no-magic-numbers: ["error", { "detectObjects": true }]*/ @@ -177,17 +173,17 @@ declare_oxc_lint!( /// tax: 0.25 /// }; /// ``` - /// + /// /// ### Example good with "detectObjects" /// ```javascript /// /*typescript no-magic-numbers: ["error", { "detectObjects": true }]*/ /// var TAX = 0.25; - /// + /// /// var magic = { /// tax: TAX /// }; /// ``` - /// + /// /// ### Example good with "ignoreEnums" /// ```typescript /// /*typescript no-magic-numbers: ["error", { "ignoreEnums": true }]*/ @@ -195,13 +191,13 @@ declare_oxc_lint!( /// SECOND = 1000, /// } /// ``` - /// + /// /// ### Example good with "ignoreNumericLiteralTypes" /// ```typescript /// /*typescript no-magic-numbers: ["error", { "ignoreNumericLiteralTypes": true }]*/ /// type SmallPrimes = 2 | 3 | 5 | 7 | 11; /// ``` - /// + /// /// ### Example good with "ignoreReadonlyClassProperties" /// ```typescript /// /*typescript no-magic-numbers: ["error", { "ignoreReadonlyClassProperties": true }]*/ @@ -212,7 +208,7 @@ declare_oxc_lint!( /// static readonly D = 1; /// } /// ``` - /// + /// /// ### Example good with "ignoreTypeIndexes" /// ```typescript /// /*typescript no-magic-numbers: ["error", { "ignoreReadonlyClassProperties": true }]*/ @@ -294,7 +290,10 @@ impl Rule for NoMagicNumbers { } impl NoMagicNumbers { - fn is_numeric_value(expression: &Expression<'_>, number: f64) -> bool { + fn is_numeric_f64(actual: f64, expected: f64) -> bool { + (actual - expected).abs() < f64::EPSILON + } + fn is_numeric_expression(expression: &Expression<'_>, number: f64) -> bool { if expression.is_number(number) { return true; } @@ -319,11 +318,11 @@ impl NoMagicNumbers { fn is_default_value(number: f64, parent_node: &AstNode<'_>) -> bool { if let AstKind::AssignmentTargetWithDefault(assignment) = parent_node.kind() { - return NoMagicNumbers::is_numeric_value(&assignment.init, number); + return NoMagicNumbers::is_numeric_expression(&assignment.init, number); } if let AstKind::AssignmentPattern(pattern) = parent_node.kind() { - return NoMagicNumbers::is_numeric_value(&pattern.right, number); + return NoMagicNumbers::is_numeric_expression(&pattern.right, number); } false @@ -331,7 +330,7 @@ impl NoMagicNumbers { fn is_class_field_initial_value(number: f64, parent_node: &AstNode<'_>) -> bool { if let AstKind::PropertyDefinition(property) = parent_node.kind() { - return NoMagicNumbers::is_numeric_value(property.value.as_ref().unwrap(), number); + return NoMagicNumbers::is_numeric_expression(property.value.as_ref().unwrap(), number); } false @@ -345,12 +344,14 @@ impl NoMagicNumbers { let argument = expression.arguments.get(1).unwrap(); return match argument { - Argument::NumericLiteral(numeric) => numeric.value == number, + Argument::NumericLiteral(numeric) => { + NoMagicNumbers::is_numeric_f64(numeric.value, number) + } Argument::UnaryExpression(unary) if unary.operator == UnaryOperator::UnaryNegation => { if let Expression::NumericLiteral(numeric) = &unary.argument { - return numeric.value == number.mul(-1.0); + return NoMagicNumbers::is_numeric_f64(numeric.value, number.mul(-1.0)); } return false; @@ -379,15 +380,15 @@ impl NoMagicNumbers { /// a[0], a[1], a[1.2e1], a[0xAB], a[0n], a[1n] /// a[-0] // (same as a[0] because -0 coerces to "0") /// a[-0n] // (-0n evaluates to 0n) - /// ``` - /// + /// ``` + /// /// Invalid examples: /// ```javascript /// a[-1], a[-0xAB], a[-1n], a[2.5], a[1.23e1], a[12e-1] /// a[4294967295] // (above the max index, it's an access to a regular property a["4294967295"]) /// a[999999999999999999999] // (even if it wasn't above the max index, it would be a["1e+21"]) /// a[1e310] // (same as a["Infinity"]) - /// ``` + /// ``` fn is_array_index<'a>(node: &AstNode<'a>, parent_node: &AstNode<'a>) -> bool { fn is_unanary_index(unary: &UnaryExpression) -> bool { match &unary.argument { @@ -401,9 +402,7 @@ impl NoMagicNumbers { } } match node.kind() { - AstKind::UnaryExpression(unary) => { - is_unanary_index(unary) - } + AstKind::UnaryExpression(unary) => is_unanary_index(unary), AstKind::NumericLiteral(numeric) => match parent_node.kind() { AstKind::MemberExpression(expression) => { if let MemberExpression::ComputedMemberExpression(computed_expression) = @@ -417,9 +416,7 @@ impl NoMagicNumbers { false } - AstKind::UnaryExpression(unary) => { - is_unanary_index(unary) - } + AstKind::UnaryExpression(unary) => is_unanary_index(unary), _ => false, }, _ => false, @@ -430,7 +427,10 @@ impl NoMagicNumbers { matches!(parent_node.kind(), AstKind::TSEnumMember(_)) } - fn is_ts_numeric_literal<'a>(parent_node: &AstNode<'a>, parent_parent_node: &AstNode<'a>) -> bool { + fn is_ts_numeric_literal<'a>( + parent_node: &AstNode<'a>, + parent_parent_node: &AstNode<'a>, + ) -> bool { if let AstKind::TSLiteralType(literal) = parent_node.kind() { if !matches!( literal.literal, @@ -460,10 +460,7 @@ impl NoMagicNumbers { false } - fn is_ts_indexed_access_type<'a>( - parent_parent_node: &AstNode<'a>, - ctx: &AstNodes<'a>, - ) -> bool { + fn is_ts_indexed_access_type<'a>(parent_parent_node: &AstNode<'a>, ctx: &AstNodes<'a>) -> bool { if matches!(parent_parent_node.kind(), AstKind::TSIndexedAccessType(_)) { return true; } @@ -486,14 +483,15 @@ impl NoMagicNumbers { if self.is_ignore_value(config.value) { return true; } - + let parent = ctx.parent_node(config.node.id()).unwrap(); if self.ignore_enums && NoMagicNumbers::is_ts_enum(parent) { return true; } - if self.ignore_readonly_class_properties && NoMagicNumbers::is_ts_readonly_property(parent) { + if self.ignore_readonly_class_properties && NoMagicNumbers::is_ts_readonly_property(parent) + { return true; } @@ -524,11 +522,14 @@ impl NoMagicNumbers { return true; } - if self.ignore_numeric_literal_types && NoMagicNumbers::is_ts_numeric_literal(parent, parent_parent) { + if self.ignore_numeric_literal_types + && NoMagicNumbers::is_ts_numeric_literal(parent, parent_parent) + { return true; } - if self.ignore_type_indexes && NoMagicNumbers::is_ts_indexed_access_type(parent_parent, ctx) { + if self.ignore_type_indexes && NoMagicNumbers::is_ts_indexed_access_type(parent_parent, ctx) + { return true; } From 066fa35828e6cde03489af39949c37db8a828f10 Mon Sep 17 00:00:00 2001 From: Sysix Date: Sun, 1 Sep 2024 22:37:04 +0200 Subject: [PATCH 11/23] feat(linter): implement typescript/no-magic-numbers --- .../src/rules/typescript/no_magic_numbers.rs | 36 ++++++++----------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs index 87cb4cf922c20..ed0ad16c03557 100644 --- a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs @@ -217,6 +217,7 @@ declare_oxc_lint!( /// ``` NoMagicNumbers, style, + pending // TODO: enforceConst, probably copy from https://github.com/oxc-project/oxc/pull/5144 ); #[derive(Debug)] @@ -427,10 +428,7 @@ impl NoMagicNumbers { matches!(parent_node.kind(), AstKind::TSEnumMember(_)) } - fn is_ts_numeric_literal<'a>( - parent_node: &AstNode<'a>, - parent_parent_node: &AstNode<'a>, - ) -> bool { + fn is_ts_numeric_literal<'a>(parent_node: &AstNode<'a>, ctx: &AstNodes<'a>) -> bool { if let AstKind::TSLiteralType(literal) = parent_node.kind() { if !matches!( literal.literal, @@ -439,32 +437,28 @@ impl NoMagicNumbers { return false; } - if matches!( - parent_parent_node.kind(), - AstKind::TSTypeAliasDeclaration(_) | AstKind::TSUnionType(_) + let mut node = ctx.parent_node(parent_node.id()).unwrap(); + + while matches!( + node.kind(), + AstKind::TSUnionType(_) + | AstKind::TSIntersectionType(_) + | AstKind::TSParenthesizedType(_) ) { - return true; + node = ctx.parent_node(node.id()).unwrap(); } - // https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/src/rules/no-magic-numbers.ts#L209 + return matches!(node.kind(), AstKind::TSTypeAliasDeclaration(_)); } false } fn is_ts_readonly_property(parent_node: &AstNode<'_>) -> bool { - if let AstKind::PropertyDefinition(property) = parent_node.kind() { - return property.readonly; - } - - false + matches!(parent_node.kind(), AstKind::PropertyDefinition(property) if property.readonly) } fn is_ts_indexed_access_type<'a>(parent_parent_node: &AstNode<'a>, ctx: &AstNodes<'a>) -> bool { - if matches!(parent_parent_node.kind(), AstKind::TSIndexedAccessType(_)) { - return true; - } - let mut node = parent_parent_node; while matches!( @@ -522,9 +516,7 @@ impl NoMagicNumbers { return true; } - if self.ignore_numeric_literal_types - && NoMagicNumbers::is_ts_numeric_literal(parent, parent_parent) - { + if self.ignore_numeric_literal_types && NoMagicNumbers::is_ts_numeric_literal(parent, ctx) { return true; } @@ -551,7 +543,7 @@ impl NoMagicNumbers { None } AstKind::AssignmentExpression(expression) => { - if let AssignmentTarget::AssignmentTargetIdentifier(_) = expression.left { + if matches!(expression.left, AssignmentTarget::AssignmentTargetIdentifier(_)) { return Some(NoMagicNumberReportReason::NoMagicNumber(span, &config.raw)); } From 25afd1475d7e42c12c3f56e4c40b4dca215ae730 Mon Sep 17 00:00:00 2001 From: Sysix Date: Mon, 2 Sep 2024 16:00:00 +0200 Subject: [PATCH 12/23] feat(linter): implement typescript/no-magic-numbers --- .../src/rules/typescript/no_magic_numbers.rs | 6 +- .../src/snapshots/no_magic_numbers.snap | 128 +++++++++--------- 2 files changed, 67 insertions(+), 67 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs index ed0ad16c03557..ef86c3e01709d 100644 --- a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs @@ -10,7 +10,7 @@ use oxc_ast::{ use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; use oxc_semantic::AstNodes; -use oxc_span::Span; +use oxc_span::{Span, GetSpan}; use oxc_syntax::operator::UnaryOperator; use crate::{context::LintContext, rule::Rule, AstNode}; @@ -266,7 +266,7 @@ impl Rule for NoMagicNumbers { Self(Box::new(NoMagicNumbersConfig::try_from(&value).unwrap())) } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { - if let AstKind::NumericLiteral(literal) = node.kind() { + if matches!(node.kind(), AstKind::NumericLiteral(_)) { let nodes = ctx.nodes(); let config = InternConfig::from(node, nodes.parent_node(node.id()).unwrap()); @@ -276,7 +276,7 @@ impl Rule for NoMagicNumbers { let parent = nodes.parent_node(config.node.id()).unwrap(); - if let Some(reason) = self.get_report_reason(parent, &config, &literal.span) { + if let Some(reason) = self.get_report_reason(parent, &config, &config.node.kind().span()) { ctx.diagnostic(match reason { NoMagicNumberReportReason::MustUseConst(span) => { must_use_const_diagnostic(*span) diff --git a/crates/oxc_linter/src/snapshots/no_magic_numbers.snap b/crates/oxc_linter/src/snapshots/no_magic_numbers.snap index 892a4449a7152..9ec1fad497bea 100644 --- a/crates/oxc_linter/src/snapshots/no_magic_numbers.snap +++ b/crates/oxc_linter/src/snapshots/no_magic_numbers.snap @@ -44,9 +44,9 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -2 - ╭─[no_magic_numbers.tsx:1:20] + ╭─[no_magic_numbers.tsx:1:19] 1 │ var foo = 0 + 1 + -2 + 2; - · ─ + · ── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 @@ -110,9 +110,9 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -60 - ╭─[no_magic_numbers.tsx:1:48] + ╭─[no_magic_numbers.tsx:1:47] 1 │ function getNegativeSecondsInMinute() {return -60;} - · ── + · ─── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 3 @@ -134,51 +134,51 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -100 - ╭─[no_magic_numbers.tsx:1:6] + ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[-100] - · ─── + · ──── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -1.5 - ╭─[no_magic_numbers.tsx:1:6] + ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[-1.5] - · ─── + · ──── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -1 - ╭─[no_magic_numbers.tsx:1:6] + ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[-1] - · ─ + · ── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -0.1 - ╭─[no_magic_numbers.tsx:1:6] + ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[-0.1] - · ─── + · ──── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -0b110 - ╭─[no_magic_numbers.tsx:1:6] + ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[-0b110] - · ───── + · ────── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -0o71 - ╭─[no_magic_numbers.tsx:1:6] + ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[-0o71] - · ──── + · ───── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -0x12 - ╭─[no_magic_numbers.tsx:1:6] + ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[-0x12] - · ──── + · ───── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -012 - ╭─[no_magic_numbers.tsx:1:6] + ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[-012] - · ─── + · ──── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 0.1 @@ -242,27 +242,27 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -1e310 - ╭─[no_magic_numbers.tsx:1:6] + ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[-1e310] - · ───── + · ────── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 0 - ╭─[no_magic_numbers.tsx:1:6] + ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[+0] - · ─ + · ── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 - ╭─[no_magic_numbers.tsx:1:6] + ╭─[no_magic_numbers.tsx:1:5] 1 │ foo[+1] - · ─ + · ── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -1 - ╭─[no_magic_numbers.tsx:1:8] + ╭─[no_magic_numbers.tsx:1:7] 1 │ foo[-(-1)] - · ─ + · ── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 100 @@ -362,9 +362,9 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -2 - ╭─[no_magic_numbers.tsx:1:18] + ╭─[no_magic_numbers.tsx:1:17] 1 │ class C { foo = -2; } - · ─ + · ── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 @@ -416,9 +416,9 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -1 - ╭─[no_magic_numbers.tsx:1:13] + ╭─[no_magic_numbers.tsx:1:12] 1 │ type Foo = -1; - · ─ + · ── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 @@ -446,9 +446,9 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -1 - ╭─[no_magic_numbers.tsx:1:17] + ╭─[no_magic_numbers.tsx:1:16] 1 │ type Foo = 1 | -1; - · ─ + · ── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 @@ -468,18 +468,18 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -1 - ╭─[no_magic_numbers.tsx:5:13] + ╭─[no_magic_numbers.tsx:5:12] 4 │ NUM = '0123456789', 5 │ NEG = -1, - · ─ + · ── 6 │ POS = +1, ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 - ╭─[no_magic_numbers.tsx:6:13] + ╭─[no_magic_numbers.tsx:6:12] 5 │ NEG = -1, 6 │ POS = +1, - · ─ + · ── 7 │ } ╰──── @@ -516,18 +516,18 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -5 - ╭─[no_magic_numbers.tsx:7:20] + ╭─[no_magic_numbers.tsx:7:19] 6 │ static readonly D = 4; 7 │ readonly E = -5; - · ─ + · ── 8 │ readonly F = +6; ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 6 - ╭─[no_magic_numbers.tsx:8:20] + ╭─[no_magic_numbers.tsx:8:19] 7 │ readonly E = -5; 8 │ readonly F = +6; - · ─ + · ── 9 │ private readonly G = 100n; ╰──── @@ -538,9 +538,9 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -1 - ╭─[no_magic_numbers.tsx:1:17] + ╭─[no_magic_numbers.tsx:1:16] 1 │ type Foo = Bar[-1]; - · ─ + · ── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 0xab @@ -562,9 +562,9 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -2 - ╭─[no_magic_numbers.tsx:1:21] + ╭─[no_magic_numbers.tsx:1:20] 1 │ type Foo = Bar[1 | -2]; - · ─ + · ── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 @@ -574,9 +574,9 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -2 - ╭─[no_magic_numbers.tsx:1:21] + ╭─[no_magic_numbers.tsx:1:20] 1 │ type Foo = Bar[1 & -2]; - · ─ + · ── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 @@ -592,9 +592,9 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -2 - ╭─[no_magic_numbers.tsx:1:23] + ╭─[no_magic_numbers.tsx:1:22] 1 │ type Foo = Bar[((1 & -2) | 3) | 4]; - · ─ + · ── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 3 @@ -678,9 +678,9 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -2 - ╭─[no_magic_numbers.tsx:1:13] + ╭─[no_magic_numbers.tsx:1:12] 1 │ type Foo = -2; - · ─ + · ── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 5.6 @@ -690,9 +690,9 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -7.8 - ╭─[no_magic_numbers.tsx:1:13] + ╭─[no_magic_numbers.tsx:1:12] 1 │ type Foo = -7.8; - · ─── + · ──── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 0x0a @@ -702,9 +702,9 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -0xbc - ╭─[no_magic_numbers.tsx:1:13] + ╭─[no_magic_numbers.tsx:1:12] 1 │ type Foo = -0xbc; - · ──── + · ───── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 1e2 @@ -714,9 +714,9 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -3e4 - ╭─[no_magic_numbers.tsx:1:13] + ╭─[no_magic_numbers.tsx:1:12] 1 │ type Foo = -3e4; - · ─── + · ──── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 5e-6 @@ -726,9 +726,9 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -7e-8 - ╭─[no_magic_numbers.tsx:1:13] + ╭─[no_magic_numbers.tsx:1:12] 1 │ type Foo = -7e-8; - · ──── + · ───── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 1.1e2 @@ -738,9 +738,9 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -3.1e4 - ╭─[no_magic_numbers.tsx:1:13] + ╭─[no_magic_numbers.tsx:1:12] 1 │ type Foo = -3.1e4; - · ───── + · ────── ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: 5.1e-6 @@ -750,7 +750,7 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-magic-numbers): No magic number: -7.1e-8 - ╭─[no_magic_numbers.tsx:1:13] + ╭─[no_magic_numbers.tsx:1:12] 1 │ type Foo = -7.1e-8; - · ────── + · ─────── ╰──── From 074e95f6d412b2770526301a59f34d9fd0d2a897 Mon Sep 17 00:00:00 2001 From: Sysix Date: Mon, 2 Sep 2024 16:00:37 +0200 Subject: [PATCH 13/23] feat(linter): implement typescript/no-magic-numbers --- crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs index ef86c3e01709d..f876fa4695002 100644 --- a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs @@ -10,7 +10,7 @@ use oxc_ast::{ use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; use oxc_semantic::AstNodes; -use oxc_span::{Span, GetSpan}; +use oxc_span::{GetSpan, Span}; use oxc_syntax::operator::UnaryOperator; use crate::{context::LintContext, rule::Rule, AstNode}; @@ -276,7 +276,9 @@ impl Rule for NoMagicNumbers { let parent = nodes.parent_node(config.node.id()).unwrap(); - if let Some(reason) = self.get_report_reason(parent, &config, &config.node.kind().span()) { + if let Some(reason) = + self.get_report_reason(parent, &config, &config.node.kind().span()) + { ctx.diagnostic(match reason { NoMagicNumberReportReason::MustUseConst(span) => { must_use_const_diagnostic(*span) From a03bafec2574decff9db2ee836e03b0356c84472 Mon Sep 17 00:00:00 2001 From: Sysix Date: Mon, 2 Sep 2024 16:10:45 +0200 Subject: [PATCH 14/23] feat(linter): implement typescript/no-magic-numbers --- .../src/rules/typescript/no_magic_numbers.rs | 57 +++++++++---------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs index f876fa4695002..ba6408ae00cc4 100644 --- a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs @@ -15,9 +15,9 @@ use oxc_syntax::operator::UnaryOperator; use crate::{context::LintContext, rule::Rule, AstNode}; -enum NoMagicNumberReportReason<'a> { - MustUseConst(&'a Span), - NoMagicNumber(&'a Span, &'a str), +enum NoMagicNumberReportReason { + MustUseConst, + NoMagicNumber, } fn must_use_const_diagnostic(span: Span) -> OxcDiagnostic { @@ -275,16 +275,13 @@ impl Rule for NoMagicNumbers { } let parent = nodes.parent_node(config.node.id()).unwrap(); + let span = config.node.kind().span(); - if let Some(reason) = - self.get_report_reason(parent, &config, &config.node.kind().span()) - { + if let Some(reason) = self.get_report_reason(parent) { ctx.diagnostic(match reason { - NoMagicNumberReportReason::MustUseConst(span) => { - must_use_const_diagnostic(*span) - } - NoMagicNumberReportReason::NoMagicNumber(span, raw) => { - no_magic_number_diagnostic(*span, raw) + NoMagicNumberReportReason::MustUseConst => must_use_const_diagnostic(span), + NoMagicNumberReportReason::NoMagicNumber => { + no_magic_number_diagnostic(span, &config.raw) } }); } @@ -430,7 +427,7 @@ impl NoMagicNumbers { matches!(parent_node.kind(), AstKind::TSEnumMember(_)) } - fn is_ts_numeric_literal<'a>(parent_node: &AstNode<'a>, ctx: &AstNodes<'a>) -> bool { + fn is_ts_numeric_literal<'a>(parent_node: &AstNode<'a>, nodes: &AstNodes<'a>) -> bool { if let AstKind::TSLiteralType(literal) = parent_node.kind() { if !matches!( literal.literal, @@ -439,7 +436,7 @@ impl NoMagicNumbers { return false; } - let mut node = ctx.parent_node(parent_node.id()).unwrap(); + let mut node = nodes.parent_node(parent_node.id()).unwrap(); while matches!( node.kind(), @@ -447,7 +444,7 @@ impl NoMagicNumbers { | AstKind::TSIntersectionType(_) | AstKind::TSParenthesizedType(_) ) { - node = ctx.parent_node(node.id()).unwrap(); + node = nodes.parent_node(node.id()).unwrap(); } return matches!(node.kind(), AstKind::TSTypeAliasDeclaration(_)); @@ -460,7 +457,10 @@ impl NoMagicNumbers { matches!(parent_node.kind(), AstKind::PropertyDefinition(property) if property.readonly) } - fn is_ts_indexed_access_type<'a>(parent_parent_node: &AstNode<'a>, ctx: &AstNodes<'a>) -> bool { + fn is_ts_indexed_access_type<'a>( + parent_parent_node: &AstNode<'a>, + nodes: &AstNodes<'a>, + ) -> bool { let mut node = parent_parent_node; while matches!( @@ -469,18 +469,18 @@ impl NoMagicNumbers { | AstKind::TSIntersectionType(_) | AstKind::TSParenthesizedType(_) ) { - node = ctx.parent_node(node.id()).unwrap(); + node = nodes.parent_node(node.id()).unwrap(); } matches!(node.kind(), AstKind::TSIndexedAccessType(_)) } - fn is_skipable<'a>(&self, config: &InternConfig<'a>, ctx: &AstNodes<'a>) -> bool { + fn is_skipable<'a>(&self, config: &InternConfig<'a>, nodes: &AstNodes<'a>) -> bool { if self.is_ignore_value(config.value) { return true; } - let parent = ctx.parent_node(config.node.id()).unwrap(); + let parent = nodes.parent_node(config.node.id()).unwrap(); if self.ignore_enums && NoMagicNumbers::is_ts_enum(parent) { return true; @@ -512,17 +512,19 @@ impl NoMagicNumbers { return true; } - let parent_parent = ctx.parent_node(parent.id()).unwrap(); + let parent_parent = nodes.parent_node(parent.id()).unwrap(); if NoMagicNumbers::is_parse_int_radix(config.value, parent_parent) { return true; } - if self.ignore_numeric_literal_types && NoMagicNumbers::is_ts_numeric_literal(parent, ctx) { + if self.ignore_numeric_literal_types && NoMagicNumbers::is_ts_numeric_literal(parent, nodes) + { return true; } - if self.ignore_type_indexes && NoMagicNumbers::is_ts_indexed_access_type(parent_parent, ctx) + if self.ignore_type_indexes + && NoMagicNumbers::is_ts_indexed_access_type(parent_parent, nodes) { return true; } @@ -530,29 +532,24 @@ impl NoMagicNumbers { false } - fn get_report_reason<'a>( - &self, - parent: &'a AstNode<'a>, - config: &'a InternConfig<'a>, - span: &'a Span, - ) -> Option> { + fn get_report_reason<'a>(&self, parent: &'a AstNode<'a>) -> Option { match parent.kind() { AstKind::VariableDeclarator(declarator) => { if self.enforce_const && declarator.kind != VariableDeclarationKind::Const { - return Some(NoMagicNumberReportReason::MustUseConst(span)); + return Some(NoMagicNumberReportReason::MustUseConst); } None } AstKind::AssignmentExpression(expression) => { if matches!(expression.left, AssignmentTarget::AssignmentTargetIdentifier(_)) { - return Some(NoMagicNumberReportReason::NoMagicNumber(span, &config.raw)); + return Some(NoMagicNumberReportReason::NoMagicNumber); } None } AstKind::JSXExpressionContainer(_) => None, - _ => return Some(NoMagicNumberReportReason::NoMagicNumber(span, &config.raw)), + _ => Some(NoMagicNumberReportReason::NoMagicNumber), } } } From 18018cbffe95e3f059ae54031e9c603acfd387e9 Mon Sep 17 00:00:00 2001 From: Sysix Date: Wed, 4 Sep 2024 01:12:03 +0200 Subject: [PATCH 15/23] feat(linter): implement typescript/no-magic-numbers --- .../src/rules/typescript/no_magic_numbers.rs | 361 ++++++++---------- 1 file changed, 149 insertions(+), 212 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs index ba6408ae00cc4..54f6a87cd0389 100644 --- a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs @@ -1,8 +1,6 @@ -use std::ops::Mul; - use oxc_ast::{ ast::{ - Argument, AssignmentTarget, Expression, MemberExpression, TSLiteral, UnaryExpression, + AssignmentTarget, Expression, MemberExpression, TSLiteral, UnaryExpression, VariableDeclarationKind, }, AstKind, @@ -235,25 +233,25 @@ impl InternConfig<'_> { let is_unary = matches!(parent_node.kind(), AstKind::UnaryExpression(_)); let is_negative = matches!(parent_node.kind(), AstKind::UnaryExpression(unary) if unary.operator == UnaryOperator::UnaryNegation); - if let AstKind::NumericLiteral(numeric) = node.kind() { - if is_negative { - Ok(InternConfig { - node: parent_node, - value: 0.0 - numeric.value, - raw: format!("-{}", numeric.raw), - }) - } else { - Ok(InternConfig { - node: if is_unary { parent_node } else { node }, - value: numeric.value, - raw: numeric.raw.into(), - }) - } - } else { - Err(OxcDiagnostic::warn(format!( + let AstKind::NumericLiteral(numeric) = node.kind() else { + return Err(OxcDiagnostic::warn(format!( "expected AstKind BingIntLiteral or NumericLiteral, got {:?}", node.kind().debug_name() - ))) + ))); + }; + + if is_negative { + Ok(InternConfig { + node: parent_node, + value: 0.0 - numeric.value, + raw: format!("-{}", numeric.raw), + }) + } else { + Ok(InternConfig { + node: if is_unary { parent_node } else { node }, + value: numeric.value, + raw: numeric.raw.into(), + }) } } @@ -266,202 +264,125 @@ impl Rule for NoMagicNumbers { Self(Box::new(NoMagicNumbersConfig::try_from(&value).unwrap())) } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { - if matches!(node.kind(), AstKind::NumericLiteral(_)) { - let nodes = ctx.nodes(); - let config = InternConfig::from(node, nodes.parent_node(node.id()).unwrap()); - - if self.is_skipable(&config, nodes) { - return; - } + if !matches!(node.kind(), AstKind::NumericLiteral(_)) { + return; + } - let parent = nodes.parent_node(config.node.id()).unwrap(); - let span = config.node.kind().span(); + let nodes = ctx.semantic().nodes(); + let config = InternConfig::from(node, nodes.parent_node(node.id()).unwrap()); - if let Some(reason) = self.get_report_reason(parent) { - ctx.diagnostic(match reason { - NoMagicNumberReportReason::MustUseConst => must_use_const_diagnostic(span), - NoMagicNumberReportReason::NoMagicNumber => { - no_magic_number_diagnostic(span, &config.raw) - } - }); - } + if self.is_skipable(&config, nodes) { + return; } - } -} -impl NoMagicNumbers { - fn is_numeric_f64(actual: f64, expected: f64) -> bool { - (actual - expected).abs() < f64::EPSILON - } - fn is_numeric_expression(expression: &Expression<'_>, number: f64) -> bool { - if expression.is_number(number) { - return true; - } + let parent_kind = nodes.parent_node(config.node.id()).unwrap().kind(); + let span = config.node.kind().span(); - if let Expression::UnaryExpression(unary) = expression { - if let Expression::NumericLiteral(_) = &unary.argument { - if unary.operator == UnaryOperator::UnaryPlus { - return unary.argument.is_number(number); - } + let Some(reason) = self.get_report_reason(&parent_kind) else { + return; + }; - if unary.operator == UnaryOperator::UnaryNegation { - return unary.argument.is_number(number * -1.0); - } + ctx.diagnostic(match reason { + NoMagicNumberReportReason::MustUseConst => must_use_const_diagnostic(span), + NoMagicNumberReportReason::NoMagicNumber => { + no_magic_number_diagnostic(span, &config.raw) } - } - - false - } - fn is_ignore_value(&self, number: f64) -> bool { - self.ignore.contains(&number) + }); } +} - fn is_default_value(number: f64, parent_node: &AstNode<'_>) -> bool { - if let AstKind::AssignmentTargetWithDefault(assignment) = parent_node.kind() { - return NoMagicNumbers::is_numeric_expression(&assignment.init, number); - } - - if let AstKind::AssignmentPattern(pattern) = parent_node.kind() { - return NoMagicNumbers::is_numeric_expression(&pattern.right, number); - } +fn is_default_value(parent_kind: &AstKind<'_>) -> bool { + matches!(parent_kind, AstKind::AssignmentTargetWithDefault(_) | AstKind::AssignmentPattern(_)) +} - false - } +fn is_class_field_initial_value(parent_kind: &AstKind<'_>) -> bool { + matches!(parent_kind, AstKind::PropertyDefinition(_)) +} - fn is_class_field_initial_value(number: f64, parent_node: &AstNode<'_>) -> bool { - if let AstKind::PropertyDefinition(property) = parent_node.kind() { - return NoMagicNumbers::is_numeric_expression(property.value.as_ref().unwrap(), number); - } +fn is_detectable_object(parent_kind: &AstKind<'_>) -> bool { + matches!(parent_kind, AstKind::ObjectExpression(_) | AstKind::ObjectProperty(_)) +} - false - } +fn is_parse_int_radix(parent_parent_node: &AstNode<'_>) -> bool { + let AstKind::CallExpression(expression) = parent_parent_node.kind() else { + return false; + }; - fn is_parse_int_radix(number: f64, parent_parent_node: &AstNode<'_>) -> bool { - if let AstKind::CallExpression(expression) = parent_parent_node.kind() { - if expression.arguments.get(1).is_none() { - return false; - } + let callee = expression.callee.without_parenthesized(); - let argument = expression.arguments.get(1).unwrap(); - return match argument { - Argument::NumericLiteral(numeric) => { - NoMagicNumbers::is_numeric_f64(numeric.value, number) - } - Argument::UnaryExpression(unary) - if unary.operator == UnaryOperator::UnaryNegation => - { - if let Expression::NumericLiteral(numeric) = &unary.argument { - return NoMagicNumbers::is_numeric_f64(numeric.value, number.mul(-1.0)); - } + callee.is_specific_id("parseInt") || callee.is_specific_member_access("Number", "parseInt") +} - return false; - } - _ => false, - }; +/// Returns whether the given node is used as an array index. +/// Value must coerce to a valid array index name: "0", "1", "2" ... "4294967294". +/// +/// All other values, like "-1", "2.5", or "4294967295", are just "normal" object properties, +/// which can be created and accessed on an array in addition to the array index properties, +/// but they don't affect array's length and are not considered by methods such as .map(), .forEach() etc. +/// +/// The maximum array length by the specification is 2 ** 32 - 1 = 4294967295, +/// thus the maximum valid index is 2 ** 32 - 2 = 4294967294. +/// +/// All notations are allowed, as long as the value coerces to one of "0", "1", "2" ... "4294967294". +/// +/// Valid examples: +/// ```javascript +/// a[0], a[1], a[1.2e1], a[0xAB], a[0n], a[1n] +/// a[-0] // (same as a[0] because -0 coerces to "0") +/// a[-0n] // (-0n evaluates to 0n) +/// ``` +/// +/// Invalid examples: +/// ```javascript +/// a[-1], a[-0xAB], a[-1n], a[2.5], a[1.23e1], a[12e-1] +/// a[4294967295] // (above the max index, it's an access to a regular property a["4294967295"]) +/// a[999999999999999999999] // (even if it wasn't above the max index, it would be a["1e+21"]) +/// a[1e310] // (same as a["Infinity"]) +/// ``` +fn is_array_index<'a>(ast_kind: &AstKind<'a>, parent_kind: &AstKind<'a>) -> bool { + fn is_unanary_index(unary: &UnaryExpression) -> bool { + match &unary.argument { + Expression::NumericLiteral(numeric) + if unary.operator == UnaryOperator::UnaryNegation => + { + numeric.value == 0.0 + } + _ => false, } - - false } - - /// Returns whether the given node is used as an array index. - /// Value must coerce to a valid array index name: "0", "1", "2" ... "4294967294". - /// - /// All other values, like "-1", "2.5", or "4294967295", are just "normal" object properties, - /// which can be created and accessed on an array in addition to the array index properties, - /// but they don't affect array's length and are not considered by methods such as .map(), .forEach() etc. - /// - /// The maximum array length by the specification is 2 ** 32 - 1 = 4294967295, - /// thus the maximum valid index is 2 ** 32 - 2 = 4294967294. - /// - /// All notations are allowed, as long as the value coerces to one of "0", "1", "2" ... "4294967294". - /// - /// Valid examples: - /// ```javascript - /// a[0], a[1], a[1.2e1], a[0xAB], a[0n], a[1n] - /// a[-0] // (same as a[0] because -0 coerces to "0") - /// a[-0n] // (-0n evaluates to 0n) - /// ``` - /// - /// Invalid examples: - /// ```javascript - /// a[-1], a[-0xAB], a[-1n], a[2.5], a[1.23e1], a[12e-1] - /// a[4294967295] // (above the max index, it's an access to a regular property a["4294967295"]) - /// a[999999999999999999999] // (even if it wasn't above the max index, it would be a["1e+21"]) - /// a[1e310] // (same as a["Infinity"]) - /// ``` - fn is_array_index<'a>(node: &AstNode<'a>, parent_node: &AstNode<'a>) -> bool { - fn is_unanary_index(unary: &UnaryExpression) -> bool { - match &unary.argument { - Expression::NumericLiteral(numeric) => { - if unary.operator == UnaryOperator::UnaryNegation { - return numeric.value == 0.0; - } - false + match ast_kind { + AstKind::UnaryExpression(unary) => is_unanary_index(unary), + AstKind::NumericLiteral(numeric) => match parent_kind { + AstKind::MemberExpression(expression) => { + if let MemberExpression::ComputedMemberExpression(computed_expression) = expression + { + return computed_expression.expression.is_number(numeric.value) + && numeric.value >= 0.0 + && numeric.value.fract() == 0.0 + && numeric.value < f64::from(u32::MAX); } - _ => false, + + false } - } - match node.kind() { AstKind::UnaryExpression(unary) => is_unanary_index(unary), - AstKind::NumericLiteral(numeric) => match parent_node.kind() { - AstKind::MemberExpression(expression) => { - if let MemberExpression::ComputedMemberExpression(computed_expression) = - expression - { - return computed_expression.expression.is_number(numeric.value) - && numeric.value >= 0.0 - && numeric.value.fract() == 0.0 - && numeric.value < f64::from(u32::MAX); - } - - false - } - AstKind::UnaryExpression(unary) => is_unanary_index(unary), - _ => false, - }, _ => false, - } - } - - fn is_ts_enum(parent_node: &AstNode<'_>) -> bool { - matches!(parent_node.kind(), AstKind::TSEnumMember(_)) + }, + _ => false, } +} - fn is_ts_numeric_literal<'a>(parent_node: &AstNode<'a>, nodes: &AstNodes<'a>) -> bool { - if let AstKind::TSLiteralType(literal) = parent_node.kind() { - if !matches!( - literal.literal, - TSLiteral::NumericLiteral(_) | TSLiteral::UnaryExpression(_) - ) { - return false; - } - - let mut node = nodes.parent_node(parent_node.id()).unwrap(); - - while matches!( - node.kind(), - AstKind::TSUnionType(_) - | AstKind::TSIntersectionType(_) - | AstKind::TSParenthesizedType(_) - ) { - node = nodes.parent_node(node.id()).unwrap(); - } +fn is_ts_enum(parent_kind: &AstKind<'_>) -> bool { + matches!(parent_kind, AstKind::TSEnumMember(_)) +} - return matches!(node.kind(), AstKind::TSTypeAliasDeclaration(_)); +fn is_ts_numeric_literal<'a>(parent_node: &AstNode<'a>, nodes: &AstNodes<'a>) -> bool { + if let AstKind::TSLiteralType(literal) = parent_node.kind() { + if !matches!(literal.literal, TSLiteral::NumericLiteral(_) | TSLiteral::UnaryExpression(_)) + { + return false; } - false - } - - fn is_ts_readonly_property(parent_node: &AstNode<'_>) -> bool { - matches!(parent_node.kind(), AstKind::PropertyDefinition(property) if property.readonly) - } - - fn is_ts_indexed_access_type<'a>( - parent_parent_node: &AstNode<'a>, - nodes: &AstNodes<'a>, - ) -> bool { - let mut node = parent_parent_node; + let mut node = nodes.parent_node(parent_node.id()).unwrap(); while matches!( node.kind(), @@ -472,68 +393,84 @@ impl NoMagicNumbers { node = nodes.parent_node(node.id()).unwrap(); } - matches!(node.kind(), AstKind::TSIndexedAccessType(_)) + return matches!(node.kind(), AstKind::TSTypeAliasDeclaration(_)); + } + + false +} + +fn is_ts_readonly_property(parent_kind: &AstKind<'_>) -> bool { + matches!(parent_kind, AstKind::PropertyDefinition(property) if property.readonly) +} + +fn is_ts_indexed_access_type<'a>(parent_parent_node: &AstNode<'a>, nodes: &AstNodes<'a>) -> bool { + let mut node = parent_parent_node; + + while matches!( + node.kind(), + AstKind::TSUnionType(_) | AstKind::TSIntersectionType(_) | AstKind::TSParenthesizedType(_) + ) { + node = nodes.parent_node(node.id()).unwrap(); } + matches!(node.kind(), AstKind::TSIndexedAccessType(_)) +} + +impl NoMagicNumbers { fn is_skipable<'a>(&self, config: &InternConfig<'a>, nodes: &AstNodes<'a>) -> bool { - if self.is_ignore_value(config.value) { + if self.ignore.contains(&config.value) { return true; } let parent = nodes.parent_node(config.node.id()).unwrap(); + let parent_kind = parent.kind(); - if self.ignore_enums && NoMagicNumbers::is_ts_enum(parent) { + if self.ignore_enums && is_ts_enum(&parent_kind) { return true; } - if self.ignore_readonly_class_properties && NoMagicNumbers::is_ts_readonly_property(parent) - { + if self.ignore_readonly_class_properties && is_ts_readonly_property(&parent_kind) { return true; } - if self.ignore_default_values && NoMagicNumbers::is_default_value(config.value, parent) { + if self.ignore_default_values && is_default_value(&parent_kind) { return true; } - if self.ignore_class_field_initial_values - && NoMagicNumbers::is_class_field_initial_value(config.value, parent) - { + if self.ignore_class_field_initial_values && is_class_field_initial_value(&parent_kind) { return true; } - if self.ignore_array_indexes && NoMagicNumbers::is_array_index(config.node, parent) { + if self.ignore_array_indexes && is_array_index(&config.node.kind(), &parent_kind) { return true; } - if !self.detect_objects - && (matches!(parent.kind(), AstKind::ObjectExpression(_)) - || matches!(parent.kind(), AstKind::ObjectProperty(_))) - { + if !self.detect_objects && is_detectable_object(&parent_kind) { return true; } let parent_parent = nodes.parent_node(parent.id()).unwrap(); - if NoMagicNumbers::is_parse_int_radix(config.value, parent_parent) { + if is_parse_int_radix(parent_parent) { return true; } - if self.ignore_numeric_literal_types && NoMagicNumbers::is_ts_numeric_literal(parent, nodes) - { + if self.ignore_numeric_literal_types && is_ts_numeric_literal(parent, nodes) { return true; } - if self.ignore_type_indexes - && NoMagicNumbers::is_ts_indexed_access_type(parent_parent, nodes) - { + if self.ignore_type_indexes && is_ts_indexed_access_type(parent_parent, nodes) { return true; } false } - fn get_report_reason<'a>(&self, parent: &'a AstNode<'a>) -> Option { - match parent.kind() { + fn get_report_reason<'a>( + &self, + parent_kind: &'a AstKind<'a>, + ) -> Option { + match parent_kind { AstKind::VariableDeclarator(declarator) => { if self.enforce_const && declarator.kind != VariableDeclarationKind::Const { return Some(NoMagicNumberReportReason::MustUseConst); From 3b43cfbcb6b88f5e0f861c9d3f673ee3a1398e5e Mon Sep 17 00:00:00 2001 From: Sysix Date: Wed, 4 Sep 2024 01:17:12 +0200 Subject: [PATCH 16/23] feat(linter): implement typescript/no-magic-numbers --- .../src/rules/typescript/no_magic_numbers.rs | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs index 54f6a87cd0389..54b71af249d80 100644 --- a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs @@ -376,27 +376,24 @@ fn is_ts_enum(parent_kind: &AstKind<'_>) -> bool { } fn is_ts_numeric_literal<'a>(parent_node: &AstNode<'a>, nodes: &AstNodes<'a>) -> bool { - if let AstKind::TSLiteralType(literal) = parent_node.kind() { - if !matches!(literal.literal, TSLiteral::NumericLiteral(_) | TSLiteral::UnaryExpression(_)) - { - return false; - } + let AstKind::TSLiteralType(literal) = parent_node.kind() else { + return false; + }; - let mut node = nodes.parent_node(parent_node.id()).unwrap(); + if !matches!(literal.literal, TSLiteral::NumericLiteral(_) | TSLiteral::UnaryExpression(_)) { + return false; + } - while matches!( - node.kind(), - AstKind::TSUnionType(_) - | AstKind::TSIntersectionType(_) - | AstKind::TSParenthesizedType(_) - ) { - node = nodes.parent_node(node.id()).unwrap(); - } + let mut node = nodes.parent_node(parent_node.id()).unwrap(); - return matches!(node.kind(), AstKind::TSTypeAliasDeclaration(_)); + while matches!( + node.kind(), + AstKind::TSUnionType(_) | AstKind::TSIntersectionType(_) | AstKind::TSParenthesizedType(_) + ) { + node = nodes.parent_node(node.id()).unwrap(); } - false + matches!(node.kind(), AstKind::TSTypeAliasDeclaration(_)) } fn is_ts_readonly_property(parent_kind: &AstKind<'_>) -> bool { From 4402749105bed7c73adac7d20bb2965c376bd6c6 Mon Sep 17 00:00:00 2001 From: Sysix Date: Wed, 4 Sep 2024 01:58:57 +0200 Subject: [PATCH 17/23] feat(linter): implement typescript/no-magic-numbers --- .../src/rules/typescript/no_magic_numbers.rs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs index 54b71af249d80..20ddb0b33ee87 100644 --- a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs @@ -1,7 +1,6 @@ use oxc_ast::{ ast::{ - AssignmentTarget, Expression, MemberExpression, TSLiteral, UnaryExpression, - VariableDeclarationKind, + AssignmentTarget, Expression, MemberExpression, UnaryExpression, VariableDeclarationKind, }, AstKind, }; @@ -375,16 +374,8 @@ fn is_ts_enum(parent_kind: &AstKind<'_>) -> bool { matches!(parent_kind, AstKind::TSEnumMember(_)) } -fn is_ts_numeric_literal<'a>(parent_node: &AstNode<'a>, nodes: &AstNodes<'a>) -> bool { - let AstKind::TSLiteralType(literal) = parent_node.kind() else { - return false; - }; - - if !matches!(literal.literal, TSLiteral::NumericLiteral(_) | TSLiteral::UnaryExpression(_)) { - return false; - } - - let mut node = nodes.parent_node(parent_node.id()).unwrap(); +fn is_ts_numeric_literal<'a>(parent_parent_node: &AstNode<'a>, nodes: &AstNodes<'a>) -> bool { + let mut node = parent_parent_node; while matches!( node.kind(), @@ -452,7 +443,7 @@ impl NoMagicNumbers { return true; } - if self.ignore_numeric_literal_types && is_ts_numeric_literal(parent, nodes) { + if self.ignore_numeric_literal_types && is_ts_numeric_literal(parent_parent, nodes) { return true; } From eed651689c965eb69127579bbe0affc79dc2ec52 Mon Sep 17 00:00:00 2001 From: Sysix Date: Fri, 6 Sep 2024 18:25:49 +0200 Subject: [PATCH 18/23] feat(linter): implement typescript/no-magic-numbers --- .../src/rules/typescript/no_magic_numbers.rs | 31 +++++++------------ 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs index 20ddb0b33ee87..9fc52056ec2ed 100644 --- a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs @@ -225,39 +225,30 @@ struct InternConfig<'a> { } impl InternConfig<'_> { - fn try_node<'a>( - node: &'a AstNode<'a>, - parent_node: &'a AstNode<'a>, - ) -> Result, OxcDiagnostic> { - let is_unary = matches!(parent_node.kind(), AstKind::UnaryExpression(_)); - let is_negative = matches!(parent_node.kind(), AstKind::UnaryExpression(unary) if unary.operator == UnaryOperator::UnaryNegation); - + pub fn from<'a>(node: &'a AstNode<'a>, parent_node: &'a AstNode<'a>) -> InternConfig<'a> { let AstKind::NumericLiteral(numeric) = node.kind() else { - return Err(OxcDiagnostic::warn(format!( - "expected AstKind BingIntLiteral or NumericLiteral, got {:?}", - node.kind().debug_name() - ))); + unreachable!("expected AstKind BingIntLiteral or NumericLiteral, got {:?}", node.kind().debug_name()); }; + let is_unary = matches!(parent_node.kind(), AstKind::UnaryExpression(_)); + let is_negative = matches!(parent_node.kind(), AstKind::UnaryExpression(unary) if unary.operator == UnaryOperator::UnaryNegation); + if is_negative { - Ok(InternConfig { + InternConfig { node: parent_node, value: 0.0 - numeric.value, raw: format!("-{}", numeric.raw), - }) + } } else { - Ok(InternConfig { + InternConfig { node: if is_unary { parent_node } else { node }, value: numeric.value, raw: numeric.raw.into(), - }) + } } } - - pub fn from<'a>(node: &'a AstNode<'a>, parent: &'a AstNode<'a>) -> InternConfig<'a> { - return InternConfig::try_node(node, parent).unwrap(); - } } + impl Rule for NoMagicNumbers { fn from_configuration(value: serde_json::Value) -> Self { Self(Box::new(NoMagicNumbersConfig::try_from(&value).unwrap())) @@ -307,7 +298,7 @@ fn is_parse_int_radix(parent_parent_node: &AstNode<'_>) -> bool { return false; }; - let callee = expression.callee.without_parenthesized(); + let callee = expression.callee.without_parentheses(); callee.is_specific_id("parseInt") || callee.is_specific_member_access("Number", "parseInt") } From bf7b95c3e697dfc49103ad444be909f9f01539c3 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Fri, 6 Sep 2024 16:27:03 +0000 Subject: [PATCH 19/23] [autofix.ci] apply automated fixes --- crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs index 9fc52056ec2ed..66e91a6612740 100644 --- a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs @@ -227,7 +227,10 @@ struct InternConfig<'a> { impl InternConfig<'_> { pub fn from<'a>(node: &'a AstNode<'a>, parent_node: &'a AstNode<'a>) -> InternConfig<'a> { let AstKind::NumericLiteral(numeric) = node.kind() else { - unreachable!("expected AstKind BingIntLiteral or NumericLiteral, got {:?}", node.kind().debug_name()); + unreachable!( + "expected AstKind BingIntLiteral or NumericLiteral, got {:?}", + node.kind().debug_name() + ); }; let is_unary = matches!(parent_node.kind(), AstKind::UnaryExpression(_)); From d6511bc81a4661f59ab6b98953d4bab4a824dac8 Mon Sep 17 00:00:00 2001 From: Sysix Date: Fri, 6 Sep 2024 18:43:47 +0200 Subject: [PATCH 20/23] feat(linter): implement typescript/no-magic-numbers --- .../src/rules/typescript/no_magic_numbers.rs | 41 ++++++++++--------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs index 9fc52056ec2ed..f33d6b31d0edb 100644 --- a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs @@ -59,7 +59,7 @@ impl TryFrom<&serde_json::Value> for NoMagicNumbersConfig { return Ok(NoMagicNumbersConfig::default()); } - raw.as_array().unwrap().first().map_or_else( + raw.get(0).map_or_else( || { Err(OxcDiagnostic::warn( "Expecting object for typescript/no-magic-numbers configuration", @@ -67,11 +67,7 @@ impl TryFrom<&serde_json::Value> for NoMagicNumbersConfig { }, |object| { fn get_bool_property(object: &serde_json::Value, index: &str) -> bool { - object - .get(index) - .unwrap_or_else(|| &serde_json::Value::Bool(false)) - .as_bool() - .unwrap() + object.get(index).and_then(serde_json::Value::as_bool).unwrap_or_default() } Ok(Self { ignore: object @@ -113,21 +109,23 @@ declare_oxc_lint!( /// /// ‘Magic numbers’ are numbers that occur multiple times in code without an explicit meaning. They should preferably be replaced by named constants. /// - /// ### Example bad + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: /// ```javascript /// /// var dutyFreePrice = 100; /// var finalPrice = dutyFreePrice + (dutyFreePrice * 0.25); /// ``` /// - /// ### Example good with "ignore" + /// Examples of **correct** code for this rule with option "ignore": /// ```javascript /// /*typescript no-magic-numbers: ["error", { "ignore": [1] }]*/ /// var data = ['foo', 'bar', 'baz']; /// var dataLast = data.length && data[data.length - 1]; /// ``` /// - /// ### Example good with "ignoreArrayIndexes" + /// Examples of **correct** code for this rule with option "ignoreArrayIndexes": /// ```javascript /// /*typescript no-magic-numbers: ["error", { "ignoreArrayIndexes": true }]*/ /// var item = data[2]; @@ -139,14 +137,14 @@ declare_oxc_lint!( /// a = data[4294967294]; // max array index /// ``` /// - /// ### Example good with "ignoreDefaultValues" + /// Examples of **correct** code for this rule with option "ignoreDefaultValues": /// ```javascript /// /*typescript no-magic-numbers: ["error", { "ignoreDefaultValues": true }]*/ /// const { tax = 0.25 } = accountancy; /// function mapParallel(concurrency = 3) { /***/ } /// ``` /// - /// ### Example good with "ignoreClassFieldInitialValues" + /// Examples of **correct** code for this rule with option "ignoreClassFieldInitialValues": /// ```javascript /// /*typescript no-magic-numbers: ["error", { "ignoreClassFieldInitialValues": true }]*/ /// class C { @@ -157,13 +155,13 @@ declare_oxc_lint!( /// } /// ``` /// - /// ### Example bad with "enforceConst" + /// Examples of **incorrect** code for this rule with option "enforceConst": /// ```javascript /// /*typescript no-magic-numbers: ["error", { "enforceConst": true }]*/ /// var TAX = 0.25; /// ``` /// - /// ### Example bad with "detectObjects" + /// Examples of **incorrect** code for this rule with option "detectObjects": /// ```javascript /// /*typescript no-magic-numbers: ["error", { "detectObjects": true }]*/ /// var magic = { @@ -171,7 +169,7 @@ declare_oxc_lint!( /// }; /// ``` /// - /// ### Example good with "detectObjects" + /// Examples of **correct** code for this rule with option "detectObjects": /// ```javascript /// /*typescript no-magic-numbers: ["error", { "detectObjects": true }]*/ /// var TAX = 0.25; @@ -181,7 +179,7 @@ declare_oxc_lint!( /// }; /// ``` /// - /// ### Example good with "ignoreEnums" + /// Examples of **correct** code for this rule with option "ignoreEnums": /// ```typescript /// /*typescript no-magic-numbers: ["error", { "ignoreEnums": true }]*/ /// enum foo { @@ -189,13 +187,13 @@ declare_oxc_lint!( /// } /// ``` /// - /// ### Example good with "ignoreNumericLiteralTypes" + /// Examples of **correct** code for this rule with option "ignoreNumericLiteralTypes": /// ```typescript /// /*typescript no-magic-numbers: ["error", { "ignoreNumericLiteralTypes": true }]*/ /// type SmallPrimes = 2 | 3 | 5 | 7 | 11; /// ``` /// - /// ### Example good with "ignoreReadonlyClassProperties" + /// Examples of **correct** code for this rule with option "ignoreReadonlyClassProperties": /// ```typescript /// /*typescript no-magic-numbers: ["error", { "ignoreReadonlyClassProperties": true }]*/ /// class Foo { @@ -206,9 +204,9 @@ declare_oxc_lint!( /// } /// ``` /// - /// ### Example good with "ignoreTypeIndexes" + /// Examples of **correct** code for this rule with option "ignoreTypeIndexes": /// ```typescript - /// /*typescript no-magic-numbers: ["error", { "ignoreReadonlyClassProperties": true }]*/ + /// /*typescript no-magic-numbers: ["error", { "ignoreTypeIndexes": true }]*/ /// type Foo = Bar[0]; /// type Baz = Parameters[2]; /// ``` @@ -227,7 +225,10 @@ struct InternConfig<'a> { impl InternConfig<'_> { pub fn from<'a>(node: &'a AstNode<'a>, parent_node: &'a AstNode<'a>) -> InternConfig<'a> { let AstKind::NumericLiteral(numeric) = node.kind() else { - unreachable!("expected AstKind BingIntLiteral or NumericLiteral, got {:?}", node.kind().debug_name()); + unreachable!( + "expected AstKind BingIntLiteral or NumericLiteral, got {:?}", + node.kind().debug_name() + ); }; let is_unary = matches!(parent_node.kind(), AstKind::UnaryExpression(_)); From 974041fec051ce12d87c8228f4b8b04d1a85844f Mon Sep 17 00:00:00 2001 From: Sysix Date: Fri, 6 Sep 2024 20:42:55 +0200 Subject: [PATCH 21/23] feat(linter): implement typescript/no-magic-numbers --- .../src/rules/typescript/no_magic_numbers.rs | 113 ++++++++++++------ .../src/snapshots/no_magic_numbers.snap | 80 +++++++++++++ 2 files changed, 157 insertions(+), 36 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs index f33d6b31d0edb..3b269b524c81b 100644 --- a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs @@ -37,9 +37,16 @@ impl std::ops::Deref for NoMagicNumbers { } } -#[derive(Debug, Clone, Default)] + +#[derive(Debug, Clone, PartialEq)] +pub enum NoMagicNumbersNumber { + Float(f64), + BigInt(String) +} + +#[derive(Debug, Default, Clone)] pub struct NoMagicNumbersConfig { - ignore: Vec, + ignore: Vec, ignore_array_indexes: bool, ignore_default_values: bool, ignore_class_field_initial_values: bool, @@ -73,7 +80,11 @@ impl TryFrom<&serde_json::Value> for NoMagicNumbersConfig { ignore: object .get("ignore") .and_then(serde_json::Value::as_array) - .map(|v| v.iter().filter_map(serde_json::Value::as_f64).collect()) + .map(|v| v.iter().map(|v| if v.is_number() { + NoMagicNumbersNumber::Float(v.as_f64().unwrap()) + } else { + NoMagicNumbersNumber::BigInt(v.as_str().unwrap().to_owned()) + }).collect()) .unwrap_or_default(), ignore_array_indexes: get_bool_property(object, "ignoreArrayIndexes"), ignore_default_values: get_bool_property(object, "ignoreDefaultValues"), @@ -103,7 +114,7 @@ declare_oxc_lint!( /// ### What it does /// /// The no-magic-numbers rule aims to make code more readable and refactoring easier by ensuring that special numbers are declared as constants to make their meaning explicit. - /// The current implementation does not support BigInt numbers. + /// The current implementation does not support BigInt numbers inside array indexes. /// /// ### Why is this bad? /// @@ -218,35 +229,60 @@ declare_oxc_lint!( #[derive(Debug)] struct InternConfig<'a> { node: &'a AstNode<'a>, - value: f64, + value: NoMagicNumbersNumber, raw: String, } impl InternConfig<'_> { pub fn from<'a>(node: &'a AstNode<'a>, parent_node: &'a AstNode<'a>) -> InternConfig<'a> { - let AstKind::NumericLiteral(numeric) = node.kind() else { - unreachable!( - "expected AstKind BingIntLiteral or NumericLiteral, got {:?}", - node.kind().debug_name() - ); - }; - let is_unary = matches!(parent_node.kind(), AstKind::UnaryExpression(_)); let is_negative = matches!(parent_node.kind(), AstKind::UnaryExpression(unary) if unary.operator == UnaryOperator::UnaryNegation); - if is_negative { - InternConfig { - node: parent_node, - value: 0.0 - numeric.value, - raw: format!("-{}", numeric.raw), + match node.kind() { + AstKind::NumericLiteral(numeric) => { + if is_negative { + InternConfig { + node: parent_node, + value: NoMagicNumbersNumber::Float(0.0 - numeric.value), + raw: format!("-{}", numeric.raw), + } + } else { + InternConfig { + node: if is_unary { parent_node } else { node }, + value: NoMagicNumbersNumber::Float(numeric.value), + raw: numeric.raw.into(), + } + } + }, + AstKind::BigIntLiteral(bigint) => { + let big_int_string = bigint.raw.clone().into_string(); + if is_negative { + let raw = format!("-{}", big_int_string); + + InternConfig { + node: parent_node, + value: NoMagicNumbersNumber::BigInt(raw.clone()), + raw: format!("-{}", raw), + } + } else { + InternConfig { + node: if is_unary { parent_node } else { node }, + value: NoMagicNumbersNumber::BigInt(big_int_string.clone()), + raw: big_int_string, + } + } } - } else { - InternConfig { - node: if is_unary { parent_node } else { node }, - value: numeric.value, - raw: numeric.raw.into(), + _ => { + unreachable!( + "expected AstKind BingIntLiteral or NumericLiteral, got {:?}", + node.kind().debug_name() + ) } } + + + + } } @@ -255,7 +291,7 @@ impl Rule for NoMagicNumbers { Self(Box::new(NoMagicNumbersConfig::try_from(&value).unwrap())) } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { - if !matches!(node.kind(), AstKind::NumericLiteral(_)) { + if !matches!(node.kind(), AstKind::NumericLiteral(_) | AstKind::BigIntLiteral(_)) { return; } @@ -316,6 +352,7 @@ fn is_parse_int_radix(parent_parent_node: &AstNode<'_>) -> bool { /// /// All notations are allowed, as long as the value coerces to one of "0", "1", "2" ... "4294967294". /// +/// This current implementation does not check for BigInt Numbers, they will always accept them as array_index /// Valid examples: /// ```javascript /// a[0], a[1], a[1.2e1], a[0xAB], a[0n], a[1n] @@ -333,6 +370,7 @@ fn is_parse_int_radix(parent_parent_node: &AstNode<'_>) -> bool { fn is_array_index<'a>(ast_kind: &AstKind<'a>, parent_kind: &AstKind<'a>) -> bool { fn is_unanary_index(unary: &UnaryExpression) -> bool { match &unary.argument { + Expression::BigIntLiteral(_) => true, Expression::NumericLiteral(numeric) if unary.operator == UnaryOperator::UnaryNegation => { @@ -341,8 +379,10 @@ fn is_array_index<'a>(ast_kind: &AstKind<'a>, parent_kind: &AstKind<'a>) -> bool _ => false, } } + match ast_kind { AstKind::UnaryExpression(unary) => is_unanary_index(unary), + AstKind::BigIntLiteral(_) => true, AstKind::NumericLiteral(numeric) => match parent_kind { AstKind::MemberExpression(expression) => { if let MemberExpression::ComputedMemberExpression(computed_expression) = expression @@ -421,10 +461,6 @@ impl NoMagicNumbers { return true; } - if self.ignore_array_indexes && is_array_index(&config.node.kind(), &parent_kind) { - return true; - } - if !self.detect_objects && is_detectable_object(&parent_kind) { return true; } @@ -443,6 +479,11 @@ impl NoMagicNumbers { return true; } + // test it at the end because of false positives with BigInt values + if self.ignore_array_indexes && is_array_index(&config.node.kind(), &parent_kind) { + return true; + } + false } @@ -673,8 +714,8 @@ fn test() { let fail = vec![ ("var foo = 42", enforce_const.clone()), // { "ecmaVersion": 6 }, ("var foo = 0 + 1;", None), - // ("var foo = 42n", enforce_const.clone()), // { "ecmaVersion": 2020 }, - // ("var foo = 0n + 1n;", None), // { "ecmaVersion": 2020 }, + ("var foo = 42n", enforce_const.clone()), // { "ecmaVersion": 2020 }, + ("var foo = 0n + 1n;", None), // { "ecmaVersion": 2020 }, ("a = a + 5;", None), ("a += 5;", None), ("var foo = 0 + 1 + -2 + 2;", None), @@ -731,12 +772,12 @@ fn test() { // ("foo[- -1n]", ignore_array_indexes.clone()), // { "ecmaVersion": 2020 }, ("100 .toString()", ignore_array_indexes.clone()), ("200[100]", ignore_array_indexes.clone()), - // ("var a =
;", None), // { "parserOptions": { "ecmaFeatures": { "jsx": true } } }, + ("var a =
;", None), // { "parserOptions": { "ecmaFeatures": { "jsx": true } } }, ("var min, max, mean; min = 1; max = 10; mean = 4;", Some(serde_json::json!([{}]))), - // ("f(100n)", Some(serde_json::json!([{ "ignore": [100] }]))), // { "ecmaVersion": 2020 }, - // ("f(-100n)", Some(serde_json::json!([{ "ignore": ["100n"] }]))), // { "ecmaVersion": 2020 }, - // ("f(100n)", Some(serde_json::json!([{ "ignore": ["-100n"] }]))), // { "ecmaVersion": 2020 }, - // ("f(100)", Some(serde_json::json!([{ "ignore": ["100n"] }]))), + ("f(100n)", Some(serde_json::json!([{ "ignore": [100] }]))), // { "ecmaVersion": 2020 }, + ("f(-100n)", Some(serde_json::json!([{ "ignore": ["100n"] }]))), // { "ecmaVersion": 2020 }, + ("f(100n)", Some(serde_json::json!([{ "ignore": ["-100n"] }]))), // { "ecmaVersion": 2020 }, + ("f(100)", Some(serde_json::json!([{ "ignore": ["100n"] }]))), ( "const func = (param = 123) => {}", Some(serde_json::json!([{ "ignoreDefaultValues": false }])), @@ -868,8 +909,8 @@ fn test() { ), ("type Foo = 1;", Some(serde_json::json!([{ "ignore": [-1] }]))), ("type Foo = -2;", Some(serde_json::json!([{ "ignore": [2] }]))), - // ("type Foo = 3n;", Some(serde_json::json!([{ "ignore": ["-3n"] }]))), - // ("type Foo = -4n;", Some(serde_json::json!([{ "ignore": ["4n"] }]))), + ("type Foo = 3n;", Some(serde_json::json!([{ "ignore": ["-3n"] }]))), + ("type Foo = -4n;", Some(serde_json::json!([{ "ignore": ["4n"] }]))), ("type Foo = 5.6;", Some(serde_json::json!([{ "ignore": [-5.6] }]))), ("type Foo = -7.8;", Some(serde_json::json!([{ "ignore": [7.8] }]))), ("type Foo = 0x0a;", Some(serde_json::json!([{ "ignore": [-0x0a] }]))), diff --git a/crates/oxc_linter/src/snapshots/no_magic_numbers.snap b/crates/oxc_linter/src/snapshots/no_magic_numbers.snap index 9ec1fad497bea..7779c9f92f8d6 100644 --- a/crates/oxc_linter/src/snapshots/no_magic_numbers.snap +++ b/crates/oxc_linter/src/snapshots/no_magic_numbers.snap @@ -19,6 +19,24 @@ source: crates/oxc_linter/src/tester.rs · ─ ╰──── + ⚠ typescript-eslint(no-magic-numbers): Number constants declarations must use 'const'. + ╭─[no_magic_numbers.tsx:1:11] + 1 │ var foo = 42n + · ─── + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 0n + ╭─[no_magic_numbers.tsx:1:11] + 1 │ var foo = 0n + 1n; + · ── + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1n + ╭─[no_magic_numbers.tsx:1:16] + 1 │ var foo = 0n + 1n; + · ── + ╰──── + ⚠ typescript-eslint(no-magic-numbers): No magic number: 5 ╭─[no_magic_numbers.tsx:1:9] 1 │ a = a + 5; @@ -277,6 +295,24 @@ source: crates/oxc_linter/src/tester.rs · ─── ╰──── + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 + ╭─[no_magic_numbers.tsx:1:26] + 1 │ var a =
; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 2 + ╭─[no_magic_numbers.tsx:1:28] + 1 │ var a =
; + · ─ + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 3 + ╭─[no_magic_numbers.tsx:1:30] + 1 │ var a =
; + · ─ + ╰──── + ⚠ typescript-eslint(no-magic-numbers): No magic number: 1 ╭─[no_magic_numbers.tsx:1:27] 1 │ var min, max, mean; min = 1; max = 10; mean = 4; @@ -295,6 +331,30 @@ source: crates/oxc_linter/src/tester.rs · ─ ╰──── + ⚠ typescript-eslint(no-magic-numbers): No magic number: 100n + ╭─[no_magic_numbers.tsx:1:3] + 1 │ f(100n) + · ──── + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: --100n + ╭─[no_magic_numbers.tsx:1:3] + 1 │ f(-100n) + · ───── + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 100n + ╭─[no_magic_numbers.tsx:1:3] + 1 │ f(100n) + · ──── + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: 100 + ╭─[no_magic_numbers.tsx:1:3] + 1 │ f(100) + · ─── + ╰──── + ⚠ typescript-eslint(no-magic-numbers): No magic number: 123 ╭─[no_magic_numbers.tsx:1:23] 1 │ const func = (param = 123) => {} @@ -531,6 +591,14 @@ source: crates/oxc_linter/src/tester.rs 9 │ private readonly G = 100n; ╰──── + ⚠ typescript-eslint(no-magic-numbers): No magic number: 100n + ╭─[no_magic_numbers.tsx:9:27] + 8 │ readonly F = +6; + 9 │ private readonly G = 100n; + · ──── + 10 │ } + ╰──── + ⚠ typescript-eslint(no-magic-numbers): No magic number: 0 ╭─[no_magic_numbers.tsx:1:16] 1 │ type Foo = Bar[0]; @@ -683,6 +751,18 @@ source: crates/oxc_linter/src/tester.rs · ── ╰──── + ⚠ typescript-eslint(no-magic-numbers): No magic number: 3n + ╭─[no_magic_numbers.tsx:1:12] + 1 │ type Foo = 3n; + · ── + ╰──── + + ⚠ typescript-eslint(no-magic-numbers): No magic number: --4n + ╭─[no_magic_numbers.tsx:1:12] + 1 │ type Foo = -4n; + · ─── + ╰──── + ⚠ typescript-eslint(no-magic-numbers): No magic number: 5.6 ╭─[no_magic_numbers.tsx:1:12] 1 │ type Foo = 5.6; From e7a02a5719ec3db3cb4ff4176e69a53834a9485c Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Fri, 6 Sep 2024 18:44:00 +0000 Subject: [PATCH 22/23] [autofix.ci] apply automated fixes --- .../src/rules/typescript/no_magic_numbers.rs | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs index 3b269b524c81b..8d955ef58f718 100644 --- a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs @@ -37,11 +37,10 @@ impl std::ops::Deref for NoMagicNumbers { } } - #[derive(Debug, Clone, PartialEq)] pub enum NoMagicNumbersNumber { Float(f64), - BigInt(String) + BigInt(String), } #[derive(Debug, Default, Clone)] @@ -80,11 +79,17 @@ impl TryFrom<&serde_json::Value> for NoMagicNumbersConfig { ignore: object .get("ignore") .and_then(serde_json::Value::as_array) - .map(|v| v.iter().map(|v| if v.is_number() { - NoMagicNumbersNumber::Float(v.as_f64().unwrap()) - } else { - NoMagicNumbersNumber::BigInt(v.as_str().unwrap().to_owned()) - }).collect()) + .map(|v| { + v.iter() + .map(|v| { + if v.is_number() { + NoMagicNumbersNumber::Float(v.as_f64().unwrap()) + } else { + NoMagicNumbersNumber::BigInt(v.as_str().unwrap().to_owned()) + } + }) + .collect() + }) .unwrap_or_default(), ignore_array_indexes: get_bool_property(object, "ignoreArrayIndexes"), ignore_default_values: get_bool_property(object, "ignoreDefaultValues"), @@ -253,7 +258,7 @@ impl InternConfig<'_> { raw: numeric.raw.into(), } } - }, + } AstKind::BigIntLiteral(bigint) => { let big_int_string = bigint.raw.clone().into_string(); if is_negative { @@ -276,13 +281,9 @@ impl InternConfig<'_> { unreachable!( "expected AstKind BingIntLiteral or NumericLiteral, got {:?}", node.kind().debug_name() - ) + ) } } - - - - } } From 584c8920441ee07d40496aa50da0cb7b9a8f7370 Mon Sep 17 00:00:00 2001 From: Sysix Date: Fri, 6 Sep 2024 20:44:08 +0200 Subject: [PATCH 23/23] feat(linter): implement typescript/no-magic-numbers --- .../src/rules/typescript/no_magic_numbers.rs | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs index 3b269b524c81b..67ddc5bc322cd 100644 --- a/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs +++ b/crates/oxc_linter/src/rules/typescript/no_magic_numbers.rs @@ -37,11 +37,10 @@ impl std::ops::Deref for NoMagicNumbers { } } - #[derive(Debug, Clone, PartialEq)] pub enum NoMagicNumbersNumber { Float(f64), - BigInt(String) + BigInt(String), } #[derive(Debug, Default, Clone)] @@ -80,11 +79,17 @@ impl TryFrom<&serde_json::Value> for NoMagicNumbersConfig { ignore: object .get("ignore") .and_then(serde_json::Value::as_array) - .map(|v| v.iter().map(|v| if v.is_number() { - NoMagicNumbersNumber::Float(v.as_f64().unwrap()) - } else { - NoMagicNumbersNumber::BigInt(v.as_str().unwrap().to_owned()) - }).collect()) + .map(|v| { + v.iter() + .map(|v| { + if v.is_number() { + NoMagicNumbersNumber::Float(v.as_f64().unwrap()) + } else { + NoMagicNumbersNumber::BigInt(v.as_str().unwrap().to_owned()) + } + }) + .collect() + }) .unwrap_or_default(), ignore_array_indexes: get_bool_property(object, "ignoreArrayIndexes"), ignore_default_values: get_bool_property(object, "ignoreDefaultValues"), @@ -253,16 +258,16 @@ impl InternConfig<'_> { raw: numeric.raw.into(), } } - }, + } AstKind::BigIntLiteral(bigint) => { let big_int_string = bigint.raw.clone().into_string(); if is_negative { - let raw = format!("-{}", big_int_string); + let raw = format!("-{big_int_string}"); InternConfig { node: parent_node, value: NoMagicNumbersNumber::BigInt(raw.clone()), - raw: format!("-{}", raw), + raw: format!("-{raw}"), } } else { InternConfig { @@ -276,13 +281,9 @@ impl InternConfig<'_> { unreachable!( "expected AstKind BingIntLiteral or NumericLiteral, got {:?}", node.kind().debug_name() - ) + ) } } - - - - } }