Skip to content

Commit

Permalink
[ruby/prism] Add warning for assignments to literals in conditionals
Browse files Browse the repository at this point in the history 
ruby/prism@ee87ed08fb
  • Loading branch information
@haldun @matzbot
haldun authored and matzbot committed Feb 16, 2024
1 parent c5f22b5 commit f4f57e1
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 0 deletions.
1 change: 1 addition & 0 deletions prism/diagnostic.c
View file
Expand Up @@ -300,6 +300,7 @@ static const pm_diagnostic_data_t diagnostic_messages[PM_DIAGNOSTIC_ID_LEN] = {
[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_EQUAL_IN_CONDITIONAL] = { "found `= literal' in conditional, should be ==", PM_WARNING_LEVEL_DEFAULT },
[PM_WARN_END_IN_METHOD] = { "END in method; use at_exit", PM_WARNING_LEVEL_DEFAULT },
};

Expand Down
1 change: 1 addition & 0 deletions prism/diagnostic.h
View file
Expand Up @@ -298,6 +298,7 @@ typedef enum {
PM_WARN_AMBIGUOUS_FIRST_ARGUMENT_PLUS,
PM_WARN_AMBIGUOUS_PREFIX_STAR,
PM_WARN_AMBIGUOUS_SLASH,
PM_WARN_EQUAL_IN_CONDITIONAL,
PM_WARN_END_IN_METHOD,

// This is the number of diagnostic codes.
Expand Down
74 changes: 74 additions & 0 deletions prism/prism.c
View file
Expand Up @@ -3572,6 +3572,68 @@ pm_hash_node_closing_loc_set(pm_hash_node_t *hash, pm_token_t *token) {
hash->closing_loc = PM_LOCATION_TOKEN_VALUE(token);
}

static pm_node_t *
pm_assignment_value_node(pm_node_t *node) {
switch (PM_NODE_TYPE(node)) {
case PM_CLASS_VARIABLE_WRITE_NODE: {
const pm_class_variable_write_node_t *cast = (pm_class_variable_write_node_t *)node;
return cast->value;
}
case PM_CONSTANT_WRITE_NODE: {
const pm_constant_write_node_t *cast = (pm_constant_write_node_t *)node;
return cast->value;
}
case PM_GLOBAL_VARIABLE_WRITE_NODE: {
const pm_global_variable_write_node_t *cast = (pm_global_variable_write_node_t *)node;
return cast->value;
}
case PM_INSTANCE_VARIABLE_WRITE_NODE: {
const pm_instance_variable_write_node_t *cast = (pm_instance_variable_write_node_t *)node;
return cast->value;
}
case PM_LOCAL_VARIABLE_WRITE_NODE: {
const pm_local_variable_write_node_t *cast = (pm_local_variable_write_node_t *)node;
return cast->value;
}
case PM_MULTI_WRITE_NODE: {
const pm_multi_write_node_t *cast = (pm_multi_write_node_t *)node;
return cast->value;
}
case PM_PARENTHESES_NODE: {
const pm_parentheses_node_t *cast = (pm_parentheses_node_t *)node;
if (cast->body != NULL) {
return pm_assignment_value_node(cast->body);
}
return NULL;
}
case PM_BEGIN_NODE: {
const pm_begin_node_t *cast = (pm_begin_node_t *)node;
if (cast->statements != NULL) {
return pm_assignment_value_node((pm_node_t *) cast->statements);
}
return NULL;
}
case PM_STATEMENTS_NODE: {
const pm_statements_node_t *cast = (const pm_statements_node_t *) node;
return pm_assignment_value_node(cast->body.nodes[cast->body.size - 1]);
}
default:
return NULL;
}
}

/**
* Check whether the predicate contains an assigment where the assigned value is a
* literal. If such an assignment is found, it generates a warning.
*/
static void
pm_check_predicate_assignment(pm_parser_t *parser, pm_node_t *predicate) {
pm_node_t *value = pm_assignment_value_node(predicate);
if ((value != NULL) && PM_NODE_FLAG_P(value, PM_NODE_FLAG_STATIC_LITERAL)) {
pm_parser_warn_token(parser, &parser->current, PM_WARN_EQUAL_IN_CONDITIONAL);
}
}

/**
* Allocate a new IfNode node.
*/
Expand All @@ -3585,6 +3647,7 @@ pm_if_node_create(pm_parser_t *parser,
const pm_token_t *end_keyword
) {
pm_conditional_predicate(predicate);
pm_check_predicate_assignment(parser, predicate);
pm_if_node_t *node = PM_ALLOC_NODE(parser, pm_if_node_t);

const uint8_t *end;
Expand Down Expand Up @@ -3624,6 +3687,7 @@ pm_if_node_create(pm_parser_t *parser,
static pm_if_node_t *
pm_if_node_modifier_create(pm_parser_t *parser, pm_node_t *statement, const pm_token_t *if_keyword, pm_node_t *predicate) {
pm_conditional_predicate(predicate);
pm_check_predicate_assignment(parser, predicate);
pm_if_node_t *node = PM_ALLOC_NODE(parser, pm_if_node_t);

pm_statements_node_t *statements = pm_statements_node_create(parser);
Expand Down Expand Up @@ -3656,6 +3720,7 @@ static pm_if_node_t *
pm_if_node_ternary_create(pm_parser_t *parser, pm_node_t *predicate, const pm_token_t *qmark, pm_node_t *true_expression, const pm_token_t *colon, pm_node_t *false_expression) {
pm_assert_value_expression(parser, predicate);
pm_conditional_predicate(predicate);
pm_check_predicate_assignment(parser, predicate);

pm_statements_node_t *if_statements = pm_statements_node_create(parser);
pm_statements_node_body_append(if_statements, true_expression);
Expand Down Expand Up @@ -5899,6 +5964,7 @@ pm_undef_node_append(pm_undef_node_t *node, pm_node_t *name) {
static pm_unless_node_t *
pm_unless_node_create(pm_parser_t *parser, const pm_token_t *keyword, pm_node_t *predicate, const pm_token_t *then_keyword, pm_statements_node_t *statements) {
pm_conditional_predicate(predicate);
pm_check_predicate_assignment(parser, predicate);
pm_unless_node_t *node = PM_ALLOC_NODE(parser, pm_unless_node_t);

const uint8_t *end;
Expand Down Expand Up @@ -5934,6 +6000,7 @@ pm_unless_node_create(pm_parser_t *parser, const pm_token_t *keyword, pm_node_t
static pm_unless_node_t *
pm_unless_node_modifier_create(pm_parser_t *parser, pm_node_t *statement, const pm_token_t *unless_keyword, pm_node_t *predicate) {
pm_conditional_predicate(predicate);
pm_check_predicate_assignment(parser, predicate);
pm_unless_node_t *node = PM_ALLOC_NODE(parser, pm_unless_node_t);

pm_statements_node_t *statements = pm_statements_node_create(parser);
Expand Down Expand Up @@ -5971,6 +6038,7 @@ pm_unless_node_end_keyword_loc_set(pm_unless_node_t *node, const pm_token_t *end
static pm_until_node_t *
pm_until_node_create(pm_parser_t *parser, const pm_token_t *keyword, const pm_token_t *closing, pm_node_t *predicate, pm_statements_node_t *statements, pm_node_flags_t flags) {
pm_until_node_t *node = PM_ALLOC_NODE(parser, pm_until_node_t);
pm_check_predicate_assignment(parser, predicate);

*node = (pm_until_node_t) {
{
Expand All @@ -5996,6 +6064,7 @@ pm_until_node_create(pm_parser_t *parser, const pm_token_t *keyword, const pm_to
static pm_until_node_t *
pm_until_node_modifier_create(pm_parser_t *parser, const pm_token_t *keyword, pm_node_t *predicate, pm_statements_node_t *statements, pm_node_flags_t flags) {
pm_until_node_t *node = PM_ALLOC_NODE(parser, pm_until_node_t);
pm_check_predicate_assignment(parser, predicate);

*node = (pm_until_node_t) {
{
Expand Down Expand Up @@ -6065,6 +6134,7 @@ pm_when_node_statements_set(pm_when_node_t *node, pm_statements_node_t *statemen
static pm_while_node_t *
pm_while_node_create(pm_parser_t *parser, const pm_token_t *keyword, const pm_token_t *closing, pm_node_t *predicate, pm_statements_node_t *statements, pm_node_flags_t flags) {
pm_while_node_t *node = PM_ALLOC_NODE(parser, pm_while_node_t);
pm_check_predicate_assignment(parser, predicate);

*node = (pm_while_node_t) {
{
Expand All @@ -6090,6 +6160,7 @@ pm_while_node_create(pm_parser_t *parser, const pm_token_t *keyword, const pm_to
static pm_while_node_t *
pm_while_node_modifier_create(pm_parser_t *parser, const pm_token_t *keyword, pm_node_t *predicate, pm_statements_node_t *statements, pm_node_flags_t flags) {
pm_while_node_t *node = PM_ALLOC_NODE(parser, pm_while_node_t);
pm_check_predicate_assignment(parser, predicate);

*node = (pm_while_node_t) {
{
Expand Down Expand Up @@ -15864,6 +15935,7 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
} else {
receiver = parse_expression(parser, PM_BINDING_POWER_COMPOSITION, true, PM_ERR_NOT_EXPRESSION);
pm_conditional_predicate(receiver);
pm_check_predicate_assignment(parser, receiver);

if (!parser->recovering) {
accept1(parser, PM_TOKEN_NEWLINE);
Expand All @@ -15874,6 +15946,7 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
} else {
receiver = parse_expression(parser, PM_BINDING_POWER_NOT, true, PM_ERR_NOT_EXPRESSION);
pm_conditional_predicate(receiver);
pm_check_predicate_assignment(parser, receiver);
}

return (pm_node_t *) pm_call_node_not_create(parser, receiver, &message, &arguments);
Expand Down Expand Up @@ -16544,6 +16617,7 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
pm_call_node_t *node = pm_call_node_unary_create(parser, &operator, receiver, "!");

pm_conditional_predicate(receiver);
pm_check_predicate_assignment(parser, receiver);
return (pm_node_t *) node;
}
case PM_TOKEN_TILDE: {
Expand Down
30 changes: 30 additions & 0 deletions test/prism/errors_test.rb
View file
Expand Up @@ -2113,6 +2113,36 @@ def ([1]).foo; end
assert_errors expression(source), source, errors, compare_ripper: false
end

def test_assignment_to_literal_in_conditionals
source = <<~RUBY
if (a = 2); end
if ($a = 2); end
if (@a = 2); end
if (@@a = 2); end
if a elsif b = 2; end
unless (a = 2); end
unless ($a = 2); end
unless (@a = 2); end
unless (@@a = 2); end
while (a = 2); end
while ($a = 2); end
while (@a = 2); end
while (@@a = 2); end
until (a = 2); end
until ($a = 2); end
until (@a = 2); end
until (@@a = 2); end
foo if a = 2
foo if (a, b = 2)
(@foo = 1) ? a : b
!(a = 2)
not a = 2
RUBY
assert_warning_messages source, [
"found `= literal' in conditional, should be =="
] * source.lines.count
end

private

def assert_errors(expected, source, errors, compare_ripper: RUBY_ENGINE == "ruby")
Expand Down

0 comments on commit f4f57e1

Please sign in to comment.