Fix dstr unparsing

* This is an entirely new approach.
* Instead to find the "correct" dstr segments we simply try all and unparse the first one
  that round trips.
* This so far guarantees we always get good concrete syntax, but it can be time intensive as
  the combinatoric space of possible dynamic string sequence is quadratic with the dstr children size.
* For this reason we try above (currently) dstr children to unparse as heredoc first.
* Passes the entire corpus and fixes bugs.

[fix #249]

Author: mbj

Gemfile

@@ -2,4 +2,6 @@
 
 source 'https://rubygems.org'
 
+gem 'mutant', path: '../mutant'
+
 gemspec

Gemfile.lock

@@ -1,3 +1,13 @@
+PATH
+  remote: ../mutant
+  specs:
+    mutant (0.12.4)
+      diff-lcs (~> 1.3)
+      parser (~> 3.3.0)
+      regexp_parser (~> 2.9.0)
+      sorbet-runtime (~> 0.5.0)
+      unparser (~> 0.6.14)
+
 PATH
   remote: .
   specs:
@@ -12,14 +22,8 @@ GEM
     diff-lcs (1.5.1)
     json (2.7.2)
     language_server-protocol (3.17.0.3)
-    mutant (0.12.3)
-      diff-lcs (~> 1.3)
-      parser (~> 3.3.0)
-      regexp_parser (~> 2.9.0)
-      sorbet-runtime (~> 0.5.0)
-      unparser (~> 0.6.14)
-    mutant-rspec (0.12.3)
-      mutant (= 0.12.3)
+    mutant-rspec (0.12.4)
+      mutant (= 0.12.4)
       rspec-core (>= 3.8.0, < 4.0.0)
     parallel (1.25.1)
     parser (3.3.2.0)
@@ -70,7 +74,7 @@ PLATFORMS
   ruby
 
 DEPENDENCIES
-  mutant (~> 0.12.2)
+  mutant!
   mutant-rspec (~> 0.12.2)
   rspec (~> 3.9)
   rspec-core (~> 3.9)

bin/corpus

@@ -1,10 +1,15 @@
 #!/usr/bin/env ruby
 # frozen_string_literal: true
 
+require 'etc'
 require 'mutant'
 require 'optparse'
+require 'pathname'
 require 'unparser'
 
+Thread.abort_on_exception = true
+Thread.report_on_exception = true
+
 module Unparser
   module Corpus
     ROOT = Pathname.new(__dir__).parent
@@ -17,16 +22,85 @@ module Unparser
       #
       # @return [Boolean]
       def verify
+        puts("Verifiying: #{name}")
         checkout
-        command = %W[unparser #{repo_path}]
-        exclude.each do |name|
-          command.push('--ignore', repo_path.join(name).to_s)
+
+        paths = Pathname.glob(Pathname.new(repo_path).join('**/*.rb'))
+
+        driver = Mutant::Parallel.async(
+          config: Mutant::Parallel::Config.new(
+            block:            method(:verify_path),
+            jobs:             Etc.nprocessors,
+            on_process_start: ->(*) {},
+            process_name:     'unparser-corpus-test',
+            sink:             Sink.new,
+            source:           Mutant::Parallel::Source::Array.new(jobs: paths),
+            thread_name:      'unparser-corpus-test',
+            timeout:          nil
+          ),
+          world:  Mutant::WORLD
+        )
+
+        loop do
+          status = driver.wait_timeout(1)
+
+          puts("Processed: #{status.payload.total}")
+
+          status.payload.errors.each do |report|
+            puts report
+            fail
+          end
+
+          break if status.done?
         end
-        Kernel.system(*command)
+
+        true
       end
 
     private
 
+      class Sink
+        include Mutant::Parallel::Sink
+
+        attr_reader :errors, :total
+
+        def initialize
+          @errors = []
+          @total  = 0
+        end
+
+        def stop?
+          !@errors.empty?
+        end
+
+        def status
+          self
+        end
+
+        def response(response)
+          if response.error
+            Mutant::WORLD.stderr.puts(response.log)
+            fail response.error
+          end
+
+          @total += 1
+
+          if response.result
+            @errors << response.result
+          end
+        end
+      end
+
+      def verify_path(path)
+        validation = Validation.from_path(path)
+
+        if original_syntax_error?(validation) || generated_encoding_error?(validation) || validation.success?
+          return
+        end
+
+        validation.report
+      end
+
       def checkout
         TMP.mkdir unless TMP.directory?
 
@@ -50,6 +124,21 @@ module Unparser
         TMP.join(name)
       end
 
+    private
+
+      # This happens if the original source contained a non UTF charset meta comment.
+      # These are not exposed to the AST in a way unparser could know about to generate a non UTF-8
+      # target and emit that meta comment itself.
+      # For the purpose of corpus testing these cases are ignored.
+      def generated_encoding_error?(validation)
+        exception = validation.generated_node.from_left { return false }
+        exception.instance_of?(Parser::SyntaxError) && exception.message.eql?('literal contains escape sequences incompatible with UTF-8')
+      end
+
+      def original_syntax_error?(validation)
+        validation.original_node.from_left { return false }.instance_of?(Parser::SyntaxError)
+      end
+
       def system(arguments)
         return if Kernel.system(*arguments)
 

bin/parser-round-trip-test

@@ -40,7 +40,8 @@ class Test
     :rubies
   )
 
-  EXPECT_FAILURE = {}.freeze
+  STATIC_LOCAL_VARIABLES = %i[foo bar baz].freeze
+  EXPECT_FAILURE         = {}.freeze
 
   def legacy_attributes
     default_builder_attributes.reject do |attribute_name, value|
@@ -77,9 +78,9 @@ class Test
 
   # rubocop:disable Metrics/AbcSize
   def validation
-    identification   = name.to_s
+    identification = name.to_s
 
-    generated_source = Unparser.unparse_either(node)
+    generated_source = Unparser.unparse_either(node, static_local_variables: STATIC_LOCAL_VARIABLES)
       .fmap { |string| string.dup.force_encoding(parser_source.encoding).freeze }
 
     generated_node = generated_source.bind do |source|
@@ -99,7 +100,7 @@ class Test
 
   def parser
     Unparser.parser.tap do |parser|
-      %w[foo bar baz].each(&parser.static_env.method(:declare))
+      STATIC_LOCAL_VARIABLES.each(&parser.static_env.method(:declare))
     end
   end
 

lib/unparser.rb

@@ -30,6 +30,11 @@ def initialize
     end
   end
 
+  # Parsed AST with additional details
+  class AST
+    include Anima.new(:comments, :explicit_encoding, :node, :static_local_variables)
+  end
+
   EMPTY_STRING = ''.freeze
   EMPTY_ARRAY  = [].freeze
 
@@ -44,39 +49,85 @@ def initialize(message, node)
       @node = node
       freeze
     end
-  end
+  end # InvalidNodeError
+
+  # Error raised when unparser encounders AST it cannot generate source for that would parse to the same AST.
+  class UnsupportedNodeError < RuntimeError
+  end # UnsupportedNodeError
 
   # Unparse an AST (and, optionally, comments) into a string
   #
   # @param [Parser::AST::Node, nil] node
-  # @param [Array] comment_array
+  # @param [Array] comments
+  # @param [Encoding, nil] explicit_encoding
+  # @param [Set<Symbol>] static_local_variables
   #
   # @return [String]
   #
   # @raise InvalidNodeError
   #   if the node passed is invalid
   #
   # @api public
-  def self.unparse(node, comment_array = [])
-    return '' if node.nil?
+  def self.unparse(node, comments: EMPTY_ARRAY, explicit_encoding: nil, static_local_variables: Set.new)
+    unparse_ast(
+      AST.new(
+        comments:               comments,
+        explicit_encoding:      explicit_encoding,
+        node:                   node,
+        static_local_variables: static_local_variables
+      )
+    )
+  end
+
+  # Unparse an AST
+  #
+  # @param [AST] ast
+  #
+  # @return [String]
+  #
+  # @raise InvalidNodeError
+  #   if the node passed is invalid
+  #
+  # @raise UnsupportedNodeError
+  #   if the node passed is valid but unparser cannot unparse it
+  #
+  # @api public
+  def self.unparse_ast(ast)
+    return EMPTY_STRING if ast.node.nil?
+
+    local_variable_scope = AST::LocalVariableScope.new(
+      node:                   ast.node,
+      static_local_variables: ast.static_local_variables
+    )
 
     Buffer.new.tap do |buffer|
       Emitter::Root.new(
-        buffer,
-        node,
-        Comments.new(comment_array)
+        buffer:               buffer,
+        comments:             Comments.new(ast.comments),
+        explicit_encoding:    ast.explicit_encoding,
+        local_variable_scope: local_variable_scope,
+        node:                 ast.node
       ).write_to_buffer
     end.content
   end
 
+  # Unparse AST either
+  #
+  # @param [AST] ast
+  #
+  # @return [Either<Exception,String>]
+  def self.unparse_ast_either(ast)
+    Either.wrap_error(Exception) { unparse_ast(ast) }
+  end
+
   # Unparse with validation
   #
   # @param [Parser::AST::Node, nil] node
-  # @param [Array] comment_array
+  # @param [Array] comments
   #
   # @return [Either<Validation,String>]
-  def self.unparse_validate(node, comment_array = [])
-    generated = unparse(node, comment_array)
+  def self.unparse_validate(node, comments: EMPTY_ARRAY)
+    generated = unparse(node, comments)
     validation = Validation.from_string(generated)
 
     if validation.success?
@@ -86,44 +137,41 @@ def self.unparse_validate(node, comment_array = [])
     end
   end
 
-  # Unparse capturing errors
-  #
-  # This is mostly useful for writing testing tools against unparser.
-  #
-  # @param [Parser::AST::Node, nil] node
-  #
-  # @return [Either<Exception, String>]
-  def self.unparse_either(node)
-    Either.wrap_error(Exception) { unparse(node) }
-  end
-
   # Parse string into AST
   #
   # @param [String] source
   #
   # @return [Parser::AST::Node, nil]
-  def self.parse(source)
-    parser.parse(buffer(source))
+  def self.parse(source, identification: '(string)')
+    parse_ast(source, identification: identification).node
   end
 
   # Parse string into either syntax error or AST
   #
   # @param [String] source
   #
-  # @return [Either<Parser::SyntaxError, (Parser::ASTNode, nil)>]
-  def self.parse_either(source)
-    Either.wrap_error(Parser::SyntaxError) do
-      parser.parse(buffer(source))
+  # @return [Either<Exception, (Parser::ASTNode, nil)>]
+  def self.parse_ast_either(source, identification: '(source)')
+    Either.wrap_error(Exception) do
+      parse_ast(source, identification: identification)
     end
   end
 
-  # Parse string into AST, with comments
+  # Parse source with ast details
   #
   # @param [String] source
   #
-  # @return [Parser::AST::Node]
-  def self.parse_with_comments(source)
-    parser.parse_with_comments(buffer(source))
+  # @return [AST]
+  def self.parse_ast(source, identification: '(source)', static_local_variables: Set.new)
+    explicit_encoding = Parser::Source::Buffer.recognize_encoding(source.dup.force_encoding(Encoding::BINARY))
+    node, comments = parser.parse_with_comments(buffer(source, identification: identification))
+
+    AST.new(
+      comments:               comments,
+      explicit_encoding:      explicit_encoding,
+      node:                   node,
+      static_local_variables: static_local_variables
+    )
   end
 
   # Parser instance that produces AST unparser understands
@@ -210,6 +258,7 @@ def self.buffer(source, identification = '(string)')
 require 'unparser/emitter/root'
 require 'unparser/emitter/send'
 require 'unparser/emitter/simple'
+require 'unparser/emitter/string'
 require 'unparser/emitter/splat'
 require 'unparser/emitter/super'
 require 'unparser/emitter/undef'
@@ -224,6 +273,7 @@ def self.buffer(source, identification = '(string)')
 require 'unparser/writer'
 require 'unparser/writer/binary'
 require 'unparser/writer/dynamic_string'
+require 'unparser/writer/regexp'
 require 'unparser/writer/resbody'
 require 'unparser/writer/rescue'
 require 'unparser/writer/send'