eslint-plugin-unicorn: Rule proposal: `no-duplicate-loops`

I’ve seen this a couple of times

Error

for (const gamma of bars.map(alpha => alpha.beta.gamma)) {
  gamma('ray')
}
 
for (const act of actions.filter(act => typeof act === 'function')) {
  act('a fool')
}

Pass

for (const alpha of bars) {
  alpha.beta.gamma('ray')
}


for (const act of actions) {
  if (typeof act === 'function') {
    act('a fool')
  }
}

About this issue

Original URL
State: open
Created 4 years ago
Reactions: 7
Comments: 23 (8 by maintainers)

Most upvoted comments

@fisker Because you end up iterating through bars twice.

sindresorhus on Sep 30, 2020

How about forcing the looped expression to be one of:

That sounds sensible, but wouldn’t that be a completely different rule? This rule is about preventing duplicate iterations.

sindresorhus on Dec 10, 2020

Should we consider this edge case

for (const gamma of 
  bars.map(someFunctionDoSomethingOrChangesThePreviousElement)
) {
  gamma('ray')
}

Hmm, I think functions with side-effects inside map and filter are incorrect anyway, so if that gives a lint error that is fine by me. Maybe the error message can be:

Do not use map in the header of a for loop. Perform the operation directly inside the loop instead. Note that your mapping must not have side-effects.

Do not use filter in the header of a for loop. Perform an if directly inside the loop instead. Note that your filter predicate must not have side-effects.

papb on Oct 4, 2020

@fregante Your last example can’t be safely converted to a single loop… (in general)

papb on Oct 1, 2020

I thought the idea was to disallow it in the body of the for loop

Apparently that part is called “header”, so perhaps it’s no-array-loops-in-for-header, just in case the feature can handle more methods in the future.

It should ignore any other construction

Agreed

fregante on Oct 1, 2020