diff --git a/fixtures/small/heredoc_modifier_conditional_actual.rb b/fixtures/small/heredoc_modifier_conditional_actual.rb new file mode 100644 index 00000000..a2850048 --- /dev/null +++ b/fixtures/small/heredoc_modifier_conditional_actual.rb @@ -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 diff --git a/fixtures/small/heredoc_modifier_conditional_expected.rb b/fixtures/small/heredoc_modifier_conditional_expected.rb new file mode 100644 index 00000000..6ba3e6ee --- /dev/null +++ b/fixtures/small/heredoc_modifier_conditional_expected.rb @@ -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 diff --git a/librubyfmt/src/format_prism.rs b/librubyfmt/src/format_prism.rs index 13b65e06..76a7cede 100644 --- a/librubyfmt/src/format_prism.rs +++ b/librubyfmt/src/format_prism.rs @@ -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> { diff --git a/librubyfmt/src/line_tokens.rs b/librubyfmt/src/line_tokens.rs index aadf4f78..fb0aa117 100644 --- a/librubyfmt/src/line_tokens.rs +++ b/librubyfmt/src/line_tokens.rs @@ -337,7 +337,7 @@ impl<'src> AbstractLineToken<'src> { } } - fn write_heredocs( + pub(crate) fn write_heredocs( heredoc_strings: Option>, out: &mut Vec>, ) { diff --git a/librubyfmt/src/parser_state.rs b/librubyfmt/src/parser_state.rs index 2261886b..d09a7712 100644 --- a/librubyfmt/src/parser_state.rs +++ b/librubyfmt/src/parser_state.rs @@ -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(&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(self, writer: &mut W) -> io::Result<()> { let rqw = RenderQueueWriter::new(self.consume_to_render_queue()); rqw.write(writer) @@ -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() diff --git a/librubyfmt/src/render_targets.rs b/librubyfmt/src/render_targets.rs index 4af82843..174820a2 100644 --- a/librubyfmt/src/render_targets.rs +++ b/librubyfmt/src/render_targets.rs @@ -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; @@ -521,6 +522,8 @@ pub enum ConditionalLayoutPhase { pub struct ConditionalLayoutEntry<'src> { predicate_tokens: Vec>, statement_tokens: Vec>, + statement_heredocs: Vec>, + predicate_heredocs: Vec>, keyword: &'static [u8], indent_depth: u32, phase: ConditionalLayoutPhase, @@ -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>) { + 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), @@ -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 @@ -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 } @@ -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, @@ -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,