Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
17 changes: 13 additions & 4 deletions lib/ast_transform/layout.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ module ASTTransform
class Layout
def initialize
@lines = []
@sealed = false
end

# The line number currently being written; the next fresh line would be +cursor + 1+.
Expand All @@ -21,7 +22,10 @@ def cursor
# +column+ — cosmetic only (leading whitespace is never significant in emitted code), but it keeps the artifact
# visually close to the source. Packed text ignores the column, as do continuation lines (they keep their own
# relative indentation).
def place(target_line, text, column: nil)
#
# +seal+ declares the text's last line unextendable (the caller's property — e.g. it terminates a heredoc,
# whose terminator must stand alone): a subsequent pack opens a fresh line instead of joining it.
def place(target_line, text, column: nil, seal: false)
first, *rest = text.split("\n")

if target_line && target_line > @lines.size
Expand All @@ -32,22 +36,27 @@ def place(target_line, text, column: nil)
end

@lines.concat(rest)
@sealed = seal
end

# Appends +text+ on a new line unconditionally — for text that must never be `;`-packed after a statement
# (e.g. keywords).
def place_on_fresh_line(text)
@lines << text
@sealed = false
end

# Appends +text+ to the current line with a `; ` separator. The last line is never blank here: padding blanks
# are only created inside +place+, which immediately overwrites the padded line.
# Appends +text+ to the current line with a `; ` separator — unless the current line is sealed, in which case
# the text opens a fresh line (alignment degrades by one more line; validity is preserved). The last line is
# never blank here: padding blanks are only created inside +place+, which immediately overwrites the padded
# line.
def pack(text)
if @lines.empty?
if @lines.empty? || @sealed
@lines << text
else
@lines[-1] = "#{@lines.last}; #{text}"
end
@sealed = false
end

# @return [String] the laid-out text, with a trailing newline.
Expand Down
7 changes: 5 additions & 2 deletions lib/ast_transform/line_aligned_emitter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ def emit_statement(node, layout, renderer)
if recursive_container?(node)
emit_container(node, layout, renderer)
else
layout.place(node.loc&.line, renderer.aligned_render(node), column: node.loc&.column)
render = renderer.aligned_render(node)
# A render in heredoc form seals its line: nothing may pack after the terminator (issue #16).
layout.place(node.loc&.line, render, column: node.loc&.column, seal: renderer.line_terminal?(render))
end
end

Expand Down Expand Up @@ -145,7 +147,8 @@ def block_assignment?(node)
# nested statements align.
def emit_container(node, layout, renderer)
opener, closer = container_delimiters(node, renderer)
layout.place(node.loc&.line, opener, column: node.loc&.column)
# An opener can carry a heredoc too (e.g. a block call with a heredoc argument) — seal it like a statement.
layout.place(node.loc&.line, opener, column: node.loc&.column, seal: renderer.line_terminal?(opener))
emit_body(container_body(node), layout, renderer)
layout.place(closer_line(node), closer, column: closer_column(node))
end
Expand Down
22 changes: 22 additions & 0 deletions lib/ast_transform/statement_renderer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,30 @@ def aligned_render(node)
compress_to_single_line(render) || render
end

# Whether +render+ must be the last text on its final line: a render ending on a heredoc terminator (Unparser
# falls back to `<<-HEREDOC` form when its quoted-string round-trip fails) cannot have anything `;`-packed
# after it — the terminator must stand alone, so a join unterminates the heredoc. Probed by re-parse, the same
# verification style as +compress_to_single_line+, behind a cheap textual gate: only a heredoc makes a last
# line unextendable, and every heredoc opener contains +<<+. A false positive (the probe failing for another
# reason, e.g. an isolated container opener carrying a heredoc argument) costs one line of alignment, never
# correctness.
def line_terminal?(render)
return false unless render.include?('<<')

# chomp: heredoc renders end with a trailing newline; the probe must extend the last *line* (where the
# layout would pack), not append below it.
!parses?("#{render.chomp}; nil")
end

