From: Steve Hancock Date: Wed, 11 Dec 2024 16:03:03 +0000 (-0800) Subject: improve breaks at long method call chains, see git #171 X-Git-Tag: 20240903.08~1 X-Git-Url: https://git.donarmstrong.com/?a=commitdiff_plain;h=e90ad172931495a3863e9ce57ba66d7a58ae03ef;p=perltidy.git improve breaks at long method call chains, see git #171 --- diff --git a/CHANGES.md b/CHANGES.md index e33397aa..47c721cb 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,25 @@ ## 2024 09 03.07 + - Line breaks at long chains of method calls now break at all calls + with args in parens, as in this example from git #171 + + # Old default + sub bla_p( $value = 42 ) { + return Mojo::Promise->resolve($value)->then( sub { shift() / 2 } ) + ->then( sub { shift() + 6 } )->then( sub { shift() / 2 } ) + ->catch( sub { warn shift } ); + } + + # New default + sub bla_p( $value = 42 ) { + return Mojo::Promise->resolve($value) + ->then( sub { shift() / 2 } ) + ->then( sub { shift() + 6 } ) + ->then( sub { shift() / 2 } ) + ->catch( sub { warn shift } ); + } + - An update for parameter --break-at-old-method-breakpoints, or -bom, has been made to insure that it only applies to lines beginning with method calls, as intended. Line breaks for all lines beginning with @@ -1700,7 +1719,7 @@ ## 2019 06 01 - - rt #128477: Prevent inconsistent owner/group and setuid/setgid bits. + - rt #128477: Prevent inconsistent owner/group and setuid/setgid bits. In the -b (--backup-and-modify-in-place) mode, an attempt is made to set ownership of the output file equal to the input file, if they differ. In all cases, if the final output file ownership differs from input file, any setuid/setgid bits are cleared. diff --git a/dev-bin/run_convergence_tests.pl.expect b/dev-bin/run_convergence_tests.pl.expect index e6f52675..20ce74ec 100644 --- a/dev-bin/run_convergence_tests.pl.expect +++ b/dev-bin/run_convergence_tests.pl.expect @@ -7083,14 +7083,17 @@ SOAP::Transport::HTTP::Daemon->new ( LocalAddr => $host, LocalPort => $port, Reuse => 1, -)->dispatch_with ({ 'urn:WishListCustomer' => 'WishListCustomer' } ) - ->objects_by_reference ('WishListCustomer')->handle ; + ) + ->dispatch_with ({ 'urn:WishListCustomer' => 'WishListCustomer' } ) + ->objects_by_reference ('WishListCustomer') + ->handle ; SOAP::Transport::HTTP::Daemon->new (LocalAddr => $host, LocalPort => $port, Reuse => 1,) ->dispatch_with ({ 'urn:WishListCustomer' => 'WishListCustomer' } ) - ->objects_by_reference ('WishListCustomer')->handle ; + ->objects_by_reference ('WishListCustomer') + ->handle ; ==> b1395 <== @@ -7486,14 +7489,16 @@ print join( "\n", ==> b1430 <== $t->post_ok( "//$auth_userid:$password@/api/v1/advanced_editor/macros" => - json => $macro_values )->status_is(400) + json => $macro_values ) + ->status_is(400) ->json_is( '/errors' => [ { message => "Read-only.", path => "/body/macro_id", } ] ); $t->post_ok( "//$auth_userid:$password@/api/v1/advanced_editor/macros" => - json => $macro_values )->status_is(400) + json => $macro_values ) + ->status_is(400) ->json_is( '/errors' => [ { message => "Read-only.", path => "/body/macro_id", } ] ); @@ -7687,7 +7692,8 @@ $last = after ( ==> b1445 <== - $cfg-> + $cfg + -> { $type } @@ -7697,7 +7703,8 @@ $last = after ( die "Unrecognized $type number: $num\n"; - $cfg-> + $cfg + -> { $type } diff --git a/lib/Perl/Tidy.pm b/lib/Perl/Tidy.pm index e974f8a8..00cb1556 100644 --- a/lib/Perl/Tidy.pm +++ b/lib/Perl/Tidy.pm @@ -5409,7 +5409,7 @@ sub Win_OS_Type { # TODO: are these more standard names? # Win32s Win95 Win98 WinMe WinNT3.51 WinNT4 Win2000 WinXP/.Net Win2003 - my $os = EMPTY_STRING; + my $os = EMPTY_STRING; return $os unless ( $OSNAME =~ /win32|dos/i ); # is it a MS box? # Systems built from Perl source may not have Win32.pm diff --git a/lib/Perl/Tidy/Formatter.pm b/lib/Perl/Tidy/Formatter.pm index d9877b1a..83cff8f6 100644 --- a/lib/Perl/Tidy/Formatter.pm +++ b/lib/Perl/Tidy/Formatter.pm @@ -653,6 +653,9 @@ BEGIN { _last_vt_type_ => $i++, _rwant_arrow_before_seqno_ => $i++, + _rseqno_arrow_call_chain_start_ => $i++, + _rarrow_call_chain_ => $i++, + # these vars are defined after call to respace tokens: _rK_package_list_ => $i++, _rK_AT_underscore_by_sub_seqno_ => $i++, @@ -1175,6 +1178,9 @@ sub new { $self->[_last_vt_type_] = 0; $self->[_rwant_arrow_before_seqno_] = {}; + $self->[_rseqno_arrow_call_chain_start_] = {}; + $self->[_rarrow_call_chain_] = {}; + $self->[_save_logfile_] = defined($logger_object) && $logger_object->get_save_logfile(); @@ -2777,9 +2783,10 @@ sub initialize_token_break_preferences { $break_after->( split_words( $rOpts->{'want-break-after'} ) ); $break_before->( split_words( $rOpts->{'want-break-before'} ) ); - # make note if breaks are before certain key types + # Make note if breaks are before certain key types + # Added '->' for git #171. %want_break_before = (); - foreach my $tok ( @all_operators, ',' ) { + foreach my $tok ( @all_operators, ',', '->' ) { $want_break_before{$tok} = $left_bond_strength{$tok} < $right_bond_strength{$tok}; } @@ -12843,10 +12850,13 @@ my $rblock_type_of_seqno; my $rwant_arrow_before_seqno; my $ris_sub_block; my $ris_asub_block; +my $rseqno_arrow_call_chain_start; +my $rarrow_call_chain; my $K_opening_container; my $K_closing_container; my @K_sequenced_token_list; +my @seqno_paren_arrow; my %K_first_here_doc_by_seqno; @@ -12943,6 +12953,8 @@ sub initialize_respace_tokens_closure { $rK_return_by_sub_seqno = $self->[_rK_return_by_sub_seqno_]; $rK_wantarray_by_sub_seqno = $self->[_rK_wantarray_by_sub_seqno_]; $rsub_call_paren_info_by_seqno = $self->[_rsub_call_paren_info_by_seqno_]; + $rseqno_arrow_call_chain_start = $self->[_rseqno_arrow_call_chain_start_]; + $rarrow_call_chain = $self->[_rarrow_call_chain_]; $rDOLLAR_underscore_by_sub_seqno = $self->[_rDOLLAR_underscore_by_sub_seqno_]; $rK_sub_by_seqno = $self->[_rK_sub_by_seqno_]; @@ -12989,6 +13001,9 @@ sub initialize_respace_tokens_closure { @K_sequenced_token_list = (); + # array for saving seqno's of ')->' for possible line breaks, git #171 + @seqno_paren_arrow = (); + return; } ## end sub initialize_respace_tokens_closure @@ -13719,43 +13734,15 @@ EOM } } - # Old patch to add space to something like "x10". - # Note: This is now done in the Tokenizer, but this code remains - # for reference. - elsif ( $type eq 'n' ) { - if ( substr( $token, 0, 1 ) eq 'x' && $token =~ /^x\d+/ ) { - $token =~ s/x/x /; - $rtoken_vars->[_TOKEN_] = $token; - if (DEVEL_MODE) { - Fault(<' ) { + if ( $last_nonblank_code_token eq ')' ) { - # check for a qw quote - elsif ( $type eq 'q' ) { - - # Trim spaces from right of qw quotes. Also trim from the left for - # safety (the tokenizer should have done this). - # To avoid trimming qw quotes use -ntqw; this causes the - # tokenizer to set them as type 'Q' instead of 'q'. - $token =~ s/^ \s+ | \s+ $//gx; - $rtoken_vars->[_TOKEN_] = $token; - if ( $self->[_save_logfile_] && $token =~ /\t/ ) { - $self->note_embedded_tab($input_line_number); - } - if ( $rwhitespace_flags->[$KK] == WS_YES - && @{$rLL_new} - && $rLL_new->[-1]->[_TYPE_] ne 'b' - && $rOpts_add_whitespace ) - { - $self->store_token(); + # save seqno of closing paren with arrow, ')->', git #171 + # (the paren seqno is still on the stack) + my $seqno_paren = $seqno_stack{$depth_next}; + if ($seqno_paren) { push @seqno_paren_arrow, $seqno_paren } } - $self->store_token($rtoken_vars); - next; - } ## end if ( $type eq 'q' ) + } # delete repeated commas if requested elsif ( $type eq ',' ) { @@ -13799,6 +13786,28 @@ EOM } } } + + # check a quote for problems + elsif ( $type eq 'Q' ) { + $self->check_Q( $KK, $Kfirst, $input_line_number ) + if ( $self->[_save_logfile_] ); + } + + # Old patch to add space to something like "x10". + # Note: This is now done in the Tokenizer, but this code remains + # for reference. + elsif ( $type eq 'n' ) { + if ( substr( $token, 0, 1 ) eq 'x' && $token =~ /^x\d+/ ) { + $token =~ s/x/x /; + $rtoken_vars->[_TOKEN_] = $token; + if (DEVEL_MODE) { + Fault(<' ) { if ( $last_nonblank_code_type eq '=>' && $rOpts->{'delete-repeated-commas'} ) @@ -13835,17 +13844,35 @@ EOM } } + # check for a qw quote + elsif ( $type eq 'q' ) { + + # Trim spaces from right of qw quotes. Also trim from the left for + # safety (the tokenizer should have done this). + # To avoid trimming qw quotes use -ntqw; this causes the + # tokenizer to set them as type 'Q' instead of 'q'. + $token =~ s/^ \s+ | \s+ $//gx; + $rtoken_vars->[_TOKEN_] = $token; + if ( $self->[_save_logfile_] && $token =~ /\t/ ) { + $self->note_embedded_tab($input_line_number); + } + if ( $rwhitespace_flags->[$KK] == WS_YES + && @{$rLL_new} + && $rLL_new->[-1]->[_TYPE_] ne 'b' + && $rOpts_add_whitespace ) + { + $self->store_token(); + } + $self->store_token($rtoken_vars); + next; + } ## end if ( $type eq 'q' ) + # change 'LABEL :' to 'LABEL:' elsif ( $type eq 'J' ) { $token =~ s/\s+//g; $rtoken_vars->[_TOKEN_] = $token; } - # check a quote for problems - elsif ( $type eq 'Q' ) { - $self->check_Q( $KK, $Kfirst, $input_line_number ) - if ( $self->[_save_logfile_] ); - } else { # no special processing for this token type } @@ -14103,6 +14130,45 @@ EOM } } + # Search for chains of method calls of the form (git #171) + # )->xxx( )->xxx( )-> + # We have previously saved the seqno of all ')->' combinations + my $in_chain_seqno = 0; + while ( my $seqno = shift @seqno_paren_arrow ) { + + # ) -> func ( + # ) -> func ( + # $Kc--^ ^--$K_test + + my $Kc = $K_closing_container->{$seqno}; + my $K_arrow = $self->K_next_nonblank( $Kc, $rLL_new ); + my $K_func = $self->K_next_nonblank( $K_arrow, $rLL_new ); + my $K_test = $self->K_next_nonblank( $K_func, $rLL_new ); + + last if ( !defined($K_test) ); + + # ignore index operation like ')->{' or ')->[' and end any chain + my $tok = $rLL_new->[$K_func]->[_TOKEN_]; + if ( $tok eq '[' || $tok eq '{' ) { $in_chain_seqno = 0; next } + + # mark seqno of parens which are part of a call chain + my $seqno_start = $in_chain_seqno ? $in_chain_seqno : $seqno; + $rseqno_arrow_call_chain_start->{$seqno} = $seqno_start; + + # save a list of the arrows, needed to set line breaks + push @{ $rarrow_call_chain->{$seqno_start} }, $K_arrow; + + # See if this chain continues + if ( @seqno_paren_arrow + && defined($K_test) + && $rLL_new->[$K_test]->[_TOKEN_] eq '(' + && $rLL_new->[$K_test]->[_TYPE_SEQUENCE_] eq $seqno_paren_arrow[0] ) + { + $in_chain_seqno ||= $seqno; + } + else { $in_chain_seqno = 0 } + } ## end while ( my $seqno = shift...) + return; } ## end sub respace_post_loop_ops @@ -27232,6 +27298,8 @@ sub break_all_chain_tokens { my %left_chain_type; my %right_chain_type; my %interior_chain_type; + my @i_arrow_breaks; + my @insert_list; my $nmax = @{$ri_right} - 1; # scan the left and right end tokens of all lines @@ -27246,6 +27314,16 @@ sub break_all_chain_tokens { $typel = '*' if ( $typel eq '/' ); # treat * and / the same $typer = '*' if ( $typer eq '/' ); + # Breaks at method calls are handled specially below (git #171) + if ( $typel eq '->' && $want_break_before{$typel} ) { + push @i_arrow_breaks, $il; + next; + } + if ( $typer eq '->' && !$want_break_before{$typer} ) { + push @i_arrow_breaks, $ir; + next; + } + my $keyl = $typel eq 'k' ? $tokens_to_go[$il] : $typel; my $keyr = $typer eq 'k' ? $tokens_to_go[$ir] : $typer; if ( $is_chain_operator{$keyl} && $want_break_before{$typel} ) { @@ -27261,7 +27339,40 @@ sub break_all_chain_tokens { $count++; } } - return unless $count; + + # Handle any method call chain breaks immediately (git #171) + if (@i_arrow_breaks) { + my %is_end_i; + @is_end_i{ @{$ri_left} } = (1) x scalar( @{$ri_left} ); + @is_end_i{ @{$ri_right} } = (1) x scalar( @{$ri_right} ); + my $one = $want_break_before{'->'} ? 1 : 0; + + foreach my $ii (@i_arrow_breaks) { + my $ip = iprev_to_go($ii); + next if ( $ip < 0 || $tokens_to_go[$ip] ne ')' ); + my $seqno = $type_sequence_to_go[$ip]; + my $seqno_start = + $self->[_rseqno_arrow_call_chain_start_]->{$seqno}; + next unless ($seqno_start); + my @Klist = @{ $self->[_rarrow_call_chain_]->{$seqno_start} }; + my $Kref = $K_to_go[0]; + foreach my $KK (@Klist) { + my $i_K = $KK - $Kref; + next if ( $i_K <= 0 || $i_K >= $max_index_to_go ); + next if ( $is_end_i{$i_K} ); + if ( $K_to_go[$i_K] != $KK ) { + ## shouldn't happen due to previous checks on i vs K + DEVEL_MODE && Fault(<name('~V:Fault')->attr( { 'xmlns' => $SOAP::Constants::NS_ENV } ) + SOAP::Data->name('~V:Fault') + ->attr( { 'xmlns' => $SOAP::Constants::NS_ENV } ) ->value( \SOAP::Data->set_value( SOAP::Data->name( diff --git a/t/snippets17.t b/t/snippets17.t index 4abdf24c..6b9b1512 100644 --- a/t/snippets17.t +++ b/t/snippets17.t @@ -861,7 +861,8 @@ Some pod after __END__ to delete with -dp and trim with -trp # This single line should break into multiple lines, even with -l=0 # sub 'tight_paren_follows' should break the do block $body = - SOAP::Data->name('~V:Fault')->attr( { 'xmlns' => $SOAP::Constants::NS_ENV } ) + SOAP::Data->name('~V:Fault') + ->attr( { 'xmlns' => $SOAP::Constants::NS_ENV } ) ->value( \SOAP::Data->set_value( SOAP::Data->name(