Add context tracking to only emit begin keyword when needed

mbj · mbj · commit 1572099a3a95 · 2013-06-15T00:07:04.000+02:00
diff --git a/.rspec b/.rspec
@@ -1,5 +1,3 @@
 --color
 --format progress
---profile
 --order random
---fail-fast
diff --git a/config/flay.yml b/config/flay.yml
@@ -1,3 +1,3 @@
 ---
 threshold: 14
-total_score: 341
+total_score: 342
diff --git a/config/reek.yml b/config/reek.yml
@@ -12,7 +12,7 @@ UncommunicativeParameterName:
 TooManyInstanceVariables:
   exclude: []
   enabled: true
-  max_instance_variables: 2
+  max_instance_variables: 3
 TooManyMethods:
   exclude:
   - Unparser::Emitter # TODO: 13 methods, mostly helpers for deduplicate sublcasses 
@@ -27,7 +27,7 @@ UncommunicativeMethodName:
   - !ruby/regexp /[0-9]$/
   - !ruby/regexp /[A-Z]/
 LongParameterList:
-  max_params: 2
+  max_params: 3
   exclude: 
   - Unparser#self.transquote
   enabled: true
diff --git a/lib/unparser.rb b/lib/unparser.rb
@@ -82,5 +82,6 @@ def self.transquote(raw_quoted, current_delimiter, target_delimiter)
 require 'unparser/emitter/case'
 require 'unparser/emitter/for'
 require 'unparser/emitter/repetition'
+require 'unparser/emitter/root'
 
 Unparser::Emitter::REGISTRY.freeze
diff --git a/lib/unparser/emitter.rb b/lib/unparser/emitter.rb
@@ -2,7 +2,8 @@ module Unparser
 
   # Emitter base class
   class Emitter
-    include Adamantium::Flat, AbstractType, Equalizer.new(:node, :buffer), Constants
+    include Adamantium::Flat, AbstractType, Constants
+    include Equalizer.new(:node, :buffer, :parent)
 
     # Registry for node emitters
     REGISTRY = {}
@@ -62,8 +63,8 @@ def self.emit(*arguments)
     #
     # @api private
     #
-    def initialize(node, buffer)
-      @node, @buffer = node, buffer
+    def initialize(node, buffer, parent)
+      @node, @buffer, @parent = node, buffer, parent
       dispatch
     end
 
@@ -78,15 +79,29 @@ def initialize(node, buffer)
     #
     # @api private
     #
-    def self.visit(node, buffer)
+    def self.visit(node, buffer, parent = Root)
       type = node.type
       emitter = REGISTRY.fetch(type) do
         raise ArgumentError, "No emitter for node: #{type.inspect}"
       end
-      emitter.emit(node, buffer)
+      emitter.emit(node, buffer, parent)
       self
     end
 
+    # Test if node needs begin
+    #
+    # @return [true]
+    #   if if node needs begin
+    #
+    # @return [false]
+    #   otherwise
+    #
+    # @api private
+    #
+    def needs_begin?
+      false
+    end
+
     # Return node
     #
     # @return [Parser::AST::Node] node
@@ -106,6 +121,15 @@ def self.visit(node, buffer)
     attr_reader :buffer
     protected :buffer
 
+    # Return parent emitter
+    #
+    # @return [Parent]
+    #
+    # @api private
+    #
+    attr_reader :parent
+    protected :parent
+
     # Emit contents of block within parentheses
     #
     # @return [undefined]
@@ -137,7 +161,7 @@ def emit_source_map
     # @api private
     #
     def visit(node)
-      self.class.visit(node, buffer)
+      self.class.visit(node, buffer, self)
     end
 
     # Emit delimited body
diff --git a/lib/unparser/emitter/begin.rb b/lib/unparser/emitter/begin.rb
@@ -150,9 +150,13 @@ def dispatch
       # @api private
       #
       def emit_normal
-        k_begin
-        indented { emit_inner }
-        k_end
+        unless parent.needs_begin?
+          emit_inner
+        else
+          k_begin
+          indented { emit_inner }
+          k_end
+        end
       end
 
       # Emit inner nodes
diff --git a/lib/unparser/emitter/root.rb b/lib/unparser/emitter/root.rb
@@ -0,0 +1,7 @@
+module Unparser
+  class Emitter
+    Root = ::Class.new(self) do
+      def initialize(); end
+    end.send(:new)
+  end
+end
diff --git a/lib/unparser/emitter/send.rb b/lib/unparser/emitter/send.rb
@@ -14,6 +14,16 @@ class Send < self
 
       AMBIGOUS = [:irange, :erange].to_set.freeze
 
+      # Tell node needs begin
+      #
+      # @return [true]
+      #
+      # @api private
+      #
+      def needs_begin?
+        true
+      end
+
     private
 
       # Perform dispatch
@@ -95,9 +105,9 @@ def binary_receiver?
       # @api private
       #
       def run(emitter)
-        emitter.emit(node, buffer)
+        emitter.emit(node, buffer, self)
       end
-      
+
       # Perform non index dispatch
       #
       # @return [undefined]
@@ -136,13 +146,13 @@ def regular_dispatch
       def emit_receiver
         return unless first_child
         emit_unambiguous_receiver
-        write(O_DOT) 
+        write(O_DOT)
       end
 
       # Test for unary operator implemented as method
       #
       # @return [true]
-      #   if node is a unary operator 
+      #   if node is a unary operator
       #
       # @return [false]
       #   otherwise
@@ -156,7 +166,7 @@ def unary?
       # Test for binary operator implemented as method
       #
       # @return [true]
-      #   if node is a binary operator 
+      #   if node is a binary operator
       #
       # @return [false]
       #   otherwise
diff --git a/spec/integration/unparser/spike_spec.rb b/spec/integration/unparser/spike_spec.rb
@@ -288,10 +288,8 @@ def self.assert_source(input, versions = RUBIES)
     assert_generates s(:begin), 'nil'
 
     assert_source <<-RUBY
-      begin
-        foo
-        bar
-      end
+      foo
+      bar
     RUBY
 
     assert_source <<-RUBY
@@ -307,6 +305,7 @@ def self.assert_source(input, versions = RUBIES)
       begin
         foo
       ensure
+        bar
         baz
       end
     RUBY
@@ -321,12 +320,11 @@ def self.assert_source(input, versions = RUBIES)
 
     assert_source <<-RUBY
       begin
-        begin
-          foo
-          bar
-        end
+        foo
+        bar
       rescue
         baz
+        bar
       end
     RUBY
 
@@ -598,15 +596,16 @@ def foo(bar, &block)
       RUBY
 
       assert_source <<-RUBY
-        def initialize(attributes, options)
-          begin
-            @attributes = freeze_object(attributes)
-            @options = freeze_object(options)
-            @attribute_for = Hash[@attributes.map do |attribute|
-              attribute.name
-            end.zip(@attributes)]
-            @keys = coerce_keys
-          end
+        def foo
+          bar
+          baz
+        end
+      RUBY
+
+      assert_source <<-RUBY
+        def self.foo
+          bar
+          baz
         end
       RUBY
     end
diff --git a/spec/unit/unparser/emitter/class_methods/visit_spec.rb b/spec/unit/unparser/emitter/class_methods/visit_spec.rb
@@ -12,7 +12,7 @@
   end
 
   class Dummy < Unparser::Emitter
-    def self.emit(node, buffer)
+    def self.emit(node, buffer, parent)
       buffer.append('foo')
     end
   end
