Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Rule Request: Forbid in operator and for...in on some iterable objects #2607

Closed
sbj42 opened this issue Apr 19, 2017 · 5 comments
Closed

Rule Request: Forbid in operator and for...in on some iterable objects #2607

sbj42 opened this issue Apr 19, 2017 · 5 comments

Comments

@sbj42
Copy link
Contributor

sbj42 commented Apr 19, 2017

Rule Request

I was converting some code from using plain objects as maps and sets to using the ES6 Map and Set classes. TypeScript handily caught all of the places where the code did things like map[key] = value and delete map[key], but it did not catch uses of key in map and for (const key in map) .... This seems like something tslint would be very useful for.

While just applying the rule to Map and Set would be nice, it might be possible to generalize it, either to any iterable type, or to any type that isn't any or indexable.

  • TSLint version: 4.5.1
  • TypeScript version: 2.2.1
  • Running TSLint via: CLI / VSCode

TypeScript code to lint

const map = new Map<string,string>();
for (const key in map) { // <-- should use `const [key, value] of map` instead
    if (map.hasOwnProperty(key)) { // <-- maybe also warn about this
        /* ... */
    }
}
if ("thing" in map) { // <-- should use `map.has("thing")` instead
    /* ... */
}
const set= new Set<string>();
for (const key in set) { // <-- should use `const key of set` instead
    if (set.hasOwnProperty(key)) { // <-- maybe also warn about this
        /* ... */
    }
}
if ("thing" in set) { // <-- should use `set.has("thing")` instead
    /* ... */
}
@adidahiya
Copy link
Contributor

we could probably generalize the no-for-in-array rule to do this. we should name it no-for-in and deprecate the existing rule

@sbj42
Copy link
Contributor Author

sbj42 commented Apr 29, 2017

In fact, these lines in the no-for-in-array test case seem to agree this is desired.

I'm having a little trouble coming up with an implementation. At one of those lines, where I think the for-in expression should have type Map, the Type obtained has no symbol, flags is 1 ("any"). I'm not sure how to find out that the type is Map, or better yet whether the type has an implementation for the Symbol.iterator property.

@andy-hanson
Copy link
Contributor

This could use microsoft/TypeScript#13502 to check for whether the object is iterable.

@JoshuaKGoldberg
Copy link
Contributor

Note: per #4534, this issue will be closed in less than a month if no PR is sent to add the rule. If you really need the rule, custom rules are always an option and can be maintained outside this repo!

@JoshuaKGoldberg
Copy link
Contributor

💀 It's time! 💀

TSLint is being deprecated and no longer accepting pull requests for new rules. See #4534. 😱

If you'd like to see this rule implemented, you have two choices:

👋 It was a pleasure open sourcing with you!

If you believe this message was posted here in error, please comment so we can re-open the issue!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants