Use the diagnostic types in the parser translation layer

Author: kddnewton

bin/prism

@@ -256,14 +256,10 @@ module Prism
       buffer.source = source
 
       puts "Parser:"
-      parser_ast, _, parser_tokens = Parser::Ruby33.new.tokenize(buffer)
-      pp parser_ast
-      pp parser_tokens
+      parser_parse(Parser::Ruby33.new, buffer)
 
       puts "Prism:"
-      prism_ast, _, prism_tokens = Prism::Translation::Parser33.new.tokenize(buffer)
-      pp prism_ast
-      pp prism_tokens
+      parser_parse(Prism::Translation::Parser33.new, buffer)
     end
 
     # bin/prism ripper [source]
@@ -302,6 +298,17 @@ module Prism
     # Helpers
     ############################################################################
 
+    # Parse a Parser::Source::Buffer with a given parser.
+    def parser_parse(parser, buffer)
+      diagnostics = []
+      parser.diagnostics.consumer = -> (diagnostic) { diagnostics << diagnostic }
+
+      parser_ast, _, parser_tokens = parser.tokenize(buffer, true)
+      pp diagnostics if diagnostics.any?
+      pp parser_ast
+      pp parser_tokens
+    end
+
     # Generate the list of values that will be used in a lookup table for a
     # given encoding.
     def lookup_table_values(encoding)

lib/prism/parse_result.rb

@@ -366,7 +366,8 @@ def inspect
 
   # This represents an error that was encountered during parsing.
   class ParseError
-    # The type of error.
+    # The type of error. This is an _internal_ symbol that is used for
+    # communicating with translation layers. It is not meant to be public API.
     attr_reader :type
 
     # The message associated with this error.
@@ -399,7 +400,8 @@ def inspect
 
   # This represents a warning that was encountered during parsing.
   class ParseWarning
-    # The type of warning.
+    # The type of warning. This is an _internal_ symbol that is used for
+    # communicating with translation layers. It is not meant to be public API.
     attr_reader :type
 
     # The message associated with this warning.

lib/prism/translation/parser.rb

@@ -9,11 +9,14 @@ module Translation
     # the parser gem, and overrides the parse* methods to parse with prism and
     # then translate.
     class Parser < ::Parser::Base
+      Diagnostic = ::Parser::Diagnostic # :nodoc:
+      private_constant :Diagnostic
+
       # The parser gem has a list of diagnostics with a hard-coded set of error
       # messages. We create our own diagnostic class in order to set our own
       # error messages.
-      class Diagnostic < ::Parser::Diagnostic
-        # The message generated by prism.
+      class PrismDiagnostic < Diagnostic
+        # This is the cached message coming from prism.
         attr_reader :message
 
         # Initialize a new diagnostic with the given message and location.
@@ -112,20 +115,109 @@ def valid_warning?(warning)
         true
       end
 
+      # Build a diagnostic from the given prism parse error.
+      def error_diagnostic(error, offset_cache)
+        location = error.location
+        diagnostic_location = build_range(location, offset_cache)
+
+        case error.type
+        when :argument_block_multi
+          Diagnostic.new(:error, :block_and_blockarg, {}, diagnostic_location, [])
+        when :argument_formal_constant
+          Diagnostic.new(:error, :formal_argument, {}, diagnostic_location, [])
+        when :argument_formal_class
+          Diagnostic.new(:error, :argument_cvar, {}, diagnostic_location, [])
+        when :argument_formal_global
+          Diagnostic.new(:error, :argument_gvar, {}, diagnostic_location, [])
+        when :argument_formal_ivar
+          Diagnostic.new(:error, :argument_ivar, {}, diagnostic_location, [])
+        when :argument_no_forwarding_amp
+          Diagnostic.new(:error, :no_anonymous_blockarg, {}, diagnostic_location, [])
+        when :argument_no_forwarding_star
+          Diagnostic.new(:error, :no_anonymous_restarg, {}, diagnostic_location, [])
+        when :begin_lonely_else
+          location = location.copy(length: 4)
+          diagnostic_location = build_range(location, offset_cache)
+          Diagnostic.new(:error, :useless_else, {}, diagnostic_location, [])
+        when :class_name, :module_name
+          Diagnostic.new(:error, :module_name_const, {}, diagnostic_location, [])
+        when :class_in_method
+          Diagnostic.new(:error, :class_in_def, {}, diagnostic_location, [])
+        when :def_endless_setter
+          Diagnostic.new(:error, :endless_setter, {}, diagnostic_location, [])
+        when :embdoc_term
+          Diagnostic.new(:error, :embedded_document, {}, diagnostic_location, [])
+        when :incomplete_variable_class, :incomplete_variable_class_3_3_0
+          location = location.copy(length: location.length + 1)
+          diagnostic_location = build_range(location, offset_cache)
+
+          Diagnostic.new(:error, :cvar_name, { name: location.slice }, diagnostic_location, [])
+        when :incomplete_variable_instance, :incomplete_variable_instance_3_3_0
+          location = location.copy(length: location.length + 1)
+          diagnostic_location = build_range(location, offset_cache)
+
+          Diagnostic.new(:error, :ivar_name, { name: location.slice }, diagnostic_location, [])
+        when :invalid_variable_global, :invalid_variable_global_3_3_0
+          Diagnostic.new(:error, :gvar_name, { name: location.slice }, diagnostic_location, [])
+        when :module_in_method
+          Diagnostic.new(:error, :module_in_def, {}, diagnostic_location, [])
+        when :numbered_parameter_ordinary
+          Diagnostic.new(:error, :ordinary_param_defined, {}, diagnostic_location, [])
+        when :numbered_parameter_outer_scope
+          Diagnostic.new(:error, :numparam_used_in_outer_scope, {}, diagnostic_location, [])
+        when :parameter_circular
+          Diagnostic.new(:error, :circular_argument_reference, { var_name: location.slice }, diagnostic_location, [])
+        when :parameter_name_repeat
+          Diagnostic.new(:error, :duplicate_argument, {}, diagnostic_location, [])
+        when :parameter_numbered_reserved
+          Diagnostic.new(:error, :reserved_for_numparam, { name: location.slice }, diagnostic_location, [])
+        when :singleton_for_literals
+          Diagnostic.new(:error, :singleton_literal, {}, diagnostic_location, [])
+        when :string_literal_eof
+          Diagnostic.new(:error, :string_eof, {}, diagnostic_location, [])
+        when :unexpected_token_ignore
+          Diagnostic.new(:error, :unexpected_token, { token: location.slice }, diagnostic_location, [])
+        when :write_target_in_method
+          Diagnostic.new(:error, :dynamic_const, {}, diagnostic_location, [])
+        else
+          PrismDiagnostic.new(error.message, :error, error.type, diagnostic_location)
+        end
+      end
+
+      # Build a diagnostic from the given prism parse warning.
+      def warning_diagnostic(warning, offset_cache)
+        diagnostic_location = build_range(warning.location, offset_cache)
+
+        case warning.type
+        when :ambiguous_first_argument_plus
+          Diagnostic.new(:warning, :ambiguous_prefix, { prefix: "+" }, diagnostic_location, [])
+        when :ambiguous_first_argument_minus
+          Diagnostic.new(:warning, :ambiguous_prefix, { prefix: "-" }, diagnostic_location, [])
+        when :ambiguous_prefix_star
+          Diagnostic.new(:warning, :ambiguous_prefix, { prefix: "*" }, diagnostic_location, [])
+        when :ambiguous_slash
+          Diagnostic.new(:warning, :ambiguous_regexp, {}, diagnostic_location, [])
+        when :dot_dot_dot_eol
+          Diagnostic.new(:warning, :triple_dot_at_eol, {}, diagnostic_location, [])
+        when :duplicated_hash_key
+          # skip, parser does this on its own
+        else
+          PrismDiagnostic.new(warning.message, :warning, warning.type, diagnostic_location)
+        end
+      end
+
       # If there was a error generated during the parse, then raise an
       # appropriate syntax error. Otherwise return the result.
       def unwrap(result, offset_cache)
         result.errors.each do |error|
           next unless valid_error?(error)
