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

Commit

Permalink
feat(rome_js_analyze): noAccumulatingSpread (#4426)
Browse files Browse the repository at this point in the history
* feat(rome_js_analyze): noAccumulatingSpread

* get a super general condition working

* all my cases pass as expected

* make analyzer semantic

* move rule & run codegen

* nicer tests

* tidy doc string

* update changelog

* update example
  • Loading branch information
Vivalldi authored May 2, 2023
1 parent fc8d58f commit 365fc59
Show file tree
Hide file tree
Showing 15 changed files with 809 additions and 129 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ output. [#4405](https://github.com/rome/tools/pull/4405).
- [`noUselessConstructor`](https://docs.rome.tools/lint/rules/noUselessConstructor/)
- [`useLiteralEnumMembers`](https://docs.rome.tools/lint/rules/useLiteralEnumMembers/)
- [`useHeadingContent`](https://docs.rome.tools/lint/rules/useHeadingContent/)
- [`noAccumulatingSpread`](https://docs.rome.tools/lint/rules/noAccumulatingSpread/)

#### Other changes

Expand Down
1 change: 1 addition & 0 deletions crates/rome_diagnostics_categories/src/categories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ define_categories! {
"lint/nursery/useNamespaceKeyword": "https://docs.rome.tools/lint/rules/useNamespaceKeyword",
"lint/nursery/noRedundantRoles": "https://docs.rome.tools/lint/rules/noRedundantRoles",
"lint/nursery/noConstantCondition": "https://docs.rome.tools/lint/rules/noConstantCondition",
"lint/nursery/noAccumulatingSpread": "https://docs.rome.tools/lint/rules/noAccumulatingSpread",

// performance
"lint/performance/noDelete": "https://docs.rome.tools/lint/rules/noDelete",
Expand Down
3 changes: 2 additions & 1 deletion crates/rome_js_analyze/src/semantic_analyzers/nursery.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
use rome_analyze::{context::RuleContext, declare_rule, Rule, RuleDiagnostic};
use rome_console::markup;
use rome_js_semantic::SemanticModel;
use rome_js_syntax::{
AnyJsFunction, JsCallArgumentList, JsCallArguments, JsCallExpression, JsFormalParameter,
JsParameterList, JsParameters, JsSpread,
};
use rome_rowan::AstNode;

use crate::semantic_services::Semantic;

declare_rule! {
/// Disallow the use of spread (`...`) syntax on accumulators.
///
/// Spread syntax allows an iterable to be expanded into its individual elements.
///
/// Spread syntax should be avoided on accumulators (like those in `.reduce`)
/// because it causes a time complexity of `O(n^2)` instead of `O(n)`.
///
/// Source: https://prateeksurana.me/blog/why-using-object-spread-with-reduce-bad-idea/
///
/// ## Examples
///
/// ### Invalid
///
/// ```js,expect_diagnostic
/// var a = ['a', 'b', 'c'];
/// a.reduce((acc, val) => [...acc, val], []);
/// ```
///
/// ```js,expect_diagnostic
/// var a = ['a', 'b', 'c'];
/// a.reduce((acc, val) => {return [...acc, val];}, []);
/// ```
///
/// ```js,expect_diagnostic
/// var a = ['a', 'b', 'c'];
/// a.reduce((acc, val) => ({...acc, [val]: val}), {});
/// ```
///
/// ## Valid
///
/// ```js
/// var a = ['a', 'b', 'c'];
/// a.reduce((acc, val) => {acc.push(val); return acc}, []);
/// ```
///
pub(crate) NoAccumulatingSpread {
version: "next",
name: "noAccumulatingSpread",
recommended: false,
}
}

impl Rule for NoAccumulatingSpread {
type Query = Semantic<JsSpread>;
type State = ();
type Signals = Option<Self::State>;
type Options = ();

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

is_known_accumulator(node, model)?.then_some(())
}

fn diagnostic(ctx: &RuleContext<Self>, _: &Self::State) -> Option<RuleDiagnostic> {
let node = ctx.query();
Some(
RuleDiagnostic::new(
rule_category!(),
node.range(),
markup! {
"Avoid the use of spread (`...`) syntax on accumulators."
},
)
.note(markup! {
"Spread syntax should be avoided on accumulators (like those in `.reduce`) because it causes a time complexity of `O(n^2)`."
}),
)
}
}

fn is_known_accumulator(node: &JsSpread, model: &SemanticModel) -> Option<bool> {
let reference = node
.argument()
.ok()?
.as_js_identifier_expression()?
.name()
.ok()?;

let parameter = model
.binding(&reference)
.and_then(|declaration| declaration.syntax().parent())
.and_then(JsFormalParameter::cast)?;
let function = parameter
.parent::<JsParameterList>()
.and_then(|list| list.parent::<JsParameters>())
.and_then(|parameters| parameters.parent::<AnyJsFunction>())?;
let call_expression = function
.parent::<JsCallArgumentList>()
.and_then(|arguments| arguments.parent::<JsCallArguments>())
.and_then(|arguments| arguments.parent::<JsCallExpression>())?;

let name = call_expression
.callee()
.ok()?
.as_js_static_member_expression()?
.member()
.ok()?
.as_js_name()?
.value_token()
.ok()?;
let name = name.text_trimmed();

if matches!(name, "reduce" | "reduceRight") {
Some(parameter.syntax().index() == 0)
} else {
None
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
[
// Array - Arrow return
"foo.reduce((acc, bar) => [...acc, bar], [])",
"foo.reduceRight((acc, bar) => [...acc, bar], [])",

// Array - Body return
"foo.reduce((acc, bar) => {return [...acc, bar];}, [])",
"foo.reduceRight((acc, bar) => {return [...acc, bar];}, [])",

// Array - Arrow return with item spread
"foo.reduce((acc, bar) => [...acc, ...bar], [])",
"foo.reduceRight((acc, bar) => [...acc, ...bar], [])",

// Array - Body return with item spread
"foo.reduce((acc, bar) => {return [...acc, ...bar];}, [])",
"foo.reduceRight((acc, bar) => {return [...acc, ...bar];}, [])",

// Object - Arrow return
"foo.reduce((acc, bar) => ({...acc, [bar.key]: bar.value}), {})",
"foo.reduceRight((acc, bar) => ({...acc, [bar.key]: bar.value}), {})",

// Object - Body return
"foo.reduce((acc, bar) => {return {...acc, [bar.key]: bar.value};}, {})",
"foo.reduceRight((acc, bar) => {return {...acc, [bar.key]: bar.value};}, {})",

// Object - Arrow return with item spread
"foo.reduce((acc, bar) => ({...acc, ...bar}), {})",
"foo.reduceRight((acc, bar) => ({...acc, ...bar}), {})",

// Object - Body return with item spread
"foo.reduce((acc, bar) => {return {...acc, ...bar};}, {})",
"foo.reduceRight((acc, bar) => {return {...acc, ...bar};}, {})"
]
Loading

0 comments on commit 365fc59

Please sign in to comment.