New compiler option to disallow for...in/in for types that do not have any properties

[not-my-profile]

The idea is to introduce a new compiler option, e.g. strictInOperator which when enabled would flag for example the following for loop as a type error:

const numbers = [1, 2, 3].entries();

for (const x in numbers) {
    // ..
}

because Array.prototype.entries returns an IterableIterator that does not have any properties so the above for loop would never iterate (you'd have to use of instead of if), which is a very easy mistake to make especially when you're used to other programming languages such as Python or Rust.

Even if this new compiler option would only check for IterableIterator this would already solve this big pitfall in the type safety of TypeScript (the keys, values and entries methods of Array.prototype, Map.prototype and Set.prototype all return IterableIterator that doesn't work with in).

Likewise just const contained = 1 in numbers; would also result in a type error if the compiler option was enabled. The new feature would be entirely backwards compatible since it would be opt-in.

[RyanCavanaugh]

Are you talking about the in operator as in p in e, the for/in construct shown in the example, or just treating them as roughly equivalent for the purposes of the suggestion?

[Josh-Cena]

I feel like this is more in linter's land. There's also typescript-eslint/typescript-eslint#5677 for the in as an operator's case. If you want to propose a rule like no-for-in-iterable, feel free to, although you should really be banning for...in loops altogether (as the airbnb preset does).

[not-my-profile]

@RyanCavanaugh I am treating them as roughly equivalent.

I feel like this is more in linter's land.

I did also consider that ... however this is a type checking issue and typescript linters afaik do not check types, so I think this does belong here.

although you should really be banning for...in loops altogether

It's not just about for...in loops but also about the in operator and that operator indeed does have its purpose in idiomatic code.

Sidenote: There also is the guard-for-in eslint lint ... however it is very lax since it checks for the presence of any if block (so e.g. if (x > 3) also satisfies the lint) but more importantly it also doesn't handle the in operator in binary expressions.

[Josh-Cena]

typescript linters afaik do not check types

typescript-eslint does. You could find other type-aware rules here by clicking "requires type information". typescript-eslint/typescript-eslint#5677 that I shared above exactly uses type information to check the RHS's validity.

Also, do note that the in in for...in is not an in operator—it's just part of the syntax.

[not-my-profile]

Thanks for pointing that out! I just found the more thorough explanation at Linting with Type Information.

I guess the bigger question is whether or not more strict type checking should be part of the TypeScript compiler vs a separate linter plugin. TypeScript already has 18 specific such rules:
allowUnreachableCode, allowUnusedLabels, alwaysStrict, exactOptionalPropertyTypes, noFallthroughCasesInSwitch, noImplicitAny, noImplicitOverride, noImplicitReturns, noImplicitThis, noPropertyAccessFromIndexSignature, noUncheckedIndexedAccess, noUnusedLocals, noUnusedParameters, strictBindCallApply, strictFunctionTypes, strictNullChecks, strictPropertyInitialization and useUnknownInCatchVariables

Looking at the typescript eslint rules that require type information I think e.g. no-unsafe-call or no-misused-promises would also fit quite well into the builtin settings (as would the setting I propose here in my opinion), because such strict checks help achieve the first design goal of TypeScript:

Statically identify constructs that are likely to be errors.

[Josh-Cena]

exactOptionalPropertyTypes, noUncheckedIndexedAccess, strictFunctionTypes, strictNullChecks, useUnknownInCatchVariables et al. actually change the perceived type of an expression or the relationships between types. Because TS-ESLint piggy-backs the TS compiler, it means the linter will see different results as well, which is why they are necessary for the linter to function as intended at all. However, as you've realized, some other TS options are very linter-y (especially noFallthroughCasesInSwitch), so it's easily imaginable that in an alternative world they would become a linter rule, had it not been implemented in tsc so early. In fact, noFallthroughCasesInSwitch was considered for deprecation because it "feels linty". In general, if a feature is not new (e.g. noImplicitOverride was added alongside override) and can be done in linters, it probably should.

[not-my-profile]

Personally I would like TypeScript to provide strict type safety by default. I think there is a clear difference between opinionated lints and type checking (that is flagging types that are used in a way they're not supposed to be used).
I also think that providing such strict type checking built into TypeScript would result in a better developer experience since it would mean one less tool to achieve the primary goal of TypeScript (which I'd argue is identifying type errors).

However since this is a bigger question about the general scope of TypeScript, I am closing this much more specific issue since as you rightfully pointed out this can already be implemented in typescript-eslint. Thanks for pointing this out and being so responsive!

[fatcerberus]

I don't consider using for..in or "foo" in obj on a {} to be a type error, though. The reason why falls into the same reasoning that Object.keys() returns string[] instead of keyof T; it's a deliberate part of the type system that objects can have more properties than are listed in their type. Thus {} is the supertype of all objects. So it's not actually provable from types alone that a given object has no properties at runtime.

That said, "foo" in obj where x is something without a declared foo property is almost certainly a code smell--which is squarely in the domain of a linter, not the type system proper.

[Josh-Cena]

@fatcerberus The request forbids them on iterables, because the idea is that they are usually ordered collections rather than keyed collections.

[fatcerberus]

@Josh-Cena I was going by the issue title which says “…for types that do not have any properties”.

[RyanCavanaugh]

I feel like as far as rules go, saying that for (let x in iterable) is an error would be pretty defensible and you should have to write for (let x in iterable as object); that's the same sort of check we have as if (functionWithoutParens) {. That said... unless iterable is of a very specific type, you're going to get an error in the loop body when x isn't the expected type.

The standalone in operator (which really should not be conflated here) is IMO totally different and doesn't merit much additional consideration in terms of its behavior. There's very rarely confusion about what's happening with that one.