private

def parses?(source)
Unparser.parse(source)
true
rescue Parser::SyntaxError
false
end

def compress_to_single_line(render)
candidate = render.split("\n").map(&:strip).join('; ')
# Both sides parsed without scope context, so lvar/send ambiguity cancels out; equality means the newline
Expand Down
25 changes: 25 additions & 0 deletions test/ast_transform/layout_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,31 @@ class LayoutTest < Minitest::Test
assert_equal "statement\nensure\n", layout.to_source
end

test "a sealed placement forces the next pack onto a fresh line" do
layout = Layout.new
layout.place(1, "value = <<-EOS\nbody\nEOS", seal: true)
layout.place(2, 'displaced')

assert_equal "value = <<-EOS\nbody\nEOS\ndisplaced\n", layout.to_source
end

test "the seal covers one pack: packing resumes on the fallback line" do
layout = Layout.new
layout.place(1, 'terminal', seal: true)
layout.place(1, 'first_displaced')
layout.place(1, 'second_displaced')

assert_equal "terminal\nfirst_displaced; second_displaced\n", layout.to_source
end

test "padding to a line ahead is unaffected by the seal" do
layout = Layout.new
layout.place(1, 'terminal', seal: true)
layout.place(3, 'ahead')

assert_equal "terminal\n\nahead\n", layout.to_source
end

test "pack onto an empty layout opens the first line" do
layout = Layout.new
layout.pack('lonely')
Expand Down
20 changes: 20 additions & 0 deletions test/ast_transform/line_aligned_emitter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,26 @@ def run; work
RUBY
end

# This dstr fails Unparser's quoted-string round-trip and re-renders as `<<-HEREDOC` form — three lines
# tall from a one-line source, displacing every following placement into packing mode (issue #16).
HEREDOC_FALLBACK_SOURCE = <<~'RUBY'
def write_learning(dir, slug, index_domain: "tooling")
line = "- [#{index_domain}/#{slug}] The #{slug} cue. → .cursor/skills/learnings/#{slug}/\n"
File.open(File.join(dir, INDEX), "a") { |index| index.write(line) }
end
RUBY

test "a render in heredoc form is line-terminal: displaced statements and closers never pack onto its terminator" do
emitted = emit(parse(HEREDOC_FALLBACK_SOURCE))

# Before the fix this was `HEREDOC; File.open(...); end` — an unterminated heredoc swallowing the file.
RubyVM::InstructionSequence.compile(emitted)
terminator_line = emitted.split("\n").find { |line| line.start_with?('HEREDOC') }

assert_equal 'HEREDOC', terminator_line
assert_includes emitted, 'File.open'
end

test "a thunk inside a def stays inside the def (scope boundary)" do
def_node = parse("def run\n helper\nend\n")
name, args, body = def_node.children
Expand Down
17 changes: 17 additions & 0 deletions test/ast_transform/statement_renderer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,23 @@ def parse(source)
assert_nil renderer.send(:compress_to_single_line, "value = <<~TXT\n hi\nTXT")
end

test "line_terminal? flags a render ending on a heredoc terminator" do
renderer = StatementRenderer.for_tree(parse("noop\n"))

assert renderer.line_terminal?("value = <<-HEREDOC\nbody\nHEREDOC")
end

test "line_terminal? passes renders whose last line can be extended" do
renderer = StatementRenderer.for_tree(parse("noop\n"))

refute renderer.line_terminal?('value = 1')
# `<<` appears without opening a heredoc: the shovel operator, and heredoc-looking text inside a string.
refute renderer.line_terminal?('queue << item')
refute renderer.line_terminal?('value = "<<-EOS"')
# A quoted multi-line string's last line closes the literal and stays extendable.
refute renderer.line_terminal?("value = \"a\nb\"")
end

test "for_tree collects assignments and every parameter flavor" do
tree = parse(<<~HEREDOC)
assigned = 1
Expand Down
Loading