Project

General

Profile

Actions

Bug #17124

closed

Wrong "ambiguous first argument" warning

Added by marcandre (Marc-Andre Lafortune) over 3 years ago. Updated over 3 years ago.

Status:
Closed
Target version:
-
[ruby-core:99633]

Description

$ ruby -v -e "x='a'; x.match? /[a-z]/"
ruby 2.8.0dev (2020-07-30T14:07:06Z master 352895b751) [x86_64-darwin18]
-e:1: warning: ambiguous first argument; put parentheses or a space even after `/' operator

There is no / operator in there and there is also no ambiguity as adding a space after the first / is a syntax error.

Is it possible to remove the warning altogether when the argument is lexed as a regexp?

The message could use a rewording too, maybe "ambiguous first argument; put parentheses around argument or add a space after `/' operator"

Actions #1

Updated by sawa (Tsuyoshi Sawada) over 3 years ago

  • Description updated (diff)

Updated by nobu (Nobuyoshi Nakada) over 3 years ago

In your example, /[a-z] looks very likely a regexp because of [, but how about /a?
Or match? /(a ...lines... )/x?
There are many possibilities.

Updated by marcandre (Marc-Andre Lafortune) over 3 years ago

I hadn't thought of method /a /x or method /a /<newline>other_method, indeed.

This warning is only there when there is ambiguity between two divisions and a regexp literal, right?

If so, could we reword the warning to "ambiguity between regexp and two divisions: wrap regexp in parentheses or add a space after `/' operator"?

Updated by jeremyevans0 (Jeremy Evans) over 3 years ago

  • Status changed from Open to Feedback

marcandre (Marc-Andre Lafortune) wrote in #note-3:

I hadn't thought of method /a /x or method /a /<newline>other_method, indeed.

This warning is only there when there is ambiguity between two divisions and a regexp literal, right?

No, the warning is used for + and - in addition to /.

If so, could we reword the warning to "ambiguity between regexp and two divisions: wrap regexp in parentheses or add a space after `/' operator"?

This requires treating the / operator differently than + and - in terms of the warning. Do you think the benefit is worth the cost?

--- a/parse.y
+++ b/parse.y
@@ -7788,7 +7788,12 @@ static int
 arg_ambiguous(struct parser_params *p, char c)
 {
 #ifndef RIPPER
-    rb_warning1("ambiguous first argument; put parentheses or a space even after `%c' operator", WARN_I(c));
+    if (c == '/') {
+        rb_warning1("ambiguity between regexp and two divisions: wrap regexp in parentheses or add a space after `%c' operator", WARN_I(c));
+    }
+    else {
+        rb_warning1("ambiguous first argument; put parentheses or a space even after `%c' operator", WARN_I(c));
+    }
 #else
     dispatch1(arg_ambiguous, rb_usascii_str_new(&c, 1));
 #endif

Updated by marcandre (Marc-Andre Lafortune) over 3 years ago

I think your wording is a big improvement 👍

I think you've paid the cost already with your diff, we should reap the benefit by committing it!

Actions #6

Updated by jeremyevans (Jeremy Evans) over 3 years ago

  • Status changed from Feedback to Closed

Applied in changeset git|f5bb9115a7f69c32089b9b970933c3507fb9f82b.


Use more specific warning for ambiguous slash

Fixes [Bug #17124]

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0