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

Unusual yoda expressions #91

Closed
dcousens opened this issue Mar 27, 2015 · 13 comments

Comments

@dcousens
Copy link
Member

commented Mar 27, 2015

I recently had a case where I was using the following code in a test:

{
  a: true,
  b: false,
  c: 1 > 2
}

Standard blew up on me with Expected literal to be on the right side of >.
Apparently this is due to it attempting to adhere a yoda condition ruling.

Is this intentional?

I understand this is most likely a question for eslint, not standard, but I thought I'd ask first.

@feross

This comment has been minimized.

Copy link
Member

commented Apr 2, 2015

It seems that eslint considers any expression where the left side is a constant to be a "yoda expression". That seems okay behavior to me, though the error is a bit misleading in this case.

@feross feross closed this Apr 2, 2015

@dcousens

This comment has been minimized.

Copy link
Member Author

commented Apr 7, 2015

So @feross we think its acceptable to not allow constant expressions? That's ridiculous no? Its a perfectly valid case of something you might want?

@dcousens dcousens referenced this issue Apr 7, 2015
@feross

This comment has been minimized.

Copy link
Member

commented Apr 9, 2015

@dcousens Constant expressions are still allowed, of course :-) For example: var x = 'my string' and var y = 42.

One side effect of the "no yoda expressions" rule is that comparison expressions that always evaluate to the same thing (like 1 > 2) should just be inlined. Is there a reason why you can't change the test code to:

{
  a: true,
  b: false,
  c: false
}
@feross

This comment has been minimized.

Copy link
Member

commented Apr 9, 2015

Btw, I don't actually think that preventing constant comparisons like 1 > 2 is very important - happy to turn that off if there's a way to do so.

If anything, eslint should make it a separate rule from the yoda expressions one, which is a bit more useful. Have you filed a bug on eslint?

@dcousens

This comment has been minimized.

Copy link
Member Author

commented Apr 9, 2015

@feross I personally don't see any difference between: 2 < 1 and 2 + 1. In my mind they are both constant expressions, and yet the former is being picked up as erroneous.

I was thinking of pushing this bug upstream to eslint, just figured I'd ask here first.

@feross

This comment has been minimized.

Copy link
Member

commented Apr 9, 2015

Ah, you're right. If 1 + 2 is allowed, then 2 < 1 should be too. This is an eslint bug, in my opinion.

@dcousens

This comment has been minimized.

Copy link
Member Author

commented Jun 3, 2015

@feross did we file this in eslint? If not, I'll assign it to myself for now as a reminder.

@dcousens dcousens added the blocked label Jun 3, 2015

@dcousens dcousens reopened this Jun 3, 2015

@dcousens dcousens self-assigned this Jun 3, 2015

@feross

This comment has been minimized.

Copy link
Member

commented Jun 3, 2015

@dcousens I don't remember filing this on eslint.

@feross

This comment has been minimized.

Copy link
Member

commented Jun 11, 2015

@dcousens Did you ever file this bug on eslint?

@dcousens

This comment has been minimized.

Copy link
Member Author

commented Jun 12, 2015

@feross I haven't yet, no.

@feross feross added the bug label Jun 28, 2015

@feross

This comment has been minimized.

Copy link
Member

commented Jun 28, 2015

Bug filed on eslint: eslint/eslint#2867

@feross

This comment has been minimized.

Copy link
Member

commented Jun 29, 2015

Bug fixed on eslint master.

@feross

This comment has been minimized.

Copy link
Member

commented Jul 10, 2015

This is fixed in eslint 0.24.1. Run npm update to get the fix.

@feross feross closed this Jul 10, 2015

@lock lock bot locked as resolved and limited conversation to collaborators May 11, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
2 participants
You can’t perform that action at this time.