Skip to content

Commit

Permalink
[ruby/prism] Warn on integers in flip-flops
Browse files Browse the repository at this point in the history 
ruby/prism@500099e896
  • Loading branch information
@kddnewton @matzbot
kddnewton authored and matzbot committed Feb 29, 2024
1 parent f9d5c60 commit 1c0c490
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 17 deletions.
3 changes: 2 additions & 1 deletion prism/diagnostic.c
View file
Expand Up @@ -310,7 +310,8 @@ static const pm_diagnostic_data_t diagnostic_messages[PM_DIAGNOSTIC_ID_LEN] = {
[PM_WARN_DUPLICATED_WHEN_CLAUSE] = { "duplicated 'when' clause with line %" PRIi32 " is ignored", 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 },
[PM_WARN_FLOAT_OUT_OF_RANGE] = { "Float %.*s%s out of range", PM_WARNING_LEVEL_VERBOSE }
[PM_WARN_FLOAT_OUT_OF_RANGE] = { "Float %.*s%s out of range", PM_WARNING_LEVEL_VERBOSE },
[PM_WARN_INTEGER_IN_FLIP_FLOP] = { "integer literal in flip-flop", PM_WARNING_LEVEL_DEFAULT }
};

static inline const char *
Expand Down
1 change: 1 addition & 0 deletions prism/diagnostic.h
View file
Expand Up @@ -309,6 +309,7 @@ typedef enum {
PM_WARN_DUPLICATED_HASH_KEY,
PM_WARN_DUPLICATED_WHEN_CLAUSE,
PM_WARN_FLOAT_OUT_OF_RANGE,
PM_WARN_INTEGER_IN_FLIP_FLOP,

// This is the number of diagnostic codes.
PM_DIAGNOSTIC_ID_LEN,
Expand Down
55 changes: 39 additions & 16 deletions prism/prism.c
View file
Expand Up @@ -584,6 +584,15 @@ pm_parser_warn_token(pm_parser_t *parser, const pm_token_t *token, pm_diagnostic
pm_parser_warn(parser, token->start, token->end, diag_id);
}

/**
* Append a warning to the list of warnings on the parser using the location of
* the given node.
*/
static inline void
pm_parser_warn_node(pm_parser_t *parser, const pm_node_t *node, pm_diagnostic_id_t diag_id) {
pm_parser_warn(parser, node->location.start, node->location.end, diag_id);
}

/******************************************************************************/
/* Node-related functions */
/******************************************************************************/
Expand Down Expand Up @@ -725,6 +734,17 @@ pm_assert_value_expression(pm_parser_t *parser, pm_node_t *node) {
}
}

/**
* Check one side of a flip-flop for integer literals. If -e was not supplied at
* the command-line, then warn.
*/
static inline void
pm_flip_flop_predicate(pm_parser_t *parser, pm_node_t *node) {
if (PM_NODE_TYPE_P(node, PM_INTEGER_NODE) && !(parser->command_line & PM_OPTIONS_COMMAND_LINE_E)) {
pm_parser_warn_node(parser, node, PM_WARN_INTEGER_IN_FLIP_FLOP);
}
}

