Rule proposal: "no-misused-in" warn against using in on values that aren't indexable

[bradzacher]

Before You File a Proposal Please Confirm You Have Done The Following...

• I have searched for related issues and found none that match my proposal.
• I have searched the current rule list and found no rules that match my proposal.
• I have read the FAQ and my problem is not listed.

My proposal is suitable for this project

• My proposal specifically checks TypeScript syntax, or it proposes a check that requires type information to be accurate.
• My proposal is not a "formatting rule"; meaning it does not just enforce how code is formatted (whitespace, brace placement, etc).
• I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Background

In languages like python use the in operator to check for key existence in sets and maps. Polyglot engineers may make the mistake to use in in JS.

With this change to TS to be released in 4.9, the in operator will now refine any object type by intersecting it with Record<key, unknown>. This means you could get some funky behaviour that previously was not allowed.
For example:

declare const map: Map<string, unknown>;

declare function acceptsAnything(arg: unknown): void;

if ('woopsie' in map) {
  acceptsAnything(map['woopsie']);
}

Currently in TS 4.8 this errors with Element implicitly has an 'any' type because type 'Map<string, unknown>' has no index signature. Did you mean to call 'map.get'? (7052)
After the above PR, this no longer errors at all because the type of map is refined to Map<string, unknown> & Record<'woopsie', unknown>, so typeof map['woopsie'] === unknown.

Rule Description

In the vast, vast majority of cases you wouldn't want to allow in to be used. I can think of the following cases that you would want to allow in:

• {}
• object
• A type with an index signature (i.e. Record<K, V>)

I think behind a (default false) flag we could also allow in to be used against types if and only if the type (or one of the types if it is a union) has that property.

Fail Cases

declare const set: Set<string>;
if ('woopsie' in set) {}

declare const map: Map<string, unknown>;
if ('woopsie' in map) {}

class Foo {}
if ('woopsie' in (new Foo()) {}

declare const array: string[];
if ('woopsie' in array) {}
if (1 in array) {}

'a' in ['a']; // some languages like python allow `in` to be used like `.includes`

With the "allow known keys in objects" flag turned OFF:

declare const obj: {a: string};
if ('a' in obj) {}
if ('b' in obj) {}

declare const union: {a: string} | {b: string};
if ('a' in union) {}

Pass Cases

for (const val in arr) {} // ForInStatement should be ignored by this rule as it has different meaning

declare const objectType: object;
if ('key' in objectType) {}

declare const emptyObjectType: {};
if ('key' in emptyObjectType) {}

declare const record1: {a: string, [k: string]: unknown};
if ('key' in record1) {}
if ('a' in record1) {}

declare const record2: Record<string, number>;
if ('key' in record2) {}

With the "allow known keys in objects" flag turned ON:

declare const obj: {a: string};
if ('a' in obj) {} // allowed --- should probably be flagged by no-unnecessary-condition?

declare const union: {a: string} | {b: string};
if ('a' in union) {}

[Josh-Cena]

What about "no-misused-in"?

[bradzacher]

If there are more types that we think it's invalid to use in on, perhaps this could be expanded to no-misused-in.

Perhaps it's worth just making this error on in usage with any type other than (explicitly) object, {}, or a type with an index signature (eg Record).
I don't know if there are any cases where you want to use in with anything other than these cases, especially after the aforementioned TS change.

One caveat to the above is that maybe it's worth allowing it for object unions in case someone wants to use it to narrow like {a: string} | {b: string} i.e. 'a' in obj would narrow obj to {a: string}. Maybe an option for this?

[Josh-Cena]

in has been notoriously unsound to begin with, because TS doesn't forbid excess properties. People should not be using it to narrow unions either.

[bradzacher]

@Josh-Cena could you clarify with some example code how it would be unsafe to use with a union?

[Josh-Cena]

Playground

TS allows using in to refine unions, which is unsafe. The only safe way is to use a tagged union.

[bradzacher]

Yeah I guess structural typing is a bitch there and it allows some dodgy code!
Worth noting that if you directly return the object instead of indirectly via a variable, then TS applies an excess property check.

[Josh-Cena]

Correct—but in an actual program it's rarely that trivial, and the makeAnObject function may come from a library and you never know what's inside it...

[HolgerJeromin]

When moving from js objects to Map we got many problems with incomplete migration which worked but was not intended.
Could this be handled with this item or should I open a new one?

this.__object = {};
// ...
this.__object['objectPropertyname'];

this.__map = new Map();
// ...
this.__map['objectPropertyname']; // ops

Disclaimer: we use ts config option suppressImplicitAnyIndexErrors https://www.typescriptlang.org/tsconfig#suppressImplicitAnyIndexErrors. eslint should report on Map/Set only.

[bradzacher]

@HolgerJeromin - originally I was thinking that the best plan for this rule was ensuring correct usage of ES6 collections - but I think there's more value in specifically just handling the in operator given the upcoming change in TS 4.9.

Given that TS has a compiler option which would handle the index access usecase in collections - I don't think it makes sense for us to build a rule in this plugin for it.
Though it would be pretty straight-forward to build one for your codebase using our tooling!

[JoshuaKGoldberg]

A note from #5474: this rule should cover the case of arrays, too.

[mahdi-farnia]

I just wanna get a background on what is exactly is a misuse of in operator.

I wanna work on this, It seems complex for me as a new one in ESLint & ast... (But I'll try)

I know one of them which is type Array<any>, what should be added to this list?

[bradzacher]

@mahdi-farnia does the issue's description contain enough information to answer your question?

[mahdi-farnia]

@bradzacher
At time that i wrote that comment, I didn't read your comment about __index and ts-ast viewer. ( about no-in-array ).

Know i do.

So Yes!

Note that Because of my college I can't work on this in weekdays.