False positives for prefer-for-of rule

[thebanjomatic]

Bug Report

There are a couple of cases where the prefer-for-of rule produces linter errors on code can not be trivially converted to a for-of loop. In particular:

1. Step values != 1
2. Start of iteration != 0
3. Reverse iteration

Additionally, there are a few other cases where errors aren't reported, but possibly should, such as: <= arr.length - 1 instead of < arr.length.

I have mixed feelings about whether or not reverse iteration should trigger this error or not. Sometimes the array can just be reversed before iteration, other times you might want to clone the array first before reversing.

I guess in any of these cases, you could write helper functions to minimize the places where you need to opt-out of this rule, but at that point you aren't really preferring for-of so much as you are not preferring bare for-loops.

/* tslint:disable:prefer-for-of */
forEachWithStep<T>(array: Array<T>, step: number, func: (T) => void) {
    for (let i = 0; i < array.length; i += step) {
        func(array[i]);
    }
}

partialForEach<T>(array: Array<T>, start: number, end: number, func: (T) => void) {
    for (let i = start; i < end; i++) {
        func(array[i]);
    }
}

reverseForEach<T>(array: Array<T>, func: (T) => void) {
    for (let i = array.length; i>= 0; i--) {
        func(array[i]);
    }
}
/* tslint:enable:prefer-for-of */

• TSLint version: 4.0.1
• TypeScript version: 2.1.1
• Running TSLint via: Node.js API

TypeScript code being linted

declare var arr: number[];
declare var step: number;

// trivial #1: array iteration [0,length)
for (let i = 0; i < arr.length; i++) {
   console.log(arr[i]);
}

// trivial #2: effectively the same as trivial #1, but not caught
for (let i = 0; i <= arr.length - 1; i++) {
   console.log(arr[i]);
}

// trivial #3: i++, ++i, and i += 1 should all be considered
for (let i = 0; i < arr.length; i += 1) {
   console.log(arr[i]);
}

/****************************************************************/

// non-trivial #1: array iteration with step size not known to be 1
for (let i = 0; i < arr.length; i += step) {
   console.log(arr[i]);
}

// non-trivial #2: non-zero start of iteration
for (let i = 1; i < arr.length; i++) {
   console.log(arr[i]);
}

// non-trivial #3: reverse iteration...
for (let i = arr.length; i >= 0; i--) {
   console.log(arr[i]);
}

with tslint.json configuration:

{
    "rules": {
        "prefer-for-of": true
    }
}

Actual behavior

Trivial Loops:
test.ts[5, 1]: Expected a 'for-of' loop instead of a 'for' loop with this simple iteration
test.ts[15, 1]: Expected a 'for-of' loop instead of a 'for' loop with this simple iteration

Non-Trivial Loops:
test.ts[22, 1]: Expected a 'for-of' loop instead of a 'for' loop with this simple iteration
test.ts[27, 1]: Expected a 'for-of' loop instead of a 'for' loop with this simple iteration
test.ts[32, 1]: Expected a 'for-of' loop instead of a 'for' loop with this simple iteration

Expected behavior

The first 3 trivial loops should be reported as errors, the last three loops should not trigger the error.

Trivial Loops:
test.ts[5, 1]: Expected a 'for-of' loop instead of a 'for' loop with this simple iteration
test.ts[10, 1]: Expected a 'for-of' loop instead of a 'for' loop with this simple iteration
test.ts[15, 1]: Expected a 'for-of' loop instead of a 'for' loop with this simple iteration

Non-Trivial Loops:
[no errors]

[thebanjomatic]

I just noticed that #1758 was merged while I was writing up the bug report. It looks like that should cover most of these cases except possibly Trivial #2.

[nchen63]

the PR does not cover Trival #2, but I don't think that is very common

[nchen63]

closed as duplicate of #1753