Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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.
  • Loading branch information
egiurleo committed Mar 23, 2023
commit 4982b6bbdb3771791f955a9413a809e01068eb49
128 changes: 39 additions & 89 deletions lib/syntax_tree/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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))
Expand All @@ -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
Expand All @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I managed to move the first two lines of this method into BlockNode, but I"m having trouble with q.remove_breaks because the tests seem to expect that all params within pipes are on the same line. Any suggestions?


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?
Expand Down Expand Up @@ -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?
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
26 changes: 19 additions & 7 deletions lib/syntax_tree/parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
)
Expand All @@ -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)
Expand All @@ -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
Expand Down
19 changes: 19 additions & 0 deletions test/node_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down