Merge pull request #2702 from ruby/better-error-message-on-invalid-def

Better error message on invalid def

Author: kddnewton

config.yml

@@ -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
@@ -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

src/prism.c

@@ -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 };
  }
 }
@@ -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
@@ -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);
@@ -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);
 
@@ -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);
@@ -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;
@@ -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 };
  }
  }

templates/src/diagnostic.c.erb

@@ -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 },
@@ -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 },

templates/src/token_type.c.erb

@@ -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:

test/prism/errors_test.rb

@@ -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