Bug #21994: If there is a local variable `foo`, calls to a method `foo` with a regexp literal as first argument is always a SyntaxError without parentheses

• Status: Open
• Assignee: -
• Target version: -
• ruby -v: 4.0.1
• Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN

[jneen (Jeanine Adkisson)]

Quick version:

ruby -e 'p /hello/' # => /hello/ (warning)
ruby -e 'p = 1; p /hello/' # syntax error
ruby -e 'p %r/hello/' # => /hello/ (no warning)
ruby -e 'p = 1; p %r/hello/' # syntax error

Context

In Rouge, our lexing DSL originally looked like this:

rule /some_regex/, Some::TokenType, :next_state
rule /some_other_regex/ do |match|
  # custom action
end

In a DSL context like this, I feel that using parentheses for every rule definition is a hassle and looks bad, so we leave them off, as is convention.

Unfortunately, this warns:

warning: ambiguous `/`; wrap regexp in parentheses or add a space after `/` operator

Some time ago, to reduce warning-spam on users who run with -w, we transitioned all of these to use %r/.../ or %r(...) syntax, which does not warn.

The issue here I think is that both cases are equivalently ambiguous in some sense, which becomes apparent with the local-scope-dependent behaviour of the parser (putting aside that dependency on local variable scope makes the Ruby syntax highlighting of Rouge more or less impossible to do correctly without a full parse). If there is a local variable named rule introduced at any point, even several block scopes above, the entire file errors out in a very confusing manner.

At the very least, I feel that the warnings here are inconsistent. If both cases truly are ambiguous and dependent on local variable scope, perhaps it would be better to warn on both - though that would make using a regex literal as a first argument always require parentheses to avoid warnings, which would make my DSL much harder. I'm honestly not entirely sure what a good solution would be, but I would like to open a discussion on the topic. Perhaps philosophically related to the other bug I have open ( #21870 ) - is this truly worthy of a warning, or should this be a linter concern?

[Earlopain (Earlopain _)]

• Related to Feature #20922: Should not we omit parentheses in assert calls? added

[jneen (Jeanine Adkisson)]

I should say that I think the worst solution to this would be to warn in both cases. It would effectively require us to rewrite our DSL across every rule for every language we support. My ideal world is one where we could go back to using rule /regex/ do ... with no additional specification, or perhaps to warn in the division/modulo case instead. I would very much like to not have to turn off warnings while loading.

EDIT: a new warning would also affect more common patterns like Set.new %w(...).

[byroot (Jean Boussier)]

I know very little about parser, so perhaps what I'm about to suggest is entirely impossible, but could we refine that warning as to only trigger it if the other interpretation would be valid syntax?

e.g.:

• p -1

  • Both p(-1) and p - 1 are valid so the warning makes sense.

• p -1, 2, (p - 1), 2 isn't valid ruby, only p(-1, 2) is so the warning could not be triggered.

I suspect there are considerations I don't know about though.

[tompng (tomoya ishida)]

I think checking code validness is hard for the code below.
Warning check is triggered at every line, each check needs to parse till the bottom.

a -1 => b # only a(-1 => b) is valid, (a - 1) => b is invalid at the `2 => 3` position
b -1 => c # only b(-1 => c) is valid
c -1 => d # only c(-1 => d) is valid
d -1 => e, 2 => 3 # invalid if local variable d is defined

[jneen (Jeanine Adkisson)]

I think checking code validness is hard

I agree, since this check happens during lexical analysis, rather than parsing. It is a bit mind-boggling to me that lexical analysis depends on local variable scope, but it's what it is.

I tend to feel that the infix operator case is nicer and easier for the end-user to fix than the method call case. a -1 is already not good style for subtraction - I would ask a contributor to format this either a-1 or a - 1 if that was in a pull request on a project of mine. However, rule /regex/ do ... I would consider to be good style, if it didn't warn. And I do feel it is fair to ask for some way to call a method with a regex as first argument without parentheses, even if it's %r/.../ syntax.

[matz (Yukihiro Matsumoto)]

I agree the current behavior is inconsistent, and I am now inclined to simply remove the ambiguous / warning.

The warning was intended to be helpful, but as this issue shows, it does not catch the genuinely confusing case (the syntax error when a local variable shadows the method name), and it discourages a reasonable DSL style such as rule /regex/ do ... end. As tompng pointed out, deciding "whether the other interpretation would be valid" during lexical analysis is not practical.

So my suggestion is to drop the ambiguous / warning and leave this kind of stylistic check to linters like RuboCop. The syntax error case is a separate problem that this warning never addressed anyway, so removing the warning does not make that situation worse.

I would like to confirm this at the next dev meeting before changing it.

Matz.