From 5234d964fb651083fdb421a9402158c2dc5307bc Mon Sep 17 00:00:00 2001 From: Yuichiro Yamashita Date: Sat, 28 Dec 2024 08:44:44 +0900 Subject: [PATCH 1/8] feat(linter): implement `eslint/no-nested-ternary` (#8150) implement: https://eslint.org/docs/latest/rules/no-nested-ternary --------- Co-authored-by: Cameron Clark --- crates/oxc_linter/src/rules.rs | 2 + .../src/rules/eslint/no_nested_ternary.rs | 100 ++++++++++++++++++ .../snapshots/eslint_no_nested_ternary.snap | 63 +++++++++++ 3 files changed, 165 insertions(+) create mode 100644 crates/oxc_linter/src/rules/eslint/no_nested_ternary.rs create mode 100644 crates/oxc_linter/src/snapshots/eslint_no_nested_ternary.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 0cfa9eb7a90da..053fec48b501e 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -93,6 +93,7 @@ mod eslint { pub mod no_loss_of_precision; pub mod no_magic_numbers; pub mod no_multi_str; + pub mod no_nested_ternary; pub mod no_new; pub mod no_new_func; pub mod no_new_native_nonconstructor; @@ -535,6 +536,7 @@ oxc_macros::declare_all_lint_rules! { eslint::max_classes_per_file, eslint::max_lines, eslint::max_params, + eslint::no_nested_ternary, eslint::no_labels, eslint::no_restricted_imports, eslint::no_object_constructor, diff --git a/crates/oxc_linter/src/rules/eslint/no_nested_ternary.rs b/crates/oxc_linter/src/rules/eslint/no_nested_ternary.rs new file mode 100644 index 0000000000000..2508793abe4cd --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/no_nested_ternary.rs @@ -0,0 +1,100 @@ +use oxc_ast::ast::Expression; +use oxc_ast::AstKind; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +fn no_nested_ternary_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("Do not nest ternary expressions.").with_label(span) +} + +#[derive(Debug, Default, Clone)] +pub struct NoNestedTernary; + +declare_oxc_lint!( + /// ### What it does + /// + /// Disallows nested ternary expressions to improve code readability and maintainability. + /// + /// ### Why is this bad? + /// + /// Nested ternary expressions make code harder to read and understand. They can lead to complex, difficult-to-debug logic. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// const result = condition1 ? (condition2 ? "a" : "b") : "c"; + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// let result; + /// if (condition1) { + /// result = condition2 ? "a" : "b"; + /// } else { + /// result = "c"; + /// } + /// ``` + NoNestedTernary, + style, +); + +impl Rule for NoNestedTernary { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + if let AstKind::ConditionalExpression(node) = node.kind() { + if matches!( + node.consequent.get_inner_expression(), + Expression::ConditionalExpression(_) + ) || matches!( + node.alternate.get_inner_expression(), + Expression::ConditionalExpression(_) + ) { + ctx.diagnostic(no_nested_ternary_diagnostic(node.span)); + } + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + "foo ? doBar() : doBaz();", + "var foo = bar === baz ? qux : quxx;", + "var result = foo && bar ? baz : qux || quux;", + "var result = foo ? bar : baz === qux;", + "foo ? doSomething(a, b) : doSomethingElse(c, d);", + // Parenthesized Expressions + "var result = (foo ? bar : baz) || qux;", + "var result = (foo ? bar : baz) && qux;", + "var result = foo === bar ? (baz || qux) : quux;", + "var result = (foo ? bar : baz) ? qux : quux;", + // TypeScript + "var result = foo! ? bar : baz;", + "var result = foo ? bar! : baz;", + "var result = (foo as boolean) ? bar : baz;", + "var result = foo ? (bar as string) : baz;", + ]; + + let fail = vec![ + "foo ? bar : baz === qux ? quxx : foobar;", + "foo ? baz === qux ? quxx : foobar : bar;", + // Parenthesized Expressions + "var result = foo ? (bar ? baz : qux) : quux;", + "var result = foo ? (bar === baz ? qux : quux) : foobar;", + "doSomething(foo ? bar : baz ? qux : quux);", + // Comment + "var result = foo /* comment */ ? bar : baz ? qux : quux;", + // TypeScript + "var result = foo! ? bar : baz! ? qux : quux;", + "var result = foo ? bar! : (baz! ? qux : quux);", + "var result = (foo as boolean) ? bar : (baz as string) ? qux : quux;", + "var result = foo ? (bar as string) : (baz as number ? qux : quux);", + ]; + + Tester::new(NoNestedTernary::NAME, NoNestedTernary::CATEGORY, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/eslint_no_nested_ternary.snap b/crates/oxc_linter/src/snapshots/eslint_no_nested_ternary.snap new file mode 100644 index 0000000000000..9bdafa63bd9d2 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/eslint_no_nested_ternary.snap @@ -0,0 +1,63 @@ +--- +source: crates/oxc_linter/src/tester.rs +snapshot_kind: text +--- + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:1] + 1 │ foo ? bar : baz === qux ? quxx : foobar; + · ─────────────────────────────────────── + ╰──── + + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:1] + 1 │ foo ? baz === qux ? quxx : foobar : bar; + · ─────────────────────────────────────── + ╰──── + + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:14] + 1 │ var result = foo ? (bar ? baz : qux) : quux; + · ────────────────────────────── + ╰──── + + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:14] + 1 │ var result = foo ? (bar === baz ? qux : quux) : foobar; + · ───────────────────────────────────────── + ╰──── + + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:13] + 1 │ doSomething(foo ? bar : baz ? qux : quux); + · ──────────────────────────── + ╰──── + + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:14] + 1 │ var result = foo /* comment */ ? bar : baz ? qux : quux; + · ────────────────────────────────────────── + ╰──── + + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:14] + 1 │ var result = foo! ? bar : baz! ? qux : quux; + · ────────────────────────────── + ╰──── + + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:14] + 1 │ var result = foo ? bar! : (baz! ? qux : quux); + · ──────────────────────────────── + ╰──── + + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:14] + 1 │ var result = (foo as boolean) ? bar : (baz as string) ? qux : quux; + · ───────────────────────────────────────────────────── + ╰──── + + ⚠ eslint(no-nested-ternary): Do not nest ternary expressions. + ╭─[no_nested_ternary.tsx:1:14] + 1 │ var result = foo ? (bar as string) : (baz as number ? qux : quux); + · ──────────────────────────────────────────────────── + ╰──── From 37c9959611c542d1ad3493c3d858682ce8acd6bc Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Sat, 28 Dec 2024 06:05:04 +0000 Subject: [PATCH 2/8] feat(minifier): normalize `Infinity` into `f64::Infinity` (#8148) --- .../oxc_minifier/src/ast_passes/normalize.rs | 23 ++++++++++++++++++- .../peephole_minimize_conditions.rs | 7 ++++-- .../ast_passes/peephole_remove_dead_code.rs | 2 +- .../src/ast_passes/statement_fusion.rs | 10 ++++---- crates/oxc_minifier/src/node_util/mod.rs | 7 ++++++ crates/oxc_minifier/src/tester.rs | 3 ++- tasks/minsize/minsize.snap | 12 +++++----- 7 files changed, 48 insertions(+), 16 deletions(-) diff --git a/crates/oxc_minifier/src/ast_passes/normalize.rs b/crates/oxc_minifier/src/ast_passes/normalize.rs index cb25270742053..964e2c548d63e 100644 --- a/crates/oxc_minifier/src/ast_passes/normalize.rs +++ b/crates/oxc_minifier/src/ast_passes/normalize.rs @@ -2,7 +2,7 @@ use oxc_ast::ast::*; use oxc_syntax::scope::ScopeFlags; use oxc_traverse::{traverse_mut_with_ctx, ReusableTraverseCtx, Traverse, TraverseCtx}; -use crate::CompressorPass; +use crate::{node_util::Ctx, CompressorPass}; /// Normalize AST /// @@ -25,6 +25,12 @@ impl<'a> Traverse<'a> for Normalize { Self::convert_while_to_for(stmt, ctx); } } + + fn exit_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) { + if let Expression::Identifier(_) = expr { + Self::convert_infinity_into_number(expr, ctx); + } + } } impl<'a> Normalize { @@ -45,6 +51,21 @@ impl<'a> Normalize { ); *stmt = Statement::ForStatement(for_stmt); } + + fn convert_infinity_into_number(expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) { + let ctx = Ctx(ctx); + match expr { + Expression::Identifier(ident) if ctx.is_identifier_infinity(ident) => { + *expr = ctx.ast.expression_numeric_literal( + ident.span, + f64::INFINITY, + None, + NumberBase::Decimal, + ); + } + _ => {} + } + } } #[cfg(test)] diff --git a/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs b/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs index 95568cd304af2..f3b4e8593bd3a 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs @@ -1276,8 +1276,11 @@ mod test { #[test] fn test_coercion_substitution_while() { // enableTypeCheck(); - test_same("var x = {}; while (x != null) throw 'a';"); - test_same("var x = 1; while (x != 0) throw 'a';"); + test( + "var x = {}; while (x != null) throw 'a';", + "var x = {}; for (;x != null;) throw 'a';", + ); + test("var x = 1; while (x != 0) throw 'a';", "var x = 1; for (;x != 0;) throw 'a';"); } #[test] diff --git a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs index 5590a62b53417..18d23ed4b9353 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs @@ -456,7 +456,7 @@ mod test { // Cases to test for empty block. // fold("while(x()){x}", "while(x());"); - fold("while(x()){x()}", "while(x())x()"); + fold("while(x()){x()}", "for(;x();)x()"); // fold("for(x=0;x<100;x++){x}", "for(x=0;x<100;x++);"); // fold("for(x in y){x}", "for(x in y);"); // fold("for (x of y) {x}", "for(x of y);"); diff --git a/crates/oxc_minifier/src/ast_passes/statement_fusion.rs b/crates/oxc_minifier/src/ast_passes/statement_fusion.rs index 017af1aa98481..4d563d94789dc 100644 --- a/crates/oxc_minifier/src/ast_passes/statement_fusion.rs +++ b/crates/oxc_minifier/src/ast_passes/statement_fusion.rs @@ -285,10 +285,10 @@ mod test { #[test] fn fuse_into_label() { - // fuse("a;b;c;label:for(x in y){}", "label:for(x in a,b,c,y){}"); - // fuse("a;b;c;label:for(;g;){}", "label:for(a,b,c;g;){}"); - // fuse("a;b;c;l1:l2:l3:for(;g;){}", "l1:l2:l3:for(a,b,c;g;){}"); - fuse_same("a;b;c;label:while(true){}"); + fuse("a;b;c;label:for(x in y){}", "label:for(x in a,b,c,y){}"); + fuse("a;b;c;label:for(;g;){}", "label:for(a,b,c;g;){}"); + fuse("a;b;c;l1:l2:l3:for(;g;){}", "l1:l2:l3:for(a,b,c;g;){}"); + fuse("a;b;c;label:while(true){}", "label:for(a,b,c;true;){}"); } #[test] @@ -304,7 +304,7 @@ mod test { #[test] fn no_fuse_into_while() { - fuse_same("a;b;c;while(x){}"); + fuse("a;b;c;while(x){}", "for(a,b,c;x;){}"); } #[test] diff --git a/crates/oxc_minifier/src/node_util/mod.rs b/crates/oxc_minifier/src/node_util/mod.rs index a3c62f6d08480..cf662b884bc52 100644 --- a/crates/oxc_minifier/src/node_util/mod.rs +++ b/crates/oxc_minifier/src/node_util/mod.rs @@ -64,4 +64,11 @@ impl<'a> Ctx<'a, '_> { } false } + + pub fn is_identifier_infinity(self, ident: &IdentifierReference) -> bool { + if ident.name == "Infinity" && ident.is_global_reference(self.symbols()) { + return true; + } + false + } } diff --git a/crates/oxc_minifier/src/tester.rs b/crates/oxc_minifier/src/tester.rs index 0ae6dcaa875d0..25e438531028a 100644 --- a/crates/oxc_minifier/src/tester.rs +++ b/crates/oxc_minifier/src/tester.rs @@ -6,7 +6,7 @@ use oxc_span::SourceType; use oxc_traverse::ReusableTraverseCtx; use crate::{ - ast_passes::{CompressorPass, RemoveSyntax}, + ast_passes::{CompressorPass, Normalize, RemoveSyntax}, CompressOptions, }; @@ -45,6 +45,7 @@ fn run<'a, P: CompressorPass<'a>>( SemanticBuilder::new().build(&program).semantic.into_symbol_table_and_scope_tree(); let mut ctx = ReusableTraverseCtx::new(scopes, symbols, allocator); RemoveSyntax::new(CompressOptions::all_false()).build(&mut program, &mut ctx); + Normalize::new().build(&mut program, &mut ctx); pass.build(&mut program, &mut ctx); } diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index 8bb240402d3bf..0d863febd2b80 100644 --- a/tasks/minsize/minsize.snap +++ b/tasks/minsize/minsize.snap @@ -9,19 +9,19 @@ Original | minified | minified | gzip | gzip | Fixture 342.15 kB | 118.76 kB | 118.14 kB | 44.54 kB | 44.37 kB | vue.js -544.10 kB | 72.05 kB | 72.48 kB | 26.19 kB | 26.20 kB | lodash.js +544.10 kB | 72.04 kB | 72.48 kB | 26.18 kB | 26.20 kB | lodash.js -555.77 kB | 274.04 kB | 270.13 kB | 91.20 kB | 90.80 kB | d3.js +555.77 kB | 273.90 kB | 270.13 kB | 91.19 kB | 90.80 kB | d3.js 1.01 MB | 461.13 kB | 458.89 kB | 126.91 kB | 126.71 kB | bundle.min.js -1.25 MB | 656.86 kB | 646.76 kB | 164.20 kB | 163.73 kB | three.js +1.25 MB | 656.81 kB | 646.76 kB | 164.16 kB | 163.73 kB | three.js -2.14 MB | 735.43 kB | 724.14 kB | 181.01 kB | 181.07 kB | victory.js +2.14 MB | 735.33 kB | 724.14 kB | 180.99 kB | 181.07 kB | victory.js -3.20 MB | 1.01 MB | 1.01 MB | 332.34 kB | 331.56 kB | echarts.js +3.20 MB | 1.01 MB | 1.01 MB | 332.27 kB | 331.56 kB | echarts.js 6.69 MB | 2.36 MB | 2.31 MB | 495.04 kB | 488.28 kB | antd.js -10.95 MB | 3.51 MB | 3.49 MB | 910.95 kB | 915.50 kB | typescript.js +10.95 MB | 3.51 MB | 3.49 MB | 910.93 kB | 915.50 kB | typescript.js From 1b9a5bae2e744c40973f274dd086a1eb91acd12d Mon Sep 17 00:00:00 2001 From: camc314 <18101008+camc314@users.noreply.github.com> Date: Sat, 28 Dec 2024 13:49:10 +0000 Subject: [PATCH 3/8] fix(linter): false positiver in private member expr in oxc/const-comparison (#8164) fixes https://github.com/oxc-project/oxc/issues/8163 --- .../oxc_linter/src/rules/oxc/const_comparisons.rs | 4 ++++ .../src/snapshots/oxc_const_comparisons.snap | 15 ++++++++++++++- crates/oxc_linter/src/utils/unicorn.rs | 11 ++++++++++- 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/crates/oxc_linter/src/rules/oxc/const_comparisons.rs b/crates/oxc_linter/src/rules/oxc/const_comparisons.rs index bc2fa4c94b643..bc998236ed52a 100644 --- a/crates/oxc_linter/src/rules/oxc/const_comparisons.rs +++ b/crates/oxc_linter/src/rules/oxc/const_comparisons.rs @@ -415,9 +415,11 @@ fn test() { "status_code > 500 && foo() && bar || status_code < 400;", // oxc specific "a < b", + "a.b.c < b.b.c", "a <= b", "a > b", "a >= b", + "class Foo { #a; #b; constructor() { this.#a = 1; }; test() { return this.#a > this.#b } }", ]; let fail = vec![ @@ -500,10 +502,12 @@ fn test() { "a <= a", "a > a", "a >= a", + "a.b.c >= a.b.c", "a == b && a == b", "a == b || a == b", "!foo && !foo", "!foo || !foo", + "class Foo { #a; #b; constructor() { this.#a = 1; }; test() { return this.#a > this.#a } }", ]; Tester::new(ConstComparisons::NAME, ConstComparisons::CATEGORY, pass, fail).test_and_snapshot(); diff --git a/crates/oxc_linter/src/snapshots/oxc_const_comparisons.snap b/crates/oxc_linter/src/snapshots/oxc_const_comparisons.snap index f6cc782939f09..baf7b23541f8b 100644 --- a/crates/oxc_linter/src/snapshots/oxc_const_comparisons.snap +++ b/crates/oxc_linter/src/snapshots/oxc_const_comparisons.snap @@ -1,6 +1,5 @@ --- source: crates/oxc_linter/src/tester.rs -snapshot_kind: text --- ⚠ oxc(const-comparisons): Unexpected constant comparison ╭─[const_comparisons.tsx:1:1] @@ -264,6 +263,13 @@ snapshot_kind: text ╰──── help: Because `a` will always be equal to itself + ⚠ oxc(const-comparisons): This comparison will always evaluate to true + ╭─[const_comparisons.tsx:1:1] + 1 │ a.b.c >= a.b.c + · ────────────── + ╰──── + help: Because `a.b.c` will always be equal to itself + ⚠ oxc(const-comparisons): Both sides of the logical operator are the same ╭─[const_comparisons.tsx:1:1] 1 │ a == b && a == b @@ -299,3 +305,10 @@ snapshot_kind: text · ╰── If this expression evaluates to true ╰──── help: This logical expression will always evaluate to the same value as the expression itself. + + ⚠ oxc(const-comparisons): This comparison will always evaluate to false + ╭─[const_comparisons.tsx:1:69] + 1 │ class Foo { #a; #b; constructor() { this.#a = 1; }; test() { return this.#a > this.#a } } + · ───────────────── + ╰──── + help: Because `this.#a` will never be greater than itself diff --git a/crates/oxc_linter/src/utils/unicorn.rs b/crates/oxc_linter/src/utils/unicorn.rs index 7edbcb543f8b2..a486ae38a55dd 100644 --- a/crates/oxc_linter/src/utils/unicorn.rs +++ b/crates/oxc_linter/src/utils/unicorn.rs @@ -274,7 +274,16 @@ pub fn is_same_member_expression( (Some(_), None) | (None, Some(_)) => { return false; } - _ => {} + (None, None) => { + if let ( + MemberExpression::PrivateFieldExpression(left), + MemberExpression::PrivateFieldExpression(right), + ) = (left, right) + { + return left.field.name == right.field.name + && is_same_expression(&left.object, &right.object, ctx); + } + } } if let ( From 1171e00a3c2d6a1374472f43cef33614bcdf7673 Mon Sep 17 00:00:00 2001 From: "Alexander S." Date: Sat, 28 Dec 2024 17:34:58 +0100 Subject: [PATCH 4/8] fix(linter): disable `react/rules-of-hooks` for vue and svelte files (#8165) because of https://github.com/oxc-project/oxc/issues/8003 I used the FrameworkFlags in hope they are only active in `.vue` and `.svelte` files. --- crates/oxc_linter/src/rules/react/rules_of_hooks.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/oxc_linter/src/rules/react/rules_of_hooks.rs b/crates/oxc_linter/src/rules/react/rules_of_hooks.rs index e6eb833f6882a..ac670d111b1f3 100644 --- a/crates/oxc_linter/src/rules/react/rules_of_hooks.rs +++ b/crates/oxc_linter/src/rules/react/rules_of_hooks.rs @@ -16,7 +16,7 @@ use crate::{ context::LintContext, rule::Rule, utils::{is_react_component_or_hook_name, is_react_function_call, is_react_hook}, - AstNode, + AstNode, FrameworkFlags, }; mod diagnostics { @@ -99,7 +99,7 @@ pub struct RulesOfHooks; declare_oxc_lint!( /// ### What it does /// - /// This enforcecs the Rules of Hooks + /// This enforces the Rules of Hooks /// /// /// @@ -108,6 +108,12 @@ declare_oxc_lint!( ); impl Rule for RulesOfHooks { + fn should_run(&self, ctx: &crate::rules::ContextHost) -> bool { + // disable this rule in vue/nuxt and svelte(kit) files + // top level useFunction are very common + !ctx.frameworks().contains(FrameworkFlags::SvelteKit | FrameworkFlags::Nuxt) + } + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { let AstKind::CallExpression(call) = node.kind() else { return }; From 65796c47a11d70cc31fcafc9e3fae8d886a4531f Mon Sep 17 00:00:00 2001 From: Yuichiro Yamashita Date: Sun, 29 Dec 2024 01:42:01 +0900 Subject: [PATCH 5/8] feat(linter): implement `eslint/prefer-rest-params` (#8155) implement: https://eslint.org/docs/latest/rules/prefer-rest-params --- crates/oxc_linter/src/rules.rs | 2 + .../src/rules/eslint/prefer_rest_params.rs | 126 ++++++++++++++++++ .../snapshots/eslint_prefer_rest_params.snap | 27 ++++ 3 files changed, 155 insertions(+) create mode 100644 crates/oxc_linter/src/rules/eslint/prefer_rest_params.rs create mode 100644 crates/oxc_linter/src/snapshots/eslint_prefer_rest_params.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 053fec48b501e..a9b681dae8796 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -141,6 +141,7 @@ mod eslint { pub mod prefer_exponentiation_operator; pub mod prefer_numeric_literals; pub mod prefer_object_has_own; + pub mod prefer_rest_params; pub mod prefer_spread; pub mod radix; pub mod require_await; @@ -630,6 +631,7 @@ oxc_macros::declare_all_lint_rules! { eslint::no_var, eslint::no_void, eslint::no_with, + eslint::prefer_rest_params, eslint::prefer_exponentiation_operator, eslint::prefer_numeric_literals, eslint::prefer_object_has_own, diff --git a/crates/oxc_linter/src/rules/eslint/prefer_rest_params.rs b/crates/oxc_linter/src/rules/eslint/prefer_rest_params.rs new file mode 100644 index 0000000000000..f7807fc8fba51 --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/prefer_rest_params.rs @@ -0,0 +1,126 @@ +use crate::{context::LintContext, rule::Rule, AstNode}; +use oxc_ast::AstKind; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::{GetSpan, Span}; + +fn prefer_rest_params_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("Use the rest parameters instead of 'arguments'.").with_label(span) +} + +#[derive(Debug, Default, Clone)] +pub struct PreferRestParams; + +declare_oxc_lint!( + /// ### What it does + /// + /// Disallows the use of the `arguments` object and instead enforces the use of rest parameters. + /// + /// ### Why is this bad? + /// + /// The `arguments` object does not have methods from `Array.prototype`, making it inconvenient for array-like operations. + /// Using rest parameters provides a more intuitive and efficient way to handle variadic arguments. + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```javascript + /// function foo() { + /// console.log(arguments); + /// } + /// + /// function foo(action) { + /// var args = Array.prototype.slice.call(arguments, 1); + /// action.apply(null, args); + /// } + /// + /// function foo(action) { + /// var args = [].slice.call(arguments, 1); + /// action.apply(null, args); + /// } + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```javascript + /// function foo(...args) { + /// console.log(args); + /// } + /// + /// function foo(action, ...args) { + /// action.apply(null, args); // Or use `action(...args)` (related to `prefer-spread` rule). + /// } + /// + /// // Note: Implicit `arguments` can be shadowed. + /// function foo(arguments) { + /// console.log(arguments); // This refers to the first argument. + /// } + /// function foo() { + /// var arguments = 0; + /// console.log(arguments); // This is a local variable. + /// } + /// ``` + PreferRestParams, + style, +); + +impl Rule for PreferRestParams { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + if let AstKind::IdentifierReference(identifier) = node.kind() { + if identifier.name != "arguments" + || !is_inside_of_function(node, ctx) + || is_not_normal_member_access(node, ctx) + { + return; + } + let binding = ctx.scopes().find_binding(node.scope_id(), "arguments"); + if binding.is_none() { + ctx.diagnostic(prefer_rest_params_diagnostic(node.span())); + } + } + } +} + +fn is_inside_of_function(node: &AstNode, ctx: &LintContext) -> bool { + let mut current = node; + while let Some(parent) = ctx.nodes().parent_node(current.id()) { + if matches!(parent.kind(), AstKind::Function(_)) { + return true; + } + current = parent; + } + false +} + +fn is_not_normal_member_access(identifier: &AstNode, ctx: &LintContext) -> bool { + let parent = ctx.nodes().parent_node(identifier.id()); + if let Some(parent) = parent { + if let AstKind::MemberExpression(member) = parent.kind() { + return member.object().span() == identifier.span() && !member.is_computed(); + } + } + false +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + "arguments;", + "function foo(arguments) { arguments; }", + "function foo() { var arguments; arguments; }", + "var foo = () => arguments;", + "function foo(...args) { args; }", + "function foo() { arguments.length; }", + "function foo() { arguments.callee; }", + ]; + + let fail = vec![ + "function foo() { arguments; }", + "function foo() { arguments[0]; }", + "function foo() { arguments[1]; }", + "function foo() { arguments[Symbol.iterator]; }", + ]; + + Tester::new(PreferRestParams::NAME, PreferRestParams::CATEGORY, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/eslint_prefer_rest_params.snap b/crates/oxc_linter/src/snapshots/eslint_prefer_rest_params.snap new file mode 100644 index 0000000000000..d5b62a95ff17d --- /dev/null +++ b/crates/oxc_linter/src/snapshots/eslint_prefer_rest_params.snap @@ -0,0 +1,27 @@ +--- +source: crates/oxc_linter/src/tester.rs +snapshot_kind: text +--- + ⚠ eslint(prefer-rest-params): Use the rest parameters instead of 'arguments'. + ╭─[prefer_rest_params.tsx:1:18] + 1 │ function foo() { arguments; } + · ───────── + ╰──── + + ⚠ eslint(prefer-rest-params): Use the rest parameters instead of 'arguments'. + ╭─[prefer_rest_params.tsx:1:18] + 1 │ function foo() { arguments[0]; } + · ───────── + ╰──── + + ⚠ eslint(prefer-rest-params): Use the rest parameters instead of 'arguments'. + ╭─[prefer_rest_params.tsx:1:18] + 1 │ function foo() { arguments[1]; } + · ───────── + ╰──── + + ⚠ eslint(prefer-rest-params): Use the rest parameters instead of 'arguments'. + ╭─[prefer_rest_params.tsx:1:18] + 1 │ function foo() { arguments[Symbol.iterator]; } + · ───────── + ╰──── From f3a36e1dbfd6e5a5cb4ef60de991ecec83ed5e50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=BF=A0=20/=20green?= Date: Sun, 29 Dec 2024 01:50:56 +0900 Subject: [PATCH 6/8] feat(minifier): fold `typeof foo != "undefined"` into `typeof foo < "u"` (#8159) --- .../peephole_substitute_alternate_syntax.rs | 32 ++++++++++++++++--- tasks/minsize/minsize.snap | 20 ++++++------ 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs b/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs index 83ddc1af628a6..f28c082a823b1 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs @@ -240,14 +240,24 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { } /// Compress `typeof foo == "undefined"` into `typeof foo > "u"` + /// + /// - `typeof foo == "undefined"` -> `typeof foo > "u"` + /// - `typeof foo != "undefined"` -> `typeof foo < "u"` + /// /// Enabled by `compress.typeofs` fn try_compress_typeof_undefined( expr: &mut BinaryExpression<'a>, ctx: Ctx<'a, 'b>, ) -> Option> { - if !matches!(expr.operator, BinaryOperator::Equality | BinaryOperator::StrictEquality) { - return None; - } + let new_op = match expr.operator { + BinaryOperator::Equality | BinaryOperator::StrictEquality => { + BinaryOperator::GreaterThan + } + BinaryOperator::Inequality | BinaryOperator::StrictInequality => { + BinaryOperator::LessThan + } + _ => return None, + }; let pair = Self::commutative_pair( (&expr.left, &expr.right), |a| a.is_specific_string_literal("undefined").then_some(()), @@ -266,7 +276,7 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax { let argument = Expression::Identifier(ctx.alloc(id_ref)); let left = ctx.ast.expression_unary(SPAN, UnaryOperator::Typeof, argument); let right = ctx.ast.expression_string_literal(SPAN, "u", None); - Some(ctx.ast.expression_binary(expr.span, left, BinaryOperator::GreaterThan, right)) + Some(ctx.ast.expression_binary(expr.span, left, new_op, right)) } /// Compress `foo === null || foo === undefined` into `foo == null`. @@ -1143,6 +1153,20 @@ mod test { test_same("const foo = () => { foo; return 'baz' }"); } + /// Port from + #[test] + fn test_fold_is_typeof_equals_undefined() { + test("typeof x !== 'undefined'", "typeof x < 'u'"); + test("typeof x != 'undefined'", "typeof x < 'u'"); + test("'undefined' !== typeof x", "typeof x < 'u'"); + test("'undefined' != typeof x", "typeof x < 'u'"); + + test("typeof x === 'undefined'", "typeof x > 'u'"); + test("typeof x == 'undefined'", "typeof x > 'u'"); + test("'undefined' === typeof x", "typeof x > 'u'"); + test("'undefined' == typeof x", "typeof x > 'u'"); + } + #[test] fn test_fold_is_null_or_undefined() { test("foo === null || foo === undefined", "foo == null"); diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index 0d863febd2b80..8b34fd75ae1fb 100644 --- a/tasks/minsize/minsize.snap +++ b/tasks/minsize/minsize.snap @@ -3,25 +3,25 @@ Original | minified | minified | gzip | gzip | Fixture ------------------------------------------------------------------------------------- 72.14 kB | 23.74 kB | 23.70 kB | 8.61 kB | 8.54 kB | react.development.js -173.90 kB | 60.22 kB | 59.82 kB | 19.49 kB | 19.33 kB | moment.js +173.90 kB | 60.16 kB | 59.82 kB | 19.49 kB | 19.33 kB | moment.js -287.63 kB | 90.61 kB | 90.07 kB | 32.19 kB | 31.95 kB | jquery.js +287.63 kB | 90.59 kB | 90.07 kB | 32.19 kB | 31.95 kB | jquery.js -342.15 kB | 118.76 kB | 118.14 kB | 44.54 kB | 44.37 kB | vue.js +342.15 kB | 118.61 kB | 118.14 kB | 44.54 kB | 44.37 kB | vue.js 544.10 kB | 72.04 kB | 72.48 kB | 26.18 kB | 26.20 kB | lodash.js -555.77 kB | 273.90 kB | 270.13 kB | 91.19 kB | 90.80 kB | d3.js +555.77 kB | 273.89 kB | 270.13 kB | 91.18 kB | 90.80 kB | d3.js -1.01 MB | 461.13 kB | 458.89 kB | 126.91 kB | 126.71 kB | bundle.min.js +1.01 MB | 461.09 kB | 458.89 kB | 126.91 kB | 126.71 kB | bundle.min.js -1.25 MB | 656.81 kB | 646.76 kB | 164.16 kB | 163.73 kB | three.js +1.25 MB | 656.66 kB | 646.76 kB | 164.14 kB | 163.73 kB | three.js -2.14 MB | 735.33 kB | 724.14 kB | 180.99 kB | 181.07 kB | victory.js +2.14 MB | 735.28 kB | 724.14 kB | 180.98 kB | 181.07 kB | victory.js -3.20 MB | 1.01 MB | 1.01 MB | 332.27 kB | 331.56 kB | echarts.js +3.20 MB | 1.01 MB | 1.01 MB | 332.23 kB | 331.56 kB | echarts.js -6.69 MB | 2.36 MB | 2.31 MB | 495.04 kB | 488.28 kB | antd.js +6.69 MB | 2.36 MB | 2.31 MB | 494.93 kB | 488.28 kB | antd.js -10.95 MB | 3.51 MB | 3.49 MB | 910.93 kB | 915.50 kB | typescript.js +10.95 MB | 3.51 MB | 3.49 MB | 910.92 kB | 915.50 kB | typescript.js From 9d62284202a3a4d9493e88834ea51bb4067c1ff9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=BF=A0=20/=20green?= Date: Sun, 29 Dec 2024 01:55:45 +0900 Subject: [PATCH 7/8] chore(tasks): print diff for minify idempotency assertion (#8161) Made the assertion error output more easier to understand. Example output: ![image](https://github.com/user-attachments/assets/445910fb-b389-4884-b4c5-70a095bc523f) --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- Cargo.lock | 19 ++++++++++++++++++- Cargo.toml | 1 + tasks/minsize/Cargo.toml | 2 ++ tasks/minsize/src/lib.rs | 23 ++++++++++++++++++++++- 4 files changed, 43 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cab6130bbc748..9461a3e989753 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -174,6 +174,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1a68f1f47cdf0ec8ee4b941b2eee2a80cb796db73118c0dd09ac63fbe405be22" dependencies = [ "memchr", + "regex-automata", "serde", ] @@ -1082,7 +1083,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fc2f4eb4bc735547cfed7c0a4922cbd04a4655978c09b54f1f7b228750664c34" dependencies = [ "cfg-if", - "windows-targets 0.48.5", + "windows-targets 0.52.6", ] [[package]] @@ -1787,6 +1788,7 @@ dependencies = [ name = "oxc_minsize" version = "0.0.0" dependencies = [ + "cow-utils", "flate2", "humansize", "oxc_allocator", @@ -1798,6 +1800,7 @@ dependencies = [ "oxc_tasks_common", "oxc_transformer", "rustc-hash", + "similar-asserts", ] [[package]] @@ -2749,6 +2752,20 @@ name = "similar" version = "2.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1de1d4f81173b03af4c0cbed3c898f6bff5b870e4a7f5d6f4057d62a7a4b686e" +dependencies = [ + "bstr", + "unicode-segmentation", +] + +[[package]] +name = "similar-asserts" +version = "1.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cfe85670573cd6f0fa97940f26e7e6601213c3b0555246c24234131f88c5709e" +dependencies = [ + "console", + "similar", +] [[package]] name = "siphasher" diff --git a/Cargo.toml b/Cargo.toml index 1bd59663b3e21..1eff0ff6634e5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -189,6 +189,7 @@ serde-wasm-bindgen = "0.6.5" sha1 = "0.10.6" simdutf8 = { version = "0.1.5", features = ["aarch64_neon"] } similar = "2.6.0" +similar-asserts = "1.6.0" string_wizard = "0.0.25" tempfile = "3.14.0" tokio = "1.42.0" diff --git a/tasks/minsize/Cargo.toml b/tasks/minsize/Cargo.toml index c842002da53fe..5a39fe439d403 100644 --- a/tasks/minsize/Cargo.toml +++ b/tasks/minsize/Cargo.toml @@ -29,5 +29,7 @@ oxc_transformer = { workspace = true } flate2 = { workspace = true } oxc_tasks_common = { workspace = true } +cow-utils = { workspace = true } humansize = { workspace = true } rustc-hash = { workspace = true } +similar-asserts = { workspace = true } diff --git a/tasks/minsize/src/lib.rs b/tasks/minsize/src/lib.rs index 3c149505e568d..91eda08f1e8df 100644 --- a/tasks/minsize/src/lib.rs +++ b/tasks/minsize/src/lib.rs @@ -16,6 +16,7 @@ use oxc_span::SourceType; use oxc_tasks_common::{project_root, TestFile, TestFiles}; use oxc_transformer::{ReplaceGlobalDefines, ReplaceGlobalDefinesConfig}; use rustc_hash::FxHashMap; +use similar_asserts::assert_eq; // #[test] // #[cfg(any(coverage, coverage_nightly))] @@ -23,6 +24,21 @@ use rustc_hash::FxHashMap; // run().unwrap(); // } +macro_rules! assert_eq_minified_code { + ($left:expr, $right:expr, $($arg:tt)*) => { + if $left != $right { + let normalized_left = $crate::normalize_minified_code($left); + let normalized_right = $crate::normalize_minified_code($right); + assert_eq!(normalized_left, normalized_right, $($arg)*); + } + }; +} + +fn normalize_minified_code(code: &str) -> String { + use cow_utils::CowUtils; + code.cow_replace(";", ";\n").cow_replace(",", ",\n").into_owned() +} + /// # Panics /// # Errors pub fn run() -> Result<(), io::Error> { @@ -131,7 +147,12 @@ fn minify_twice(file: &TestFile) -> String { }; let source_text1 = minify(&file.source_text, source_type, options); let source_text2 = minify(&source_text1, source_type, options); - assert_eq!(source_text1, source_text2, "Minification failed for {}", &file.file_name); + assert_eq_minified_code!( + &source_text1, + &source_text2, + "Minification failed for {}", + &file.file_name + ); source_text2 } From d8d2ec625729582b58633e0e17e39059a248dd81 Mon Sep 17 00:00:00 2001 From: "Alexander S." Date: Sat, 28 Dec 2024 18:03:11 +0100 Subject: [PATCH 8/8] perf(linter): run rules which require typescript syntax only when source type is actually typescript (#8166) --- .../src/rules/typescript/consistent_generic_constructors.rs | 4 ++++ .../oxc_linter/src/rules/typescript/no_empty_object_type.rs | 4 ++++ crates/oxc_linter/src/rules/typescript/no_misused_new.rs | 4 ++++ .../src/rules/typescript/no_unsafe_function_type.rs | 4 ++++ .../src/rules/typescript/no_wrapper_object_types.rs | 4 ++++ 5 files changed, 20 insertions(+) diff --git a/crates/oxc_linter/src/rules/typescript/consistent_generic_constructors.rs b/crates/oxc_linter/src/rules/typescript/consistent_generic_constructors.rs index 8ee66e9916c97..bcb86b4f21156 100644 --- a/crates/oxc_linter/src/rules/typescript/consistent_generic_constructors.rs +++ b/crates/oxc_linter/src/rules/typescript/consistent_generic_constructors.rs @@ -118,6 +118,10 @@ impl Rule for ConsistentGenericConstructors { .unwrap_or_default(), })) } + + fn should_run(&self, ctx: &crate::rules::ContextHost) -> bool { + ctx.source_type().is_typescript() + } } impl ConsistentGenericConstructors { diff --git a/crates/oxc_linter/src/rules/typescript/no_empty_object_type.rs b/crates/oxc_linter/src/rules/typescript/no_empty_object_type.rs index 12c1bc5f91fe6..8e479d1d8e83f 100644 --- a/crates/oxc_linter/src/rules/typescript/no_empty_object_type.rs +++ b/crates/oxc_linter/src/rules/typescript/no_empty_object_type.rs @@ -140,6 +140,10 @@ impl Rule for NoEmptyObjectType { _ => {} } } + + fn should_run(&self, ctx: &crate::rules::ContextHost) -> bool { + ctx.source_type().is_typescript() + } } fn check_interface_declaration( diff --git a/crates/oxc_linter/src/rules/typescript/no_misused_new.rs b/crates/oxc_linter/src/rules/typescript/no_misused_new.rs index acc5d646d2992..64e5c77c35b98 100644 --- a/crates/oxc_linter/src/rules/typescript/no_misused_new.rs +++ b/crates/oxc_linter/src/rules/typescript/no_misused_new.rs @@ -113,6 +113,10 @@ impl Rule for NoMisusedNew { _ => {} } } + + fn should_run(&self, ctx: &crate::rules::ContextHost) -> bool { + ctx.source_type().is_typescript() + } } #[test] diff --git a/crates/oxc_linter/src/rules/typescript/no_unsafe_function_type.rs b/crates/oxc_linter/src/rules/typescript/no_unsafe_function_type.rs index 23ff43c4704f0..dd7fe7af08a3b 100644 --- a/crates/oxc_linter/src/rules/typescript/no_unsafe_function_type.rs +++ b/crates/oxc_linter/src/rules/typescript/no_unsafe_function_type.rs @@ -76,6 +76,10 @@ impl Rule for NoUnsafeFunctionType { _ => {} } } + + fn should_run(&self, ctx: &crate::rules::ContextHost) -> bool { + ctx.source_type().is_typescript() + } } fn handle_function_type<'a>(identifier: &'a IdentifierReference<'a>, ctx: &LintContext<'a>) { diff --git a/crates/oxc_linter/src/rules/typescript/no_wrapper_object_types.rs b/crates/oxc_linter/src/rules/typescript/no_wrapper_object_types.rs index f8adc33f1f4b7..d320944f24c17 100644 --- a/crates/oxc_linter/src/rules/typescript/no_wrapper_object_types.rs +++ b/crates/oxc_linter/src/rules/typescript/no_wrapper_object_types.rs @@ -99,6 +99,10 @@ impl Rule for NoWrapperObjectTypes { } } } + + fn should_run(&self, ctx: &crate::rules::ContextHost) -> bool { + ctx.source_type().is_typescript() + } } #[test]