Bug #18396: An unexpected "hash value omission" syntax error when without parentheses call expr follows

• Status: Open
• Assignee: -
• Target version: -
• ruby -v: ruby 3.1.0dev (2021-12-07T23:18:11Z master 4a3e7984bf) [x86_64-darwin19]
• Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN

[koic (Koichi ITO)]

Summary

I encountered an unexpected "hash value omission" syntax error when parentheses call expression follows:

% cat example.rb
foo key:
foo arg

% ruby -cv /tmp/b.rb
ruby 3.1.0dev (2021-12-07T23:18:11Z master 4a3e7984bf) [x86_64-darwin19]
example.rb:2: syntax error, unexpected local variable or method, expecting `do' or '{' or '('
foo arg

Additional Information

The following is a traditional usage.

# No errors.
foo key: key
foo arg

A syntax error is unexpectedly raised when hash value omission argument without parentheses is followed by a method call without parentheses.

# No errors is expected, but syntax error is raised.
foo key:
foo arg

No error occurs if any of the calls have parentheses.

# No errors.
foo(key:)
foo arg

# Also no errors.
foo key:
foo(arg)

No error occurs when calling alone.

# No errors.
foo key:

I encountered this error while trying to apply hash value omission to RSpec code of a real-world application (proprietary) .
But this is a new Ruby 3.1 syntax and may not be supported yet. Thank you.

[koic (Koichi ITO)]

• Subject changed from An unexpected "hash value omission" syntax error when parentheses call expr follows to An unexpected "hash value omission" syntax error when without parentheses call expr follows

[mame (Yusuke Endoh)]

• Status changed from Open to Rejected

It is by design.

foo key:
bar

was parsed as foo(key: bar) in Ruby 3.0 or before. It is incompatible to change it to foo(key: key); bar.

Except some traditional exceptions like puts "foo" and require "foo", it would be good to write parentheses when a method call has any argument.

[koic (Koichi ITO)]

I get it. Thank you for the explanation!

[matz (Yukihiro Matsumoto)]

• Status changed from Rejected to Open

Although it's incompatible, I think it's worth improving. I estimate that the compatibility issue is minimal.
After 3.1 release, we experiment to measure how big the issue is.

Matz.

[zverok (Victor Shepelev)]

The current state of things is indeed quite confusing (Wrote a small blog post on it: https://zverok.github.io/blog/2021-12-08-value-omission-debug.html).

As far as I can understand, the kind of old code that would be affected should look like this:

method_call foo:
  bar

While using parenthesis in non-DSL method calls is the most common style currently, it is not the only one.
For example, Seattle.rb's style (minitest) have optional parenthesis

Also, I suspect there could be a frequently/sometimes used style at least when using Rails DSL:

validate :something, if: -> {condition }

# ...when `condition` grows longer, I suspect _some_ codebases might do this:
validate :something, if: 
                     -> {condition }

Another parenthesis-less DSLs are assertions (of the same minitest and other "testunit"-alike frameworks).

The worst thing here, I can't think of a way of catching this incompatibility automatically, just after updating on 3.2 some code might break, and in a quite subtle way (for example, DSL methods receiving something accidentally available in current scope), or in a baffling one like undefined local variable or method 'if' — ugh what.

As an alternative, I was thinking maybe prohibit methods with keyword argument omission and parenthesis omission? E.g. make this:

p x: 1, y: 1  # valid
p x:, y: 1    # valid
p x: 1, y:    # invalid, ambiguity!
p(x: 1, y:)   # valid

[mame (Yusuke Endoh)]

I think this patch will change the meaning of p x: as matz said.

diff --git a/parse.y b/parse.y
index 0ff3ddbb4e..d92decfc1c 100644
--- a/parse.y
+++ b/parse.y
@@ -9290,7 +9290,7 @@ parser_yylex(struct parser_params *p)
        p->token_seen = token_seen;
        c = (IS_lex_state(EXPR_BEG|EXPR_CLASS|EXPR_FNAME|EXPR_DOT) &&
             !IS_lex_state(EXPR_LABELED));
-       if (c || IS_lex_state_all(EXPR_ARG|EXPR_LABELED)) {
+       if (c || (IS_lex_state_all(EXPR_ARG|EXPR_LABELED) && p->lex.paren_nest)) {
             if (!fallthru) {
                 dispatch_scan_event(p, tIGNORED_NL);
             }

Example:

x = 1
p x:
2

#=> {:x=>2}  # before the patch
#=> {:x=>1}  # after the patch

If the expression is within parentheses, the behavior is be changed.

x = 1
p(x:
2
)

#=> {:x=>2}  # not changed

Before introducing this change, we definitely need to prepare migration path. The following patch will keep the current behavior and print a warning against code that will be changed. This can be used to estimate the impact of the compatibility.

diff --git a/parse.y b/parse.y
index 0ff3ddbb4e..73deae8627 100644
--- a/parse.y
+++ b/parse.y
@@ -9291,6 +9291,9 @@ parser_yylex(struct parser_params *p)
        c = (IS_lex_state(EXPR_BEG|EXPR_CLASS|EXPR_FNAME|EXPR_DOT) &&
             !IS_lex_state(EXPR_LABELED));
        if (c || IS_lex_state_all(EXPR_ARG|EXPR_LABELED)) {
+            if (IS_lex_state_all(EXPR_ARG|EXPR_LABELED) && !p->lex.paren_nest) {
+               rb_warn0("keyword label followed by newline without parentheses will be changed in Ruby 3.3");
+            }
             if (!fallthru) {
                 dispatch_scan_event(p, tIGNORED_NL);
             }

[Dan0042 (Daniel DeLorme)]

matz (Yukihiro Matsumoto) wrote in #note-4:

Although it's incompatible, I think it's worth improving. I estimate that the compatibility issue is minimal.
After 3.1 release, we experiment to measure how big the issue is.

Please, no.
To my eye, foo key: looks like it continues on the next line. Changing that is both incompatible and, more importantly to me, unreadable. hash value omission is a great feature but in order to be readable imho it really requires open and close tokens. So {a:, b:} and p(a:, b:) are both readable but p a:, b: is not. -- 2¢

[mame (Yusuke Endoh)]

I have just noticed that Ruby 3.1's right-assignment pattern can end with x:.

# Works as expected on Ruby 3.1

hsh = { x: 42 }
hsh => x:
p x  #=> 42

[tycooon (Yuri Smirnov)]

Personally I would suggest only allowing shorthand syntax in case when brackets are present. So that

x = 15
p x: # Syntax error
p(x:) # Works

OR maybe change current behavior and require trailing backslash in order to make it use the expression on the next line:

a = 1, b = 2

p a:, b: # Always works like p(a:, b:)

p a:, b: \
15
# => { a: 1, b: 15 }

[jeremyevans0 (Jeremy Evans)]

mame (Yusuke Endoh) wrote in #note-6:

diff --git a/parse.y b/parse.y
index 0ff3ddbb4e..73deae8627 100644
--- a/parse.y
+++ b/parse.y
@@ -9291,6 +9291,9 @@ parser_yylex(struct parser_params *p)
        c = (IS_lex_state(EXPR_BEG|EXPR_CLASS|EXPR_FNAME|EXPR_DOT) &&
             !IS_lex_state(EXPR_LABELED));
        if (c || IS_lex_state_all(EXPR_ARG|EXPR_LABELED)) {
+            if (IS_lex_state_all(EXPR_ARG|EXPR_LABELED) && !p->lex.paren_nest) {
+               rb_warn0("keyword label followed by newline without parentheses will be changed in Ruby 3.3");
+            }
             if (!fallthru) {
                 dispatch_scan_event(p, tIGNORED_NL);
             }

I did some testing with this and found it that warns for method definitions with required keywords, such as:

def obj.foo a:
  a
end

Since the behavior for that wouldn't change, I don't think this warning will work. We'll have to figure out a way to issue the warning only in the method call case and not in the method definition case. I guess that means we'll have to move the warning from the lexer to the parser.