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

bradzacher · 2022-09-20T02:41:52Z

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) {}

The text was updated successfully, but these errors were encountered:

Josh-Cena · 2022-09-20T02:44:05Z

What about "no-misused-in"?

bradzacher · 2022-09-20T02:44:11Z

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 · 2022-09-20T02:45:08Z

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 · 2022-09-20T02:48:15Z

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

Josh-Cena · 2022-09-20T02:52:34Z

Playground

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

bradzacher · 2022-09-20T03:40:04Z

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 · 2022-09-20T03:56:33Z

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 · 2022-09-20T06:52:44Z

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 · 2022-09-20T09:02:01Z

@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 · 2022-10-20T14:20:33Z

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

mahdi-farnia · 2022-10-20T15:26:38Z

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 · 2022-10-24T11:41:10Z

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

mahdi-farnia · 2022-10-24T12:17:47Z

@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.

karlhorky · 2024-07-19T07:57:59Z

I was going to submit a new rule proposal just now for no-unsafe-in, following the existing rules:

Something that would ban usage of the in operator with any-typed RHS, eg. this code which causes a JavaScript runtime error:

if ('x' in (undefined as any)) {

But maybe comment #1251768783 by @bradzacher alludes to no-misused-in already becoming this?

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).

Or should this rule proposal for no-misused-in be renamed to no-unsafe-in for API symmetry with the other rules?

bradzacher · 2024-07-19T09:23:47Z

I think that would be the best thing yeah - one rule for safe in usage.

bradzacher mentioned this issue Sep 20, 2022

Improve checking of in operator microsoft/TypeScript#50666

Merged

bradzacher changed the title ~~Rule proposal: "no-misused-collection" warn against using in on Set and Map~~ Rule proposal: "no-misused-in" warn against using in on values that aren't indexable Sep 20, 2022

bradzacher mentioned this issue Oct 20, 2022

Rule proposal: Ban 'in' operator for arrays #5474

Closed

6 tasks

JoshuaKGoldberg mentioned this issue Oct 20, 2022

feat(no-in-array): Disallow using in operator for arrays #5851

Closed

3 tasks

Josh-Cena mentioned this issue Mar 8, 2023

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

Closed

Josh-Cena mentioned this issue Mar 31, 2023

Rule proposal: Prevent Object.values(...) usage with Map #6807

Open

6 tasks

bradzacher mentioned this issue Jun 18, 2023

Bug: [no-unnecessary-condition] does not report an error for typeof/instanceof or "in" typeguards #7064

Closed

4 tasks

SimonSchick mentioned this issue Jun 21, 2024

feat(eslint-plugin): [no-misused-object-like] add rule #9369

Closed

3 tasks

This was referenced Oct 27, 2024

feat(eslint-plugin): new rule no-unsafe-type-assertion #10051

Merged

Rule proposal: no-excess-properties #10234

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

bradzacher commented Sep 20, 2022 •

edited

Loading

Josh-Cena commented Sep 20, 2022

bradzacher commented Sep 20, 2022 •

edited

Loading

Josh-Cena commented Sep 20, 2022

bradzacher commented Sep 20, 2022

Josh-Cena commented Sep 20, 2022 •

edited

Loading

bradzacher commented Sep 20, 2022

Josh-Cena commented Sep 20, 2022

HolgerJeromin commented Sep 20, 2022 •

edited

Loading

bradzacher commented Sep 20, 2022

JoshuaKGoldberg commented Oct 20, 2022

mahdi-farnia commented Oct 20, 2022 •

edited

Loading

bradzacher commented Oct 24, 2022

mahdi-farnia commented Oct 24, 2022 •

edited

Loading

karlhorky commented Jul 19, 2024 •

edited

Loading

bradzacher commented Jul 19, 2024

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

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

Comments

bradzacher commented Sep 20, 2022 • edited Loading

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

My proposal is suitable for this project

Background

Rule Description

Fail Cases

Pass Cases

Josh-Cena commented Sep 20, 2022

bradzacher commented Sep 20, 2022 • edited Loading

Josh-Cena commented Sep 20, 2022

bradzacher commented Sep 20, 2022

Josh-Cena commented Sep 20, 2022 • edited Loading

bradzacher commented Sep 20, 2022

Josh-Cena commented Sep 20, 2022

HolgerJeromin commented Sep 20, 2022 • edited Loading

bradzacher commented Sep 20, 2022

JoshuaKGoldberg commented Oct 20, 2022

mahdi-farnia commented Oct 20, 2022 • edited Loading

bradzacher commented Oct 24, 2022

mahdi-farnia commented Oct 24, 2022 • edited Loading

karlhorky commented Jul 19, 2024 • edited Loading

bradzacher commented Jul 19, 2024

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

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

bradzacher commented Sep 20, 2022 •

edited

Loading

bradzacher commented Sep 20, 2022 •

edited

Loading

Josh-Cena commented Sep 20, 2022 •

edited

Loading

HolgerJeromin commented Sep 20, 2022 •

edited

Loading

mahdi-farnia commented Oct 20, 2022 •

edited

Loading

mahdi-farnia commented Oct 24, 2022 •

edited

Loading

karlhorky commented Jul 19, 2024 •

edited

Loading