Bug #19392: Endless method vs and/or

• Status: Closed
• Priority: Normal
• Assignee: -
• Target version: -
• Backport: 2.7: DONTNEED, 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN

[zverok (Victor Shepelev)]

Discovered by Lucian Ghinda:

def test = puts("foo") and puts("bar")
# prints "bar" immediately
test
# prints "foo"

It seems that it is a parser error, right?..

RubyVM::AbstractSyntaxTree.parse('def test = puts("foo") and puts("bar")')
#  => 
# (SCOPE@1:0-1:38                                                         
#  tbl: []                                                                
#  args: nil                                                              
#  body:                                                                  
#    (AND@1:0-1:38                                                        
#       (DEFN@1:0-1:22                                                    
#        mid: :test                                                       
#        body:                                                            
#          (SCOPE@1:0-1:22                                                
#           tbl: []                                                       
#           args:                                                         
#             (ARGS@1:0-1:8 pre_num: 0 pre_init: nil opt: nil first_post: nil post_num: 0 post_init: nil rest: nil kw: nil kwrest: nil block: nil)
#           body: (FCALL@1:11-1:22 :puts (LIST@1:16-1:21 (STR@1:16-1:21 "foo") nil))))
#       (FCALL@1:27-1:38 :puts (LIST@1:32-1:37 (STR@1:32-1:37 "bar") nil)))) 

E.g. it is parsed as

(def test = puts("foo")) and (puts("bar"))

...which is hardly intentional or have any practical use. The rightly parsed code in this case can have practical use, like

def write(data) = File.write(@filename, data) == data.size or raise "Something went wrong"

[austin (Austin Ziegler)]

Note that this does not happen with && / ||, so it’s a binding precedence issue.

[ufuk (Ufuk Kayserilioglu)]

Given that the precendence of and and or is already low, I think this is expected behaviour and fully replicates the operation of local variable assignment as well. That is:

foo = some_method and puts("bar")

would also be parsed as:

(foo = some_method) and puts("bar")

I would expect that do be the same when "assigning" a method.

The "fix" for the use-case you are describing is to be explicit with grouping:

def write(data) = (File.write(@filename, data) == data.size or raise "Something went wrong")

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Rejected

This precedence is needed to allow:

public def test = puts("foo")

[zverok (Victor Shepelev)]

• Status changed from Rejected to Open

@nobu (Nobuyoshi Nakada) Sorry, can we talk about it a bit more?..

I am not an expert in parser's internals, so I can believe that it is 100% impossible to solve, but is it?..

From developer's experience point of view:

• There is a high possibility of using def foo = statement or statement, it is quite regularly used idiom for shortcutting and validations;
• Depending on the second part of such expression, the wrong behavior might be noticed very late;
• There are no useful consequences (that I can think of) for breaking the one-line method body on and/or, so it can't be justified by "it doesn't work this way, because it actually does this interesting thing!"

Therefore it definitely perceives like a bug and a harmful one, not just "useless trivia" kind.

Moreover:

• One-line method definitions aren't any expressions. They introduce a new scope, for example, and having a scope suddenly ending mid-line on operator (and not at least on ;) is behavior that screams "bug", even if it can be explained in "it is internal precedence" terms:

def test = x = 5 and puts x 
# in `<main>': undefined local variable or method `x' for main:Object (NameError)

• Again, I am not a Ruby internal expert (though if there is no other choice, I can take on studying this problem), but I have a hunch that it isn't impossible to introduce a simple rule "endless method's definition ends on ; or \n, and nothing else". That's the user expectation, anyway.

[nobu (Nobuyoshi Nakada)]

zverok (Victor Shepelev) wrote in #note-4:

Moreover:

• One-line method definitions aren't any expressions.

It depends on how you define "expressions".
At least in Ruby's terms, any method definition can be an argument of other method calls.

public def test1(x) = x = 5
public def test2(x)
  x = 5
end

In the above examples, both methods return the method ID and the result will be passed to the method public.
To utilize this, def returns the ID since 2.3.

[zverok (Victor Shepelev)]

@nobu (Nobuyoshi Nakada)

It depends on how you define "expressions".
At least in Ruby's terms, any method definition can be an argument of other method calls.

Thank you, I am well aware of this. (I maintain Ruby Changelog, including detailed explanations on how and why methods as expressions can be used.)

To utilize this, def returns the ID since 2.3.

Since 2.1, actually :)

That's why I didn't say, "method definitions aren't expressions", I said, "method definitions aren't any expressions", meaning that while being expressions indeed, they still behave differently in several important aspects from most the other expressions.

One of those aspects is the creation of its own scope.
Endless methods do it without explicit ending sign (like, } or end). This makes "where the expression ends" an extremely sensitive question, which, I believe, shouldn't be brushed off easily.

[zverok (Victor Shepelev)]

and is actually not alone.

The code like this is also parsed not the way one would intuitively expect:

def save = File.write(name, self.to_yaml) unless invalid?

Actual parsing is this:

(def save = File.write(name, self.to_yaml)) unless invalid?

I believe that combining postfix if/unless with one-line methods is extremely possible, while intention to conditionally define methods with postfix condition is much less so.

In general, I believe that

def method
  expression
end

and

def method = expression

should be equivalent as long as it is humanly possible, because the simplification of one-expression method to endless one is what many codebases would try to do, especially for simple utility classes.

And if they can't be made equivalent (due to some parser complexities), the only other acceptable behavior is an error or at least warning (though the latter is much less desirable), not a silent change of semantics.

[mame (Yusuke Endoh)]

• Status changed from Open to Feedback

@nobu (Nobuyoshi Nakada) says it would be difficult to implement. If someone creates a patch successfully, we may be able to consider it.

[matz (Yukihiro Matsumoto)]

• Status changed from Feedback to Closed

Analogous to a = b and c parsed as (a = b) and c, def a = b and c should be parsed as (def a = b) and c.
Considering the fact no one really want to write (def a = b) and c, it may be better to cause a warning in the future.
I understand there's some confusion regarding the precedence of and, or operators, but if we change the precedence it may break a huge amount of existing code.

Matz.

[zverok (Victor Shepelev)]

@matz (Yukihiro Matsumoto) With all due respect, I don't believe the current behaviour is acceptable. Considering the behaviour affects not only control flow and/or (relatively rarely used), but also simple if (https://bugs.ruby-lang.org/issues/19392#note-7), I believe the behaviour should be considered a bug.

I don't believe the fix lies in changing priorities of some operators, rather in adjustment of endless methods parsing so everything till the end of the line (or expression, if some way of linebreaking used) would be considered the body of such method, always.

I don't believe it to be impossible, but due to obvious reasons can't experiment with implementation myself currently.

I should say though that this behaviour would always be perceived as a bug, and I believe can have an effect on Ruby's reputation, when one of the "cool new features" behaves in an obviously wrong way.

[mame (Yusuke Endoh)]

• Related to Bug #19731: Can’t call an instance method inside an Endless method definition added

[yui-knk (Kaneko Yuichiro)]

JFYI: https://github.com/ruby/ruby/pull/8054 doesn't change the precedence globally, however def test = puts("foo") or puts("bar") is interrupted as def test = (puts("foo") or puts("bar")).

[Eregon (Benoit Daloze)]

I agree with @zverok (Victor Shepelev), the current behavior is highly confusing and undesirable. No Ruby user wants the current behavior.

I think if the body of an endless method doesn't cover the full line, we should get a warning (enabled by default) or an error, or better: accept the full line as the endless method's body.