Bug #19392
closedEndless method vs and/or
Description
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"
Updated by austin (Austin Ziegler) 8 months ago
Note that this does not happen with &&
/ ||
, so it’s a binding precedence issue.
Updated by ufuk (Ufuk Kayserilioglu) 8 months ago
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")
Updated by nobu (Nobuyoshi Nakada) 8 months ago
- Status changed from Open to Rejected
This precedence is needed to allow:
public def test = puts("foo")
Updated by zverok (Victor Shepelev) 8 months ago
- 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.
Updated by nobu (Nobuyoshi Nakada) 8 months ago
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.
Updated by zverok (Victor Shepelev) 8 months ago
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.
Updated by zverok (Victor Shepelev) 7 months ago
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.
Updated by mame (Yusuke Endoh) 7 months ago
- 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.
Updated by matz (Yukihiro Matsumoto) 6 months ago
- 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.
Updated by zverok (Victor Shepelev) 6 months ago
@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.
Updated by mame (Yusuke Endoh) 4 months ago
- Related to Bug #19731: Can’t call an instance method inside an Endless method definition added
Updated by yui-knk (Kaneko Yuichiro) 3 months ago
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"))
.
Updated by Eregon (Benoit Daloze) 3 months ago
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.