Add a cop for Yoda conditions

[jonas054]

I mentioned this in a review comment for #4014.

A new cop should be added to the Style department. It could be called YodaCondition and report contructs like

if 42 == answer
  # ...
end

x = 32 < value ? something : other

and auto-correct them to

if answer == 42
  # ...
end

x = value > 32 ? something : other

See https://en.wikipedia.org/wiki/Yoda_conditions

[smakagon]

I'm going to work on this one.

[pocke]

Is 32 < value yoda conditions?
42 == answer(literal == variable) is unnatural, but I like 32 < value(small < large) style. I think it is natural that lhs is small, rhs is large.

[Drenmi]

Is 32 < value yoda conditions?

Might be personal preference depending on the grammar of ones first language, but in English, this is like asking, for example, "is 30 smaller than your age?" instead of "are you over 30?" 😊

[betesh]

This seems to be very much a matter of personal preference. There should definitely be a SupportedStyle for requiring Yoda conditions.

From https://en.wikipedia.org/wiki/Yoda_conditions:

Some programming languages as Python and Swift do not allow variable assignments
within conditionals, by defining assignments to not return a value, in which
case this error is impossible to make.[6] Many compilers produce a warning
for code such as if (myNumber = 42) (e.g., the GCC -Wall option warns suggest
parentheses around assignment used as truth value), which alerts the programmer
to the likely mistake. In JavaScript, linters such as ESLint can warn on 
assignment inside a conditional.[7]

So there are better ways to protect your code from unintentional assignments in C++, Python, Swift and JS. But not in Ruby (if there is another way, please let me know). So I prefer if 42 == age to if age == 42.

[jonas054]

if there is another way, please let me know

RuboCop's Lint/AssignmentInCondition and running ruby -w are two ways to guard against unintended assignments. I think that's enough, and there's no need to resort to Yoda conditions. Having said that, I'm OK with a style option to require them.

[betesh]

RuboCop's Lint/AssignmentInCondition

Fair enough

running ruby -w

Unpractical for a few reasons:

• #! usr/bin/env ruby -w doesn't work (https://www.ruby-forum.com/topic/96121). I can turn this on by adding export RUBYOPT="-w $RUBYOPT" to my ~/.bashrc, but I can't turn it on for all developers on a given project without replacing that shebang with some pretty ugly stuff in bin files (which may be installed by bundler)

• produces a lot of noise from gems that I don't maintain:

  rack-test-0.6.3/lib/rack/test.rb:284: warning: instance variable @digest_username not initialized
  jwt-1.5.6/lib/jwt/decode.rb:26: warning: instance variable @signature not initialized
  aws-sdk-core-2.8.7/lib/aws-sdk-core/shared_config.rb:215: warning: assigned but unused variable - default
  headless-2.3.1/lib/headless.rb:246: warning: instance variable @at_exit_hook_installed not initialized
  actionpack-5.0.2/lib/abstract_controller/helpers.rb:67: warning: method redefined; discarding old current_user
  devise-4.2.1/lib/devise/controllers/helpers.rb:127: warning: previous definition of user_session was here
  

• Outputs a warning each time the offending code is executed, instead of once in a clean summary at the end of the output

• No cross-platform way to make this trigger a CI failure (See http://stackoverflow.com/a/662436/1633753

[jonas054]

I stand corrected regarding ruby -w. I should have said ruby -wc, which avoids most of these problems although not the last one.

[smakagon]

Guys I've been struggling with this cop for a while. I have working code and green specs, but I feel like I should add more specs and make sure that i didn't miss any edge cases. I'll appreciate any feedback.