Skip to content

Commit

Permalink
[ruby/prism] Better error message on invalid def
Browse files Browse the repository at this point in the history
  • Loading branch information
kddnewton authored and matzbot committed Apr 12, 2024
1 parent 52b8623 commit 1521af3
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 19 deletions.
2 changes: 1 addition & 1 deletion prism/config.yml
Expand Up @@ -60,7 +60,6 @@ errors:
- DEF_ENDLESS
- DEF_ENDLESS_SETTER
- DEF_NAME
- DEF_NAME_AFTER_RECEIVER
- DEF_PARAMS_TERM
- DEF_PARAMS_TERM_PAREN
- DEF_RECEIVER
Expand Down Expand Up @@ -97,6 +96,7 @@ errors:
- EXPECT_EXPRESSION_AFTER_STAR
- EXPECT_IDENT_REQ_PARAMETER
- EXPECT_LPAREN_REQ_PARAMETER
- EXPECT_MESSAGE
- EXPECT_RBRACKET
- EXPECT_RPAREN
- EXPECT_RPAREN_AFTER_MULTI
Expand Down
23 changes: 10 additions & 13 deletions prism/prism.c
Expand Up @@ -15281,6 +15281,7 @@ parse_method_definition_name(pm_parser_t *parser) {
parser_lex(parser);
return parser->previous;
default:
PM_PARSER_ERR_TOKEN_FORMAT(parser, parser->current, PM_ERR_DEF_NAME, pm_token_type_human(parser->current.type));
return (pm_token_t) { .type = PM_TOKEN_MISSING, .start = parser->current.start, .end = parser->current.end };
}
}
Expand Down Expand Up @@ -17722,7 +17723,7 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b

pm_node_t *receiver = NULL;
pm_token_t operator = not_provided(parser);
pm_token_t name = (pm_token_t) { .type = PM_TOKEN_MISSING, .start = def_keyword.end, .end = def_keyword.end };
pm_token_t name;

// This context is necessary for lexing `...` in a bare params
// correctly. It must be pushed before lexing the first param, so it
Expand Down Expand Up @@ -17804,7 +17805,7 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
receiver = (pm_node_t *) pm_true_node_create(parser, &identifier);
break;
case PM_TOKEN_KEYWORD_FALSE:
receiver = (pm_node_t *)pm_false_node_create(parser, &identifier);
receiver = (pm_node_t *) pm_false_node_create(parser, &identifier);
break;
case PM_TOKEN_KEYWORD___FILE__:
receiver = (pm_node_t *) pm_source_file_node_create(parser, &identifier);
Expand All @@ -17826,9 +17827,10 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
break;
}
case PM_TOKEN_PARENTHESIS_LEFT: {
// The current context is `PM_CONTEXT_DEF_PARAMS`, however the inner expression
// of this parenthesis should not be processed under this context.
// Thus, the context is popped here.
// The current context is `PM_CONTEXT_DEF_PARAMS`, however
// the inner expression of this parenthesis should not be
// processed under this context. Thus, the context is popped
// here.
context_pop(parser);
parser_lex(parser);

Expand All @@ -17845,7 +17847,8 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
operator = parser->previous;
receiver = (pm_node_t *) pm_parentheses_node_create(parser, &lparen, expression, &rparen);

// To push `PM_CONTEXT_DEF_PARAMS` again is for the same reason as described the above.
// To push `PM_CONTEXT_DEF_PARAMS` again is for the same
// reason as described the above.
pm_parser_scope_push(parser, true);
context_push(parser, PM_CONTEXT_DEF_PARAMS);
name = parse_method_definition_name(parser);
Expand All @@ -17857,12 +17860,6 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
break;
}

// If, after all that, we were unable to find a method name, add an
// error to the error list.
if (name.type == PM_TOKEN_MISSING) {
pm_parser_err_previous(parser, PM_ERR_DEF_NAME);
}

