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 comparing typeof expressions against string literals #629

Closed
feross opened this issue Sep 16, 2016 · 8 comments
Closed

Enforce comparing typeof expressions against string literals #629

feross opened this issue Sep 16, 2016 · 8 comments

Comments

@feross
Copy link
Member

feross commented Sep 16, 2016

I propose adding { "requireStringLiterals": true } to the valid-typeof rule that we already enforce.

This requires typeof expressions to only be compared to string literals, and disallows comparisons to any other value.

Examples of incorrect code:

typeof foo === undefined
typeof bar == Object
typeof baz === 'strnig'
typeof qux === 'some invalid type'
typeof baz === anotherVariable
typeof foo == 5
typeof bar === typeof qux

Examples of correct code:

typeof foo === 'undefined'
typeof bar == 'object'
typeof baz === 'string'

http://eslint.org/docs/rules/valid-typeof

@feross
Copy link
Member Author

feross commented Sep 16, 2016

Essentially no ecosystem impact:

# tests 422
# pass  421
# fail  1

@feross feross added this to the standard v9 milestone Sep 16, 2016
@Flet
Copy link
Member

Flet commented Sep 18, 2016

:shipit:

@dcousens
Copy link
Member

Make it so

@dcousens
Copy link
Member

Why not standard <9 too?

@feross
Copy link
Member Author

feross commented Sep 18, 2016

@dcousens I fear users will get fatigue from too many releases of standard. Rule changes in a non-major version are especially onerous.

Also, frankly, it's a lot of work to do a release. It takes at least an hour to properly test everything, write the changelog, update the various packages, etc. I'd prefer to hold back most of these rule changes until standard v9.

@dcousens
Copy link
Member

dcousens commented Sep 19, 2016

@feross I feel like we don't thank you enough. Thanks so much for the work, totally understandable. Looking forward to v9.0 😄 👍

@feross
Copy link
Member Author

feross commented Sep 19, 2016

@dcousens Thanks. I appreciate your contributions, too! It's really nice to not be in this all alone. 😄

feross added a commit to standard/eslint-config-standard that referenced this issue Feb 9, 2017
…-typeof)

Adds { "requireStringLiterals": true } to the 'valid-typeof' rule that
we already enforce.

Fixes: standard/standard#629
@feross
Copy link
Member Author

feross commented Feb 9, 2017

This will be part of standard v9. Only one repo was impacted and it was doing this:

const OBJ = 'object'

module.exports = function merge (a, b, checked) {
  if (checked || typeof a === OBJ) {

Can't see a reason that's preferable to typeof a === 'object', so not too worried about this case.

@feross feross closed this as completed Feb 9, 2017
feross added a commit to standard/eslint-config-standard that referenced this issue Feb 9, 2017
…-typeof)

Adds { "requireStringLiterals": true } to the 'valid-typeof' rule that
we already enforce.

Fixes: standard/standard#629
@lock lock bot locked as resolved and limited conversation to collaborators May 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Archived in project
Development

No branches or pull requests

3 participants