diff --git a/fixtures/small/prism/multiline_attr_write_actual.rb b/fixtures/small/prism/multiline_attr_write_actual.rb new file mode 100644 index 00000000..ddc2fe67 --- /dev/null +++ b/fixtures/small/prism/multiline_attr_write_actual.rb @@ -0,0 +1,8 @@ +v2_request + .additional_data_private + .sub_elements + .specific_collect_pointer = Constant::Thing + +v2_request.additional_data_private.sub_elements.specific_collect_pointer.lets_make_this_quite_a_bit_longer = Constant::Thing + +v2_request.additional_data_private.sub_elements.specific_collect_pointer.lets_make_this_quite_a_bit_longer.over_120_chars_now = Constant::Thing diff --git a/fixtures/small/prism/multiline_attr_write_expected.rb b/fixtures/small/prism/multiline_attr_write_expected.rb new file mode 100644 index 00000000..f5623a44 --- /dev/null +++ b/fixtures/small/prism/multiline_attr_write_expected.rb @@ -0,0 +1,13 @@ +v2_request + .additional_data_private + .sub_elements + .specific_collect_pointer = Constant::Thing + +v2_request.additional_data_private.sub_elements.specific_collect_pointer.lets_make_this_quite_a_bit_longer = Constant::Thing + +v2_request + .additional_data_private + .sub_elements + .specific_collect_pointer + .lets_make_this_quite_a_bit_longer + .over_120_chars_now = Constant::Thing diff --git a/librubyfmt/src/format.rs b/librubyfmt/src/format.rs index 0de4399d..402d55cf 100644 --- a/librubyfmt/src/format.rs +++ b/librubyfmt/src/format.rs @@ -1590,13 +1590,17 @@ pub fn format_field(ps: &mut ParserState, f: Field) { ps.emit_indent(); } + let Field(_, expr, dot, ident_or_const) = f; + + // Format as a call chain for proper multiline handling + let mut chain = (*expr).into_call_chain(); + chain.push(CallChainElement::DotTypeOrOp(dot)); + chain.push(CallChainElement::IdentOrOpOrKeywordOrConst( + ident_or_const.into_ident_or_op_or_keyword_or_const(), + )); + ps.with_start_of_line(false, |ps| { - format_expression(ps, *f.1); - format_dot(ps, f.2); - match f.3 { - IdentOrConst::Const(c) => format_const(ps, c), - IdentOrConst::Ident(i) => format_ident(ps, i), - } + format_call_chain(ps, chain, None); }); if ps.at_start_of_line() { diff --git a/librubyfmt/src/format_prism.rs b/librubyfmt/src/format_prism.rs index 9f60395b..292b15d3 100644 --- a/librubyfmt/src/format_prism.rs +++ b/librubyfmt/src/format_prism.rs @@ -86,10 +86,10 @@ pub fn format_node<'src>(ps: &mut ParserState<'src>, node: prism::Node<'src>) { if is_last_call_in_chain && has_block { ps.breakable_call_chain_of(MultilineHandling::Prism(false), |ps| { - format_call_node(ps, call_node, false, is_last_call_in_chain); + format_call_node(ps, call_node, false, is_last_call_in_chain, false); }); } else { - format_call_node(ps, call_node, false, is_last_call_in_chain) + format_call_node(ps, call_node, false, is_last_call_in_chain, false) } } Node::CallOperatorWriteNode { .. } => { @@ -1711,6 +1711,7 @@ fn format_call_node<'src>( call_node: prism::CallNode<'src>, skip_receiver: bool, is_final_call_in_chain: bool, + skip_attr_write_value: bool, ) { let method_name = const_to_str(call_node.name()); let end_offset = call_node.location().end_offset(); @@ -1785,13 +1786,15 @@ fn format_call_node<'src>( ); ps.with_start_of_line(false, |ps| format_node(ps, last_arg)); } else if call_node.is_attribute_write() { - ps.emit_ident(" = "); - let value = arguments - .arguments() - .iter() - .next() - .expect("Attribute writes must have a value"); - ps.with_start_of_line(false, |ps| format_node(ps, value)); + if !skip_attr_write_value { + ps.emit_ident(" = "); + let value = arguments + .arguments() + .iter() + .next() + .expect("Attribute writes must have a value"); + ps.with_start_of_line(false, |ps| format_node(ps, value)); + } } else { let should_use_parens = use_parens_for_call_node( ps, @@ -2108,41 +2111,66 @@ fn format_call_chain<'src>(ps: &mut ParserState<'src>, call_node: ruby_prism::Ca ps.with_start_of_line(false, |ps| { let segments = split_node_into_call_chains(call_node.as_node()); - let is_attribute_write = segments - .last() - .and_then(|seg| seg.last()) - .and_then(|n| n.as_call_node()) - .map(|c| c.is_attribute_write()) - .unwrap_or(false); - - format_call_chain_segments(ps, segments, is_attribute_write); + format_call_chain_segments(ps, segments); }); } fn format_call_chain_segments<'src>( ps: &mut ParserState<'src>, mut segments: Vec>>, - is_attr_write: bool, ) { if let Some(current) = segments.pop() { let has_inner = !segments.is_empty(); let (chain_elements, trailing_arefs) = extract_trailing_arefs(current); + // Render the right-hand side of the write in its own breakable, so that each side can + // break independently of each other + let trailing_attr_write_value = chain_elements.last().and_then(|last| { + last.as_call_node().and_then(|call| { + let method_name = const_to_str(call.name()); + // arefs are handled in format_call_node + if call.is_attribute_write() && method_name != "[]=" { + call.arguments().and_then(|args| { + debug_assert!( + args.arguments().len() == 1, + "Expected attr_write to have exactly one argument" + ); + + args.arguments().iter().next() + }) + } else { + None + } + }) + }); + let is_user_multilined = call_chain_elements_are_user_multilined(ps, &chain_elements); ps.breakable_call_chain_of(MultilineHandling::Prism(is_user_multilined), |ps| { // Recurse and format previous segments inside this breakable - // Inner segments are never attribute writes - format_call_chain_segments(ps, segments, false); + format_call_chain_segments(ps, segments); - format_call_body(ps, chain_elements, is_attr_write, has_inner); + format_call_body( + ps, + chain_elements, + has_inner, + trailing_attr_write_value.is_some(), + ); }); + // Attr_write values are formatted after the breakable so they break independently + if let Some(value) = trailing_attr_write_value { + ps.emit_space(); + ps.emit_op("=".to_string()); + ps.emit_space(); + ps.with_start_of_line(false, |ps| format_node(ps, value)); + } + // Trailing arefs are formatted after the breakable for aref in trailing_arefs { let aref = aref.as_call_node().unwrap(); - format_call_node(ps, aref, true, false); + format_call_node(ps, aref, true, false, false); } } } @@ -2173,8 +2201,8 @@ fn extract_trailing_arefs<'src>( fn format_call_body<'src>( ps: &mut ParserState<'src>, mut call_chain_elements: Vec>, - is_attr_write: bool, is_continuation: bool, + skip_final_attr_write_value: bool, ) { if call_chain_elements.is_empty() { return; @@ -2196,7 +2224,7 @@ fn format_call_body<'src>( && call_node.call_operator_loc().is_none() { call_chain_elements.remove(0); - format_call_node(ps, call_node, true, false); + format_call_node(ps, call_node, true, false, false); continue; } break; @@ -2209,8 +2237,8 @@ fn format_call_body<'src>( ps.render_heredocs(true); } - // Attribute writes like `self.foo = bar` should have both sides - // rendered separately so they can break independently + // Attribute writes like `self.foo = bar` format the LHS without call chain indent + // since there's only one element. if !is_continuation && call_chain_elements.len() == 1 && let Some(attr_write) = call_chain_elements @@ -2218,7 +2246,6 @@ fn format_call_body<'src>( .and_then(|n| n.as_call_node()) .filter(|c| c.is_attribute_write()) { - // Format `.attr_name = ` directly without call chain indent let call_operator = attr_write.call_operator_loc().map(|loc| loc_to_str(loc)); if let Some(call_operator) = call_operator { match call_operator { @@ -2232,14 +2259,9 @@ fn format_call_body<'src>( ps.at_offset(start_loc_for_call_node_in_chain(&attr_write)); ps.shift_comments(); - // Format just the attribute name and ` = `, then the value separately - format_call_node(ps, attr_write, true, true); + format_call_node(ps, attr_write, true, true, skip_final_attr_write_value); } else if !call_chain_elements.is_empty() { - // attr_writes are not wrapped in a breakable, so they - // cannot emit abstract token types - if !is_attr_write { - ps.start_indent_for_call_chain(); - } + ps.start_indent_for_call_chain(); ps.with_start_of_line(false, |ps| { let call_chain_element_count = call_chain_elements.len(); @@ -2251,7 +2273,7 @@ fn format_call_body<'src>( // it may be None in the case of arefs, e.g. foo[bar] let call_operator = element.call_operator_loc().map(|loc| loc_to_str(loc)); if let Some(call_operator) = call_operator { - if call_operator != "::" && !is_attr_write { + if call_operator != "::" { ps.emit_collapsing_newline(); ps.emit_soft_indent(); } @@ -2270,12 +2292,12 @@ fn format_call_body<'src>( ps.at_offset(start_loc_for_call_node_in_chain(&element)); ps.shift_comments(); - format_call_node(ps, element, true, is_final_call); + let skip_value = + is_final_call && skip_final_attr_write_value && element.is_attribute_write(); + format_call_node(ps, element, true, is_final_call, skip_value); } }); - if !is_attr_write { - ps.end_indent_for_call_chain(); - } + ps.end_indent_for_call_chain(); } } @@ -4207,7 +4229,7 @@ fn format_match_write_node<'src>( ps: &mut ParserState<'src>, match_write_node: prism::MatchWriteNode<'src>, ) { - format_call_node(ps, match_write_node.call(), false, true); + format_call_node(ps, match_write_node.call(), false, true, false); } fn format_multi_target_node<'src>( diff --git a/librubyfmt/src/ripper_tree_types.rs b/librubyfmt/src/ripper_tree_types.rs index c5209887..c47e2a90 100644 --- a/librubyfmt/src/ripper_tree_types.rs +++ b/librubyfmt/src/ripper_tree_types.rs @@ -299,6 +299,32 @@ impl Expression { Expression::AnonBlockArg(AnonBlockArg(.., line_start)) => Some(*line_start), } } + + pub fn into_call_chain(self) -> Vec { + match self { + Expression::MethodCall(MethodCall(_, mut chain, method, _, args, start_end)) => { + chain.extend([ + CallChainElement::IdentOrOpOrKeywordOrConst(method), + CallChainElement::ArgsAddStarOrExpressionListOrArgsForward(args, start_end), + ]); + chain + } + Expression::Call(c) => CallLeft::Call(c).into_call_chain(), + Expression::MethodAddArg(m) => CallLeft::MethodAddArg(m).into_call_chain(), + Expression::MethodAddBlock(m) => CallLeft::MethodAddBlock(m).into_call_chain(), + Expression::VarRef(v) => CallLeft::VarRef(v).into_call_chain(), + Expression::VCall(v) => CallLeft::VCall(v).into_call_chain(), + Expression::Paren(p) => CallLeft::Paren(p).into_call_chain(), + Expression::Command(c) => CallLeft::Command(c).into_call_chain(), + Expression::CommandCall(c) => CallLeft::CommandCall(c).into_call_chain(), + Expression::Super(s) => CallLeft::Super(s).into_call_chain(), + Expression::ZSuper(z) => CallLeft::ZSuper(z).into_call_chain(), + Expression::Next(n) => CallLeft::Next(n).into_call_chain(), + Expression::Yield(y) => CallLeft::Yield(y).into_call_chain(), + Expression::Yield0(y) => CallLeft::Yield0(y).into_call_chain(), + other => vec![CallChainElement::Expression(Box::new(other))], + } + } } def_tag!(mlhs_tag, "mlhs");