From d170616bd95e217b8711bac78c30956e632080b7 Mon Sep 17 00:00:00 2001 From: Reese Williams Date: Wed, 23 Aug 2023 09:12:58 -0700 Subject: [PATCH] Model call chains as an `AbstractTokenTarget` (#443) This PR implements call chains as a new AbstractTokenTarget: BreakableCallChainEntry. This means that multilining for call chains -- just like for existing breakables -- will be decided on-the-fly during rendering, where we have more contextual information and can better enforce line length. This is done with a combination of the normal breakable machinery and some additional "magic tokens." Specifically, this adds tokens at the beginning and end of the indent-able portion of the call chain, and then during rendering, we maintain a count of how many nested call chains we're in and adjust indentation accordingly. This isn't exactly the cleanest implementation, but given the complexity in use-cases for call chains, it was the best I could muster in a way that functioned how users would generally expect. This is a fairly large PR, but there's primarily three main portions which each are about a third of the total diff each: fixtures, the boilerplate in ripper_tree_types to support getting expression starting lines, and finally the main formatting changes. I've done my best to comment these areas appropriately, but it's helpful to add further explanation there, I'm more than happy to add more. --- fixtures/small/aref_in_call_actual.rb | 9 +- fixtures/small/aref_in_call_expected.rb | 7 + fixtures/small/begin_end_stack_actual.rb | 4 + fixtures/small/begin_end_stack_expected.rb | 3 + .../small/brace_blocks_with_no_args_actual.rb | 9 + .../brace_blocks_with_no_args_expected.rb | 9 + fixtures/small/conditional_actual.rb | 4 + fixtures/small/conditional_expected.rb | 6 + fixtures/small/heredoc_method_call_actual.rb | 11 + .../small/heredoc_method_call_expected.rb | 11 + fixtures/small/heredocs_actual.rb | 21 + fixtures/small/heredocs_expected.rb | 22 + fixtures/small/method_annotation_expected.rb | 6 +- fixtures/small/method_chains_actual.rb | 36 +- fixtures/small/method_chains_expected.rb | 44 +- .../multiline_chain_in_block_expected.rb | 3 +- .../small/multiline_chained_call_expected.rb | 3 +- ...line_method_chain_with_arguments_actual.rb | 8 + ...ne_method_chain_with_arguments_expected.rb | 12 +- fixtures/small/paren_expr_calls_actual.rb | 14 + fixtures/small/paren_expr_calls_expected.rb | 16 + .../small/pathological_heredocs_expected.rb | 5 +- .../single_line_method_call_chain_actual.rb | 19 + .../single_line_method_call_chain_expected.rb | 29 ++ fixtures/small/visibility_modifier_actual.rb | 6 + .../small/visibility_modifier_expected.rb | 6 + librubyfmt/rubyfmt_lib.rb | 8 +- librubyfmt/src/format.rs | 457 +++++------------- librubyfmt/src/heredoc_string.rs | 7 +- librubyfmt/src/intermediary.rs | 13 +- librubyfmt/src/line_tokens.rs | 95 +++- librubyfmt/src/parser_state.rs | 112 +++-- librubyfmt/src/render_queue_writer.rs | 134 ++++- librubyfmt/src/render_targets.rs | 335 +++++++++++-- librubyfmt/src/ripper_tree_types.rs | 397 +++++++++++++++ 35 files changed, 1385 insertions(+), 496 deletions(-) diff --git a/fixtures/small/aref_in_call_actual.rb b/fixtures/small/aref_in_call_actual.rb index ebf1f50b2..8d3b469d0 100644 --- a/fixtures/small/aref_in_call_actual.rb +++ b/fixtures/small/aref_in_call_actual.rb @@ -9,4 +9,11 @@ ) .void end -def foo; end \ No newline at end of file +def foo; end + +Array[ + @root_fragment, + @lemon_tea_fragment, + @green_tea_fragment, + @cake_fragment +].sort_by(&:name) diff --git a/fixtures/small/aref_in_call_expected.rb b/fixtures/small/aref_in_call_expected.rb index 7b09871b4..51923026f 100644 --- a/fixtures/small/aref_in_call_expected.rb +++ b/fixtures/small/aref_in_call_expected.rb @@ -11,3 +11,10 @@ end def foo end + +Array[ + @root_fragment, + @lemon_tea_fragment, + @green_tea_fragment, + @cake_fragment +].sort_by(&:name) diff --git a/fixtures/small/begin_end_stack_actual.rb b/fixtures/small/begin_end_stack_actual.rb index 756e7ff43..a7d873f6f 100644 --- a/fixtures/small/begin_end_stack_actual.rb +++ b/fixtures/small/begin_end_stack_actual.rb @@ -2,3 +2,7 @@ a = begin end end + +begin +end + .freeze diff --git a/fixtures/small/begin_end_stack_expected.rb b/fixtures/small/begin_end_stack_expected.rb index 4b55ef913..de1b04a8b 100644 --- a/fixtures/small/begin_end_stack_expected.rb +++ b/fixtures/small/begin_end_stack_expected.rb @@ -2,3 +2,6 @@ a = begin end end + +begin +end.freeze diff --git a/fixtures/small/brace_blocks_with_no_args_actual.rb b/fixtures/small/brace_blocks_with_no_args_actual.rb index 431f006ec..59a972af9 100644 --- a/fixtures/small/brace_blocks_with_no_args_actual.rb +++ b/fixtures/small/brace_blocks_with_no_args_actual.rb @@ -1,3 +1,12 @@ def func brace_block_with_no_args { p 'hi' } end + +{ + "/v1/transfers" => lambda { + foo + bar + # TODO: add baz here + # Oh and maybe quux if you have time + } +} \ No newline at end of file diff --git a/fixtures/small/brace_blocks_with_no_args_expected.rb b/fixtures/small/brace_blocks_with_no_args_expected.rb index 8484107b3..bc10ebc3d 100644 --- a/fixtures/small/brace_blocks_with_no_args_expected.rb +++ b/fixtures/small/brace_blocks_with_no_args_expected.rb @@ -1,3 +1,12 @@ def func brace_block_with_no_args { p("hi") } end + +{ + "/v1/transfers" => lambda { + foo + bar + # TODO: add baz here + # Oh and maybe quux if you have time + } +} diff --git a/fixtures/small/conditional_actual.rb b/fixtures/small/conditional_actual.rb index a83647b32..42c4c509b 100644 --- a/fixtures/small/conditional_actual.rb +++ b/fixtures/small/conditional_actual.rb @@ -7,3 +7,7 @@ def foo puts("hi") end end + +do_stuff! if it_isnt_dangerous( + i_promise: true +) \ No newline at end of file diff --git a/fixtures/small/conditional_expected.rb b/fixtures/small/conditional_expected.rb index a83647b32..6c3aa1415 100644 --- a/fixtures/small/conditional_expected.rb +++ b/fixtures/small/conditional_expected.rb @@ -7,3 +7,9 @@ def foo puts("hi") end end + +if it_isnt_dangerous( + i_promise: true + ) + do_stuff! +end diff --git a/fixtures/small/heredoc_method_call_actual.rb b/fixtures/small/heredoc_method_call_actual.rb index 2de4a44e8..23a23ee13 100644 --- a/fixtures/small/heredoc_method_call_actual.rb +++ b/fixtures/small/heredoc_method_call_actual.rb @@ -25,3 +25,14 @@ class William::Carlos::Williams Icarus drowning LANDSCAPE end + +optp + .on do |value| + <<~EOF + There's some lines here + + But that one's a blank line! + + There shouldn't be any whitespace on those + EOF + end \ No newline at end of file diff --git a/fixtures/small/heredoc_method_call_expected.rb b/fixtures/small/heredoc_method_call_expected.rb index 3c7d3186a..903e4ff95 100644 --- a/fixtures/small/heredoc_method_call_expected.rb +++ b/fixtures/small/heredoc_method_call_expected.rb @@ -30,3 +30,14 @@ class William::Carlos::Williams Williams ) end + +optp + .on do |value| + <<~EOF + There's some lines here + + But that one's a blank line! + + There shouldn't be any whitespace on those + EOF + end diff --git a/fixtures/small/heredocs_actual.rb b/fixtures/small/heredocs_actual.rb index 4601b128e..7cd49d14b 100644 --- a/fixtures/small/heredocs_actual.rb +++ b/fixtures/small/heredocs_actual.rb @@ -68,6 +68,27 @@ class Foo rubocop.smash(bad_thing) end +this_one_is + .in_a_call_chain { + # some stuff + } + .each do + <<~MYHEREDOC + Words are pale shadows of forgotten names. + As names have power, words have power. + Words can light fires in the minds of men. + Words can wring tears from the hardest hearts. + MYHEREDOC + end + +def foo + "#{stuff.each do + <<~MESSAGE.strip + #{message.text} + MESSAGE + end.join("\n\n")}" +end + puts a puts b diff --git a/fixtures/small/heredocs_expected.rb b/fixtures/small/heredocs_expected.rb index 5b71d0967..496b59119 100644 --- a/fixtures/small/heredocs_expected.rb +++ b/fixtures/small/heredocs_expected.rb @@ -69,6 +69,28 @@ class Foo rubocop.smash(bad_thing) end +this_one_is + .in_a_call_chain { + # some stuff + } + .each do + <<~MYHEREDOC + Words are pale shadows of forgotten names. + As names have power, words have power. + Words can light fires in the minds of men. + Words can wring tears from the hardest hearts. + MYHEREDOC + end + +def foo + "#{stuff.each do + <<~MESSAGE + #{message.text} + MESSAGE + .strip + end.join("\n\n")}" +end + puts(a) puts(b) foo diff --git a/fixtures/small/method_annotation_expected.rb b/fixtures/small/method_annotation_expected.rb index 47fe010e5..4dc9c9156 100644 --- a/fixtures/small/method_annotation_expected.rb +++ b/fixtures/small/method_annotation_expected.rb @@ -32,7 +32,8 @@ def do_the_thing(a, b) params( a: String, b: String - ).void + ) + .void end def example(a, b) end @@ -95,7 +96,8 @@ class Bees first_param: MyClass, # This one is the second one, nice second_param: YourClass - ).void + ) + .void # Please not the bees! } def not_the_bees! diff --git a/fixtures/small/method_chains_actual.rb b/fixtures/small/method_chains_actual.rb index ece2d64e7..0f79b15f4 100644 --- a/fixtures/small/method_chains_actual.rb +++ b/fixtures/small/method_chains_actual.rb @@ -18,8 +18,6 @@ foo.bar .baz -# If they're all on the same line but different from -# the first receiver, consider that "on one line" foo .bar.baz @@ -97,6 +95,24 @@ def example b: "" ) +ThisIs::OnlyOneCall + # but it's explicitly multilined with a + .comment! + +OnlyOneCall + # but it's explicitly multilined with a + .comment! + + [ + # foo + ] + # bar + .baz + + [] + # Please don't do this + .freeze + Paul::Blart::Mall::Cop::PerformedByTheLegendaryKevinJamesWhoIsAnAbsoluteLegendInAllOfHisFilmsWhichAreAbsolutelyIncredible.consume_pixie_sticks(mall: "downtown").each do |punch_list_type| end @@ -129,3 +145,19 @@ def gather_thanes! puts "h" end &.foo + +My::Error.soft( + "", + stuff: { + message_token: message.token, + + # Some comments! + value: id_or_email.name + } +) + +# rubocop:disable PrisonGuard/PrivateModule +(foo.load_one + # rubocop:enable PrisonGuard/PrivateModule + .bar) + .thing \ No newline at end of file diff --git a/fixtures/small/method_chains_expected.rb b/fixtures/small/method_chains_expected.rb index 99ed221da..149fdbe78 100644 --- a/fixtures/small/method_chains_expected.rb +++ b/fixtures/small/method_chains_expected.rb @@ -21,11 +21,12 @@ .bar .baz -# If they're all on the same line but different from -# the first receiver, consider that "on one line" -foo.bar.baz +foo + .bar + .baz -foo::bar&.nil? +foo::bar + &.nil? foo::bar &.nil?::klass @@ -120,6 +121,24 @@ def example b: "" ) +ThisIs::OnlyOneCall + # but it's explicitly multilined with a + .comment! + +OnlyOneCall + # but it's explicitly multilined with a + .comment! + +[ + # foo +] + # bar + .baz + +[] + # Please don't do this + .freeze + Paul::Blart::Mall::Cop::PerformedByTheLegendaryKevinJamesWhoIsAnAbsoluteLegendInAllOfHisFilmsWhichAreAbsolutelyIncredible .consume_pixie_sticks(mall: "downtown") .each do |punch_list_type| @@ -160,3 +179,20 @@ def gather_thanes! puts("h") end &.foo + +My::Error.soft( + "", + stuff: { + message_token: message.token, + + # Some comments! + value: id_or_email.name + } +) + +# rubocop:disable PrisonGuard/PrivateModule +(foo + .load_one + # rubocop:enable PrisonGuard/PrivateModule + .bar) + .thing diff --git a/fixtures/small/multiline_chain_in_block_expected.rb b/fixtures/small/multiline_chain_in_block_expected.rb index 95bee5bd4..7303c03d7 100644 --- a/fixtures/small/multiline_chain_in_block_expected.rb +++ b/fixtures/small/multiline_chain_in_block_expected.rb @@ -1,7 +1,8 @@ sig { params( route: String - ).void + ) + .void } def ajax_get(route) super diff --git a/fixtures/small/multiline_chained_call_expected.rb b/fixtures/small/multiline_chained_call_expected.rb index 5068b4f53..7e1548798 100644 --- a/fixtures/small/multiline_chained_call_expected.rb +++ b/fixtures/small/multiline_chained_call_expected.rb @@ -2,5 +2,6 @@ def blorp method_call( arg_a, arg_b - ).chained_call + ) + .chained_call end diff --git a/fixtures/small/multiline_method_chain_with_arguments_actual.rb b/fixtures/small/multiline_method_chain_with_arguments_actual.rb index 8d10d7e2d..ab5262972 100644 --- a/fixtures/small/multiline_method_chain_with_arguments_actual.rb +++ b/fixtures/small/multiline_method_chain_with_arguments_actual.rb @@ -5,3 +5,11 @@ def stub_server(path:, body: {}) "https://example.com#{path}" ).to_return(body: body.to_json) end + +{ + "original_fields" => foo, + "alternative_fields" => (thing_one(id, api) + thing_two( + id, + api + )).sort +} \ No newline at end of file diff --git a/fixtures/small/multiline_method_chain_with_arguments_expected.rb b/fixtures/small/multiline_method_chain_with_arguments_expected.rb index 8d10d7e2d..7ee990f9b 100644 --- a/fixtures/small/multiline_method_chain_with_arguments_expected.rb +++ b/fixtures/small/multiline_method_chain_with_arguments_expected.rb @@ -3,5 +3,15 @@ def stub_server(path:, body: {}) stub_request( :get, "https://example.com#{path}" - ).to_return(body: body.to_json) + ) + .to_return(body: body.to_json) end + +{ + "original_fields" => foo, + "alternative_fields" => (thing_one(id, api) + thing_two( + id, + api + )) + .sort +} diff --git a/fixtures/small/paren_expr_calls_actual.rb b/fixtures/small/paren_expr_calls_actual.rb index 308abbba3..30469c8bc 100644 --- a/fixtures/small/paren_expr_calls_actual.rb +++ b/fixtures/small/paren_expr_calls_actual.rb @@ -1,2 +1,16 @@ a (1) other_cool_method (a + b).round(4) + +# rubocop:disable PrisonGuard/PrivateModule +(foo( + foo # rubocop:enable PrisonGuard/PrivateModule +)).flatten + +# rubocop:disable Style/Stuff +(MyModel::InSomeNamespace + .load_one( + # rubocop:enable Style/Stuff + {name: "name"} + ) + &.rules) + .freeze diff --git a/fixtures/small/paren_expr_calls_expected.rb b/fixtures/small/paren_expr_calls_expected.rb index 8d419989d..614ab5303 100644 --- a/fixtures/small/paren_expr_calls_expected.rb +++ b/fixtures/small/paren_expr_calls_expected.rb @@ -1,2 +1,18 @@ a(1) other_cool_method((a + b).round(4)) + +# rubocop:disable PrisonGuard/PrivateModule +(foo( + # rubocop:enable PrisonGuard/PrivateModule + foo +)) + .flatten + +# rubocop:disable Style/Stuff +(MyModel::InSomeNamespace + .load_one( + # rubocop:enable Style/Stuff + {name: "name"} + ) + &.rules) + .freeze diff --git a/fixtures/small/pathological_heredocs_expected.rb b/fixtures/small/pathological_heredocs_expected.rb index 456aa06d0..d04a7a836 100644 --- a/fixtures/small/pathological_heredocs_expected.rb +++ b/fixtures/small/pathological_heredocs_expected.rb @@ -32,8 +32,11 @@ def foo end ASSIGNED_MESSAGE = lambda do |assignee| - <<-END.lines.join.strip + <<-END This heredoc chains together in a weird way, talk to [~#{assignee}] about it. Otherwise, it's probably in your best interest not to write things like this. END + .lines + .join + .strip end diff --git a/fixtures/small/single_line_method_call_chain_actual.rb b/fixtures/small/single_line_method_call_chain_actual.rb index 437266b22..fbcb16f48 100644 --- a/fixtures/small/single_line_method_call_chain_actual.rb +++ b/fixtures/small/single_line_method_call_chain_actual.rb @@ -1,3 +1,22 @@ def blorp method_call(arg_a, arg_b).chained_call end + +This::Is::Some::SuperDuperLongConstantThatOnlyHasOneCallInTheChain.build( + reason: Because::ThisCouldCauseUsSome::SeriousProblemsIfWeTriedToMultilineIt::EventThoughItActuallyDoesExtendBeyondTheLimit +) + +This::Is::Some::SuperDuperLongConstantThatOnlyHasOneCallInTheChain + .build( + reason: Because::IPutThisOnMultipleLines + ) + +this.surprisingly.can_break +# due to this comment being here! + +this.wont.break # If there's a super long comment here that goes over the line limit weeeeeeeeeeeeeeeeeeee look at this one go + +this.is_really_long.and_will_go_beyond_the_maximum_line_length_and_break.across_multiple_lines.and_it_has_a_comment_after_it! # And there's a comment here + +this.is_really_long.and_will_go_beyond_the_maximum_line_length_and_break.across_multiple_lines.and_it_has_a_comment_after_it! +# See what happens to this comment here! \ No newline at end of file diff --git a/fixtures/small/single_line_method_call_chain_expected.rb b/fixtures/small/single_line_method_call_chain_expected.rb index 437266b22..d287f5b6c 100644 --- a/fixtures/small/single_line_method_call_chain_expected.rb +++ b/fixtures/small/single_line_method_call_chain_expected.rb @@ -1,3 +1,32 @@ def blorp method_call(arg_a, arg_b).chained_call end + +This::Is::Some::SuperDuperLongConstantThatOnlyHasOneCallInTheChain.build( + reason: Because::ThisCouldCauseUsSome::SeriousProblemsIfWeTriedToMultilineIt::EventThoughItActuallyDoesExtendBeyondTheLimit +) + +This::Is::Some::SuperDuperLongConstantThatOnlyHasOneCallInTheChain + .build( + reason: Because::IPutThisOnMultipleLines + ) + +this.surprisingly.can_break +# due to this comment being here! + +# If there's a super long comment here that goes over the line limit weeeeeeeeeeeeeeeeeeee look at this one go +this.wont.break + +# And there's a comment here +this + .is_really_long + .and_will_go_beyond_the_maximum_line_length_and_break + .across_multiple_lines + .and_it_has_a_comment_after_it! + +this + .is_really_long + .and_will_go_beyond_the_maximum_line_length_and_break + .across_multiple_lines + .and_it_has_a_comment_after_it! +# See what happens to this comment here! diff --git a/fixtures/small/visibility_modifier_actual.rb b/fixtures/small/visibility_modifier_actual.rb index 575cc1139..95d39b00a 100644 --- a/fixtures/small/visibility_modifier_actual.rb +++ b/fixtures/small/visibility_modifier_actual.rb @@ -6,6 +6,12 @@ class Foo private def baz(a, b, c) a + b + c end + + private def with_comments + # beginning + stuff! + # end + end end module WhiteTeaBowl diff --git a/fixtures/small/visibility_modifier_expected.rb b/fixtures/small/visibility_modifier_expected.rb index f46268936..371279089 100644 --- a/fixtures/small/visibility_modifier_expected.rb +++ b/fixtures/small/visibility_modifier_expected.rb @@ -6,6 +6,12 @@ class Foo private def baz(a, b, c) a + b + c end + + private def with_comments + # beginning + stuff! + # end + end end module WhiteTeaBowl diff --git a/librubyfmt/rubyfmt_lib.rb b/librubyfmt/rubyfmt_lib.rb index 385fa1eda..62c8cae5c 100644 --- a/librubyfmt/rubyfmt_lib.rb +++ b/librubyfmt/rubyfmt_lib.rb @@ -65,16 +65,10 @@ def initialize(file_data) @rbracket_stack = [] @lbrace_stack = [] @comments = {} - @last_ln = 0 # binary contents comming after a `__END__` node @data_contents_start_line = nil end - def on_nl(*args) - @last_ln = lineno+1 - super(*args) - end - # This method has incorrect behavior inside Ripper, # so we patch it for now # original: https://github.com/ruby/ruby/blob/118368c1dd9304c0c21a4437016af235bd9b8438/ext/ripper/lib/ripper/sexp.rb#L144-L155 @@ -112,7 +106,7 @@ def parse nil end - [res, @comments, @lines_with_any_ruby, @last_ln, data_contents] + [res, @comments, @lines_with_any_ruby, @file_lines.count, data_contents] end end diff --git a/librubyfmt/src/format.rs b/librubyfmt/src/format.rs index 51bcf06ff..8e3c45e3c 100644 --- a/librubyfmt/src/format.rs +++ b/librubyfmt/src/format.rs @@ -830,8 +830,7 @@ pub fn format_method_call(ps: &mut dyn ConcreteParserState, method_call: MethodC ps.emit_indent(); } - let should_multiline_call_chain = should_multiline_call_chain(ps, &method_call); - let MethodCall(_, chain, method, original_used_parens, args, start_end) = method_call; + let MethodCall(_, mut chain, method, original_used_parens, args, start_end) = method_call; debug!("method call!!"); let use_parens = use_parens_for_method_call( @@ -842,78 +841,15 @@ pub fn format_method_call(ps: &mut dyn ConcreteParserState, method_call: MethodC original_used_parens, ps.current_formatting_context(), ); + chain.extend([ + CallChainElement::IdentOrOpOrKeywordOrConst(method), + CallChainElement::ArgsAddStarOrExpressionListOrArgsForward(args, start_end), + ]); ps.with_start_of_line( false, Box::new(|ps| { - let is_indented = format_call_chain(ps, chain, should_multiline_call_chain); - if is_indented { - ps.start_indent(); - } - - if use_parens && method.get_name() == ".()" { - ps.emit_ident(".".to_string()); - } else { - format_ident(ps, method.into_ident()); - } - let delims = if use_parens { - BreakableDelims::for_method_call() - } else { - BreakableDelims::for_kw() - }; - if !args.is_empty() { - if args_has_single_def_expression(&args) { - // If we match `def ...` as the first argument, just - // emit it without any delimiters. - ps.emit_space(); - - if let ArgsAddStarOrExpressionListOrArgsForward::ExpressionList(mut el) = args { - let expr = el.pop().expect("checked the list is not empty"); - - if let Expression::Def(def_expression) = expr { - format_def(ps, def_expression); - } else if let Expression::Defs(defs_expression) = expr { - format_defs(ps, defs_expression); - } - } - } else { - let force_single_line = matches!( - args, - ArgsAddStarOrExpressionListOrArgsForward::ArgsForward(..) - ); - - let maybe_end_line = start_end.map(|se| se.1); - - ps.breakable_of( - delims, - Box::new(|ps| { - ps.with_formatting_context( - FormattingContext::ArgsList, - Box::new(|ps| { - format_list_like_thing( - ps, - args, - maybe_end_line, - force_single_line, - ); - ps.emit_collapsing_newline(); - }), - ); - debug!("end of format method call"); - }), - ); - if let Some(end_line) = maybe_end_line { - ps.wind_dumping_comments_until_line(end_line); - } - } - } else if use_parens { - ps.emit_open_paren(); - ps.emit_close_paren(); - } - - if is_indented { - ps.end_indent(); - } + format_call_chain(ps, chain, Some(use_parens)); }), ); @@ -1228,6 +1164,7 @@ pub fn format_begin(ps: &mut dyn ConcreteParserState, begin: Begin) { ps.emit_indent() } + let end_line = begin.1.end_line(); ps.on_line(begin.1 .0); ps.emit_begin(); @@ -1236,7 +1173,10 @@ pub fn format_begin(ps: &mut dyn ConcreteParserState, begin: Begin) { ps.emit_newline(); ps.with_start_of_line( true, - Box::new(|ps| format_bodystmt(ps, begin.2, begin.1.end_line())), + Box::new(|ps| { + format_bodystmt(ps, begin.2, end_line); + ps.wind_dumping_comments_until_line(end_line); + }), ); })); @@ -1621,6 +1561,7 @@ pub fn format_inner_string( if peekable.peek().is_none() && contents.ends_with('\n') { contents.pop(); } + ps.on_line((t.2).0); ps.emit_string_content(contents); } _ => { @@ -1701,14 +1642,10 @@ pub fn format_heredoc_string_literal( Box::new(|ps| { let heredoc_type = (hd.1).0; let heredoc_symbol = (hd.1).1; - ps.emit_heredoc_start(heredoc_type.clone(), heredoc_symbol.clone()); + let kind = HeredocKind::from_string(&heredoc_type); + ps.emit_heredoc_start(heredoc_symbol.clone(), kind); - ps.push_heredoc_content( - heredoc_symbol, - HeredocKind::from_string(heredoc_type), - parts, - end_line, - ); + ps.push_heredoc_content(heredoc_symbol, kind, parts, end_line); }), ); @@ -2807,73 +2744,128 @@ fn can_elide_parens_for_reserved_names(cc: &[CallChainElement]) -> bool { fn format_call_chain( ps: &mut dyn ConcreteParserState, cc: Vec, - should_multiline_call_chain: bool, -) -> bool { + last_call_use_parens: Option, +) { if cc.is_empty() { - return false; + return; } - format_call_chain_elements(ps, cc, should_multiline_call_chain); + let first_elem_line = cc.first().unwrap().start_line(); + if let Some(first_elem_line) = first_elem_line { + ps.on_line(first_elem_line); + } + + ps.breakable_call_chain_of( + cc.clone(), + Box::new(|ps| format_call_chain_elements(ps, cc, last_call_use_parens)), + ); ps.emit_after_call_chain(); - should_multiline_call_chain } fn format_call_chain_elements( ps: &mut dyn ConcreteParserState, cc: Vec, - render_multiline_chain: bool, + // Whether or not to force the last call to use parens. By default, falls back to normal call chain rules. + // This is necessary for supporting things like parenthesized methods in `self.method` method chains where + // the parens are sometimes semantically meaningful. However, we leave this as optional since not all callers + // require this (e.g. `MethodAddArg` doesn't enforce invariants like those). + last_call_use_parens: Option, ) { let elide_parens = can_elide_parens_for_reserved_names(&cc); - let mut has_indented = false; // When set, force all `CallChainElement::ArgsAddStarOrExpressionListOrArgsForward` // to use parens, even when empty. This handles cases like `super()` where parens matter let mut next_args_list_must_use_parens = false; - for cc_elem in cc { - let mut element_is_super_keyword = false; + let last_call_index = cc + .iter() + .rposition(|cce| matches!(cce, CallChainElement::IdentOrOpOrKeywordOrConst(..))); + let mut has_indented = false; + + for (index, cc_elem) in cc.into_iter().enumerate() { + let is_last_call_args = if let Some(last_call_index) = last_call_index { + index == (last_call_index + 1) + } else { + false + }; match cc_elem { CallChainElement::Paren(p) => format_paren(ps, p), CallChainElement::IdentOrOpOrKeywordOrConst(i) => { let ident = i.into_ident(); - element_is_super_keyword = ident.1 == "super"; + next_args_list_must_use_parens = ident.1 == "super" || ident.1 == ".()"; - format_ident(ps, ident) + if ident.1 == ".()" { + ps.emit_ident(".".to_string()); + } else { + format_ident(ps, ident); + } + ps.shift_comments(); } CallChainElement::Block(b) => { ps.emit_space(); format_block(ps, b) + // Shifting comments should be handled by `format_block`, so we don't + // need to shift again here. + } + CallChainElement::VarRef(vr) => { + format_var_ref(ps, vr); + ps.shift_comments(); } - CallChainElement::VarRef(vr) => format_var_ref(ps, vr), CallChainElement::ArgsAddStarOrExpressionListOrArgsForward(aas, start_end) => { + let end_line = start_end.map(|se| se.1); + if !aas.is_empty() || next_args_list_must_use_parens { - let delims = if elide_parens { - BreakableDelims::for_kw() + let use_parens = if next_args_list_must_use_parens { + // Reset for next call + next_args_list_must_use_parens = false; + true + } else if is_last_call_args && last_call_use_parens.is_some() { + last_call_use_parens.unwrap() } else { + !elide_parens + }; + let delims = if use_parens { BreakableDelims::for_method_call() + } else { + BreakableDelims::for_kw() }; - let end_line = start_end.map(|se| se.1); + // For def visiblity modifiers, e.g. `private def ...` + if args_has_single_def_expression(&aas) { + ps.emit_space(); - ps.breakable_of( - delims, - Box::new(|ps| { - format_list_like_thing(ps, aas, end_line, false); - }), - ); - if let Some(end_line) = end_line { - // If we're rendering a single-line chain, force a reset so - // that comments end up at the current indentation level - if !render_multiline_chain { + if let ArgsAddStarOrExpressionListOrArgsForward::ExpressionList(el) = aas { + // Cloning here so format_def{s} can take ownership + let expr = el.last().expect("checked the list is not empty").clone(); + + if let Expression::Def(def_expression) = expr { + format_def(ps, def_expression); + } else if let Expression::Defs(defs_expression) = expr { + format_defs(ps, defs_expression); + } + ps.shift_comments(); + } + } else { + ps.breakable_of( + delims, + Box::new(|ps| { + format_list_like_thing(ps, aas, end_line, false); + }), + ); + if let Some(end_line) = end_line { + // If we're rendering a single-line chain, force a reset so + // that comments end up at the current indentation level ps.reset_space_count(); + ps.wind_dumping_comments_until_line(end_line); } - ps.wind_dumping_comments_until_line(end_line); } + } else if is_last_call_args && last_call_use_parens.unwrap_or(false) { + ps.emit_single_line_delims(BreakableDelims::for_method_call()); } } CallChainElement::DotTypeOrOp(d) => { - if render_multiline_chain && !has_indented { - ps.start_indent(); + if !has_indented { + ps.start_indent_for_call_chain(); has_indented = true; } let is_double_colon = match &d { @@ -2881,213 +2873,25 @@ fn format_call_chain_elements( DotTypeOrOp::StringDot(val) => val == "::", _ => false, }; - if render_multiline_chain - // Separating `::` calls with a newline - // isn't valid syntax - && !is_double_colon - { - ps.emit_newline(); - ps.emit_indent(); + if !is_double_colon { + ps.emit_collapsing_newline(); + ps.emit_soft_indent(); } format_dot(ps, d); } - CallChainElement::Expression(e) => format_expression(ps, *e), - } - next_args_list_must_use_parens = element_is_super_keyword; - } - - if has_indented { - ps.end_indent(); - } -} - -/// Checks whether a call chain both starts with a heredoc expression -/// *and* contains a call chain element with a breakable. -/// -/// In practice, this generally means something like the call chain having something -/// like a method call with args or a block, e.g. -/// -/// ```ruby -/// # `|line|` here is the breakable -/// <<~FOO.lines.map { |line| p(line) } -/// FOO -/// ``` -/// -/// Breakables don't play very nicely with heredoc rendering in call chains, -/// and it would likely be a pretty hefty refactor to properly support this. -fn is_heredoc_call_chain_with_breakables(cc_elements: &[CallChainElement]) -> bool { - if let Some(CallChainElement::Expression(expr)) = cc_elements.first() { - if let Expression::StringLiteral(string_literal) = &**expr { - if matches!(string_literal, StringLiteral::Heredoc(..)) { - let contains_breakables = cc_elements.iter().any(|cc_elem| match cc_elem { - CallChainElement::ArgsAddStarOrExpressionListOrArgsForward( - ArgsAddStarOrExpressionListOrArgsForward::ExpressionList(list), - .., - ) => !list.is_empty(), - CallChainElement::Block(..) => true, - _ => false, - }); - return contains_breakables; - } - } - } - - false -} - -/// When to multiling call chains generally relies on a few broadly-applicable rules, but in practice -/// it has *many* special-cases, because multilining them ends up colliding with other language features in awkward ways. -/// -/// ## High-level rules -/// -/// The three main rules that govern whether or not to multiline a method chain is to split across multiple lines if -/// (1) the user multilined the chain, -/// (2) the whole chain exceeds the maximum line length, or -/// (3) the chain contains blocks that are split across multiple lines -/// -/// That said, both of these have some *very large* asterisks, since there are a lot of contexts in which these -/// have special cases for various reasons (see below). -/// -/// ## Special conditions -/// -/// This is a best-effort listing for the exceptional cases and their rationales -/// -/// * String embedded expressions -/// * We currently _never_ multiline in string embexprs, mostly because multilining makes it more difficult -/// to grok the final whitespace of the string. -/// * Chains consisting only of var/const refs -/// * It's pretty common to have something of the shape of `Class.method(args)`, and even if it exceeds the max line length, -/// multilining generally makes this only more confusing at first glance. -/// * Chains starting with heredocs -/// * With the current way breakables work, heredocs at the beginning of call chains will will often render really awkwardly -/// (for example, in the middle of the argument parameters), so we default to multilining if the chain has both a heredoc -/// and breakables to work around this. -/// * Long expressions/blocks at the beggining/end -/// * It's not uncommon to have call chains that start with extremely long expressions (e.g. a long array literal) but -/// but have very little following it; similarly, it's fairly common to have a short expression but a *very* long -/// block call following it (e.g. an `each` or `map` call with most of the logic in it). In these cases, we ignore -/// these long CallChainElements when calculating the length of the final expression to make some common idioms render nicely. -fn should_multiline_call_chain(ps: &mut dyn ConcreteParserState, method_call: &MethodCall) -> bool { - // Never multiline if we're in an embedded expression - if ps.current_formatting_context() == FormattingContext::StringEmbexpr { - return false; - } - - let MethodCall(_, mut call_chain_to_check, ident, _, args, start_end) = method_call.clone(); - - // Add the original method as a call chain element purely for the sake of determining multiling - call_chain_to_check.append(&mut vec![ - CallChainElement::IdentOrOpOrKeywordOrConst(ident), - CallChainElement::ArgsAddStarOrExpressionListOrArgsForward(args, start_end), - ]); - - let all_op_locations = call_chain_to_check - .iter() - .filter_map(|cc_elem| match cc_elem { - CallChainElement::DotTypeOrOp(dot_type_or_op) => { - match dot_type_or_op { - // ColonColon is specially represented in the parser, and - // it can't be properly multilined anyways, so we ignore it here - DotTypeOrOp::ColonColon(..) => None, - DotTypeOrOp::StringDot(..) => None, - DotTypeOrOp::Op(Op(.., start_end)) - | DotTypeOrOp::DotType( - DotType::LonelyOperator(LonelyOperator(_, start_end)) - | DotType::Dot(Dot(_, start_end)), - ) => Some(start_end.clone()), - DotTypeOrOp::Period(Period(.., linecol)) => { - Some(StartEnd(linecol.0, linecol.0)) - } - } - } - _ => None, - }) - .collect::>(); - // Multiline the chain if all the operators (dots, double colons, etc.) are not on the same line - if let Some(first_op_start_end) = all_op_locations.first() { - let chain_is_user_multilined = !all_op_locations - .iter() - .all(|op_start_end| op_start_end == first_op_start_end); - if chain_is_user_multilined { - return true; - } - } - - // Ignore chains that are basically only method calls, e.g. - // ````ruby - // Thing.foo(args) - // Thing.foo(args) { block! } - // ``` - // These should always stay inline - match call_chain_to_check.as_slice() { - [CallChainElement::VarRef(..) | CallChainElement::IdentOrOpOrKeywordOrConst(..), CallChainElement::DotTypeOrOp(..), CallChainElement::IdentOrOpOrKeywordOrConst(..)] - | [CallChainElement::VarRef(..) | CallChainElement::IdentOrOpOrKeywordOrConst(..), CallChainElement::DotTypeOrOp(..), CallChainElement::IdentOrOpOrKeywordOrConst(..), CallChainElement::ArgsAddStarOrExpressionListOrArgsForward(..)] - | [CallChainElement::VarRef(..) | CallChainElement::IdentOrOpOrKeywordOrConst(..), CallChainElement::DotTypeOrOp(..), CallChainElement::IdentOrOpOrKeywordOrConst(..), CallChainElement::Block(..)] - | [CallChainElement::VarRef(..) | CallChainElement::IdentOrOpOrKeywordOrConst(..), CallChainElement::DotTypeOrOp(..), CallChainElement::IdentOrOpOrKeywordOrConst(..), CallChainElement::ArgsAddStarOrExpressionListOrArgsForward(..), CallChainElement::Block(..)] => - { - return false; - } - [CallChainElement::Expression(maybe_const_ref), CallChainElement::DotTypeOrOp(..), CallChainElement::IdentOrOpOrKeywordOrConst(..)] - | [CallChainElement::Expression(maybe_const_ref), CallChainElement::DotTypeOrOp(..), CallChainElement::IdentOrOpOrKeywordOrConst(..), CallChainElement::ArgsAddStarOrExpressionListOrArgsForward(..)] - | [CallChainElement::Expression(maybe_const_ref), CallChainElement::DotTypeOrOp(..), CallChainElement::IdentOrOpOrKeywordOrConst(..), CallChainElement::Block(..)] - | [CallChainElement::Expression(maybe_const_ref), CallChainElement::DotTypeOrOp(..), CallChainElement::IdentOrOpOrKeywordOrConst(..), CallChainElement::ArgsAddStarOrExpressionListOrArgsForward(..), CallChainElement::Block(..)] => { - if matches!(maybe_const_ref.as_ref(), Expression::ConstPathRef(..)) { - return false; + CallChainElement::Expression(e) => { + format_expression(ps, *e); + // Eagerly render heredocs if they're in the first expression. + // We want the full heredoc to get rendered _before_ we emit the + // BeginCallChainIndent token so that it gets correctly indented + // (or in the case of it being the first expression, _not_ indented). + ps.render_heredocs(true); } } - _ => {} - } - - if is_heredoc_call_chain_with_breakables(&call_chain_to_check) { - return true; - } - - // If the first item in the chain is a multiline expression (like a hash or array), - // ignore it when checking line length - if let Some(CallChainElement::Expression(expr)) = call_chain_to_check.first() { - let is_multiline_expression = ps.will_render_as_multiline(Box::new(|ps| { - format_expression(ps, expr.as_ref().clone()); - })); - - if is_multiline_expression { - call_chain_to_check.remove(0); - } } - - // We don't always want to multiline blocks if their only usage - // is at the end of a chain, since it's common to have chains - // that end with long blocks, but those blocks don't mean we should - // multiline the rest of the chain. - // - // example: - // ``` - // items.get_all.each do - // end - // ``` - if let Some(CallChainElement::Block(..)) = call_chain_to_check.last() { - call_chain_to_check.pop(); - } - - let chain_is_too_long = ps.will_render_beyond_max_line_length(Box::new(|ps| { - format_call_chain_elements(ps, call_chain_to_check.clone(), false); - })); - if chain_is_too_long { - return true; + if has_indented { + ps.end_indent_for_call_chain(); } - - let chain_blocks_are_multilined = call_chain_to_check - .iter() - .filter_map(|elem| match elem { - CallChainElement::Block(block) => Some(block.clone()), - _ => None, - }) - .any(|block| { - ps.will_render_as_multiline(Box::new(|ps| { - format_block(ps, block); - })) - }); - - chain_blocks_are_multilined } pub fn format_block(ps: &mut dyn ConcreteParserState, b: Block) { @@ -3102,14 +2906,13 @@ pub fn format_method_add_block(ps: &mut dyn ConcreteParserState, mab: MethodAddB ps.emit_indent(); } - let should_multiline_chain = should_multiline_call_chain(ps, &mab.clone().to_method_call()); let mut chain = (mab.1).into_call_chain(); chain.push(CallChainElement::Block(mab.2)); ps.with_start_of_line( false, Box::new(|ps| { - format_call_chain(ps, chain, should_multiline_chain); + format_call_chain(ps, chain, None); }), ); @@ -3208,25 +3011,28 @@ fn render_block_contents( ps.emit_soft_newline(); ps.emit_soft_indent() } - BraceBlockRenderMethod::MultipleExpressions => ps.emit_newline(), + BraceBlockRenderMethod::MultipleExpressions => { + ps.emit_newline(); + ps.emit_indent() + } } - let always_multiline = brace_block_render_method == BraceBlockRenderMethod::MultipleExpressions; - ps.with_start_of_line( - always_multiline, + false, Box::new(|ps| { - for expr in body.into_iter() { - format_expression(ps, expr); + let mut peekable = body.into_iter().peekable(); + while peekable.peek().is_some() { + format_expression(ps, peekable.next().unwrap()); + ps.emit_soft_newline(); + if peekable.peek().is_some() { + ps.emit_soft_indent(); + } } ps.shift_comments(); }), ); // This is assuming that we're always inside of an `inline_breakable_of` block, which // doesn't handle the indentation for the closing delimeter for us. - if !always_multiline { - ps.emit_soft_newline(); - } ps.dedent(Box::new(|ps| ps.emit_soft_indent())); ps.wind_dumping_comments_until_line(end_line); @@ -3279,7 +3085,8 @@ pub fn format_do_block(ps: &mut dyn ConcreteParserState, do_block: DoBlock) { true, Box::new(|ps| { ps.wind_dumping_comments_until_line(end_line); - ps.emit_end() + ps.emit_end(); + ps.shift_comments(); }), ); } @@ -3411,20 +3218,8 @@ pub fn format_multilinable_mod( body: Box, name: String, ) { - let new_body = body.clone(); - let is_multiline = ps.will_render_as_multiline(Box::new(|next_ps| { - let exprs = match *new_body { - Expression::Paren(p) => match p.1 { - ParenExpressionOrExpressions::Expressions(exprs) => exprs, - ParenExpressionOrExpressions::Expression(e) => vec![*e], - }, - e => vec![e], - }; - - for expr in exprs { - format_expression(next_ps, expr); - } + format_inline_mod(next_ps, conditional.clone(), body.clone(), name.clone()) })); if is_multiline { diff --git a/librubyfmt/src/heredoc_string.rs b/librubyfmt/src/heredoc_string.rs index 09bea6428..7fb7e1349 100644 --- a/librubyfmt/src/heredoc_string.rs +++ b/librubyfmt/src/heredoc_string.rs @@ -1,6 +1,6 @@ use crate::types::ColNumber; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum HeredocKind { Bare, Dash, @@ -8,7 +8,7 @@ pub enum HeredocKind { } impl HeredocKind { - pub fn from_string(kind_str: String) -> Self { + pub fn from_string(kind_str: &str) -> Self { if kind_str.contains('~') { HeredocKind::Squiggly } else if kind_str.contains('-') { @@ -47,10 +47,9 @@ impl HeredocString { pub fn render_as_string(self) -> String { let indent = self.indent; - let kind = self.kind.clone(); let string = String::from_utf8(self.buf).expect("heredoc is utf8"); - if kind.is_squiggly() { + if self.kind.is_squiggly() { string .split('\n') .map(|l| { diff --git a/librubyfmt/src/intermediary.rs b/librubyfmt/src/intermediary.rs index bac5865f0..af05fc9f6 100644 --- a/librubyfmt/src/intermediary.rs +++ b/librubyfmt/src/intermediary.rs @@ -17,6 +17,7 @@ pub struct Intermediary { index_of_last_hard_newline: usize, current_line_metadata: LineMetadata, previous_line_metadata: Option, + pub additional_indent: u32, } impl Intermediary { @@ -26,6 +27,7 @@ impl Intermediary { current_line_metadata: LineMetadata::new(), previous_line_metadata: None, index_of_last_hard_newline: 0, + additional_indent: 0, } } @@ -53,12 +55,7 @@ impl Intermediary { self.index_of_last_hard_newline = self.tokens.len() - 1; } - pub fn fix_heredoc_delim_indent_mistake(&mut self) { - // Remove duplicate indent - self.tokens.remove(self.tokens.len() - 2); - } - - pub fn fix_heredoc_direct_part_indent_mistake(&mut self) { + pub fn fix_heredoc_duplicate_indent_mistake(&mut self) { // Remove duplicate indent self.tokens.remove(self.tokens.len() - 3); } @@ -228,7 +225,9 @@ impl Intermediary { // [.., Comma, Space, DirectPart {part: ""}, ] // so we remove items at positions length-2 until there is nothing // in that position that is garbage. - while self.tokens[self.len() - 2].is_single_line_breakable_garbage() { + while self.tokens.len() > 2 + && self.tokens[self.len() - 2].is_single_line_breakable_garbage() + { self.tokens.remove(self.len() - 2); } } diff --git a/librubyfmt/src/line_tokens.rs b/librubyfmt/src/line_tokens.rs index 6fcfefce5..1d9723a31 100644 --- a/librubyfmt/src/line_tokens.rs +++ b/librubyfmt/src/line_tokens.rs @@ -1,5 +1,7 @@ -use crate::heredoc_string::HeredocString; -use crate::render_targets::{AbstractTokenTarget, BreakableEntry, ConvertType}; +use crate::heredoc_string::{HeredocKind, HeredocString}; +use crate::render_targets::{ + AbstractTokenTarget, BreakableCallChainEntry, BreakableEntry, ConvertType, +}; use crate::types::ColNumber; pub fn cltats_hard_newline() -> ConcreteLineTokenAndTargets { @@ -51,10 +53,15 @@ pub enum ConcreteLineToken { SingleSlash, Comment { contents: String }, Delim { contents: String }, - AfterCallChain, End, HeredocClose { symbol: String }, DataEnd, + // These are "magic" tokens. They have no concrete representation, + // but they're meaningful inside of the render queue + AfterCallChain, + BeginCallChainIndent, + EndCallChainIndent, + HeredocStart { kind: HeredocKind, symbol: String }, } impl ConcreteLineToken { @@ -92,9 +99,20 @@ impl ConcreteLineToken { Self::End => "end".to_string(), Self::HeredocClose { symbol } => symbol, Self::DataEnd => "__END__".to_string(), + Self::HeredocStart { kind, symbol } => { + let mut kind_str = match kind { + HeredocKind::Bare => "<<".to_string(), + HeredocKind::Dash => "<<-".to_string(), + HeredocKind::Squiggly => "<<~".to_string(), + }; + kind_str.push_str(&symbol); + kind_str + } // no-op, this is purely semantic information // for the render queue - Self::AfterCallChain => "".to_string(), + Self::AfterCallChain | Self::BeginCallChainIndent | Self::EndCallChainIndent => { + "".to_string() + } } } @@ -105,7 +123,14 @@ impl ConcreteLineToken { // each individual string token, which would increase the allocations of rubyfmt // by an order of magnitude match self { - AfterCallChain => 0, // purely semantic token, doesn't render + AfterCallChain | BeginCallChainIndent | EndCallChainIndent => 0, // purely semantic tokens, don't render + HeredocStart { kind, symbol } => { + symbol.len() + + match kind { + HeredocKind::Bare => 2, // << + HeredocKind::Dash | HeredocKind::Squiggly => 3, // <<- or <<~ + } + } Indent { depth } => *depth as usize, Keyword { keyword: contents } | Op { op: contents } @@ -117,8 +142,9 @@ impl ConcreteLineToken { | HeredocClose { symbol: contents } | ModKeyword { contents } => contents.len(), HardNewLine | Comma | Space | Dot | OpenSquareBracket | CloseSquareBracket - | OpenCurlyBracket | CloseCurlyBracket | OpenParen | CloseParen | SingleSlash => 1, - DoKeyword | CommaSpace | LonelyOperator | ColonColon | DoubleQuote => 2, + | OpenCurlyBracket | CloseCurlyBracket | OpenParen | CloseParen | SingleSlash + | DoubleQuote => 1, + DoKeyword | CommaSpace | LonelyOperator | ColonColon => 2, DefKeyword | Ellipsis | End => 3, // "def"/"..."/"end" ClassKeyword => 5, // "class" ModuleKeyword => 6, // "module" @@ -195,6 +221,9 @@ impl From for AbstractLineToken { ConcreteLineTokenAndTargets::ConcreteLineToken(clt) => { AbstractLineToken::ConcreteLineToken(clt) } + ConcreteLineTokenAndTargets::BreakableCallChainEntry(bcce) => { + AbstractLineToken::BreakableCallChainEntry(bcce) + } } } } @@ -203,6 +232,7 @@ impl From for AbstractLineToken { pub enum ConcreteLineTokenAndTargets { ConcreteLineToken(ConcreteLineToken), BreakableEntry(BreakableEntry), + BreakableCallChainEntry(BreakableCallChainEntry), } impl ConcreteLineTokenAndTargets { @@ -228,6 +258,12 @@ impl ConcreteLineTokenAndTargets { .fold("".to_string(), |accum, tok| { format!("{}{}", accum, tok.into_ruby()) }), + Self::BreakableCallChainEntry(bcce) => bcce + .into_tokens(ConvertType::SingleLine) + .into_iter() + .fold("".to_string(), |accum, tok| { + format!("{}{}", accum, tok.into_ruby()) + }), Self::ConcreteLineToken(clt) => clt.into_ruby(), } } @@ -241,31 +277,35 @@ pub enum AbstractLineToken { SoftNewline(Option>), SoftIndent { depth: u32 }, BreakableEntry(BreakableEntry), + BreakableCallChainEntry(BreakableCallChainEntry), } impl AbstractLineToken { - pub fn into_single_line(self) -> ConcreteLineTokenAndTargets { + pub fn into_single_line(self) -> Vec { match self { - Self::CollapsingNewLine(_) => { - // we ignore the heredoc part of the collapsing newline here because the - // line length check is only used to calculate if we're going to render - // the breakable as multiline, and we always render heredoc strings as - // multiline - ConcreteLineTokenAndTargets::ConcreteLineToken(ConcreteLineToken::DirectPart { - part: "".to_string(), - }) + Self::CollapsingNewLine(heredoc_strings) => { + let mut res = Vec::new(); + if heredoc_strings.is_some() { + res.push(cltats_hard_newline()); + } + res.extend(Self::shimmy_and_shake_heredocs(heredoc_strings)); + res + } + Self::SoftNewline(heredoc_strings) => { + let mut res = vec![ConcreteLineTokenAndTargets::ConcreteLineToken( + ConcreteLineToken::Space, + )]; + res.extend(Self::shimmy_and_shake_heredocs(heredoc_strings)); + res } - Self::SoftNewline(_) => { - // see comment above - ConcreteLineTokenAndTargets::ConcreteLineToken(ConcreteLineToken::Space) + Self::SoftIndent { .. } => Vec::new(), + Self::ConcreteLineToken(clt) => { + vec![ConcreteLineTokenAndTargets::ConcreteLineToken(clt)] } - Self::SoftIndent { .. } => { - ConcreteLineTokenAndTargets::ConcreteLineToken(ConcreteLineToken::DirectPart { - part: "".to_string(), - }) + Self::BreakableEntry(be) => vec![ConcreteLineTokenAndTargets::BreakableEntry(be)], + Self::BreakableCallChainEntry(bcce) => { + vec![ConcreteLineTokenAndTargets::BreakableCallChainEntry(bcce)] } - Self::ConcreteLineToken(clt) => ConcreteLineTokenAndTargets::ConcreteLineToken(clt), - Self::BreakableEntry(be) => ConcreteLineTokenAndTargets::BreakableEntry(be), } } @@ -290,6 +330,9 @@ impl AbstractLineToken { vec![ConcreteLineTokenAndTargets::ConcreteLineToken(clt)] } Self::BreakableEntry(be) => vec![ConcreteLineTokenAndTargets::BreakableEntry(be)], + Self::BreakableCallChainEntry(bcce) => { + vec![ConcreteLineTokenAndTargets::BreakableCallChainEntry(bcce)] + } } } @@ -300,7 +343,7 @@ impl AbstractLineToken { if let Some(values) = heredoc_strings { for hds in values { let indent = hds.indent; - let kind = hds.kind.clone(); + let kind = hds.kind; let symbol = hds.closing_symbol(); let s = hds.render_as_string(); diff --git a/librubyfmt/src/parser_state.rs b/librubyfmt/src/parser_state.rs index 7ee215d2c..e160edc4a 100644 --- a/librubyfmt/src/parser_state.rs +++ b/librubyfmt/src/parser_state.rs @@ -5,8 +5,10 @@ use crate::format::{format_inner_string, StringType}; use crate::heredoc_string::{HeredocKind, HeredocString}; use crate::line_tokens::*; use crate::render_queue_writer::{RenderQueueWriter, MAX_LINE_LENGTH}; -use crate::render_targets::{AbstractTokenTarget, BaseQueue, BreakableEntry}; -use crate::ripper_tree_types::StringContentPart; +use crate::render_targets::{ + AbstractTokenTarget, BaseQueue, BreakableCallChainEntry, BreakableEntry, +}; +use crate::ripper_tree_types::{CallChainElement, StringContentPart}; use crate::types::{ColNumber, LineNumber}; use log::debug; use std::io::{self, Cursor, Write}; @@ -91,10 +93,11 @@ where fn emit_op(&mut self, op: String); fn emit_def(&mut self, def_name: String); fn emit_indent(&mut self); - fn emit_heredoc_start(&mut self, hd_type: String, symbol: String); + fn emit_heredoc_start(&mut self, symbol: String, kind: HeredocKind); fn emit_after_call_chain(&mut self); fn emit_data_end(&mut self); fn emit_data(&mut self, data: &str); + fn emit_single_line_delims(&mut self, delims: BreakableDelims); // other state changers fn bind_variable(&mut self, s: String); @@ -125,13 +128,20 @@ where // blocks fn start_indent(&mut self); + fn start_indent_for_call_chain(&mut self); + fn end_indent_for_call_chain(&mut self); fn end_indent(&mut self); fn with_formatting_context(&mut self, fc: FormattingContext, f: RenderFunc); fn new_scope(&mut self, f: RenderFunc); fn new_block(&mut self, f: RenderFunc); fn with_start_of_line(&mut self, start_of_line: bool, f: RenderFunc); - fn breakable_of(&mut self, delims: BreakableDelims, f: RenderFunc) -> bool; + fn breakable_of(&mut self, delims: BreakableDelims, f: RenderFunc); fn inline_breakable_of(&mut self, delims: BreakableDelims, f: RenderFunc); + fn breakable_call_chain_of( + &mut self, + call_chain_elements: Vec, + f: RenderFunc, + ); fn dedent(&mut self, f: RenderFunc); fn reset_space_count(&mut self); fn with_absorbing_indent_block(&mut self, f: RenderFunc); @@ -217,9 +227,8 @@ impl ConcreteParserState for BaseParserState { )); } - fn emit_heredoc_start(&mut self, hd_type: String, symbol: String) { - self.push_concrete_token(ConcreteLineToken::DirectPart { part: hd_type }); - self.push_concrete_token(ConcreteLineToken::DirectPart { part: symbol }); + fn emit_heredoc_start(&mut self, symbol: String, kind: HeredocKind) { + self.push_concrete_token(ConcreteLineToken::HeredocStart { kind, symbol }); } fn magic_handle_comments_for_multiline_arrays<'a>( @@ -270,7 +279,7 @@ impl ConcreteParserState for BaseParserState { let data = next_ps.render_to_buffer(); let s = str::from_utf8(&data).expect("string is utf8").to_string(); - s.trim().contains('\n') + s.trim().contains('\n') || s.len() > MAX_LINE_LENGTH } fn will_render_beyond_max_line_length<'a>(&mut self, f: RenderFunc) -> bool { @@ -302,6 +311,14 @@ impl ConcreteParserState for BaseParserState { self.depth_stack[ds_length - 1].increment(); } + fn start_indent_for_call_chain(&mut self) { + self.push_concrete_token(ConcreteLineToken::BeginCallChainIndent) + } + + fn end_indent_for_call_chain(&mut self) { + self.push_concrete_token(ConcreteLineToken::EndCallChainIndent) + } + fn end_indent(&mut self) { let ds_length = self.depth_stack.len(); self.depth_stack[ds_length - 1].decrement(); @@ -313,13 +330,9 @@ impl ConcreteParserState for BaseParserState { self.start_of_line.pop(); } - fn breakable_of<'a>(&mut self, delims: BreakableDelims, f: RenderFunc) -> bool { + fn breakable_of<'a>(&mut self, delims: BreakableDelims, f: RenderFunc) { self.shift_comments(); - let mut be = BreakableEntry::new( - self.current_spaces(), - delims, - self.current_formatting_context(), - ); + let mut be = BreakableEntry::new(delims, self.formatting_context.clone()); be.push_line_number(self.current_orig_line_number); self.breakable_entry_stack.push(Box::new(be)); @@ -343,21 +356,16 @@ impl ConcreteParserState for BaseParserState { .breakable_entry_stack .pop() .expect("cannot have empty here because we just pushed") - .to_breakable_entry(); - let is_multiline = insert_be.is_multiline(); + .to_breakable_entry() + .expect("This should be the BreakableEntry we just pushed"); self.push_target(ConcreteLineTokenAndTargets::BreakableEntry(insert_be)); - is_multiline } /// A version of `breakable_of` for list-like things that use whitespace delimiters. /// At the moment, this is only for conditions in a `when` clause fn inline_breakable_of<'a>(&mut self, delims: BreakableDelims, f: RenderFunc) { self.shift_comments(); - let mut be = BreakableEntry::new( - self.current_spaces(), - delims, - self.current_formatting_context(), - ); + let mut be = BreakableEntry::new(delims, self.formatting_context.clone()); be.push_line_number(self.current_orig_line_number); self.breakable_entry_stack.push(Box::new(be)); @@ -375,10 +383,35 @@ impl ConcreteParserState for BaseParserState { .breakable_entry_stack .pop() .expect("cannot have empty here because we just pushed") - .to_breakable_entry(); + .to_breakable_entry() + .expect("This should be the BreakableEntry we just pushed"); self.push_target(ConcreteLineTokenAndTargets::BreakableEntry(insert_be)); } + fn breakable_call_chain_of<'a>( + &mut self, + call_chain_elements: Vec, + f: RenderFunc, + ) { + self.shift_comments(); + let mut be = + BreakableCallChainEntry::new(self.formatting_context.clone(), call_chain_elements); + be.push_line_number(self.current_orig_line_number); + self.breakable_entry_stack.push(Box::new(be)); + + f(self); + + let insert_bcce = self + .breakable_entry_stack + .pop() + .expect("cannot have empty here because we just pushed") + .to_breakable_call_chain() + .expect("This should be the BreakableCallChainEntry we just pushed"); + self.push_target(ConcreteLineTokenAndTargets::BreakableCallChainEntry( + insert_bcce, + )); + } + fn with_suppress_comments<'a>(&mut self, suppress: bool, f: RenderFunc) { self.suppress_comments_stack.push(suppress); f(self); @@ -587,10 +620,7 @@ impl ConcreteParserState for BaseParserState { fn shift_comments_at_index(&mut self, index: usize) { if let Some(new_comments) = self.comments_to_insert.take() { - self.insert_concrete_tokens( - index, - new_comments.into_line_tokens().into_iter().collect(), - ); + self.insert_concrete_tokens(index, new_comments.into_line_tokens()); } } @@ -629,6 +659,11 @@ impl ConcreteParserState for BaseParserState { self.push_concrete_token(ConcreteLineToken::OpenParen); } + fn emit_single_line_delims(&mut self, delims: BreakableDelims) { + self.push_concrete_token(delims.single_line_open()); + self.push_concrete_token(delims.single_line_close()); + } + fn emit_comma_space(&mut self) { self.push_concrete_token(ConcreteLineToken::CommaSpace) } @@ -765,7 +800,7 @@ impl ConcreteParserState for BaseParserState { self.push_concrete_token(ConcreteLineToken::HardNewLine); } - let kind = next_heredoc.kind.clone(); + let kind = next_heredoc.kind; let symbol = next_heredoc.closing_symbol(); let space_count = next_heredoc.indent; let string_contents = next_heredoc.render_as_string(); @@ -869,19 +904,13 @@ impl BaseParserState { let idx = self.index_of_prev_hard_newline(); let insert_idx = idx.unwrap_or(0); - // Insert SoftNewlines in breakable entries instead of - // HardNewlines. They're functionally the same (since at - // this point we know that the breakable will be multiline), - // but inserting newlines for comments is special-cased - // for soft newlines inside of breakables (and this special casing - // is sometimes broken by mixing newline types), so we should - // use SoftNewlines here instead. if self.breakable_entry_stack.last().is_some() { - let hd = self.gather_heredocs(); - self.breakable_entry_stack - .last_mut() - .unwrap() - .insert_at(insert_idx, &mut vec![AbstractLineToken::SoftNewline(hd)]); + self.breakable_entry_stack.last_mut().unwrap().insert_at( + insert_idx, + &mut vec![AbstractLineToken::ConcreteLineToken( + ConcreteLineToken::HardNewLine, + )], + ); } else { self.insert_concrete_tokens(insert_idx, vec![ConcreteLineToken::HardNewLine]); } @@ -945,6 +974,9 @@ impl BaseParserState { AbstractLineToken::BreakableEntry(be) => { ConcreteLineTokenAndTargets::BreakableEntry(be) } + AbstractLineToken::BreakableCallChainEntry(bcce) => { + ConcreteLineTokenAndTargets::BreakableCallChainEntry(bcce) + } _ => panic!("failed to convert"), } } diff --git a/librubyfmt/src/render_queue_writer.rs b/librubyfmt/src/render_queue_writer.rs index df7c95da2..686e3be82 100644 --- a/librubyfmt/src/render_queue_writer.rs +++ b/librubyfmt/src/render_queue_writer.rs @@ -1,7 +1,9 @@ +use crate::heredoc_string::HeredocKind; use crate::intermediary::{BlanklineReason, Intermediary}; use crate::line_tokens::*; -use crate::parser_state::FormattingContext; -use crate::render_targets::{AbstractTokenTarget, BreakableEntry, ConvertType}; +use crate::render_targets::{ + AbstractTokenTarget, BreakableCallChainEntry, BreakableEntry, ConvertType, +}; #[cfg(debug_assertions)] use log::debug; use std::io::{self, Write}; @@ -29,13 +31,95 @@ impl RenderQueueWriter { fn render_as(accum: &mut Intermediary, tokens: Vec) { use ConcreteLineToken::*; + let token_len = tokens.len(); + let mut peekable = tokens.into_iter().enumerate().peekable(); + let mut current_heredoc_kind: Option = None; + + while let Some((index, mut next_token)) = peekable.next() { + // Do any additional indentation changes caused by call chain rendering + match &next_token { + ConcreteLineTokenAndTargets::ConcreteLineToken(Indent { depth }) => { + let is_ending_heredoc_token = token_len > index + && matches!( + peekable.peek(), + Some(( + _, + ConcreteLineTokenAndTargets::ConcreteLineToken(HeredocClose { .. }) + )) + ); + if !is_ending_heredoc_token { + next_token = clats_indent(depth + (accum.additional_indent * 2)) + } + } + ConcreteLineTokenAndTargets::ConcreteLineToken(ConcreteLineToken::Comment { + contents, + }) => { + if !contents.is_empty() { + let mut new_contents: String = + (0..(accum.additional_indent * 2)).map(|_| ' ').collect(); + new_contents.push_str(contents.as_str()); + next_token = ConcreteLineTokenAndTargets::ConcreteLineToken( + ConcreteLineToken::Comment { + contents: new_contents, + }, + ) + } + } + ConcreteLineTokenAndTargets::ConcreteLineToken(ConcreteLineToken::DirectPart { + part, + }) => { + if current_heredoc_kind + .map(|k| k.is_squiggly()) + .unwrap_or(false) + { + let indent: String = + (0..(accum.additional_indent * 2)).map(|_| ' ').collect(); + let new_contents = part + .split('\n') + .map(|p| { + if p.is_empty() { + return p.to_string(); + } + let mut line = indent.clone(); + line.push_str(p); + line + }) + .collect::>() + .join("\n"); + next_token = clats_direct_part(new_contents) + } + } + ConcreteLineTokenAndTargets::ConcreteLineToken( + ConcreteLineToken::HeredocStart { kind, .. }, + ) => current_heredoc_kind = Some(*kind), + ConcreteLineTokenAndTargets::ConcreteLineToken( + ConcreteLineToken::HeredocClose { symbol }, + ) => { + // Bare heredocs (e.g. < {} + } - for next_token in tokens.into_iter() { match next_token { ConcreteLineTokenAndTargets::BreakableEntry(be) => { Self::format_breakable_entry(accum, be) } - ConcreteLineTokenAndTargets::ConcreteLineToken(x) => accum.push(x), + ConcreteLineTokenAndTargets::BreakableCallChainEntry(bcce) => { + Self::format_breakable_call_chain_entry(accum, bcce) + } + ConcreteLineTokenAndTargets::ConcreteLineToken(x) => match x { + BeginCallChainIndent => accum.additional_indent += 1, + EndCallChainIndent => accum.additional_indent -= 1, + _ => accum.push(x), + }, } if let Some( @@ -92,17 +176,12 @@ impl RenderQueueWriter { } if let Some( - [&ConcreteLineToken::HeredocClose { .. }, &ConcreteLineToken::HardNewLine, &ConcreteLineToken::Indent { .. }, &ConcreteLineToken::Indent { .. }, &ConcreteLineToken::Delim { .. }], + [&ConcreteLineToken::HeredocClose { .. }, &ConcreteLineToken::HardNewLine, &ConcreteLineToken::Indent { .. }, &ConcreteLineToken::Indent { .. }, &ConcreteLineToken::Delim { .. } + | &ConcreteLineToken::Dot + | &ConcreteLineToken::DirectPart { .. }], ) = accum.last::<5>() { - accum.fix_heredoc_delim_indent_mistake(); - } - - if let Some( - [&ConcreteLineToken::HeredocClose { .. }, &ConcreteLineToken::HardNewLine, &ConcreteLineToken::Indent { .. }, &ConcreteLineToken::Indent { .. }, &ConcreteLineToken::DirectPart { .. }], - ) = accum.last::<5>() - { - accum.fix_heredoc_direct_part_indent_mistake(); + accum.fix_heredoc_duplicate_indent_mistake(); } if let Some( @@ -115,11 +194,14 @@ impl RenderQueueWriter { } fn format_breakable_entry(accum: &mut Intermediary, be: BreakableEntry) { - let length = accum.current_line_length() + be.single_line_string_length(); + let length = be.single_line_string_length(accum.current_line_length()); + // We generally will force expressions embedded in strings to be on a single line, + // but if that expression has a heredoc nested in it, we should let it render across lines + // so that the collapsing newlines render properly. + let force_single_line = + !be.any_collapsing_newline_has_heredoc_content() && be.in_string_embexpr(); - if (length > MAX_LINE_LENGTH || be.is_multiline()) - && be.entry_formatting_context() != FormattingContext::StringEmbexpr - { + if !force_single_line && (length > MAX_LINE_LENGTH || be.is_multiline()) { Self::render_as(accum, be.into_tokens(ConvertType::MultiLine)); } else { Self::render_as(accum, be.into_tokens(ConvertType::SingleLine)); @@ -131,6 +213,24 @@ impl RenderQueueWriter { } } + fn format_breakable_call_chain_entry( + accum: &mut Intermediary, + mut bcce: BreakableCallChainEntry, + ) { + let length = bcce.single_line_string_length(accum.current_line_length()); + let must_multiline = + bcce.any_collapsing_newline_has_heredoc_content() && bcce.in_string_embexpr(); + if must_multiline + || ((length > MAX_LINE_LENGTH || bcce.is_multiline()) && !bcce.in_string_embexpr()) + { + let tokens = bcce.into_tokens(ConvertType::MultiLine); + Self::render_as(accum, tokens); + } else { + bcce.remove_call_chain_magic_tokens(); + Self::render_as(accum, bcce.into_tokens(ConvertType::SingleLine)); + } + } + fn write_final_tokens( writer: &mut W, mut tokens: Vec, diff --git a/librubyfmt/src/render_targets.rs b/librubyfmt/src/render_targets.rs index 0794e97e9..f17e71c01 100644 --- a/librubyfmt/src/render_targets.rs +++ b/librubyfmt/src/render_targets.rs @@ -1,7 +1,8 @@ use crate::delimiters::BreakableDelims; use crate::line_tokens::{AbstractLineToken, ConcreteLineToken, ConcreteLineTokenAndTargets}; use crate::parser_state::FormattingContext; -use crate::types::{ColNumber, LineNumber}; +use crate::ripper_tree_types::{CallChainElement, Expression, StringLiteral}; +use crate::types::LineNumber; use std::collections::HashSet; fn insert_at(idx: usize, target: &mut Vec, input: &mut Vec) { @@ -51,27 +52,60 @@ pub trait AbstractTokenTarget: std::fmt::Debug { fn into_tokens(self, ct: ConvertType) -> Vec; fn is_multiline(&self) -> bool; fn push_line_number(&mut self, number: LineNumber); - fn single_line_string_length(&self) -> usize; - fn index_of_prev_newline(&self) -> Option; - fn last_token_is_a_newline(&self) -> bool; - fn to_breakable_entry(self: Box) -> BreakableEntry; - fn len(&self) -> usize; + fn single_line_string_length(&self, current_line_length: usize) -> usize; + fn to_breakable_entry(self: Box) -> Option; + fn to_breakable_call_chain(self: Box) -> Option; + fn tokens(&self) -> &Vec; + fn any_collapsing_newline_has_heredoc_content(&self) -> bool; + + fn len(&self) -> usize { + self.tokens().len() + } + + fn index_of_prev_newline(&self) -> Option { + let first_idx = self + .tokens() + .iter() + .rposition(|v| v.is_newline() || v.is_comment()); + match first_idx { + Some(x) => { + if matches!(self.tokens()[x], AbstractLineToken::CollapsingNewLine(_)) + || matches!(self.tokens()[x], AbstractLineToken::SoftNewline(_)) + { + Some(x + 1) + } else { + Some(x) + } + } + None => None, + } + } + + fn last_token_is_a_newline(&self) -> bool { + match self.tokens().last() { + Some(x) => x.is_newline(), + _ => false, + } + } } #[derive(Debug, Clone)] pub struct BreakableEntry { - #[allow(dead_code)] - spaces: ColNumber, tokens: Vec, line_numbers: HashSet, delims: BreakableDelims, - context: FormattingContext, + context: Vec, } impl AbstractTokenTarget for BreakableEntry { - fn to_breakable_entry(self: Box) -> BreakableEntry { - *self + fn to_breakable_entry(self: Box) -> Option { + Some(*self) + } + + fn to_breakable_call_chain(self: Box) -> Option { + None } + fn push(&mut self, lt: AbstractLineToken) { self.tokens.push(lt); } @@ -96,7 +130,7 @@ impl AbstractTokenTarget for BreakableEntry { let mut new_tokens: Vec<_> = self .tokens .into_iter() - .map(|t| t.into_single_line()) + .flat_map(|t| t.into_single_line()) .collect(); new_tokens.insert(0, self.delims.single_line_open().into()); new_tokens.push(self.delims.single_line_close().into()); @@ -105,39 +139,14 @@ impl AbstractTokenTarget for BreakableEntry { } } - fn last_token_is_a_newline(&self) -> bool { - match self.tokens.last() { - Some(x) => x.is_newline(), - _ => false, - } - } - - fn index_of_prev_newline(&self) -> Option { - let first_idx = self - .tokens - .iter() - .rposition(|v| v.is_newline() || v.is_comment()); - match first_idx { - Some(x) => { - if matches!(self.tokens[x], AbstractLineToken::CollapsingNewLine(_)) - || matches!(self.tokens[x], AbstractLineToken::SoftNewline(_)) - { - Some(x + 1) - } else { - Some(x) - } - } - None => None, - } - } - - fn single_line_string_length(&self) -> usize { + fn single_line_string_length(&self, current_line_length: usize) -> usize { self.tokens .iter() - .map(|tok| tok.clone().into_single_line()) + .flat_map(|tok| tok.clone().into_single_line()) .map(|tok| tok.into_ruby().len()) .sum::() + self.delims.single_line_len() + + current_line_length } fn push_line_number(&mut self, number: LineNumber) { @@ -150,15 +159,25 @@ impl AbstractTokenTarget for BreakableEntry { || self.contains_hard_newline() } - fn len(&self) -> usize { - self.tokens.len() + fn tokens(&self) -> &Vec { + &self.tokens + } + + fn any_collapsing_newline_has_heredoc_content(&self) -> bool { + self.tokens().iter().any(|t| match t { + AbstractLineToken::CollapsingNewLine(Some(..)) => true, + AbstractLineToken::SoftNewline(Some(..)) => true, + AbstractLineToken::BreakableEntry(be) => { + be.any_collapsing_newline_has_heredoc_content() + } + _ => false, + }) } } impl BreakableEntry { - pub fn new(spaces: ColNumber, delims: BreakableDelims, context: FormattingContext) -> Self { + pub fn new(delims: BreakableDelims, context: Vec) -> Self { BreakableEntry { - spaces, tokens: Vec::new(), line_numbers: HashSet::new(), delims, @@ -166,27 +185,241 @@ impl BreakableEntry { } } - pub fn entry_formatting_context(&self) -> FormattingContext { + pub fn in_string_embexpr(&self) -> bool { self.context + .iter() + .any(|fc| fc == &FormattingContext::StringEmbexpr) + } + + fn contains_hard_newline(&self) -> bool { + self.tokens.iter().any(|t| { + matches!( + t, + AbstractLineToken::ConcreteLineToken(ConcreteLineToken::HardNewLine) + ) + }) + } +} + +#[derive(Debug, Clone)] +pub struct BreakableCallChainEntry { + tokens: Vec, + line_numbers: HashSet, + call_chain: Vec, + context: Vec, +} + +impl AbstractTokenTarget for BreakableCallChainEntry { + fn to_breakable_entry(self: Box) -> Option { + None + } + + fn tokens(&self) -> &Vec { + &self.tokens + } + + fn to_breakable_call_chain(self: Box) -> Option { + Some(*self) + } + + fn push(&mut self, lt: AbstractLineToken) { + self.tokens.push(lt); + } + + fn insert_at(&mut self, idx: usize, tokens: &mut Vec) { + insert_at(idx, &mut self.tokens, tokens) + } + + fn into_tokens(self, ct: ConvertType) -> Vec { + match ct { + ConvertType::MultiLine => self + .tokens + .into_iter() + .flat_map(|t| t.into_multi_line()) + .collect(), + ConvertType::SingleLine => self + .tokens + .into_iter() + .flat_map(|t| t.into_single_line()) + .collect(), + } + } + + fn single_line_string_length(&self, current_line_length: usize) -> usize { + // Render all tokens to strings, but since these are call chains, they may + // have multiline blocks (which will often be quite long vertically, even if + // they're under 120 characters horizontally). In this case, look for the longest + // individual line and get _that_ max length. + let mut tokens = self.tokens.clone(); + if tokens.len() > 2 { + if let Some(AbstractLineToken::ConcreteLineToken(ConcreteLineToken::End)) = + tokens.get(tokens.len() - 2) + { + // Pop off all tokens that make up the `do`/`end` block (but not `do`!), + // since we assume that the block contents will handle their own line + // length appropriately. + while let Some(token) = tokens.last() { + if matches!( + token, + AbstractLineToken::ConcreteLineToken(ConcreteLineToken::DoKeyword) + ) { + break; + } + tokens.pop(); + } + } + } + + if let Some(AbstractLineToken::BreakableEntry(_)) = tokens.first() { + tokens.remove(0); + } + // EndCallChainIndent, which we don't care about + tokens.pop(); + // If the last breakable extends beyond the line length but the call chain doesn't, + // the breakable will break itself, e.g. + // ```ruby + // # ↓ if the break is here, we'll break the parens instead of the call chain + // AssumeThisIs.one_hundred_twenty_characters(breaks_here) + // ``` + if let Some(AbstractLineToken::BreakableEntry(_)) = tokens.last() { + tokens.pop(); + } + tokens.insert( + 0, + AbstractLineToken::ConcreteLineToken( + // Push the starting indentation for the first line -- other + // lines will already have the appropriate indentation + ConcreteLineToken::Indent { + depth: current_line_length as u32, + }, + ), + ); + + tokens + .into_iter() + .flat_map(|t| t.into_single_line()) + .map(|t| t.into_ruby()) + .collect::() + .len() + } + + fn push_line_number(&mut self, number: LineNumber) { + self.line_numbers.insert(number); + } + + fn is_multiline(&self) -> bool { + if self.begins_with_heredoc() { + return true; + } + + let mut call_chain_to_check = self.call_chain.as_slice(); + // We don't always want to multiline blocks if their only usage + // is at the end of a chain, since it's common to have chains + // that end with long blocks, but those blocks don't mean we should + // multiline the rest of the chain. + // + // example: + // ``` + // items.get_all.each do + // end + // ``` + if let Some(CallChainElement::Block(..)) = call_chain_to_check.last() { + call_chain_to_check = &call_chain_to_check[..call_chain_to_check.len() - 1]; + } + + let has_leading_expression = match call_chain_to_check.first() { + Some(CallChainElement::Expression(expr)) => !expr.is_constant_reference(), + _ => false, + }; + let has_comments = self.tokens.iter().any(|t| { + matches!( + t, + AbstractLineToken::ConcreteLineToken(ConcreteLineToken::Comment { .. }) + ) + }); + + // If the first item in the chain is a multiline expression (like a hash or array), + // ignore it when checking line length. + // Don't ignore this if there are comments in the call chain though; this check may + // cause it to single-lined, which breaks comment rendering. + if has_leading_expression && !has_comments { + call_chain_to_check = &call_chain_to_check[1..]; + } + + let chain_is_user_multilined = call_chain_to_check + .iter() + .filter_map(|cc_elem| cc_elem.start_line()) + .collect::>() + .len() + > 1; + + if chain_is_user_multilined { + return true; + } + + false } fn any_collapsing_newline_has_heredoc_content(&self) -> bool { - self.tokens.iter().any(|t| match t { + self.tokens().iter().any(|t| match t { AbstractLineToken::CollapsingNewLine(Some(..)) => true, AbstractLineToken::SoftNewline(Some(..)) => true, AbstractLineToken::BreakableEntry(be) => { be.any_collapsing_newline_has_heredoc_content() } _ => false, + }) || self.call_chain.iter().any(|cce| match cce { + // In cases where the heredoc is the first item in the call chain, + // it won't get stored in an abstract token; instead, it'll be directly + // in the call chain as a concrete token. + CallChainElement::Expression(expr) => { + matches!( + expr.as_ref(), + Expression::StringLiteral(StringLiteral::Heredoc(..)) + ) + } + _ => false, }) } +} - fn contains_hard_newline(&self) -> bool { - self.tokens.iter().any(|t| { - matches!( +impl BreakableCallChainEntry { + pub fn new(context: Vec, call_chain: Vec) -> Self { + BreakableCallChainEntry { + tokens: Vec::new(), + line_numbers: HashSet::new(), + context, + call_chain, + } + } + + /// Removes `BeginCallChainIndent` and `EndCallChainIndent`, which is only really + /// necessary when rendering a call chain as single-line. This prevents unnecessariliy + /// increasing the indentation for a trailing block in e.g. `thing.each do; /* block */; end` + pub fn remove_call_chain_magic_tokens(&mut self) { + self.tokens.retain(|t| { + !matches!( t, - AbstractLineToken::ConcreteLineToken(ConcreteLineToken::HardNewLine) + AbstractLineToken::ConcreteLineToken( + ConcreteLineToken::BeginCallChainIndent | ConcreteLineToken::EndCallChainIndent + ) ) - }) + }); + } + + pub fn in_string_embexpr(&self) -> bool { + self.context + .iter() + .any(|fc| fc == &FormattingContext::StringEmbexpr) + } + + fn begins_with_heredoc(&self) -> bool { + if let Some(CallChainElement::Expression(expr)) = self.call_chain.first() { + if let Expression::StringLiteral(string_literal) = &**expr { + return matches!(string_literal, StringLiteral::Heredoc(..)); + } + } + + false } } diff --git a/librubyfmt/src/ripper_tree_types.rs b/librubyfmt/src/ripper_tree_types.rs index 26f47cff4..a24e0fbed 100644 --- a/librubyfmt/src/ripper_tree_types.rs +++ b/librubyfmt/src/ripper_tree_types.rs @@ -160,6 +160,146 @@ pub enum Expression { Yield0(Yield0), } +impl Expression { + pub fn is_constant_reference(&self) -> bool { + use Expression::*; + matches!( + self, + VarRef(..) | TopConstRef(..) | Ident(..) | Const(..) | ConstPathRef(..) + ) + } + + /// This _is_ a lot of boilerplate, but there's some method to this madness. + /// Notably, not all expressions have a "trustworthy" `StartEnd`. There's an + /// important distinction between which of these contructs that come from the parser + /// and which are lexical constructions. For example, a hash will have an accurate + /// StartEnd because it's beginning and end are clearly defined in the grammar. + /// + /// However, some things -- take a Command for example -- are ambiguous by design. + /// In the case of Commands, since they don't use parens, their StartEnd will continue + /// all the way until the next expression, since technically a method argument could + /// be anywhere in the interim, so it's StartEnd is far longer than the actual call. + /// + /// Many such constructs here are unreliable and thus use a proxy instead, e.g. the first + /// nested expression, the nearest identifier, etc. These are broadly grouped into "categories", + /// but there are unfortunately many cases of ambiguity, hence this gigantic match statement + /// (and the similar functions on other related structs). + pub fn start_line(&self) -> Option { + match self { + // Expressions with a StartEnd (ideally most/all of them would end up here) + Expression::Class(Class(.., start_end)) + | Expression::If(If(.., start_end)) + | Expression::VCall(VCall(.., start_end)) + | Expression::Def(Def(.., start_end)) + | Expression::Defs(Defs(.., start_end)) + | Expression::SymbolLiteral(SymbolLiteral(.., start_end)) + | Expression::DynaSymbol(DynaSymbol(.., start_end)) + | Expression::Begin(Begin(_, start_end, ..)) + | Expression::Array(Array(.., start_end)) + | Expression::Next(Next(.., start_end)) + | Expression::Super(Super(.., start_end)) + | Expression::Module(Module(.., start_end)) + | Expression::Return(Return(.., start_end)) + | Expression::Return0(Return0(_, start_end)) + | Expression::Hash(Hash(.., start_end)) + | Expression::Yield(Yield(.., start_end)) + | Expression::While(While(.., start_end)) + | Expression::Case(Case(.., start_end)) + | Expression::Retry(Retry(.., start_end)) + | Expression::Redo(Redo(.., start_end)) + | Expression::SClass(SClass(.., start_end)) + | Expression::Break(Break(.., start_end)) + | Expression::StabbyLambda(StabbyLambda(.., start_end)) + | Expression::Unless(Unless(.., start_end)) + | Expression::ZSuper(ZSuper(.., start_end)) + | Expression::Yield0(Yield0(.., start_end)) => Some(start_end.start_line()), + // Expressions with a LineCol + Expression::TopConstRef(TopConstRef(_, Const(.., linecol))) + | Expression::Ident(Ident(.., linecol)) + | Expression::Int(Int(.., linecol)) + | Expression::Const(Const(.., linecol)) + | Expression::Kw(Kw(.., linecol)) + | Expression::Float(Float(.., linecol)) + | Expression::Char(Char(.., linecol)) + | Expression::Backref(Backref(.., linecol)) + | Expression::Imaginary(Imaginary(.., linecol)) + | Expression::Rational(Rational(.., linecol)) => Some(linecol.0), + // Expressions with locations defined by nested expressions + Expression::RescueMod(RescueMod(_, expr, _)) + | Expression::ToProc(ToProc(_, expr)) + | Expression::Unary(Unary(.., expr)) + | Expression::ConstPathRef(ConstPathRef(_, expr, ..)) + | Expression::Defined(Defined(.., expr)) + | Expression::Binary(Binary(_, expr, ..)) + | Expression::WhileMod(WhileMod(_, expr, ..)) + | Expression::UntilMod(UntilMod(_, expr, ..)) + | Expression::IfMod(IfMod(_, expr, ..)) + | Expression::UnlessMod(UnlessMod(_, expr, ..)) + | Expression::Until(Until(_, expr, ..)) + | Expression::For(For(_, _, expr, _)) + | Expression::IfOp(IfOp(_, expr, ..)) => expr.start_line(), + // Miscellaneous expressions with special cases + Expression::VoidStmt(..) => None, + Expression::Paren(ParenExpr(.., paren_expr, _)) => paren_expr.start_line(), + Expression::MLhs(MLhs(mlhs_inners)) => { + mlhs_inners.first().and_then(|mlhs| mlhs.start_line()) + } + Expression::MethodAddBlock(MethodAddBlock(_, call_left, ..)) => call_left.start_line(), + Expression::RegexpLiteral(RegexpLiteral(_, string_content_parts, _)) => { + string_content_parts + .first() + .and_then(|scp| scp.start_line()) + } + Expression::OpAssign(OpAssign(_, assignable, ..)) => assignable.start_line(), + Expression::Params(params) => Some(params.as_ref().8.start_line()), + Expression::MethodCall(MethodCall(_, call_chain_elements, ..)) => { + call_chain_elements.first().and_then(|cce| cce.start_line()) + } + Expression::MethodAddArg(MethodAddArg(_, call_left, ..)) + | Expression::CommandCall(CommandCall(_, call_left, ..)) + | Expression::Call(Call(_, call_left, ..)) => call_left.start_line(), + Expression::BareAssocHash(BareAssocHash(_, assocs)) => { + assocs.first().and_then(|a| a.start_line()) + } + Expression::Symbol(Symbol(_, symbol_type)) => symbol_type.start_line(), + Expression::BeginBlock(BeginBlock(_, exprs)) + | Expression::EndBlock(EndBlock(_, exprs)) => { + exprs.first().and_then(|expr| expr.start_line()) + } + // Pick the first of either expression, since these can be e.g. `foo..bar` or `foo..` or `..bar` + Expression::Dot2(Dot2(_, maybe_first_expr, maybe_second_expr)) + | Expression::Dot3(Dot3(_, maybe_first_expr, maybe_second_expr)) => maybe_first_expr + .as_ref() + .map(|mfe| mfe.start_line()) + .or_else(|| maybe_second_expr.as_ref().map(|mse| mse.start_line())) + .flatten(), + Expression::Alias(Alias(_, symbol, ..)) => Some(symbol.start_line()), + Expression::StringLiteral(string_literal) => Some(string_literal.start_line()), + Expression::XStringLiteral(XStringLiteral(_, string_parts)) => { + string_parts.first().and_then(|sp| sp.start_line()) + } + Expression::VarRef(VarRef(_, var_ref_type)) => Some(var_ref_type.start_line()), + Expression::Assign(Assign(_, assignable, ..)) => assignable.start_line(), + Expression::MAssign(MAssign(_, assignable_list_or_mlhs, ..)) => { + assignable_list_or_mlhs.start_line() + } + Expression::Command(Command(_, ident_or_const, ..)) => { + Some(ident_or_const.start_line()) + } + Expression::MRHSAddStar(MRHSAddStar(_, mrhs, ..)) => mrhs.start_line(), + Expression::StringConcat(StringConcat(_, concat_or_literal, ..)) => { + concat_or_literal.start_line() + } + Expression::Undef(Undef(_, symbol_literals)) => { + symbol_literals.first().map(|s| s.start_line()) + } + // Arefs only have an accurate closing line and not a starting line, + // so don't use it here + Expression::Aref(Aref(_, expr, ..)) => expr.start_line(), + } + } +} + def_tag!(mlhs_tag, "mlhs"); #[derive(Debug, Clone)] pub struct MLhs(pub Vec); @@ -173,6 +313,24 @@ pub enum MLhsInner { MLhs(Box), } +impl MLhsInner { + pub fn start_line(&self) -> Option { + match self { + MLhsInner::VarField(VarField(_, var_ref_type)) => Some(var_ref_type.start_line()), + MLhsInner::Field(Field(_, expr, ..)) => expr.start_line(), + MLhsInner::RestParam(RestParam(.., rest_param_assignable)) => rest_param_assignable + .as_ref() + .and_then(|rpa| rpa.start_line()), + MLhsInner::Ident(Ident(_, _, linecol)) => Some(linecol.0), + MLhsInner::MLhs(mlhs) => mlhs + .as_ref() + .0 + .first() + .and_then(|mlhs_inner| mlhs_inner.start_line()), + } + } +} + impl<'de> Deserialize<'de> for MLhs { fn deserialize(deserializer: D) -> Result where @@ -279,6 +437,15 @@ pub enum StringConcatOrStringLiteral { StringLiteral(StringLiteral), } +impl StringConcatOrStringLiteral { + pub fn start_line(&self) -> Option { + match self { + StringConcatOrStringLiteral::StringConcat(sc) => sc.1.start_line(), + StringConcatOrStringLiteral::StringLiteral(sl) => Some(sl.start_line()), + } + } +} + def_tag!(mrhs_add_star_tag, "mrhs_add_star"); #[derive(Deserialize, Debug, Clone)] pub struct MRHSAddStar( @@ -293,6 +460,17 @@ pub enum MRHSNewFromArgsOrEmpty { Empty(Vec), } +impl MRHSNewFromArgsOrEmpty { + pub fn start_line(&self) -> Option { + match self { + MRHSNewFromArgsOrEmpty::MRHSNewFromArgs(MRHSNewFromArgs(_, args_add_star, ..)) => { + args_add_star.start_line() + } + MRHSNewFromArgsOrEmpty::Empty(exprs) => exprs.first().and_then(|e| e.start_line()), + } + } +} + def_tag!(mrhs_new_from_args_tag, "mrhs_new_from_args"); #[derive(Deserialize, Debug, Clone)] pub struct MRHSNewFromArgs( @@ -392,6 +570,17 @@ pub enum AssignableListOrMLhs { MLhs(MLhs), } +impl AssignableListOrMLhs { + pub fn start_line(&self) -> Option { + match self { + AssignableListOrMLhs::AssignableList(al) => { + al.first().and_then(|assignable| assignable.start_line()) + } + AssignableListOrMLhs::MLhs(mlhs) => mlhs.0.first().and_then(|inner| inner.start_line()), + } + } +} + #[derive(RipperDeserialize, Debug, Clone)] pub enum MRHSOrArray { MRHS(MRHS), @@ -405,6 +594,18 @@ pub enum RestParamAssignable { ArefField(ArefField), } +impl RestParamAssignable { + pub fn start_line(&self) -> Option { + match self { + RestParamAssignable::ArefField(ArefField(.., linecol)) + | RestParamAssignable::Ident(Ident(.., linecol)) => Some(linecol.0), + RestParamAssignable::VarField(VarField(.., var_ref_type)) => { + Some(var_ref_type.start_line()) + } + } + } +} + #[derive(RipperDeserialize, Debug, Clone)] pub enum Assignable { VarField(VarField), @@ -418,6 +619,25 @@ pub enum Assignable { Ident(Ident), } +impl Assignable { + pub fn start_line(&self) -> Option { + match self { + Assignable::VarField(VarField(.., var_ref_type)) => Some(var_ref_type.start_line()), + Assignable::RestParam(RestParam(.., rest_param_assignable)) => rest_param_assignable + .as_ref() + .and_then(|rpa| rpa.start_line()), + Assignable::ConstPathField(ConstPathField(.., Const(.., linecol))) + | Assignable::Ident(Ident(.., linecol)) + | Assignable::TopConstField(TopConstField(.., Const(.., linecol))) => Some(linecol.0), + Assignable::ArefField(ArefField(_, expr, ..)) => expr.start_line(), + Assignable::Field(Field(_, expr, ..)) => expr.start_line(), + Assignable::MLhs(MLhs(mlhs_inners)) => mlhs_inners + .first() + .and_then(|mlhs_inner| mlhs_inner.start_line()), + } + } +} + def_tag!(begin_block, "BEGIN"); #[derive(Deserialize, Debug, Clone)] pub struct BeginBlock(pub begin_block, pub Vec); @@ -467,6 +687,17 @@ pub enum VarRefType { } impl VarRefType { + pub fn start_line(&self) -> u64 { + match self { + VarRefType::GVar(GVar(.., linecol)) + | VarRefType::IVar(IVar(.., linecol)) + | VarRefType::CVar(CVar(.., linecol)) + | VarRefType::Ident(Ident(.., linecol)) + | VarRefType::Const(Const(.., linecol)) + | VarRefType::Kw(Kw(.., linecol)) => linecol.0, + } + } + pub fn to_local_string(self) -> String { match self { VarRefType::GVar(v) => v.1, @@ -506,6 +737,15 @@ pub enum StringLiteral { Heredoc(string_literal_tag, HeredocStringLiteral, StringContent), } +impl StringLiteral { + pub fn start_line(&self) -> u64 { + match self { + StringLiteral::Heredoc(_, HeredocStringLiteral(.., start_end), ..) + | StringLiteral::Normal(.., start_end) => start_end.start_line(), + } + } +} + def_tag!(xstring_literal_tag, "xstring_literal"); #[derive(Deserialize, Debug, Clone)] pub struct XStringLiteral(pub xstring_literal_tag, pub Vec); @@ -558,6 +798,18 @@ pub enum StringContentPart { StringDVar(StringDVar), } +impl StringContentPart { + pub fn start_line(&self) -> Option { + match self { + StringContentPart::TStringContent(TStringContent(.., linecol)) => Some(linecol.0), + StringContentPart::StringEmbexpr(StringEmbexpr(_, exprs)) => { + exprs.first().and_then(|expr| expr.start_line()) + } + StringContentPart::StringDVar(StringDVar(_, expr)) => expr.as_ref().start_line(), + } + } +} + def_tag!(string_content_tag, "string_content"); #[derive(Debug, Clone)] pub struct StringContent(pub string_content_tag, pub Vec); @@ -628,6 +880,18 @@ impl ArgsAddStarOrExpressionListOrArgsForward { pub fn empty() -> Self { ArgsAddStarOrExpressionListOrArgsForward::ExpressionList(vec![]) } + + pub fn start_line(&self) -> Option { + match self { + ArgsAddStarOrExpressionListOrArgsForward::ExpressionList(exprs) => { + exprs.first().and_then(|e| e.start_line()) + } + ArgsAddStarOrExpressionListOrArgsForward::ArgsAddStar(ArgsAddStar(_, aas, ..)) => { + aas.start_line() + } + ArgsAddStarOrExpressionListOrArgsForward::ArgsForward(ArgsForward(..)) => None, + } + } } def_tag!(args_add_star_tag, "args_add_star"); @@ -692,12 +956,34 @@ pub enum SymbolLiteralOrDynaSymbol { SymbolLiteral(SymbolLiteral), } +impl SymbolLiteralOrDynaSymbol { + pub fn start_line(&self) -> u64 { + match self { + SymbolLiteralOrDynaSymbol::DynaSymbol(DynaSymbol(.., start_end)) + | SymbolLiteralOrDynaSymbol::SymbolLiteral(SymbolLiteral(.., start_end)) => { + start_end.start_line() + } + } + } +} + #[derive(RipperDeserialize, Debug, Clone)] pub enum ParenExpressionOrExpressions { Expressions(Vec), Expression(Box), } +impl ParenExpressionOrExpressions { + pub fn start_line(&self) -> Option { + match self { + ParenExpressionOrExpressions::Expressions(exprs) => { + exprs.first().and_then(|e| e.start_line()) + } + ParenExpressionOrExpressions::Expression(expr) => expr.as_ref().start_line(), + } + } +} + def_tag!(paren_expr_tag, "paren"); #[derive(Deserialize, Debug, Clone)] pub struct ParenExpr( @@ -1195,6 +1481,15 @@ pub enum AssocNewOrAssocSplat { AssocSplat(Box), } +impl AssocNewOrAssocSplat { + pub fn start_line(&self) -> Option { + match self { + AssocNewOrAssocSplat::AssocNew(assoc_new) => assoc_new.as_ref().1.start_line(), + AssocNewOrAssocSplat::AssocSplat(assoc_splat) => assoc_splat.as_ref().1.start_line(), + } + } +} + def_tag!(assoc_new_tag, "assoc_new"); #[derive(Deserialize, Debug, Clone)] pub struct AssocNew(pub assoc_new_tag, pub AssocKey, pub Option); @@ -1209,6 +1504,15 @@ pub enum AssocKey { Expression(Expression), } +impl AssocKey { + pub fn start_line(&self) -> Option { + match self { + AssocKey::Label(Label(.., linecol)) => Some(linecol.0), + AssocKey::Expression(expr) => expr.start_line(), + } + } +} + def_tag!(label_tag, "@label"); #[derive(Deserialize, Debug, Clone)] pub struct Label(pub label_tag, pub String, pub LineCol); @@ -1239,6 +1543,14 @@ impl IdentOrConst { IdentOrConst::Const(c) => IdentOrOpOrKeywordOrConst::Const(c), } } + + pub fn start_line(&self) -> u64 { + match self { + IdentOrConst::Ident(Ident(.., linecol)) | IdentOrConst::Const(Const(.., linecol)) => { + linecol.0 + } + } + } } #[derive(RipperDeserialize, Debug, Clone)] @@ -1253,6 +1565,23 @@ pub enum IdentOrConstOrKwOrOpOrIvarOrGvarOrCvarOrBacktick { Backtick(Backtick), } +impl IdentOrConstOrKwOrOpOrIvarOrGvarOrCvarOrBacktick { + pub fn start_line(&self) -> Option { + match self { + IdentOrConstOrKwOrOpOrIvarOrGvarOrCvarOrBacktick::Ident(Ident(.., linecol)) + | IdentOrConstOrKwOrOpOrIvarOrGvarOrCvarOrBacktick::Const(Const(.., linecol)) + | IdentOrConstOrKwOrOpOrIvarOrGvarOrCvarOrBacktick::Keyword(Kw(.., linecol)) + | IdentOrConstOrKwOrOpOrIvarOrGvarOrCvarOrBacktick::Op(Op(_, _, linecol, _)) + | IdentOrConstOrKwOrOpOrIvarOrGvarOrCvarOrBacktick::IVar(IVar(.., linecol)) + | IdentOrConstOrKwOrOpOrIvarOrGvarOrCvarOrBacktick::GVar(GVar(.., linecol)) + | IdentOrConstOrKwOrOpOrIvarOrGvarOrCvarOrBacktick::CVar(CVar(.., linecol)) + | IdentOrConstOrKwOrOpOrIvarOrGvarOrCvarOrBacktick::Backtick(Backtick(.., linecol)) => { + Some(linecol.0) + } + } + } +} + def_tag!(symbol_tag, "symbol"); #[derive(Deserialize, Debug, Clone)] pub struct Symbol( @@ -1290,6 +1619,31 @@ pub enum CallLeft { } impl CallLeft { + pub fn start_line(&self) -> Option { + match self { + CallLeft::ZSuper(ZSuper(.., start_end)) + | CallLeft::Next(Next(.., start_end)) + | CallLeft::Yield(Yield(.., start_end)) + | CallLeft::Yield0(Yield0(.., start_end)) + | CallLeft::Super(Super(.., start_end)) => Some(start_end.start_line()), + CallLeft::Paren(ParenExpr(_, paren_expr_or_exprs, ..)) => { + paren_expr_or_exprs.start_line() + } + CallLeft::SingleParen(_, expr) => expr.start_line(), + CallLeft::Command(Command(_, ident_or_const, ..)) + | CallLeft::VCall(VCall(_, ident_or_const, _)) + | CallLeft::FCall(FCall(_, ident_or_const)) => Some(match ident_or_const { + IdentOrConst::Ident(Ident(.., linecol)) + | IdentOrConst::Const(Const(.., linecol)) => linecol.0, + }), + CallLeft::CommandCall(CommandCall(_, call_left, ..)) + | CallLeft::MethodAddArg(MethodAddArg(_, call_left, ..)) + | CallLeft::Call(Call(_, call_left, ..)) + | CallLeft::MethodAddBlock(MethodAddBlock(_, call_left, ..)) => call_left.start_line(), + CallLeft::VarRef(VarRef(_, var_ref_type)) => Some(var_ref_type.start_line()), + CallLeft::Expression(expr) => expr.start_line(), + } + } pub fn into_call_chain(self) -> Vec { match self { CallLeft::Paren(p) => vec![CallChainElement::Paren(p)], @@ -1350,6 +1704,26 @@ pub enum CallChainElement { Expression(Box), } +impl CallChainElement { + pub fn start_line(&self) -> Option { + match self { + CallChainElement::IdentOrOpOrKeywordOrConst(ident) => { + Some(ident.clone().to_def_parts().1 .0) + } + CallChainElement::Block(block) => Some(block.start_line()), + CallChainElement::VarRef(VarRef(.., var_ref_type)) => Some(var_ref_type.start_line()), + CallChainElement::ArgsAddStarOrExpressionListOrArgsForward(..) => { + // The start_end for these is generally incorrect, since the parser event + // only fires at the _end_ of the expression list + None + } + CallChainElement::DotTypeOrOp(d) => d.start_line(), + CallChainElement::Paren(pe) => pe.1.start_line(), + CallChainElement::Expression(expr) => expr.start_line(), + } + } +} + pub type DotCall = call_tag; def_tag!(method_add_arg_tag, "method_add_arg"); @@ -1540,6 +1914,20 @@ pub enum DotTypeOrOp { StringDot(String), } +impl DotTypeOrOp { + pub fn start_line(&self) -> Option { + match self { + DotTypeOrOp::Period(Period(.., linecol)) => Some(linecol.0), + DotTypeOrOp::DotType( + DotType::Dot(Dot(.., start_end)) + | DotType::LonelyOperator(LonelyOperator(.., start_end)), + ) + | DotTypeOrOp::Op(Op(.., start_end)) => Some(start_end.start_line()), + DotTypeOrOp::ColonColon(..) | DotTypeOrOp::StringDot(..) => None, + } + } +} + def_tag!(period_tag, "@period"); #[derive(Deserialize, Debug, Clone)] pub struct Period(pub period_tag, pub String, pub LineCol); @@ -1870,6 +2258,15 @@ pub enum Block { DoBlock(DoBlock), } +impl Block { + pub fn start_line(&self) -> u64 { + match self { + Block::BraceBlock(BraceBlock(.., start_end)) + | Block::DoBlock(DoBlock(.., start_end)) => start_end.start_line(), + } + } +} + // block local variables are a nightmare, they can be false, nil, or an array // of idents: //