Merge pull request #3087 from ruby/more-error-messages

Fix up more error messages to more closely match parse.y

bin/prism

@@ -162,23 +162,32 @@ module Prism
     # bin/prism error [name] [source]
     def error(argv)
       name = argv.shift
-      source, _ = read_source(argv)
-
-      result = Prism.parse(source)
-      raise "Expected #{source.inspect} to have errors" if result.success?
 
+      source = nil
       filepath = File.expand_path("../test/prism/errors/#{name}.txt", __dir__)
 
-      if File.file?(filepath)
-        counter = 1
+      if argv.empty?
+        raise "Expected #{filepath} to exist" unless File.file?(filepath)
 
-        begin
-          current = "#{File.dirname(filepath)}/#{File.basename(filepath, ".txt")}_#{counter += 1}.txt"
-        end while File.file?(current)
+        source = File.read(filepath, binmode: true, external_encoding: Encoding::UTF_8)
+        source = source.lines.grep_v(/^\s*\^/).join.gsub(/\n*\z/, "")
+      else
+        if File.file?(filepath)
+          counter = 1
 
-        filepath = current
+          begin
+            current = "#{File.dirname(filepath)}/#{File.basename(filepath, ".txt")}_#{counter += 1}.txt"
+          end while File.file?(current)
+
+          filepath = current
+        end
+
+        source, _ = read_source(argv)
       end
 
+      result = Prism.parse(source)
+      raise "Expected #{source.inspect} to have errors" if result.success?
+
       File.write(filepath, result.errors_format)
     end
 

src/prism.c

@@ -15421,7 +15421,7 @@ parse_arguments_list(pm_parser_t *parser, pm_arguments_t *arguments, bool accept
         // then we have a trailing comma where we need to check whether it is
         // allowed or not.
         if (parser->previous.type == PM_TOKEN_COMMA && !match1(parser, PM_TOKEN_SEMICOLON)) {
-            pm_parser_err_previous(parser, PM_ERR_EXPECT_ARGUMENT);
+            PM_PARSER_ERR_TOKEN_FORMAT(parser, parser->previous, PM_ERR_EXPECT_ARGUMENT, pm_token_type_human(parser->current.type));
         }
 
         pm_accepts_block_stack_pop(parser);
@@ -17649,7 +17649,8 @@ pm_parser_err_prefix(pm_parser_t *parser, pm_diagnostic_id_t diag_id) {
             PM_PARSER_ERR_TOKEN_FORMAT(parser, parser->previous, diag_id, human, parser->previous.start[0]);
             break;
         }
-        case PM_ERR_UNARY_DISALLOWED: {
+        case PM_ERR_UNARY_DISALLOWED:
+        case PM_ERR_EXPECT_ARGUMENT: {
             PM_PARSER_ERR_TOKEN_FORMAT(parser, parser->current, diag_id, pm_token_type_human(parser->current.type));
             break;
         }
@@ -17999,7 +18000,13 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b
             }
 
             accept1(parser, PM_TOKEN_NEWLINE);
-            expect1(parser, PM_TOKEN_BRACKET_RIGHT, PM_ERR_ARRAY_TERM);
+
+            if (!accept1(parser, PM_TOKEN_BRACKET_RIGHT)) {
+                PM_PARSER_ERR_TOKEN_FORMAT(parser, parser->current, PM_ERR_ARRAY_TERM, pm_token_type_human(parser->current.type));
+                parser->previous.start = parser->previous.end;
+                parser->previous.type = PM_TOKEN_MISSING;
+            }
+
             pm_array_node_close_set(array, &parser->previous);
             pm_accepts_block_stack_pop(parser);
 

templates/src/diagnostic.c.erb

@@ -113,7 +113,7 @@ static const pm_diagnostic_data_t diagnostic_messages[PM_DIAGNOSTIC_ID_MAX] = {
     [PM_ERR_ARRAY_EXPRESSION]                   = { "expected an expression for the array element", PM_ERROR_LEVEL_SYNTAX },
     [PM_ERR_ARRAY_EXPRESSION_AFTER_STAR]        = { "expected an expression after `*` in the array", PM_ERROR_LEVEL_SYNTAX },
     [PM_ERR_ARRAY_SEPARATOR]                    = { "unexpected %s; expected a `,` separator for the array elements", PM_ERROR_LEVEL_SYNTAX },
-    [PM_ERR_ARRAY_TERM]                         = { "expected a `]` to close the array", PM_ERROR_LEVEL_SYNTAX },
+    [PM_ERR_ARRAY_TERM]                         = { "unexpected %s; expected a `]` to close the array", PM_ERROR_LEVEL_SYNTAX },
     [PM_ERR_BEGIN_LONELY_ELSE]                  = { "unexpected `else` in `begin` block; else without rescue is useless", PM_ERROR_LEVEL_SYNTAX },
     [PM_ERR_BEGIN_TERM]                         = { "expected an `end` to close the `begin` statement", PM_ERROR_LEVEL_SYNTAX },
     [PM_ERR_BEGIN_UPCASE_BRACE]                 = { "expected a `{` after `BEGIN`", PM_ERROR_LEVEL_SYNTAX },
@@ -170,7 +170,7 @@ static const pm_diagnostic_data_t diagnostic_messages[PM_DIAGNOSTIC_ID_MAX] = {
     [PM_ERR_ESCAPE_INVALID_UNICODE_LONG]        = { "invalid Unicode escape sequence; maximum length is 6 digits", PM_ERROR_LEVEL_SYNTAX },
     [PM_ERR_ESCAPE_INVALID_UNICODE_SHORT]       = { "too short escape sequence: %.*s", PM_ERROR_LEVEL_SYNTAX },
     [PM_ERR_ESCAPE_INVALID_UNICODE_TERM]        = { "unterminated Unicode escape", PM_ERROR_LEVEL_SYNTAX },
-    [PM_ERR_EXPECT_ARGUMENT]                    = { "expected an argument", PM_ERROR_LEVEL_SYNTAX },
+    [PM_ERR_EXPECT_ARGUMENT]                    = { "unexpected %s; expected an argument", PM_ERROR_LEVEL_SYNTAX },
     [PM_ERR_EXPECT_EOL_AFTER_STATEMENT]         = { "unexpected %s, expecting end-of-input", PM_ERROR_LEVEL_SYNTAX },
     [PM_ERR_EXPECT_EXPRESSION_AFTER_AMPAMPEQ]   = { "expected an expression after `&&=`", PM_ERROR_LEVEL_SYNTAX },
     [PM_ERR_EXPECT_EXPRESSION_AFTER_PIPEPIPEEQ] = { "expected an expression after `||=`", PM_ERROR_LEVEL_SYNTAX },

test/prism/errors/break_1.txt

@@ -1,4 +1,4 @@
 break 1,;
-       ^ expected an argument
+        ^ unexpected ';'; expected an argument
 ^~~~~~~~ Invalid break
 

test/prism/errors/next_1.txt

@@ -1,4 +1,4 @@
 next 1,;
-      ^ expected an argument
+       ^ unexpected ';'; expected an argument
 ^~~~~~~ Invalid next
 

test/prism/errors/return_1.txt

@@ -1,3 +1,3 @@
 return 1,;
-        ^ expected an argument
+         ^ unexpected ';'; expected an argument
 

test/prism/errors/trailing_comma_in_calls.txt

@@ -1,3 +1,3 @@
 foo 1,
-     ^ expected an argument
+     ^ unexpected end-of-input; expected an argument