Combine the LambdaVar and BlockVar nodes

Both `LambdaVar` and `BlockVar` represent a collection of arguments
(positional, optional, keyword, block) as well as block or lambda-local
variables. This commit removes the `LambdaVar` node and folds its
functionality into the `BlockVar` node, which reduces the number of
nodes and simplifies logic.

Author: egiurleo

lib/syntax_tree/node.rb

@@ -2131,13 +2131,18 @@ def ===(other)
     end
   end
 
-  # BlockVar represents the parameters being declared for a block. Effectively
-  # this node is everything contained within the pipes. This includes all of the
-  # various parameter types, as well as block-local variable declarations.
+  # BlockVar represents the parameters being declared for a block or lambda.
+  # This includes all of the various parameter types, as well as
+  # block-local/lambda-local variable declarations.
   #
   #     method do |positional, optional = value, keyword:, █ local|
   #     end
   #
+  #     OR
+  #
+  #    -> (positional, optional = value, keyword:, █ local) do
+  #    end
+  #
   class BlockVar < Node
     # [Params] the parameters being declared with the block
     attr_reader :params
@@ -2148,10 +2153,15 @@ class BlockVar < Node
     # [Array[ Comment | EmbDoc ]] the comments attached to this node
     attr_reader :comments
 
-    def initialize(params:, locals:, location:)
+    # [boolean] whether or not the variables are within pipes
+    attr_reader :pipe
+    alias pipe? pipe
+
+    def initialize(params:, locals:, location:, pipe:)
       @params = params
       @locals = locals
       @location = location
+      @pipe = pipe
       @comments = []
     end
 
@@ -2163,12 +2173,13 @@ def child_nodes
       [params, *locals]
     end
 
-    def copy(params: nil, locals: nil, location: nil)
+    def copy(params: nil, locals: nil, location: nil, pipe: nil)
       node =
         BlockVar.new(
           params: params || self.params,
           locals: locals || self.locals,
-          location: location || self.location
+          location: location || self.location,
+          pipe: pipe || self.pipe
         )
 
       node.comments.concat(comments.map(&:copy))
@@ -2178,7 +2189,13 @@ def copy(params: nil, locals: nil, location: nil)
     alias deconstruct child_nodes
 
     def deconstruct_keys(_keys)
-      { params: params, locals: locals, location: location, comments: comments }
+      {
+        params: params,
+        locals: locals,
+        location: location,
+        comments: comments,
+        pipe: pipe
+      }
     end
 
     # Within the pipes of the block declaration, we don't want any spaces. So
@@ -2194,23 +2211,23 @@ def call(q)
     SEPARATOR = Separator.new.freeze
 
     def format(q)
-      q.text("|")
-      q.group do
-        q.remove_breaks(q.format(params))
+      pipe? ? q.remove_breaks(q.format(params)) : q.format(params)
 
-        if locals.any?
-          q.text("; ")
-          q.seplist(locals, SEPARATOR) { |local| q.format(local) }
-        end
+      if locals.any?
+        q.text("; ")
+        q.seplist(locals, SEPARATOR) { |local| q.format(local) }
       end
-      q.text("|")
     end
 
     def ===(other)
       other.is_a?(BlockVar) && params === other.params &&
         ArrayMatch.call(locals, other.locals)
     end
 
+    def empty?
+      params.empty? && locals.empty?
+    end
+
     # When a single required parameter is declared for a block, it gets
     # automatically expanded if the values being yielded into it are an array.
     def arg0?
@@ -4486,8 +4503,9 @@ def format_break(q, break_opening, break_closing)
       q.format(BlockOpenFormatter.new(break_opening, opening), stackable: false)
 
       if block_var
-        q.text(" ")
-        q.format(block_var)
+        q.text(" |")
+        q.group { q.format(block_var) }
+        q.text("|")
       end
 
       unless bodystmt.empty?
@@ -4507,7 +4525,9 @@ def format_flat(q, flat_opening, flat_closing)
 
       if block_var
         q.breakable_space
-        q.format(block_var)
+        q.text("|")
+        q.group { q.format(block_var) }
+        q.text("|")
         q.breakable_space
       end
 
@@ -7140,7 +7160,7 @@ def ===(other)
   #     ->(value) { value * 2 }
   #
   class Lambda < Node
-    # [LambdaVar | Paren] the parameter declaration for this lambda
+    # [BlockVar | Paren] the parameter declaration for this lambda
     attr_reader :params
 
     # [BodyStmt | Statements] the expressions to be executed in this lambda
@@ -7258,76 +7278,6 @@ def ===(other)
     end
   end
 
