From 51428db5975f27b49efe996310077bc330a48fa9 Mon Sep 17 00:00:00 2001 From: Steve Hancock Date: Sat, 17 Oct 2020 08:11:03 -0700 Subject: [PATCH] fix missing line break for hash of subs with signatures --- lib/Perl/Tidy/Formatter.pm | 35 ++++++++++++++++++----------- local-docs/BugLog.pod | 39 +++++++++++++++++++++++++++++---- t/snippets/expect/rt112534.def | 6 ++--- t/snippets/expect/signature.def | 4 +--- t/snippets/packing_list.txt | 6 ++--- t/snippets17.t | 4 +--- t/snippets7.t | 6 ++--- 7 files changed, 66 insertions(+), 34 deletions(-) diff --git a/lib/Perl/Tidy/Formatter.pm b/lib/Perl/Tidy/Formatter.pm index ebcda5e8..50817341 100644 --- a/lib/Perl/Tidy/Formatter.pm +++ b/lib/Perl/Tidy/Formatter.pm @@ -8899,9 +8899,10 @@ sub starting_one_line_block { my ( $self, $Kj, $K_last_nonblank, $K_last, $level, $slevel, $ci_level ) = @_; - my $rbreak_container = $self->[_rbreak_container_]; - my $rshort_nested = $self->[_rshort_nested_]; - my $rLL = $self->[_rLL_]; + my $rbreak_container = $self->[_rbreak_container_]; + my $rshort_nested = $self->[_rshort_nested_]; + my $rLL = $self->[_rLL_]; + my $K_opening_container = $self->[_K_opening_container_]; # kill any current block - we can only go 1 deep destroy_one_line_block(); @@ -8964,17 +8965,25 @@ sub starting_one_line_block { # If this brace follows a parenthesized list, we should look back to # find the keyword before the opening paren because otherwise we might # form a one line block which stays intack, and cause the parenthesized - # expression to break open. That looks bad. However, actually - # searching for the opening paren is slow and tedius. - # The actual keyword is often at the start of a line, but might not be. - # For example, we might have an anonymous sub with signature list - # following a =>. It is safe to mark the start anywhere before the - # opening paren, so we just go back to the prevoious break (or start of - # the line) if that is before the opening paren. The minor downside is - # that we may very occasionally break open a block unnecessarily. + # expression to break open. That looks bad. if ( $tokens_to_go[$i_start] eq ')' ) { - $i_start = $index_max_forced_break + 1; - if ( $types_to_go[$i_start] eq 'b' ) { $i_start++; } + + # Find the opening paren + my $K_start = $K_to_go[$i_start]; + return 0 unless defined($K_start); + my $seqno = $type_sequence_to_go[$i_start]; + return 0 unless ($seqno); + my $K_opening = $K_opening_container->{$seqno}; + return 0 unless defined($K_opening); + my $i_opening = $i_start + ( $K_opening - $K_start ); + + # give up if not on this line + return 0 unless ( $i_opening >= 0 ); + $i_start = $i_opening; ##$index_max_forced_break + 1; + + # go back one token before the opening paren + if ( $i_start > 0 ) { $i_start-- } + if ( $types_to_go[$i_start] eq 'b' && $i_start > 0 ) { $i_start--; } my $lev = $levels_to_go[$i_start]; if ( $lev > $level ) { return 0 } } diff --git a/local-docs/BugLog.pod b/local-docs/BugLog.pod index 1f0b7229..376baf5e 100644 --- a/local-docs/BugLog.pod +++ b/local-docs/BugLog.pod @@ -1,10 +1,39 @@ =head1 Issues fixed after release 20201001 +=item b + +During testing the following error was found and fixed. +Given the following input snippet: + + get( + on_ready => sub ($worker) { + $on_ready->end; + return; + }, + on_exit => sub ( $worker, $status ) { return; }, + ); + +The resulting formatting was + + get( + on_ready => sub ($worker) { + $on_ready->end; + return; + }, on_exit => sub ( $worker, $status ) { return; }, + ); + +Notice that the break after the comma has been lost. The problem was traced to +a short-cut taken by the code looking for one-line blocks. The unique +circumstances in which this occured involved a hash of anonymous subs, one with +a signature with multiple parameters and short enough to be a one-line block, +as in the last sub definition line. This was fixed 17 Oct 2020. + =item b Problems with parsing prototypes and signatures were found during testing and -fixed. For example the following snippet was mis-parsed because of the hash -mark. +fixed 17 Oct 2020 in 'fixed problem parsing multi-line signatures with +comments', 017fd07. For example the following snippet was mis-parsed because +of the hash mark. sub test ( # comment ))) $x, $x) { $x+$y } @@ -37,7 +66,7 @@ were parsed as a pattern. This problem was discovered in random testing. When a slash follows a bareword whose prototype is not known to perltidy, it has to guess whether the slash starts a pattern or is a division. The guessing logic was rewritten and -improved. +improved 14 Oct 2020 in 'rewrote logic to guess if divide or pattern', afebe2f. =item b @@ -48,7 +77,8 @@ at old isolated semicolons. For example ; In testing it was found not to be doing this after braces which require -semicolons, such as 'do' and anonymous subs. This was fixed. For example +semicolons, such as 'do' and anonymous subs. This was fixed 12 Oct 2020 in +'fix -bos to work with semicolons after braces', 03ee7fc. For example my $dist = sub { $z = sqrt( $x**2 + $y**2 ) @@ -75,6 +105,7 @@ it was dropped. For example, this would be kept intact: This keeps the list from shifting to the right and can avoid problems in formatting the list with certain styles, including with the -xci flag. +Fixed 12 Oct 2020 in 'keep break after use overload statement', 8485afa. =item B diff --git a/t/snippets/expect/rt112534.def b/t/snippets/expect/rt112534.def index 183fcea1..84c05727 100644 --- a/t/snippets/expect/rt112534.def +++ b/t/snippets/expect/rt112534.def @@ -1,7 +1,5 @@ get( on_ready => sub ($worker) { $on_ready->end; return; }, - on_exit => sub ( $worker, $status ) { - return; - }, - on_data => sub ($data) { $self->_on_data(@_) if $self; return; } + on_exit => sub ( $worker, $status ) { return; }, + on_data => sub ($data) { $self->_on_data(@_) if $self; return; } ); diff --git a/t/snippets/expect/signature.def b/t/snippets/expect/signature.def index de0dc882..5f11ab9d 100644 --- a/t/snippets/expect/signature.def +++ b/t/snippets/expect/signature.def @@ -11,9 +11,7 @@ sub foo ( $x, $y = do { {} }, $z = 42, $w = do { "abcd" } ) { } # This signature should get put back on one line -sub t022 ( $p = do { $z += 10; 222 }, $a = do { $z++; 333 } ) { - "$p/$a"; -} +sub t022 ( $p = do { $z += 10; 222 }, $a = do { $z++; 333 } ) { "$p/$a" } # anonymous sub with signature my $subref = sub ( $cat, $id = do { state $auto_id = 0; $auto_id++ } ) { diff --git a/t/snippets/packing_list.txt b/t/snippets/packing_list.txt index 18becc13..cdb09985 100644 --- a/t/snippets/packing_list.txt +++ b/t/snippets/packing_list.txt @@ -277,6 +277,9 @@ ../snippets22.t bbhb.bbhb4 ../snippets22.t bbhb.bbhb5 ../snippets22.t braces.braces7 +../snippets22.t xci.def +../snippets22.t xci.xci1 +../snippets22.t xci.xci2 ../snippets3.t ce_wn1.ce_wn ../snippets3.t ce_wn1.def ../snippets3.t colin.colin @@ -417,6 +420,3 @@ ../snippets9.t rt98902.def ../snippets9.t rt98902.rt98902 ../snippets9.t rt99961.def -../snippets22.t xci.def -../snippets22.t xci.xci1 -../snippets22.t xci.xci2 diff --git a/t/snippets17.t b/t/snippets17.t index 27b37534..b6686e7c 100644 --- a/t/snippets17.t +++ b/t/snippets17.t @@ -951,9 +951,7 @@ sub foo ( $x, $y = do { {} }, $z = 42, $w = do { "abcd" } ) { } # This signature should get put back on one line -sub t022 ( $p = do { $z += 10; 222 }, $a = do { $z++; 333 } ) { - "$p/$a"; -} +sub t022 ( $p = do { $z += 10; 222 }, $a = do { $z++; 333 } ) { "$p/$a" } # anonymous sub with signature my $subref = sub ( $cat, $id = do { state $auto_id = 0; $auto_id++ } ) { diff --git a/t/snippets7.t b/t/snippets7.t index d2ee8790..e89b4995 100644 --- a/t/snippets7.t +++ b/t/snippets7.t @@ -329,10 +329,8 @@ my $y = 2; expect => <<'#8...........', get( on_ready => sub ($worker) { $on_ready->end; return; }, - on_exit => sub ( $worker, $status ) { - return; - }, - on_data => sub ($data) { $self->_on_data(@_) if $self; return; } + on_exit => sub ( $worker, $status ) { return; }, + on_data => sub ($data) { $self->_on_data(@_) if $self; return; } ); #8........... }, -- 2.39.5