Skip to content

Commit

Permalink
[ruby/prism] Add warning for chained comparisons
Browse files Browse the repository at this point in the history
  • Loading branch information
kddnewton authored and matzbot committed Mar 12, 2024
1 parent 157733b commit e914fa0
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 10 deletions.
1 change: 1 addition & 0 deletions prism/config.yml
Expand Up @@ -235,6 +235,7 @@ warnings:
- AMBIGUOUS_FIRST_ARGUMENT_PLUS
- AMBIGUOUS_PREFIX_STAR
- AMBIGUOUS_SLASH
- COMPARISON_AFTER_COMPARISON
- DOT_DOT_DOT_EOL
- EQUAL_IN_CONDITIONAL
- EQUAL_IN_CONDITIONAL_3_3_0
Expand Down
32 changes: 23 additions & 9 deletions prism/prism.c
Expand Up @@ -1959,6 +1959,11 @@ pm_break_node_create(pm_parser_t *parser, const pm_token_t *keyword, pm_argument
return node;
}

// There are certain flags that we want to use internally but don't want to
// expose because they are not relevant beyond parsing. Therefore we'll define
// them here and not define them in config.yml/a header file.
static const pm_node_flags_t PM_CALL_NODE_FLAGS_COMPARISON = 0x10;

/**
* Allocate and initialize a new CallNode node. This sets everything to NULL or
* PM_TOKEN_NOT_PROVIDED as appropriate such that its values can be overridden
Expand Down Expand Up @@ -2026,11 +2031,11 @@ pm_call_node_aref_create(pm_parser_t *parser, pm_node_t *receiver, pm_arguments_
* Allocate and initialize a new CallNode node from a binary expression.
*/
static pm_call_node_t *
pm_call_node_binary_create(pm_parser_t *parser, pm_node_t *receiver, pm_token_t *operator, pm_node_t *argument) {
pm_call_node_binary_create(pm_parser_t *parser, pm_node_t *receiver, pm_token_t *operator, pm_node_t *argument, pm_node_flags_t flags) {
pm_assert_value_expression(parser, receiver);
pm_assert_value_expression(parser, argument);

pm_call_node_t *node = pm_call_node_create(parser, pm_call_node_ignore_visibility_flag(receiver));
pm_call_node_t *node = pm_call_node_create(parser, pm_call_node_ignore_visibility_flag(receiver) | flags);

node->base.location.start = MIN(receiver->location.start, argument->location.start);
node->base.location.end = MAX(receiver->location.end, argument->location.end);
Expand Down Expand Up @@ -17507,7 +17512,7 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
if (accept1(parser, PM_TOKEN_STAR_STAR)) {
pm_token_t exponent_operator = parser->previous;
pm_node_t *exponent = parse_expression(parser, pm_binding_powers[exponent_operator.type].right, false, PM_ERR_EXPECT_ARGUMENT);
node = (pm_node_t *) pm_call_node_binary_create(parser, node, &exponent_operator, exponent);
node = (pm_node_t *) pm_call_node_binary_create(parser, node, &exponent_operator, exponent, 0);
node = (pm_node_t *) pm_call_node_unary_create(parser, &operator, node, "-@");
} else {
switch (PM_NODE_TYPE(node)) {
Expand Down Expand Up @@ -18262,7 +18267,7 @@ parse_expression_infix(pm_parser_t *parser, pm_node_t *node, pm_binding_power_t
pm_node_t *argument = parse_expression(parser, binding_power, false, PM_ERR_EXPECT_EXPRESSION_AFTER_OPERATOR);

// By default, we're going to create a call node and then return it.
pm_call_node_t *call = pm_call_node_binary_create(parser, node, &token, argument);
pm_call_node_t *call = pm_call_node_binary_create(parser, node, &token, argument, 0);
pm_node_t *result = (pm_node_t *) call;

// If the receiver of this =~ is a regular expression node, then we
Expand Down Expand Up @@ -18327,10 +18332,6 @@ parse_expression_infix(pm_parser_t *parser, pm_node_t *node, pm_binding_power_t
case PM_TOKEN_EQUAL_EQUAL:
case PM_TOKEN_EQUAL_EQUAL_EQUAL:
case PM_TOKEN_LESS_EQUAL_GREATER:
case PM_TOKEN_GREATER:
case PM_TOKEN_GREATER_EQUAL:
case PM_TOKEN_LESS:
case PM_TOKEN_LESS_EQUAL:
case PM_TOKEN_CARET:
case PM_TOKEN_PIPE:
case PM_TOKEN_AMPERSAND:
Expand All @@ -18343,9 +18344,20 @@ parse_expression_infix(pm_parser_t *parser, pm_node_t *node, pm_binding_power_t
case PM_TOKEN_STAR:
case PM_TOKEN_STAR_STAR: {
parser_lex(parser);
pm_node_t *argument = parse_expression(parser, binding_power, false, PM_ERR_EXPECT_EXPRESSION_AFTER_OPERATOR);
return (pm_node_t *) pm_call_node_binary_create(parser, node, &token, argument, 0);
}
case PM_TOKEN_GREATER:
case PM_TOKEN_GREATER_EQUAL:
case PM_TOKEN_LESS:
case PM_TOKEN_LESS_EQUAL: {
if (PM_NODE_TYPE_P(node, PM_CALL_NODE) && PM_NODE_FLAG_P(node, PM_CALL_NODE_FLAGS_COMPARISON)) {
PM_PARSER_WARN_TOKEN_FORMAT_CONTENT(parser, parser->current, PM_WARN_COMPARISON_AFTER_COMPARISON);
}

parser_lex(parser);
pm_node_t *argument = parse_expression(parser, binding_power, false, PM_ERR_EXPECT_EXPRESSION_AFTER_OPERATOR);
return (pm_node_t *) pm_call_node_binary_create(parser, node, &token, argument);
return (pm_node_t *) pm_call_node_binary_create(parser, node, &token, argument, PM_CALL_NODE_FLAGS_COMPARISON);
}
case PM_TOKEN_AMPERSAND_DOT:
case PM_TOKEN_DOT: {
Expand Down Expand Up @@ -18670,6 +18682,7 @@ parse_expression(pm_parser_t *parser, pm_binding_power_t binding_power, bool acc
current_binding_powers.binary
) {
node = parse_expression_infix(parser, node, binding_power, current_binding_powers.right, accepts_command_call);

if (current_binding_powers.nonassoc) {
bool endless_range_p = PM_NODE_TYPE_P(node, PM_RANGE_NODE) && ((pm_range_node_t *) node)->right == NULL;
pm_binding_power_t left = endless_range_p ? PM_BINDING_POWER_TERM : current_binding_powers.left;
Expand All @@ -18683,6 +18696,7 @@ parse_expression(pm_parser_t *parser, pm_binding_power_t binding_power, bool acc
break;
}
}

if (accepts_command_call) {
// A command-style method call is only accepted on method chains.
// Thus, we check whether the parsed node can continue method chains.
Expand Down
1 change: 1 addition & 0 deletions prism/templates/src/diagnostic.c.erb
Expand Up @@ -315,6 +315,7 @@ static const pm_diagnostic_data_t diagnostic_messages[PM_DIAGNOSTIC_ID_MAX] = {
[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_STAR] = { "ambiguous `*` has been interpreted as an argument prefix", PM_WARNING_LEVEL_VERBOSE },
[PM_WARN_AMBIGUOUS_SLASH] = { "ambiguous `/`; wrap regexp in parentheses or add a space after `/` operator", PM_WARNING_LEVEL_VERBOSE },
[PM_WARN_COMPARISON_AFTER_COMPARISON] = { "comparison '%.*s' after comparison", PM_WARNING_LEVEL_VERBOSE },
[PM_WARN_DOT_DOT_DOT_EOL] = { "... at EOL, should be parenthesized?", PM_WARNING_LEVEL_DEFAULT },
[PM_WARN_DUPLICATED_HASH_KEY] = { "key %.*s is duplicated and overwritten on line %" PRIi32, PM_WARNING_LEVEL_DEFAULT },
[PM_WARN_DUPLICATED_WHEN_CLAUSE] = { "duplicated 'when' clause with line %" PRIi32 " is ignored", PM_WARNING_LEVEL_VERBOSE },
Expand Down
2 changes: 1 addition & 1 deletion test/prism/format_errors_test.rb
Expand Up @@ -15,7 +15,7 @@ def test_format_errors

assert_equal <<~'ERROR', Debug.format_errors('"%W"\u"', false)
> 1 | "%W"\u"
| ^ invalid character `\`
| ^ unexpected backslash, ignoring it
| ^ unexpected local variable or method, expecting end-of-input
| ^ unterminated string meets end of file
ERROR
Expand Down

0 comments on commit e914fa0

Please sign in to comment.