-  # LambdaVar represents the parameters being declared for a lambda. Effectively
-  # this node is everything contained within the parentheses. This includes all
-  # of the various parameter types, as well as block-local variable
-  # declarations.
-  #
-  #     -> (positional, optional = value, keyword:, &block; local) do
-  #     end
-  #
-  class LambdaVar < Node
-    # [Params] the parameters being declared with the block
-    attr_reader :params
-
-    # [Array[ Ident ]] the list of block-local variable declarations
-    attr_reader :locals
-
-    # [Array[ Comment | EmbDoc ]] the comments attached to this node
-    attr_reader :comments
-
-    def initialize(params:, locals:, location:)
-      @params = params
-      @locals = locals
-      @location = location
-      @comments = []
-    end
-
-    def accept(visitor)
-      visitor.visit_lambda_var(self)
-    end
-
-    def child_nodes
-      [params, *locals]
-    end
-
-    def copy(params: nil, locals: nil, location: nil)
-      node =
-        LambdaVar.new(
-          params: params || self.params,
-          locals: locals || self.locals,
-          location: location || self.location
-        )
-
-      node.comments.concat(comments.map(&:copy))
-      node
-    end
-
-    alias deconstruct child_nodes
-
-    def deconstruct_keys(_keys)
-      { params: params, locals: locals, location: location, comments: comments }
-    end
-
-    def empty?
-      params.empty? && locals.empty?
-    end
-
-    def format(q)
-      q.format(params)
-
-      if locals.any?
-        q.text("; ")
-        q.seplist(locals, BlockVar::SEPARATOR) { |local| q.format(local) }
-      end
-    end
-
-    def ===(other)
-      other.is_a?(LambdaVar) && params === other.params &&
-        ArrayMatch.call(locals, other.locals)
-    end
-  end
-
   # LBrace represents the use of a left brace, i.e., {.
   class LBrace < Node
     # [String] the left brace

lib/syntax_tree/parser.rb

@@ -937,7 +937,8 @@ def on_block_var(params, locals)
       BlockVar.new(
         params: params,
         locals: locals || [],
-        location: beginning.location.to(ending.location)
+        location: beginning.location.to(ending.location),
+        pipe: true
       )
     end
 
@@ -2296,10 +2297,11 @@ def on_lambda(params, statements)
             Paren.new(
               lparen: params.lparen,
               contents:
-                LambdaVar.new(
+                BlockVar.new(
                   params: params.contents,
                   locals: locals,
-                  location: location
+                  location: location,
+                  pipe: false
                 ),
               location: params.location
             )
@@ -2324,8 +2326,13 @@ def on_lambda(params, statements)
 
           # In this case we've gotten to the plain set of parameters. In this
           # case there cannot be lambda locals, so we will wrap the parameters
-          # into a lambda var that has no locals.
-          LambdaVar.new(params: params, locals: [], location: params.location)
+          # into a block var that has no locals.
+          BlockVar.new(
+            params: params,
+            locals: [],
+            location: params.location,
+            pipe: false
+          )
         end
 
       start_char = find_next_statement_start(opening.location.end_char)
@@ -2345,12 +2352,17 @@ def on_lambda(params, statements)
     end
 
     # :call-seq:
-    #   on_lambda_var: (Params params, Array[ Ident ] locals) -> LambdaVar
+    #   on_lambda_var: (Params params, Array[ Ident ] locals) -> BlockVar
     def on_lambda_var(params, locals)
       location = params.location
       location = location.to(locals.last.location) if locals.any?
 
-      LambdaVar.new(params: params, locals: locals || [], location: location)
+      BlockVar.new(
+        params: params,
+        locals: locals || [],
+        location: location,
+        pipe: false
+      )
     end
 
     # Ripper doesn't support capturing lambda local variables until 3.2. To

test/node_test.rb

@@ -635,6 +635,25 @@ def test_lambda
       source = "->(value) { value * 2 }"
 
       assert_node(Lambda, source)
+
+      at = location(chars: 3..8)
+      assert_node(BlockVar, source, at: at) { |node| node.params.contents }
+    end
+
+    def test_lambda_no_parens
+      source = "-> value { value * 2 }"
+
+      assert_node(Lambda, source)
+
+      at = location(chars: 3..8)
+      assert_node(BlockVar, source, at: at, &:params)
+    end
+
+    def test_lambda_braces
+      source = "lambda { |value| value * 2 }"
+
+      at = location(chars: 9..16)
+      assert_node(BlockVar, source, at: at) { |node| node.block.block_var }
     end
 
     def test_lambda_do