pm_token_t lparen;
pm_token_t rparen;
pm_parameters_node_t *params;
Expand Down Expand Up @@ -19856,7 +19853,7 @@ parse_expression_infix(pm_parser_t *parser, pm_node_t *node, pm_binding_power_t
break;
}
default: {
pm_parser_err_current(parser, PM_ERR_DEF_NAME);
PM_PARSER_ERR_TOKEN_FORMAT(parser, parser->current, PM_ERR_EXPECT_MESSAGE, pm_token_type_human(parser->current.type));
message = (pm_token_t) { .type = PM_TOKEN_MISSING, .start = parser->previous.end, .end = parser->previous.end };
}
}
Expand Down
4 changes: 2 additions & 2 deletions prism/templates/src/diagnostic.c.erb
Expand Up @@ -144,8 +144,7 @@ static const pm_diagnostic_data_t diagnostic_messages[PM_DIAGNOSTIC_ID_MAX] = {
[PM_ERR_CONSTANT_PATH_COLON_COLON_CONSTANT] = { "expected a constant after the `::` operator", PM_ERROR_LEVEL_SYNTAX },
[PM_ERR_DEF_ENDLESS] = { "could not parse the endless method body", PM_ERROR_LEVEL_SYNTAX },
[PM_ERR_DEF_ENDLESS_SETTER] = { "invalid method name; a setter method cannot be defined in an endless method definition", PM_ERROR_LEVEL_SYNTAX },
[PM_ERR_DEF_NAME] = { "expected a method name", PM_ERROR_LEVEL_SYNTAX },
[PM_ERR_DEF_NAME_AFTER_RECEIVER] = { "expected a method name after the receiver", PM_ERROR_LEVEL_SYNTAX },
[PM_ERR_DEF_NAME] = { "unexpected %s; expected a method name", PM_ERROR_LEVEL_SYNTAX },
[PM_ERR_DEF_PARAMS_TERM] = { "expected a delimiter to close the parameters", PM_ERROR_LEVEL_SYNTAX },
[PM_ERR_DEF_PARAMS_TERM_PAREN] = { "expected a `)` to close the parameters", PM_ERROR_LEVEL_SYNTAX },
[PM_ERR_DEF_RECEIVER] = { "expected a receiver for the method definition", PM_ERROR_LEVEL_SYNTAX },
Expand Down Expand Up @@ -181,6 +180,7 @@ static const pm_diagnostic_data_t diagnostic_messages[PM_DIAGNOSTIC_ID_MAX] = {
[PM_ERR_EXPECT_EXPRESSION_AFTER_STAR] = { "expected an expression after `*`", PM_ERROR_LEVEL_SYNTAX },
[PM_ERR_EXPECT_IDENT_REQ_PARAMETER] = { "expected an identifier for the required parameter", PM_ERROR_LEVEL_SYNTAX },
[PM_ERR_EXPECT_LPAREN_REQ_PARAMETER] = { "expected a `(` to start a required parameter", PM_ERROR_LEVEL_SYNTAX },
[PM_ERR_EXPECT_MESSAGE] = { "unexpected %s; expecting a message to send to the receiver", PM_ERROR_LEVEL_SYNTAX },
[PM_ERR_EXPECT_RBRACKET] = { "expected a matching `]`", PM_ERROR_LEVEL_SYNTAX },
[PM_ERR_EXPECT_RPAREN] = { "expected a matching `)`", PM_ERROR_LEVEL_SYNTAX },
[PM_ERR_EXPECT_RPAREN_AFTER_MULTI] = { "expected a `)` after multiple assignment", PM_ERROR_LEVEL_SYNTAX },
Expand Down
4 changes: 2 additions & 2 deletions prism/templates/src/token_type.c.erb
Expand Up @@ -326,13 +326,13 @@ pm_token_type_human(pm_token_type_t token_type) {
case PM_TOKEN_STAR_STAR_EQUAL:
return "'**='";
case PM_TOKEN_STRING_BEGIN:
return "string beginning";
return "string literal";
case PM_TOKEN_STRING_CONTENT:
return "string content";
case PM_TOKEN_STRING_END:
return "string ending";
case PM_TOKEN_SYMBOL_BEGIN:
return "symbol beginning";
return "symbol literal";
case PM_TOKEN_TILDE:
return "'~'";
case PM_TOKEN_UAMPERSAND:
Expand Down
2 changes: 1 addition & 1 deletion test/prism/errors_test.rb
Expand Up @@ -341,7 +341,7 @@ def test_aliasing_global_variable_with_global_number_variable
def test_def_with_expression_receiver_and_no_identifier
assert_errors expression("def (a); end"), "def (a); end", [
["expected a `.` or `::` after the receiver in a method definition", 7..7],
["expected a method name", 7..7]
["unexpected ';'; expected a method name", 7..8]
]
end

Expand Down

0 comments on commit 1521af3

Please sign in to comment.