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

- fix a false positive for endless method definition #796

Conversation

koic
Copy link
Collaborator

@koic koic commented Apr 20, 2021

Fixes: #795.

This PR fixes false positives that comparison methods recognizes as a setter method.

@koic koic force-pushed the fix_false_positive_for_endless_method_definition branch from 7fa7dd9 to 9dd8521 Compare April 20, 2021 02:07
@@ -3071,7 +3071,7 @@ require 'parser'
end

def endless_method_name(name_t)
if name_t[0].end_with?('=')
if !%w[== === >= <= !=].include?(name_t[0]) && name_t[0].end_with?('=')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[]= should also be in this list (and who knows what else):

> ObjectSpace.each_object(Class).flat_map(&:instance_methods).uniq.select { |mid| mid.to_s.end_with?('=') && !(mid.to_s[-2] =~ /\w/) }
 => [:===, :==, :!=, :<=, :>=, :[]=]

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait, or is it a setter? 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[]= will also result in the error on MRI :-)

% ruby -ve "def []=(index, value) = 42"
ruby 3.0.1p64 (2021-04-05 revision 0fb782ee38) [x86_64-darwin19]
-e:1: setter method cannot be defined in an endless method definition
def []=(index, value) = 42
% ruby -ve "def []=(index, value) = 42"
ruby 3.1.0dev (2021-04-06T07:03:20Z master 31ba817887) [x86_64-darwin19]
-e:1: setter method cannot be defined in an endless method definition
def []=(index, value) = 42

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you're right. It's a setter, so it's an error and it's expected. I updated this PR again.

@@ -10107,6 +10107,73 @@ def test_endless_setter
SINCE_3_0)
end

def test_endless_comparison_method
assert_parses(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you rewrite it into a loop that iterates over method names please? Also, source maps are redundant here.

@koic koic force-pushed the fix_false_positive_for_endless_method_definition branch from 9dd8521 to fc635a8 Compare April 20, 2021 02:34
@koic
Copy link
Collaborator Author

koic commented Apr 20, 2021

Thank you for your review. I've updated this PR.

Fixes: whitequark#795.

This PR fixes false positives that comparison methods recognizes as
a setter method.
@koic koic force-pushed the fix_false_positive_for_endless_method_definition branch from fc635a8 to 63fd724 Compare April 20, 2021 02:47
@koic koic changed the title - fix a false positive for endless method definition Fix a false positive for endless method definition Apr 20, 2021
@iliabylich iliabylich changed the title Fix a false positive for endless method definition - fix a false positive for endless method definition Apr 20, 2021
@iliabylich iliabylich merged commit 49ed4dd into whitequark:master Apr 20, 2021
@iliabylich
Copy link
Collaborator

@koic Thanks!

@koic koic deleted the fix_false_positive_for_endless_method_definition branch April 20, 2021 10:06
@koic
Copy link
Collaborator Author

koic commented Apr 20, 2021

Thanks too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix Parser::SyntaxError error with endless method for object equality
2 participants