-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Description
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) {}