From e90e8f8bd35391233f43d196d23b46ba8c20d56e Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Mon, 20 May 2024 09:54:14 -0400 Subject: [PATCH] [ruby/prism] Implement ambiguous binary operator warning https://github.com/ruby/prism/commit/6258c3695f --- prism/config.yml | 1 + prism/prism.c | 36 +++++++++++++++++++++++++++- prism/templates/src/diagnostic.c.erb | 1 + test/prism/newline_test.rb | 1 + test/prism/warnings_test.rb | 17 +++++++++++++ 5 files changed, 55 insertions(+), 1 deletion(-) diff --git a/prism/config.yml b/prism/config.yml index 0652cc4a9f..6b4b1aa9c0 100644 --- 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 index 02c2088089..b1d584d0ab 100644 --- 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 index d9e195e08f..57c52602f4 100644 --- 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 index f7511f665c..96374ce6a7 100644 --- a/test/prism/newline_test.rb +++ b/test/prism/newline_test.rb @@ -15,6 +15,7 @@ module Prism 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 index b138a3cb43..3e26cd2227 100644 --- a/test/prism/warnings_test.rb +++ b/test/prism/warnings_test.rb @@ -23,6 +23,23 @@ module Prism 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