[Fix #7569] Make `Style/YodaCondition` accept `FILE == $0` #7583

koic · 2019-12-22T08:38:50Z

This PR makes Style/YodaCondition cop accept __FILE__ == $0

I think that both operands __FILE__ == $0 can be assumed to be read-only, as mentioned in #7569. If users try to assign __FILE__, an error will occur. Also, it is not usually assigned to $0 by users.

Therefore, this PR will make Style/YodaCondition cop accept both __FILE__ and $0 on the left side.

Also, this PR will change only an idiom __FILE__ == $0.

Before submitting the PR make sure the following are checked:

Wrote good commit messages.
Commit message starts with [Fix #issue-number] (if the related issue exists).
Feature branch is up-to-date with master (if not - rebase it).
Squashed related commits together.
Added tests.
Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

buehmann · 2019-12-22T08:56:48Z

lib/rubocop/cop/style/yoda_condition.rb

+        def_node_matcher :file_constant_equal_doller_zero?, <<~PATTERN
+          (send #source_file_path_constant? :== (gvar #doller_zero?))


The currency is called dollar with a.

I changed to program_name? based on feedback #7583 (comment).

buehmann · 2019-12-22T08:58:54Z

lib/rubocop/cop/style/yoda_condition.rb

@@ -67,9 +67,14 @@ class YodaCondition < Cop

        NONCOMMUTATIVE_OPERATORS = %i[===].freeze

+        def_node_matcher :file_constant_equal_doller_zero?, <<~PATTERN
+          (send #source_file_path_constant? :== (gvar #doller_zero?))


I assume you used (gvar #dollar_zero?) because (gvar :$0) is currently not supported syntax in node patterns. I wonder if we should extend node pattern to support symbols like :$0. But this could be done later in a separate PR.

Yes. This is because (gvar: $0) could not be used. Perhaps an extension of node patterns needs to be considered.

buehmann · 2019-12-22T08:59:47Z

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Bug fixes

 * [#7193](https://github.com/rubocop-hq/rubocop/issues/7193): Prevent `Style/PercentLiteralDelimiters` from changing `%i` literals that contain escaped delimiters. ([@buehmann][])
+* [#7569](https://github.com/rubocop-hq/rubocop/issues/7569): Make `Style/YodaCondition` to accepts `__FILE__ == $0`. ([@koic][])


English grammar: Make sth accept sth

buehmann · 2019-12-22T09:02:31Z

lib/rubocop/cop/style/yoda_condition.rb

@@ -67,9 +67,14 @@ class YodaCondition < Cop

        NONCOMMUTATIVE_OPERATORS = %i[===].freeze

+        def_node_matcher :file_constant_equal_doller_zero?, <<~PATTERN
+          (send #source_file_path_constant? :== (gvar #doller_zero?))


Shouldn't we cover some of the other comparison operators as well? < and friends probably don't make sense, but I could imagine seeing __FILE__ != $0 in the wild.

And what about the English name of $0, that is $PROGRAM_NAME?

Sure! I change to accept != and $PROGRAM_NAME.

buehmann

Looks good, thanks! (PS: file_constant_equal_program_name? should probably be private)

koic · 2019-12-22T09:37:19Z

Thanks for the review!
def_node_matcher :file_constant_equal_program_name? is actually a private method. However, it is declared at the upper part of the file because it is a declarative code by DSL.

buehmann · 2019-12-22T09:42:11Z

def_node_matcher :file_constant_equal_program_name? is actually a private method. However, it is declared at the top of the file because it is a declarative code by DSL.

That does not seem to be correct:

[4] pry(RuboCop)> Cop::Style::YodaCondition.public_instance_methods.grep /program_name/
=> [:file_constant_equal_program_name?]

I think you would need to move the definition to after private.

koic · 2019-12-22T09:43:43Z

Oops!

a private method

This is a typo that should be written as public method.

bbatsov · 2019-12-23T07:11:38Z

Is __FILE__ the only such variable? I thought there were more.

koic · 2019-12-23T07:20:52Z

Yeah, this PR only covers __FILE__ where a pragmatic use case is shown, but there may be others.

Fixes rubocop#7569. This PR makes `Style/YodaCondition` cop accept `__FILE__ == $0` I think that both operands `__FILE__ == $0` can be assumed to be read-only, as mentioned in rubocop#7569. If users try to assign `__FILE__`, an error will occur. Also, it is not usually assigned to `$0` by users. Therefore, this PR will make `Style/YodaCondition` cop accept both `__FILE__` and `$0` on the left side. Also, this PR will change only an idiom `__FILE__ == $0`.

bbatsov · 2019-12-27T09:35:35Z

👍

see rubocop/rubocop#7583

buehmann suggested changes Dec 22, 2019

View reviewed changes

koic changed the title ~~[Fix #7569] Make Style/YodaCondition to accepts __FILE__ == $0~~ [Fix #7569] Make Style/YodaCondition accept __FILE__ == $0 Dec 22, 2019

koic force-pushed the make_style_yoda_condition_aware_of_file_eq_doller_zero branch 2 times, most recently from 47a55ba to 205dabc Compare December 22, 2019 09:25

buehmann approved these changes Dec 22, 2019

View reviewed changes

koic force-pushed the make_style_yoda_condition_aware_of_file_eq_doller_zero branch from 205dabc to 848f9bc Compare December 22, 2019 09:29

koic force-pushed the make_style_yoda_condition_aware_of_file_eq_doller_zero branch from 848f9bc to 098c722 Compare December 25, 2019 14:08

bbatsov merged commit a7dcd1c into rubocop:master Dec 27, 2019

koic deleted the make_style_yoda_condition_aware_of_file_eq_doller_zero branch December 27, 2019 11:09

matkoniecz added a commit to matkoniecz/matkoniecz-ruby-style that referenced this pull request Dec 27, 2019

note that Yoda detector can be restored in the future

ddd53ce

see rubocop/rubocop#7583

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fix #7569] Make `Style/YodaCondition` accept `FILE == $0` #7583

[Fix #7569] Make `Style/YodaCondition` accept `FILE == $0` #7583

koic commented Dec 22, 2019 •

edited

buehmann Dec 22, 2019

koic Dec 22, 2019

buehmann Dec 22, 2019

koic Dec 22, 2019

buehmann Dec 22, 2019 •

edited

buehmann Dec 22, 2019

koic Dec 22, 2019

buehmann left a comment

koic commented Dec 22, 2019 •

edited

buehmann commented Dec 22, 2019

koic commented Dec 22, 2019

bbatsov commented Dec 23, 2019

koic commented Dec 23, 2019

bbatsov commented Dec 27, 2019

		def_node_matcher :file_constant_equal_doller_zero?, <<~PATTERN
		(send #source_file_path_constant? :== (gvar #doller_zero?))

[Fix #7569] Make Style/YodaCondition accept __FILE__ == $0 #7583

[Fix #7569] Make Style/YodaCondition accept __FILE__ == $0 #7583

Conversation

koic commented Dec 22, 2019 • edited

buehmann Dec 22, 2019

Choose a reason for hiding this comment

koic Dec 22, 2019

Choose a reason for hiding this comment

buehmann Dec 22, 2019

Choose a reason for hiding this comment

koic Dec 22, 2019

Choose a reason for hiding this comment

buehmann Dec 22, 2019 • edited

Choose a reason for hiding this comment

buehmann Dec 22, 2019

Choose a reason for hiding this comment

koic Dec 22, 2019

Choose a reason for hiding this comment

buehmann left a comment

Choose a reason for hiding this comment

koic commented Dec 22, 2019 • edited

buehmann commented Dec 22, 2019

koic commented Dec 22, 2019

bbatsov commented Dec 23, 2019

koic commented Dec 23, 2019

bbatsov commented Dec 27, 2019

[Fix #7569] Make `Style/YodaCondition` accept `FILE == $0` #7583

[Fix #7569] Make `Style/YodaCondition` accept `FILE == $0` #7583

koic commented Dec 22, 2019 •

edited

buehmann Dec 22, 2019 •

edited

koic commented Dec 22, 2019 •

edited