[ruby/prism] Implement ambiguous binary operator warning

ruby/prism@6258c3695f
ruby · May 20, 2024 · e90e8f8 · e90e8f8
1 parent 8248268
commit e90e8f8
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 1 deletion.
diff --git a/prism/config.yml b/prism/config.yml
@@ -274,6 +274,7 @@ errors:
   - WRITE_TARGET_UNEXPECTED
   - XSTRING_TERM
 warnings:
+  - AMBIGUOUS_BINARY_OPERATOR
   - AMBIGUOUS_FIRST_ARGUMENT_MINUS
   - AMBIGUOUS_FIRST_ARGUMENT_PLUS
   - AMBIGUOUS_PREFIX_AMPERSAND

diff --git a/prism/prism.c b/prism/prism.c
@@ -423,10 +423,18 @@ lex_mode_pop(pm_parser_t *parser) {
  * This is the equivalent of IS_lex_state is CRuby.
  */
 static inline bool
-lex_state_p(pm_parser_t *parser, pm_lex_state_t state) {
+lex_state_p(const pm_parser_t *parser, pm_lex_state_t state) {
     return parser->lex_state & state;
 }
 
+/**
+ * This is equivalent to the predicate of warn_balanced in CRuby.
+ */
+static inline bool
+ambiguous_operator_p(const pm_parser_t *parser, bool space_seen) {
+    return !lex_state_p(parser, PM_LEX_STATE_CLASS | PM_LEX_STATE_DOT | PM_LEX_STATE_FNAME | PM_LEX_STATE_ENDFN) && space_seen && !pm_char_is_whitespace(*parser->current.end);
+}
+
 typedef enum {
     PM_IGNORED_NEWLINE_NONE = 0,
     PM_IGNORED_NEWLINE_ALL,
@@ -10821,6 +10829,8 @@ parser_lex(pm_parser_t *parser) {
                             type = PM_TOKEN_USTAR_STAR;
                         } else if (lex_state_beg_p(parser)) {
                             type = PM_TOKEN_USTAR_STAR;
+                        } else if (ambiguous_operator_p(parser, space_seen)) {
+                            PM_PARSER_WARN_TOKEN_FORMAT(parser, parser->current, PM_WARN_AMBIGUOUS_BINARY_OPERATOR, "**", "argument prefix");
                         }
 
                         if (lex_state_operator_p(parser)) {
@@ -10844,6 +10854,8 @@ parser_lex(pm_parser_t *parser) {
                         type = PM_TOKEN_USTAR;
                     } else if (lex_state_beg_p(parser)) {
                         type = PM_TOKEN_USTAR;
+                    } else if (ambiguous_operator_p(parser, space_seen)) {
+                        PM_PARSER_WARN_TOKEN_FORMAT(parser, parser->current, PM_WARN_AMBIGUOUS_BINARY_OPERATOR, "*", "argument prefix");
                     }
 
                     if (lex_state_operator_p(parser)) {
@@ -11017,6 +11029,10 @@ parser_lex(pm_parser_t *parser) {
                             LEX(PM_TOKEN_LESS_LESS_EQUAL);
                         }
 
+                        if (ambiguous_operator_p(parser, space_seen)) {
+                            PM_PARSER_WARN_TOKEN_FORMAT(parser, parser->current, PM_WARN_AMBIGUOUS_BINARY_OPERATOR, "<<", "here document");
+                        }
+
                         if (lex_state_operator_p(parser)) {
                             lex_state_set(parser, PM_LEX_STATE_ARG);
                         } else {
@@ -11130,6 +11146,8 @@ parser_lex(pm_parser_t *parser) {
                         type = PM_TOKEN_UAMPERSAND;
                     } else if (lex_state_beg_p(parser)) {
                         type = PM_TOKEN_UAMPERSAND;
+                    } else if (ambiguous_operator_p(parser, space_seen)) {
+                        PM_PARSER_WARN_TOKEN_FORMAT(parser, parser->current, PM_WARN_AMBIGUOUS_BINARY_OPERATOR, "&", "argument prefix");
                     }
 
                     if (lex_state_operator_p(parser)) {
@@ -11204,6 +11222,10 @@ parser_lex(pm_parser_t *parser) {
                         LEX(PM_TOKEN_UPLUS);
                     }
 
+                    if (ambiguous_operator_p(parser, space_seen)) {
+                        PM_PARSER_WARN_TOKEN_FORMAT(parser, parser->current, PM_WARN_AMBIGUOUS_BINARY_OPERATOR, "+", "unary operator");
+                    }
+
                     lex_state_set(parser, PM_LEX_STATE_BEG);
                     LEX(PM_TOKEN_PLUS);
                 }
@@ -11241,6 +11263,10 @@ parser_lex(pm_parser_t *parser) {
                         LEX(pm_char_is_decimal_digit(peek(parser)) ? PM_TOKEN_UMINUS_NUM : PM_TOKEN_UMINUS);
                     }
 
+                    if (ambiguous_operator_p(parser, space_seen)) {
+                        PM_PARSER_WARN_TOKEN_FORMAT(parser, parser->current, PM_WARN_AMBIGUOUS_BINARY_OPERATOR, "-", "unary operator");
+                    }
+
                     lex_state_set(parser, PM_LEX_STATE_BEG);
                     LEX(PM_TOKEN_MINUS);
                 }
@@ -11339,6 +11365,10 @@ parser_lex(pm_parser_t *parser) {
                         LEX(PM_TOKEN_REGEXP_BEGIN);
                     }
 
+                    if (ambiguous_operator_p(parser, space_seen)) {
+                        PM_PARSER_WARN_TOKEN_FORMAT(parser, parser->current, PM_WARN_AMBIGUOUS_BINARY_OPERATOR, "/", "regexp literal");
+                    }
+
                     if (lex_state_operator_p(parser)) {
                         lex_state_set(parser, PM_LEX_STATE_ARG);
                     } else {
@@ -11520,6 +11550,10 @@ parser_lex(pm_parser_t *parser) {
                         }
                     }
 
+                    if (ambiguous_operator_p(parser, space_seen)) {
+                        PM_PARSER_WARN_TOKEN_FORMAT(parser, parser->current, PM_WARN_AMBIGUOUS_BINARY_OPERATOR, "%", "string literal");
+                    }
+
                     lex_state_set(parser, lex_state_operator_p(parser) ? PM_LEX_STATE_ARG : PM_LEX_STATE_BEG);
                     LEX(PM_TOKEN_PERCENT);
                 }

diff --git a/prism/templates/src/diagnostic.c.erb b/prism/templates/src/diagnostic.c.erb
@@ -356,6 +356,7 @@ static const pm_diagnostic_data_t diagnostic_messages[PM_DIAGNOSTIC_ID_MAX] = {
     [PM_ERR_XSTRING_TERM]                       = { "expected a closing delimiter for the `%x` or backtick string", PM_ERROR_LEVEL_SYNTAX },
 
     // Warnings
+    [PM_WARN_AMBIGUOUS_BINARY_OPERATOR]         = { "'%s' after local variable or literal is interpreted as binary operator even though it seems like %s", PM_WARNING_LEVEL_VERBOSE },
     [PM_WARN_AMBIGUOUS_FIRST_ARGUMENT_MINUS]    = { "ambiguous first argument; put parentheses or a space even after `-` operator", PM_WARNING_LEVEL_VERBOSE },
     [PM_WARN_AMBIGUOUS_FIRST_ARGUMENT_PLUS]     = { "ambiguous first argument; put parentheses or a space even after `+` operator", PM_WARNING_LEVEL_VERBOSE },
     [PM_WARN_AMBIGUOUS_PREFIX_AMPERSAND]        = { "ambiguous `&` has been interpreted as an argument prefix", PM_WARNING_LEVEL_VERBOSE },

diff --git a/test/prism/newline_test.rb b/test/prism/newline_test.rb
@@ -15,6 +15,7 @@ class NewlineTest < TestCase
       regexp_test.rb
       static_literals_test.rb
       unescape_test.rb
+      warnings_test.rb
     ]
 
     filepaths.each do |relative|

diff --git a/test/prism/warnings_test.rb b/test/prism/warnings_test.rb
@@ -23,6 +23,23 @@ def test_ambiguous_regexp
       assert_warning("a /b/", "wrap regexp in parentheses")
     end
 
+    def test_binary_operator
+      [
+        [:**, "argument prefix"],
+        [:*, "argument prefix"],
+        [:<<, "here document"],
+        [:&, "argument prefix"],
+        [:+, "unary operator"],
+        [:-, "unary operator"],
+        [:/, "regexp literal"],
+        [:%, "string literal"]
+      ].each do |(operator, warning)|
+        assert_warning("puts 1 #{operator}0", warning)
+        assert_warning("puts :a #{operator}0", warning)
+        assert_warning("m = 1; puts m #{operator}0", warning)
+      end
+    end
+
     def test_equal_in_conditional
       assert_warning("if a = 1; end; a = a", "should be ==")
     end