Skip to content
Draft
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
47 changes: 47 additions & 0 deletions fixtures/small/heredoc_modifier_conditional_actual.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<<~HEREDOC if Method.call
text
HEREDOC

<<~HEREDOC unless Method.call
text
HEREDOC

<<~HEREDOC if Method.call || other_condition
text
HEREDOC

<<~HEREDOC if other_condition && Method.call
text
HEREDOC

x = <<~HEREDOC if Method.call
text
HEREDOC

@y = <<~HEREDOC if Method.call
text
HEREDOC

<<~HEREDOC while x.foo
text
HEREDOC

<<~HEREDOC until x.foo
text
HEREDOC

something if <<~PRED.empty?
pred
PRED

<<~BODY if <<~PRED.empty?
body
BODY
pred
PRED

puts(<<~OUT) if <<~PRED.include?("yes")
output
OUT
is this enabled?
PRED
70 changes: 70 additions & 0 deletions fixtures/small/heredoc_modifier_conditional_expected.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
if Method.call
<<~HEREDOC
text
HEREDOC
end

unless Method.call
<<~HEREDOC
text
HEREDOC
end

if Method.call || other_condition
<<~HEREDOC
text
HEREDOC
end

if other_condition && Method.call
<<~HEREDOC
text
HEREDOC
end

if Method.call
x = <<~HEREDOC
text
HEREDOC
end

if Method.call
@y = <<~HEREDOC
text
HEREDOC
end

<<~HEREDOC while x.foo
text
HEREDOC

<<~HEREDOC until x.foo
text
HEREDOC

if <<~PRED
pred
PRED
.empty?
something
end

if <<~PRED
pred
PRED
.empty?
<<~BODY
body
BODY
end

if <<~PRED
is this enabled?
PRED
.include?("yes")
puts(
<<~OUT
output
OUT
)
end
7 changes: 6 additions & 1 deletion librubyfmt/src/format_prism.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3767,7 +3767,12 @@ fn format_inline_conditional<'src>(
}
ps.emit_conditional_keyword(keyword);
ps.emit_space();
ps.with_start_of_line(false, |ps| format_node(ps, predicate));
// Hide any pending heredocs (e.g. a heredoc statement before `if cond.foo`)
// so an eager `render_heredocs` triggered while formatting the predicate
// doesn't render the heredoc body into the predicate's output.
ps.with_preserved_pending_heredocs(|ps| {
ps.with_start_of_line(false, |ps| format_node(ps, predicate));
});
}

enum Conditional<'pr> {
Expand Down
2 changes: 1 addition & 1 deletion librubyfmt/src/line_tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ impl<'src> AbstractLineToken<'src> {
}
}

fn write_heredocs(
pub(crate) fn write_heredocs(
heredoc_strings: Option<Vec<HeredocString>>,
out: &mut Vec<ConcreteLineTokenAndTargets<'src>>,
) {
Expand Down
39 changes: 36 additions & 3 deletions librubyfmt/src/parser_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -859,6 +859,21 @@ impl<'src> ParserState<'src> {
!self.heredoc_strings.is_empty()
}

/// Run `f` with the current pending heredocs hidden, then restore them
/// (followed by any heredocs `f` itself queued). This prevents a
/// `render_heredocs` call inside `f` from accidentally draining heredocs
/// that belong to an earlier formatting phase.
pub(crate) fn with_preserved_pending_heredocs<F>(&mut self, f: F)
where
F: FnOnce(&mut ParserState<'src>),
{
let saved = std::mem::take(&mut self.heredoc_strings);
f(self);
let from_f = std::mem::take(&mut self.heredoc_strings);
self.heredoc_strings = saved;
self.heredoc_strings.extend(from_f);
}

pub(crate) fn write<W: Write>(self, writer: &mut W) -> io::Result<()> {
let rqw = RenderQueueWriter::new(self.consume_to_render_queue());
rqw.write(writer)
Expand Down Expand Up @@ -964,17 +979,35 @@ impl<'src> ParserState<'src> {
ps.new_block(format_statement);
});

self.breakable_entry_stack
// Pull any heredocs the statement queued onto the entry. Otherwise a
// downstream `render_heredocs` call during predicate formatting
// (e.g. the eager one in `format_call_body` for `Method.call`)
// would splice the body into the predicate phase's token bucket.
let statement_heredocs = std::mem::take(&mut self.heredoc_strings);
let entry = self
.breakable_entry_stack
.last_mut()
.expect("just pushed InlineConditional")
.as_conditional_layout_mut()
.expect("just pushed InlineConditional")
.switch_to_predicate();
.expect("just pushed InlineConditional");
entry.gather_heredocs(statement_heredocs);
entry.switch_to_predicate();

self.with_start_of_line(false, |ps| {
ps.new_block(format_predicate);
});

// Any heredocs left after predicate formatting belong to the
// predicate phase (most predicate-side heredocs get rendered
// eagerly by call-chain formatting, so this is usually empty).
let predicate_heredocs = std::mem::take(&mut self.heredoc_strings);
self.breakable_entry_stack
.last_mut()
.expect("just pushed InlineConditional")
.as_conditional_layout_mut()
.expect("just pushed InlineConditional")
.gather_heredocs(predicate_heredocs);

let cle = self
.breakable_entry_stack
.pop()
Expand Down
36 changes: 36 additions & 0 deletions librubyfmt/src/render_targets.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::delimiters::BreakableDelims;
use crate::heredoc_string::HeredocString;
use crate::line_tokens::{AbstractLineToken, ConcreteLineToken, ConcreteLineTokenAndTargets};
use crate::parser_state::FormattingContext;
use crate::types::LineNumber;
Expand Down Expand Up @@ -521,6 +522,8 @@ pub enum ConditionalLayoutPhase {
pub struct ConditionalLayoutEntry<'src> {
predicate_tokens: Vec<AbstractLineToken<'src>>,
statement_tokens: Vec<AbstractLineToken<'src>>,
statement_heredocs: Vec<HeredocString<'src>>,
predicate_heredocs: Vec<HeredocString<'src>>,
keyword: &'static [u8],
indent_depth: u32,
phase: ConditionalLayoutPhase,
Expand All @@ -531,12 +534,21 @@ impl<'src> ConditionalLayoutEntry<'src> {
ConditionalLayoutEntry {
predicate_tokens: Vec::new(),
statement_tokens: Vec::new(),
statement_heredocs: Vec::new(),
predicate_heredocs: Vec::new(),
keyword,
indent_depth,
phase: ConditionalLayoutPhase::Statement,
}
}

pub fn gather_heredocs(&mut self, heredocs: Vec<HeredocString<'src>>) {
match self.phase {
ConditionalLayoutPhase::Statement => self.statement_heredocs.extend(heredocs),
ConditionalLayoutPhase::Predicate => self.predicate_heredocs.extend(heredocs),
}
}

pub fn push(&mut self, token: AbstractLineToken<'src>) {
match self.phase {
ConditionalLayoutPhase::Predicate => self.predicate_tokens.push(token),
Expand Down Expand Up @@ -648,6 +660,12 @@ impl<'src> ConditionalLayoutEntry<'src> {
}

pub fn should_use_block_form(&self, current_line_length: usize) -> bool {
// A statement-side heredoc body makes the conditional inherently
// multi-line, so promote to `if … end` block form. (Predicate-side
// heredocs alone don't force this — the predicate is just longer.)
if !self.statement_heredocs.is_empty() {
return true;
}
self.is_multiline()
|| current_line_length + self.inline_single_line_len()
> crate::render_queue_writer::MAX_LINE_LENGTH
Expand All @@ -673,6 +691,18 @@ impl<'src> ConditionalLayoutEntry<'src> {
token.write_single_line(&mut result);
}

// Heredocs gathered during either phase render after the inline
// line — statement heredocs first, then predicate heredocs (source
// order). We don't expect to reach inline form when there are
// statement heredocs (`should_use_block_form` forces block) but
// handle it defensively.
if !self.statement_heredocs.is_empty() || !self.predicate_heredocs.is_empty() {
result.push(ConcreteLineToken::HardNewLine.into());
let mut combined = self.statement_heredocs;
combined.extend(self.predicate_heredocs);
AbstractLineToken::write_heredocs(Some(combined), &mut result);
}

result
}

Expand All @@ -692,6 +722,9 @@ impl<'src> ConditionalLayoutEntry<'src> {
}

result.push(ConcreteLineToken::HardNewLine.into());
if !self.predicate_heredocs.is_empty() {
AbstractLineToken::write_heredocs(Some(self.predicate_heredocs), &mut result);
}
result.push(
ConcreteLineToken::Indent {
depth: self.indent_depth + 2,
Expand All @@ -704,6 +737,9 @@ impl<'src> ConditionalLayoutEntry<'src> {
}

result.push(ConcreteLineToken::HardNewLine.into());
if !self.statement_heredocs.is_empty() {
AbstractLineToken::write_heredocs(Some(self.statement_heredocs), &mut result);
}
result.push(
ConcreteLineToken::Indent {
depth: self.indent_depth,
Expand Down