/**
* The predicate of conditional nodes can change what would otherwise be regular
* nodes into specialized nodes. For example:
Expand All @@ -735,37 +755,40 @@ pm_assert_value_expression(pm_parser_t *parser, pm_node_t *node) {
* if /foo #{bar}/ => InterpolatedRegularExpressionNode becomes InterpolatedMatchLastLineNode
*/
static void
pm_conditional_predicate(pm_node_t *node) {
pm_conditional_predicate(pm_parser_t *parser, pm_node_t *node) {
switch (PM_NODE_TYPE(node)) {
case PM_AND_NODE: {
pm_and_node_t *cast = (pm_and_node_t *) node;
pm_conditional_predicate(cast->left);
pm_conditional_predicate(cast->right);
pm_conditional_predicate(parser, cast->left);
pm_conditional_predicate(parser, cast->right);
break;
}
case PM_OR_NODE: {
pm_or_node_t *cast = (pm_or_node_t *) node;
pm_conditional_predicate(cast->left);
pm_conditional_predicate(cast->right);
pm_conditional_predicate(parser, cast->left);
pm_conditional_predicate(parser, cast->right);
break;
}
case PM_PARENTHESES_NODE: {
pm_parentheses_node_t *cast = (pm_parentheses_node_t *) node;

if ((cast->body != NULL) && PM_NODE_TYPE_P(cast->body, PM_STATEMENTS_NODE)) {
pm_statements_node_t *statements = (pm_statements_node_t *) cast->body;
if (statements->body.size == 1) pm_conditional_predicate(statements->body.nodes[0]);
if (statements->body.size == 1) pm_conditional_predicate(parser, statements->body.nodes[0]);
}

break;
}
case PM_RANGE_NODE: {
pm_range_node_t *cast = (pm_range_node_t *) node;

if (cast->left) {
pm_conditional_predicate(cast->left);
pm_flip_flop_predicate(parser, cast->left);
pm_conditional_predicate(parser, cast->left);
}
if (cast->right) {
pm_conditional_predicate(cast->right);
pm_flip_flop_predicate(parser, cast->right);
pm_conditional_predicate(parser, cast->right);
}

// Here we change the range node into a flip flop node. We can do
Expand Down Expand Up @@ -3778,7 +3801,7 @@ pm_if_node_create(pm_parser_t *parser,
pm_node_t *consequent,
const pm_token_t *end_keyword
) {
pm_conditional_predicate(predicate);
pm_conditional_predicate(parser, predicate);
pm_predicate_check(parser, predicate);
pm_if_node_t *node = PM_ALLOC_NODE(parser, pm_if_node_t);

Expand Down Expand Up @@ -3818,7 +3841,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_conditional_predicate(parser, predicate);
pm_predicate_check(parser, predicate);
pm_if_node_t *node = PM_ALLOC_NODE(parser, pm_if_node_t);

Expand Down Expand Up @@ -3851,7 +3874,7 @@ pm_if_node_modifier_create(pm_parser_t *parser, pm_node_t *statement, const pm_t
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_conditional_predicate(parser, predicate);
pm_predicate_check(parser, predicate);

pm_statements_node_t *if_statements = pm_statements_node_create(parser);
Expand Down Expand Up @@ -6161,7 +6184,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_conditional_predicate(parser, predicate);
pm_predicate_check(parser, predicate);
pm_unless_node_t *node = PM_ALLOC_NODE(parser, pm_unless_node_t);

Expand Down Expand Up @@ -6197,7 +6220,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_conditional_predicate(parser, predicate);
pm_predicate_check(parser, predicate);
pm_unless_node_t *node = PM_ALLOC_NODE(parser, pm_unless_node_t);

Expand Down Expand Up @@ -16317,7 +16340,7 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
arguments.closing_loc = PM_LOCATION_TOKEN_VALUE(&parser->previous);
} else {
receiver = parse_expression(parser, PM_BINDING_POWER_COMPOSITION, true, PM_ERR_NOT_EXPRESSION);
pm_conditional_predicate(receiver);
pm_conditional_predicate(parser, receiver);
pm_predicate_check(parser, receiver);

if (!parser->recovering) {
Expand All @@ -16328,7 +16351,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_conditional_predicate(parser, receiver);
pm_predicate_check(parser, receiver);
}

Expand Down Expand Up @@ -17006,7 +17029,7 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
pm_node_t *receiver = parse_expression(parser, pm_binding_powers[parser->previous.type].right, binding_power < PM_BINDING_POWER_MATCH, PM_ERR_UNARY_RECEIVER);
pm_call_node_t *node = pm_call_node_unary_create(parser, &operator, receiver, "!");

pm_conditional_predicate(receiver);
pm_conditional_predicate(parser, receiver);
pm_predicate_check(parser, receiver);
return (pm_node_t *) node;
}
Expand Down
8 changes: 8 additions & 0 deletions test/prism/command_line_test.rb
View file
Expand Up @@ -57,5 +57,13 @@ def test_command_line_l
assert_equal :$/, arguments.first.name
assert_equal "chomp", arguments.last.elements.first.key.unescaped
end

def test_command_line_e
result = Prism.parse("1 if 2..3")
assert_equal 2, result.warnings.length

result = Prism.parse("1 if 2..3", command_line: "e")
assert_equal 0, result.warnings.length
end
end
end

0 comments on commit 1c0c490

Please sign in to comment.