-
-          location = build_range(error.location, offset_cache)
-          diagnostics.process(Diagnostic.new(error.message, :error, :prism_error, location))
+          diagnostics.process(error_diagnostic(error, offset_cache))
         end
+
         result.warnings.each do |warning|
           next unless valid_warning?(warning)
-
-          location = build_range(warning.location, offset_cache)
-          diagnostics.process(Diagnostic.new(warning.message, :warning, :prism_warning, location))
+          diagnostic = warning_diagnostic(warning, offset_cache)
+          diagnostics.process(diagnostic) if diagnostic
         end
 
         result

lib/prism/translation/parser/lexer.rb

@@ -213,9 +213,11 @@ def initialize(source_buffer, lexed, offset_cache)
         # Convert the prism tokens into the expected format for the parser gem.
         def to_a
           tokens = []
+
           index = 0
+          length = lexed.length
 
-          while index < lexed.length
+          while index < length
             token, state = lexed[index]
             index += 1
             next if %i[IGNORED_NEWLINE __END__ EOF].include?(token.type)
@@ -229,14 +231,18 @@ def to_a
               value.delete_prefix!("?")
             when :tCOMMENT
               if token.type == :EMBDOC_BEGIN
-                until (next_token = lexed[index][0]) && next_token.type == :EMBDOC_END
+                start_index = index
+
+                while !((next_token = lexed[index][0]) && next_token.type == :EMBDOC_END) && (index < length - 1)
                   value += next_token.value
                   index += 1
                 end
 
-                value += next_token.value
-                location = Range.new(source_buffer, offset_cache[token.location.start_offset], offset_cache[lexed[index][0].location.end_offset])
-                index += 1
+                if start_index != index
+                  value += next_token.value
+                  location = Range.new(source_buffer, offset_cache[token.location.start_offset], offset_cache[lexed[index][0].location.end_offset])
+                  index += 1
+                end
               else
                 value.chomp!
                 location = Range.new(source_buffer, offset_cache[token.location.start_offset], offset_cache[token.location.end_offset - 1])

templates/src/diagnostic.c.erb

@@ -331,6 +331,9 @@ pm_diagnostic_id_human(pm_diagnostic_id_t diag_id) {
         case PM_WARN_<%= warning.name %>: return "<%= warning.name.downcase %>";
         <%- end -%>
     }
+
+    assert(false && "unreachable");
+    return "";
 }
 
 static inline const char *

test/prism/parser_test.rb

@@ -95,21 +95,6 @@ class ParserTest < TestCase
       end
     end
 
-    def test_warnings
-      buffer = Parser::Source::Buffer.new("inline ruby with warning", 1)
-      buffer.source = "do_something *array"
-
-      parser = Prism::Translation::Parser33.new
-      parser.diagnostics.all_errors_are_fatal = false
-
-      warning = nil
-      parser.diagnostics.consumer = ->(received) { warning = received }
-      parser.parse(buffer)
-
-      assert_equal :warning, warning.level
-      assert_includes warning.message, "has been interpreted as"
-    end
-
     private
 
     def assert_equal_parses(filepath, compare_tokens: true)
@@ -186,8 +171,6 @@ def assert_equal_tokens(expected_tokens, actual_tokens)
             actual_token[0] = expected_token[0] if %i[kDO_BLOCK kDO_LAMBDA].include?(expected_token[0])
           when :tLPAREN
             actual_token[0] = expected_token[0] if expected_token[0] == :tLPAREN2
-          when :tLCURLY
-            actual_token[0] = expected_token[0] if %i[tLBRACE tLBRACE_ARG].include?(expected_token[0])
           when :tPOW
             actual_token[0] = expected_token[0] if expected_token[0] == :tDSTAR
           end