Bug: [no-for-in-array] rule does not detect complex non-array situation

[HolgerJeromin]

Before You File a Bug Report Please Confirm You Have Done The Following...

• I have tried restarting my IDE and the issue persists.
• I have updated to the latest version of the packages.
• I have searched for related issues and found none that matched my issue.
• I have read the FAQ and my problem is not listed.

Playground Link

https://typescript-eslint.io/play/#ts=5.7.3&fileType=.ts&code=MYGwhgzhAEBmD28A8ARaBTAHgF3QOwBMYJsAnASzwHMBtAXWgB9oAldYeUgpEi6gGmh4ArgFsARulIA%2BadADeAKGjQADqXiqAXNDTMRIEE2jDC6WJXQEA3Mui4S0ALzQAFAEpncpSpUc8jmCkpGAAns72ABbkEAB06pq2vtDksG4AgsFhsTGZIaGuQfnunj7J0AD0FULwALQIpLWUtUVh0KToqpzYMBzB7NggoXbJDW7%2BjuQpeNCtoaUj5RPwIOixIPBUhVmhNOR07knlfvAB2NAA%2BuS4ohFze3SLxypVAHoA-E-QAL6L3xggCDoBRfKo1eqcJp4Fo7aAEeDoGB4eDnDpdUjnPodYCDUKxaAAFWiMBiNVmOy%2BY1cE3OU0o5OKIOe0GWq3Wm22%2BQehy%2BJzOl2u6FuLnu%2B155Ten2Ov2SMp%2Bil%2BQA&eslintrc=N4KABGBEBOCuA2BTAzpAXGUEKQAIBcBPABxQGNoBLY-AWhXkoDt8B6Jge1oDMPpbmtAIbRoQwuiiJRfSODABfEAqA&tsconfig=N4KABGBEDGD2C2AHAlgGwKYCcDyiAuysAdgM6QBcYoEEkJemy0eAcgK6qoDCAFutAGsylBm3TgwAXxCSgA&tokens=false

Repro Code

class foo<D extends string[] | Record<string, number>> {
  prop: D | null | undefined;
  test = () => {
    const array = this.prop;
    if (Array.isArray(array)) {
      // no-for-in-array reports correctly
      for (const i in array) {
        console.log(array[i]);
        const _item = array[i]
            //^?
      }
    } else {
      // no-for-in-array does not report correctly. This is no array
      for (const i in array) {
        console.log(array[i]);
        const _item = array[i]
            //^?
      }
    }
  }
}

ESLint Config

module.exports = {
  parser: "@typescript-eslint/parser",
  rules: {
    "@typescript-eslint/no-for-in-array": "error",
  },
};

tsconfig

{
  "compilerOptions": {
    // ...
  }
}

Expected Result

We have a check Array.isArray(), but TS does not detect it is no array-like.

Actual Result

Since #10535 ("report on any type which may be an array or array-like") this code get reported as an error.

I am totally fine to accept that this complex thing is failing the "may be an array" test.
I know Typescript is not perfect in flow analytics.

But perhaps you are interested in this.

Additional Info

No response

[kirkwaiblinger]

Hi @HolgerJeromin !

It looks like this is specifically an issue to do with the type parameter/generic.... We can simplify the repro to

function foo<T extends string[] | Record<string, number>>(array: T) {
  if (Array.isArray(array)) {
    // no-for-in-array reports correctly
    for (const i in array) {
    }
  } else {
    // no-for-in-array does not report correctly. This is no array
    for (const i in array) {
    }
  }  
}

Compare to

declare const x: string[] | Record<string, number> | null | undefined;
if (Array.isArray(x)) {
  // reports correctly
  for (const i in x) {
  }
} else {
  // no false positive
  for (const i in x) {
  }
}

(playground)

---

We could possibly investigate this further, but I'm not sure whether it'll be feasible to fix on our side, since TS appears to give the type of the array in the else branch as T, not as Exclude<T, string[]> (which the rule would correctly ignore). So I think most likely this will get closed as external/wontfix. But good catch!

FWIW - I would personally recommend just banning for...in entirely but that's just me 🙃 (#10558).

[HolgerJeromin]

Yeah, TS is quite unsure about isArray on this variable.
The type in the isArray branch is D & any[] which is not exact either.
(lib.es5.d.ts has isArray(arg: any): arg is any[]; ) There are some issues in the ts tracker on this topic.

So I think most likely this will get closed as external/wontfix. But good catch!

As I wrote: fine with me :)

[bradzacher]

This is a TS limitation. TS does not refine the generic like you're expecting. In the true branch TS's "refinement" is D & any[]. But in the false branch the type is just D | null | undefined - it does not refine D.

So when we inspect the type we see a plain generic that could be an array.

So this is working as expected.