Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce length check style in the explicit-length-check rule #56

Closed
jfmengels opened this issue Oct 12, 2016 · 2 comments · Fixed by #96
Closed

Enforce length check style in the explicit-length-check rule #56

jfmengels opened this issue Oct 12, 2016 · 2 comments · Fixed by #96

Comments

@jfmengels
Copy link
Contributor

explicit-length-check checks that length properties get compared to a value instead of checking the truthiness of the property.

In avajs/eslint-plugin-ava#149 (review), we remembered that there are some opinions on how that value then gets compared 😅

For instance:

a.length === 0
// vs
a.length < 1 // not sure I've ever seen this one TBH

a.length !== 0
// vs
a.length > 0
// vs
a.length >= 1

// + some I might have forgotten

It could be nice to be able to specify how you want your comparisons to be made, and make that consistent across your project.

I'm not sure what the options would look like. At the top of my head, I'm thinking:

{
  "unicorn/explicit-length-check": ["error", {
    "empty": ["eq", 0] | ["lt", 1], // one or the other
    "not-empty": ["ne", 0] | ["gt", 0] | ["gte", 1]
  }]
}

Should one be omitted (empty for instance), then you'd simply get no errors reports on how that one gets checked (both a.length < 1 a.length === 0 would be fine).

This sounds pretty complicated at the moment (especially if we want to support == too), so I'm not too fond of it at the moment, especially it would not make sense to have empty: ["equal", 1200] as the config. Maybe we could simplify it to

{
  "unicorn/explicit-length-check": ["error", {
    "empty": "eq" | "lt",
    "not-empty": "ne" | "gt" | "gte"
  }]
}

and then we have predefined values associated to each of those operators. We could also have longer names for the operators, but you'd then get greater-than-or-equal (which is okay, but pretty long).

I'm kind of proposing this system so that we could have the same system for a different rule checking the indexOf uses. It might be nice to think of a similar "API" for it.

Oh, and obviously, this should not report any check for specific indices: a.length === 2 and a.length == 2 should always be fine, whatever the settings may be.

@sindresorhus
Copy link
Owner

This sounds very complicated. I wonder if we could just enforce a.length > 0 and wait adding options until someone complains?

@jfmengels
Copy link
Contributor Author

Yes, it's way too complicated for what it is. I'm fine with just enforcing a.length > 0, and I'll cross my fingers hoping for nobody to complain.

@sindresorhus sindresorhus changed the title Add an option for how to check the length object Enforce length check style in the explicit-length-check rule Oct 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants