Project

General

Profile

Actions

Bug #18396

open

An unexpected "hash value omission" syntax error when without parentheses call expr follows

Added by koic (Koichi ITO) over 2 years ago. Updated almost 2 years ago.

Status:
Open
Assignee:
-
Target version:
-
ruby -v:
ruby 3.1.0dev (2021-12-07T23:18:11Z master 4a3e7984bf) [x86_64-darwin19]
[ruby-core:106543]

Description

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.

Actions #1

Updated by koic (Koichi ITO) over 2 years ago

  • 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

Updated by mame (Yusuke Endoh) over 2 years ago

  • 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.

Updated by koic (Koichi ITO) over 2 years ago

I get it. Thank you for the explanation!

Updated by matz (Yukihiro Matsumoto) over 2 years ago

  • 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.

Updated by zverok (Victor Shepelev) over 2 years ago

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

Updated by mame (Yusuke Endoh) over 2 years ago

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);
             }

Updated by Dan0042 (Daniel DeLorme) over 2 years ago

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¢

Updated by mame (Yusuke Endoh) about 2 years ago

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

Updated by tycooon (Yuri Smirnov) about 2 years ago

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 }

Updated by jeremyevans0 (Jeremy Evans) almost 2 years ago

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.

Actions

Also available in: Atom PDF

Like1
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0