typescript:S2310 triggers in `for…of`

  • Using SonarQube 8.7.1 (with SonarTS 2.1.0.4359, I believe).

If I understand typescript:S2310 (“Loop counters should not be assigned to from within the loop body”) correctly, it tries to disallow messing with the iteration order by modifying the loop counter variable (as in the rule’s non-compliant code example).

However, the rule is currently triggered also in for…of iteration (and I guess in for…in as well). AFAICT, the reasoning does not apply there at all. By modifying the iteration variable (which is no “loop counter” here), we are modifying just a copy/reference of an object contained within an iterable, without affecting the iteration order at all. E.g. this is, IMHO, a completely fine code:

const array1 = [' a', 'b  ', ' c '];

for (let element of array1) {
  element = element.trim();
  console.log(element);
  somethingsomething(element);
}

A possible workaround here would be to make another variable to hold a copy of the iteration variable, i.e.

for (const element of array1) {
  let trimmed = element.trim();
  console.log(trimmed);
  somethingsomething(trimmed);
}

which is nice as far as immutable variables go, but I don’t think this is necessary or that it should be the same rule / the same level of problematic as the loop counter modification reported normally by S2310. So I believe S2310 should keep reporting only the “old-style” for-loops.

Hello,

I see your reasoning, but I don’t fully agree. The point is that it is expected by the future maintainer of the code that iterator variable holds an element of the collection, so it will be harder to maintain if variable is reassigned. Indeed your case might be borderline as transformation is minimal and loop body is small (then I suggest you to “won’t fix” the issue in the SQ UI), but in general case I think reporting on for ... of is making sense for the rule.