Tslint: Rule Suggestion: prefer-in-operator

Created on 20 Feb 2019  ·  5Comments  ·  Source: palantir/tslint

Rule Suggestion

Is your rule for a general problem or is it specific to your development style? Not quite sure.

What does your suggested rule do?

The rule would disallow usage of accessing via index in ifs to check whether property exists in hash map / object or not, effectively requiring to use in operator instead.

Why would you like to do this?

  • in operator should work a bit faster (test)
  • It reads nicely in the code:

    • This: !("a" in object): NO "a" in object

    • Versus: !object.a: NOT object's property a

List several examples where your rule could be used

const sample = Object.create(null);

sample.a = "hello, world";

// FORBIDS:
if (sample.a) console.log(sample.a);
if (!sample.b) throw new Error("No \"b\" property found in sample")

// ALLOWS:
if ("a" in sample) console.log(sample.a);
if (!("b" in sample)) throw new Error("No \"b\" property found in sample")

And why this rule can be questionable:

sample.a = undefined;

Boolean(sample.a); // false
"a" in sample; // true 🙁

// Actually you must use delete operator: 
delete sample.a;

Boolean(sample.a); // false
"a" in sample; // false 👍

For the case above, comparing to undefined / null would be an exception to this rule:

sample.a = undefined;

// ALLOWS:
if (sample.a == null) console.log("\"a\" was deleted! No more hello to this world..");

This rule must probably requires type information (?) to avoid false-reports such as in:

const user = await rest.users.request(id);

// SHOULD NOT FORBID:
if (user.isPremium) …
Requires Type Checker In Discussion Rule Suggestion

Most helpful comment

Relevant discussion: https://github.com/eslint/eslint/issues/2723

in searches the entire prototype chain, not just the immediate prototype

It's also normal to use sample.hasOwnProperty("a"), or more safely, {}.hasOwnProperty.call(sample, "a"). Should this rule have an option to prefer either both or just one?

All 5 comments

Relevant discussion: https://github.com/eslint/eslint/issues/2723

in searches the entire prototype chain, not just the immediate prototype

It's also normal to use sample.hasOwnProperty("a"), or more safely, {}.hasOwnProperty.call(sample, "a"). Should this rule have an option to prefer either both or just one?

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!

Oh my… I forgot about this issue and somehow GitHub did not send me any notification about your comment. Thanks now I checked my Issues tab :man_facepalming:

Now after a long time, reviewing this I actually think this rule is too edgy and hard to implement and not worth effort in ratio to its usefulness, so whoever really really needs it for some reason could implement it as custom rule on their own.

So, @JoshuaKGoldberg, are we closing this issue now or leaving it for some time so maybe someone may pick it up for pull request (doubt it)?

Up to you @Sasha-Sorokin! You're welcome to close this issue; someone else can always refile it if they're interested.

Okay. Let's close it for now!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

DanielKucal picture DanielKucal  ·  3Comments

dashmug picture dashmug  ·  3Comments

jacob-robertson picture jacob-robertson  ·  3Comments

allbto picture allbto  ·  3Comments

ghost picture ghost  ·  3Comments