diff --git a/crates/oxc_linter/src/rules/unicorn/prefer_set_size.rs b/crates/oxc_linter/src/rules/unicorn/prefer_set_size.rs index fc8f46752ee91..acb3445b55e52 100644 --- a/crates/oxc_linter/src/rules/unicorn/prefer_set_size.rs +++ b/crates/oxc_linter/src/rules/unicorn/prefer_set_size.rs @@ -1,13 +1,17 @@ use oxc_ast::{ AstKind, - ast::{ArrayExpressionElement, Expression}, + ast::{ArrayExpressionElement, CallExpression, Expression}, }; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; -use oxc_span::Span; +use oxc_span::{GetSpan, Span}; use crate::{ - AstNode, ast_util::get_declaration_of_variable, context::LintContext, fixer::Fix, rule::Rule, + AstNode, + ast_util::{get_declaration_of_variable, is_method_call}, + context::LintContext, + fixer::Fix, + rule::Rule, }; fn prefer_set_size_diagnostic(span: Span) -> OxcDiagnostic { @@ -57,40 +61,72 @@ impl Rule for PreferSetSize { return; } - let Expression::ArrayExpression(array_expr) = member_expr.object.without_parentheses() + let Some((conversion_span, maybe_set)) = + get_set_node(member_expr.object.without_parentheses()) else { return; }; - if array_expr.elements.len() != 1 { + if !is_set(maybe_set.get_inner_expression(), ctx) { return; } - let ArrayExpressionElement::SpreadElement(spread_element) = &array_expr.elements[0] else { - return; - }; - - let maybe_set = &spread_element.argument.get_inner_expression(); - - if !is_set(maybe_set, ctx) { + if ctx.comments_range(conversion_span.start..conversion_span.end).count() + > ctx.comments_range(maybe_set.span().start..maybe_set.span().end).count() + { + ctx.diagnostic(prefer_set_size_diagnostic(span)); return; } ctx.diagnostic_with_fix(prefer_set_size_diagnostic(span), |fixer| { + let replacement = maybe_set.span().source_text(ctx.source_text()); + let mut fix = fixer - .new_fix_with_capacity(3) - .with_message("Remove spread and replace with `Set.size`"); - // remove [... - fix.push(Fix::delete(Span::new(array_expr.span.start, spread_element.span.start + 3))); - // remove everything after the end of the spread element (including the `]` ) - fix.push(Fix::delete(Span::new(spread_element.span.end, array_expr.span.end))); - // replace .length with .size + .new_fix_with_capacity(2) + .with_message("Replace array conversion with direct `Set.size` access"); + fix.push(Fix::new(replacement.to_string(), conversion_span)); fix.push(Fix::new("size", span)); fix }); } } +fn get_set_node<'a>(expression: &'a Expression<'a>) -> Option<(Span, &'a Expression<'a>)> { + // `[...set].length` + if let Expression::ArrayExpression(array_expr) = expression + && array_expr.elements.len() == 1 + && let ArrayExpressionElement::SpreadElement(spread_element) = &array_expr.elements[0] + { + return Some((array_expr.span, &spread_element.argument)); + } + + // `Array.from(set).length` + if let Expression::CallExpression(call_expr) = expression + && is_array_from_call(call_expr) + { + let set_expr = call_expr.arguments.first()?.as_expression()?; + return Some((call_expr.span, set_expr)); + } + + None +} + +fn is_array_from_call(call_expr: &CallExpression) -> bool { + if call_expr.optional { + return false; + } + + let Some(callee_member_expr) = call_expr.callee.get_member_expr() else { + return false; + }; + + if callee_member_expr.optional() || callee_member_expr.is_computed() { + return false; + } + + is_method_call(call_expr, Some(&["Array"]), Some(&["from"]), Some(1), Some(1)) +} + fn is_set<'a>(maybe_set: &Expression<'a>, ctx: &LintContext<'a>) -> bool { if let Expression::NewExpression(new_expr) = maybe_set { if let Expression::Identifier(identifier) = &new_expr.callee { @@ -190,17 +226,16 @@ fn test() { ;[...new Set(array)].length", "[/* comment */...new Set(array)].length", "[...new /* comment */ Set(array)].length", - // TODO: Update the rule to handle Array.from cases. - // "Array.from(new Set(array)).length", - // "const foo = new Set([]); - // console.log(Array.from(foo).length);", - // "Array.from((( new Set(array) ))).length", - // "(( Array.from(new Set(array)) )).length", - // "Array.from(/* comment */ new Set(array)).length", - // "Array.from(new /* comment */ Set(array)).length", - // "function isUnique(array) { - // return Array.from(new Set(array)).length === array.length - // }", + "Array.from(new Set(array)).length", + "const foo = new Set([]); + console.log(Array.from(foo).length);", + "Array.from((( new Set(array) ))).length", + "(( Array.from(new Set(array)) )).length", + "Array.from(/* comment */ new Set(array)).length", + "Array.from(new /* comment */ Set(array)).length", + "function isUnique(array) { + return Array.from(new Set(array)).length === array.length + }", ]; let fix = vec![ @@ -213,6 +248,30 @@ fn test() { r"[...(( new Set(array) as foo ) ) ] .length;", r"(( new Set(array) as foo ) ) .size;", ), + (r"Array.from(new Set(array)).length", r"new Set(array).size"), + (r"Array.from((( new Set(array) ))).length", r"(( new Set(array) )).size"), + (r"(( Array.from(new Set(array)) )).length", r"(( new Set(array) )).size"), + (r"Array.from(new /* comment */ Set(array)).length", r"new /* comment */ Set(array).size"), + ( + r"const foo = new Set([]); + console.log(Array.from(foo).length);", + r"const foo = new Set([]); + console.log(foo.size);", + ), + ( + r"function isUnique(array) { + return Array.from(new Set(array)).length === array.length + }", + r"function isUnique(array) { + return new Set(array).size === array.length + }", + ), + ( + r"foo + ;[...new Set(array)].length", + r"foo + ;new Set(array).size", + ), ]; Tester::new(PreferSetSize::NAME, PreferSetSize::PLUGIN, pass, fail) diff --git a/crates/oxc_linter/src/snapshots/unicorn_prefer_set_size.snap b/crates/oxc_linter/src/snapshots/unicorn_prefer_set_size.snap index 0e72b01423953..34f755ea14965 100644 --- a/crates/oxc_linter/src/snapshots/unicorn_prefer_set_size.snap +++ b/crates/oxc_linter/src/snapshots/unicorn_prefer_set_size.snap @@ -1,5 +1,6 @@ --- source: crates/oxc_linter/src/tester.rs +assertion_line: 444 --- ⚠ eslint-plugin-unicorn(prefer-set-size): Use `Set#size` instead of converting a `Set` to an array and using its `length` property. @@ -7,7 +8,7 @@ source: crates/oxc_linter/src/tester.rs 1 │ [...new Set(array)].length · ────── ╰──── - help: Remove spread and replace with `Set.size` + help: Replace array conversion with direct `Set.size` access ⚠ eslint-plugin-unicorn(prefer-set-size): Use `Set#size` instead of converting a `Set` to an array and using its `length` property. ╭─[prefer_set_size.tsx:2:34] @@ -15,7 +16,7 @@ source: crates/oxc_linter/src/tester.rs 2 │ console.log([...foo].length); · ────── ╰──── - help: Remove spread and replace with `Set.size` + help: Replace array conversion with direct `Set.size` access ⚠ eslint-plugin-unicorn(prefer-set-size): Use `Set#size` instead of converting a `Set` to an array and using its `length` property. ╭─[prefer_set_size.tsx:2:43] @@ -24,28 +25,28 @@ source: crates/oxc_linter/src/tester.rs · ────── 3 │ } ╰──── - help: Remove spread and replace with `Set.size` + help: Replace array conversion with direct `Set.size` access ⚠ eslint-plugin-unicorn(prefer-set-size): Use `Set#size` instead of converting a `Set` to an array and using its `length` property. ╭─[prefer_set_size.tsx:1:22] 1 │ [...new Set(array),].length · ────── ╰──── - help: Remove spread and replace with `Set.size` + help: Replace array conversion with direct `Set.size` access ⚠ eslint-plugin-unicorn(prefer-set-size): Use `Set#size` instead of converting a `Set` to an array and using its `length` property. ╭─[prefer_set_size.tsx:1:27] 1 │ [...(( new Set(array) ))].length · ────── ╰──── - help: Remove spread and replace with `Set.size` + help: Replace array conversion with direct `Set.size` access ⚠ eslint-plugin-unicorn(prefer-set-size): Use `Set#size` instead of converting a `Set` to an array and using its `length` property. ╭─[prefer_set_size.tsx:1:27] 1 │ (( [...new Set(array)] )).length · ────── ╰──── - help: Remove spread and replace with `Set.size` + help: Replace array conversion with direct `Set.size` access ⚠ eslint-plugin-unicorn(prefer-set-size): Use `Set#size` instead of converting a `Set` to an array and using its `length` property. ╭─[prefer_set_size.tsx:2:34] @@ -53,18 +54,68 @@ source: crates/oxc_linter/src/tester.rs 2 │ ;[...new Set(array)].length · ────── ╰──── - help: Remove spread and replace with `Set.size` + help: Replace array conversion with direct `Set.size` access ⚠ eslint-plugin-unicorn(prefer-set-size): Use `Set#size` instead of converting a `Set` to an array and using its `length` property. ╭─[prefer_set_size.tsx:1:34] 1 │ [/* comment */...new Set(array)].length · ────── ╰──── - help: Remove spread and replace with `Set.size` ⚠ eslint-plugin-unicorn(prefer-set-size): Use `Set#size` instead of converting a `Set` to an array and using its `length` property. ╭─[prefer_set_size.tsx:1:35] 1 │ [...new /* comment */ Set(array)].length · ────── ╰──── - help: Remove spread and replace with `Set.size` + help: Replace array conversion with direct `Set.size` access + + ⚠ eslint-plugin-unicorn(prefer-set-size): Use `Set#size` instead of converting a `Set` to an array and using its `length` property. + ╭─[prefer_set_size.tsx:1:28] + 1 │ Array.from(new Set(array)).length + · ────── + ╰──── + help: Replace array conversion with direct `Set.size` access + + ⚠ eslint-plugin-unicorn(prefer-set-size): Use `Set#size` instead of converting a `Set` to an array and using its `length` property. + ╭─[prefer_set_size.tsx:2:41] + 1 │ const foo = new Set([]); + 2 │ console.log(Array.from(foo).length); + · ────── + ╰──── + help: Replace array conversion with direct `Set.size` access + + ⚠ eslint-plugin-unicorn(prefer-set-size): Use `Set#size` instead of converting a `Set` to an array and using its `length` property. + ╭─[prefer_set_size.tsx:1:34] + 1 │ Array.from((( new Set(array) ))).length + · ────── + ╰──── + help: Replace array conversion with direct `Set.size` access + + ⚠ eslint-plugin-unicorn(prefer-set-size): Use `Set#size` instead of converting a `Set` to an array and using its `length` property. + ╭─[prefer_set_size.tsx:1:34] + 1 │ (( Array.from(new Set(array)) )).length + · ────── + ╰──── + help: Replace array conversion with direct `Set.size` access + + ⚠ eslint-plugin-unicorn(prefer-set-size): Use `Set#size` instead of converting a `Set` to an array and using its `length` property. + ╭─[prefer_set_size.tsx:1:42] + 1 │ Array.from(/* comment */ new Set(array)).length + · ────── + ╰──── + + ⚠ eslint-plugin-unicorn(prefer-set-size): Use `Set#size` instead of converting a `Set` to an array and using its `length` property. + ╭─[prefer_set_size.tsx:1:42] + 1 │ Array.from(new /* comment */ Set(array)).length + · ────── + ╰──── + help: Replace array conversion with direct `Set.size` access + + ⚠ eslint-plugin-unicorn(prefer-set-size): Use `Set#size` instead of converting a `Set` to an array and using its `length` property. + ╭─[prefer_set_size.tsx:2:51] + 1 │ function isUnique(array) { + 2 │ return Array.from(new Set(array)).length === array.length + · ────── + 3 │ } + ╰──── + help: Replace array conversion with direct `Set.size` access