From 39d65e8dbdc2d01665126adcc72642cc5ca81d73 Mon Sep 17 00:00:00 2001 From: Steve Hancock Date: Mon, 4 Sep 2023 20:44:27 -0700 Subject: [PATCH] add parameters --add-missing-else and --warn-missing-else --- CHANGES.md | 23 ++++++++ bin/perltidy | 70 ++++++++++++++++++++++++ lib/Perl/Tidy.pm | 3 + lib/Perl/Tidy/Formatter.pm | 106 ++++++++++++++++++++++++++++++++---- t/snippets/ame.in | 2 + t/snippets/ame.par | 1 + t/snippets/expect/ame.ame | 5 ++ t/snippets/expect/ame.def | 2 + t/snippets/packing_list.txt | 2 + t/snippets28.t | 29 ++++++++++ 10 files changed, 231 insertions(+), 12 deletions(-) create mode 100644 t/snippets/ame.in create mode 100644 t/snippets/ame.par create mode 100644 t/snippets/expect/ame.ame create mode 100644 t/snippets/expect/ame.def diff --git a/CHANGES.md b/CHANGES.md index 14c53e75..1e66f82d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,29 @@ ## 2023 07 01.03 + - Add parameters -wme, or --warn-missing-else, and -ame, + or --add-missing else. The parameter -wme tells perltidy to issue + a warning to the error output if an if-elsif-elsif-... chain does + not end in an else block. The parameter -ame tells perltidy to + insert an else block at the end of such a chain if there is none. + + For example, given the following snippet: + + if ( $level == 3 ) { $val = $global{'section'} } + elsif ( $level == 2 ) { $val = $global{'chapter'} } + + # perltidy -ame + if ( $level == 3 ) { $val = $global{'section'} } + elsif ( $level == 2 ) { $val = $global{'chapter'} } + else { + ##FIXME - added with perltidy -ame + } + + The resulting code should be carefully reviewed, and the comment should + be updated as appropriate. The comment can be changed with parameter + -amec=s, where 's' is the comment in the else block. The man pages + have more details. + - The syntax of the parameter --use-feature=class, or -uf=class, which new in the previous release, has been changed slightly for clarity. The default behavior, which occurs if this flag is not entered, is diff --git a/bin/perltidy b/bin/perltidy index 4a54bc41..fd188ed0 100755 --- a/bin/perltidy +++ b/bin/perltidy @@ -3858,6 +3858,76 @@ commas are removed. =back + +=head2 Missing Else Blocks + +One defensive programming technique is to require every B chain be +terminated with an B block, even if it is not strictly required, +to insure that there are no holes in the logic. + +For example, consider the following snippet from an actual program: + + if ( $level == 3 ) { $val = $global{'section'} } + elsif ( $level == 2 ) { $val = $global{'chapter'} } + +What if the variable B<$level> is neither 2 nor 3? Maybe the original +programmer knew that this was okay, but a new programmer might not be sure +sure. + +Perltidy has always written this information in its B file (search for +B). But a problem is that you have to turn on the log file and +look for it. The parameters in this section can either issue a warning if an +B is missing, or even insert an empty B block where one is missing, +or both. + +=over 4 + +=item B<-wme>, B<--warn-missing-else> + +This flag tells perltidy to issue a warning if a program is missing a terminal B block. + +=item B<-ame>, B<--add-missing-else> + +This flag tells perltidy to output an empty else block wherever a program is missing a terminal B block. + +=item B<-amec=s>, B<--add-missing-else-comment=s> + +This string is an optional side comment which will be placed within a new empty else block. The default is: + + -amec='##FIXME - added with perltidy -ame' + +=back + +For example, on the above example we get + + # perltidy -ame + if ( $level == 3 ) { $val = $global{'section'} } + elsif ( $level == 2 ) { $val = $global{'chapter'} } + else { + ##FIXME - added with perltidy -ame + } + +The comment should be changed appropriately after the code is inspected. For +example, we might decide that it fine as is: + + if ( $level == 3 ) { $val = $global{'section'} } + elsif ( $level == 2 ) { $val = $global{'chapter'} } + else { + # ok - ignore other level values are safely ignored + } + +Or maybe it is a potential problem: + + if ( $level == 3 ) { $val = $global{'section'} } + elsif ( $level == 2 ) { $val = $global{'chapter'} } + else { + die("unexpected value of level=$level\n); + } + +Note that this operation cannot be undone, so be careful to inspect the new +code carefully. + + =head2 Retaining or Ignoring Existing Line Breaks Several additional parameters are available for controlling the extent diff --git a/lib/Perl/Tidy.pm b/lib/Perl/Tidy.pm index c3f8c01c..1d504d36 100644 --- a/lib/Perl/Tidy.pm +++ b/lib/Perl/Tidy.pm @@ -3561,6 +3561,9 @@ sub generate_options { ######################################## $category = 9; # Other controls ######################################## + $add_option->( 'warn-missing-else', 'wme', '!' ); + $add_option->( 'add-missing-else', 'ame', '!' ); + $add_option->( 'add-missing-else-comment', 'amec', '=s' ); $add_option->( 'delete-block-comments', 'dbc', '!' ); $add_option->( 'delete-closing-side-comments', 'dcsc', '!' ); $add_option->( 'delete-pod', 'dp', '!' ); diff --git a/lib/Perl/Tidy/Formatter.pm b/lib/Perl/Tidy/Formatter.pm index 7c95c458..dbb07c05 100644 --- a/lib/Perl/Tidy/Formatter.pm +++ b/lib/Perl/Tidy/Formatter.pm @@ -231,6 +231,8 @@ my ( $rOpts_space_prototype_paren, $rOpts_stack_closing_block_brace, $rOpts_static_block_comments, + $rOpts_add_missing_else, + $rOpts_warn_missing_else, $rOpts_tee_block_comments, $rOpts_tee_pod, $rOpts_tee_side_comments, @@ -1413,6 +1415,21 @@ sub check_options { ## ok - no -csc issues } + my $comment = $rOpts->{'add-missing-else-comment'}; + if ( !$comment ) { + $comment = "##FIXME - added with perltidy -ame"; + } + else { + $comment = substr( $comment, 0, 60 ); + $comment =~ s/^\s+//; + $comment =~ s/\s+$//; + $comment =~ s/\n/ /g; + if ( substr( $comment, 0, 1 ) ne '#' ) { + $comment = '#' . $comment; + } + } + $rOpts->{'add-missing-else-comment'} = $comment; + make_bli_pattern(); make_bl_pattern(); @@ -2490,6 +2507,8 @@ sub initialize_global_option_vars { $rOpts_space_prototype_paren = $rOpts->{'space-prototype-paren'}; $rOpts_stack_closing_block_brace = $rOpts->{'stack-closing-block-brace'}; $rOpts_static_block_comments = $rOpts->{'static-block-comments'}; + $rOpts_add_missing_else = $rOpts->{'add-missing-else'}; + $rOpts_warn_missing_else = $rOpts->{'warn-missing-else'}; $rOpts_tee_block_comments = $rOpts->{'tee-block-comments'}; $rOpts_tee_pod = $rOpts->{'tee-pod'}; $rOpts_tee_side_comments = $rOpts->{'tee-side-comments'}; @@ -15547,7 +15566,7 @@ EOM # past stored nonblank tokens and flags my ( - $K_last_nonblank_code, $looking_for_else, + $K_last_nonblank_code, $K_dangling_elsif, $is_static_block_comment, $last_CODE_type, $last_line_had_side_comment, $next_parent_seqno, $next_slevel, @@ -15556,7 +15575,7 @@ EOM # Called once at the start of a new file sub initialize_process_line_of_CODE { $K_last_nonblank_code = undef; - $looking_for_else = 0; + $K_dangling_elsif = 0; $is_static_block_comment = 0; $last_line_had_side_comment = 0; $next_parent_seqno = SEQ_ROOT; @@ -16023,6 +16042,44 @@ EOM $is_assignment_or_fat_comma{'=>'} = 1; } + sub add_missing_else { + + # Add a missing 'else' block. + # $K_dangling_elsif = index of closing elsif brace not followed by else + my ($self) = @_; + + # Make sure everything looks okay + if ( !$K_dangling_elsif + || $K_dangling_elsif < $K_first + || $rLL->[$K_dangling_elsif]->[_TYPE_] ne '}' ) + { + DEVEL_MODE && Fault("could not find closing elsif brace\n"); + } + + my $comment = $rOpts->{'add-missing-else-comment'}; + + # Safety check + if ( substr( $comment, 0, 1 ) ne '#' ) { $comment = '#' . $comment } + + # Calculate indentation + my $level = $radjusted_levels->[$K_dangling_elsif]; + my $spaces = SPACE x ( $level * $rOpts_indent_columns ); + my $line1 = $spaces . "else {\n"; + my $line3 = $spaces . "}\n"; + $spaces .= SPACE x $rOpts_indent_columns; + my $line2 = $spaces . $comment . "\n"; + + # clear the output pipeline + $self->flush(); + + my $file_writer_object = $self->[_file_writer_object_]; + + $file_writer_object->write_code_line($line1); + $file_writer_object->write_code_line($line2); + $file_writer_object->write_code_line($line3); + return; + } + sub process_line_of_CODE { my ( $self, $my_line_of_tokens ) = @_; @@ -16248,15 +16305,7 @@ EOM # Handle all other lines ... #--------------------------- - # If we just saw the end of an elsif block, write nag message - # if we do not see another elseif or an else. - if ($looking_for_else) { - - if ( !$is_elsif_else{ $rLL->[$K_first_true]->[_TOKEN_] } ) { - write_logfile_entry("(No else block)\n"); - } - $looking_for_else = 0; - } + $K_dangling_elsif = 0; # This is a good place to kill incomplete one-line blocks if ( $max_index_to_go >= 0 ) { @@ -16394,6 +16443,10 @@ EOM } } + if ( $K_dangling_elsif && $rOpts_add_missing_else ) { + $self->add_missing_else(); + } + return; } ## end sub process_line_of_CODE @@ -16816,12 +16869,41 @@ EOM # check for 'elsif' or 'else' if ( !$is_elsif_else{$next_nonblank_token} ) { write_logfile_entry("(No else block)\n"); + + # Note that we cannot add a missing else block + # in this case because more code follows the + # closing elsif brace on the same line. + if ( $rOpts_warn_missing_else && !DEVEL_MODE ) { + my $lno = + $rLL->[$Ktoken_vars]->[_LINE_INDEX_] + 1; + warning("$lno: No else block\n"); + } } } # no more code on this line, so check on next line else { - $looking_for_else = 1; + my $K_next = $self->K_next_code($K_last); + if ( !defined($K_next) + || $rLL->[$K_next]->[_TYPE_] ne 'k' + || !$is_elsif_else{ $rLL->[$K_next]->[_TOKEN_] } ) + { + $K_dangling_elsif = $Ktoken_vars; + write_logfile_entry("(No else block)\n"); + if ( $rOpts_warn_missing_else && !DEVEL_MODE ) { + my $lno = + $rLL->[$Ktoken_vars]->[_LINE_INDEX_] + 1; + if ($rOpts_add_missing_else) { + warning( + "$lno: Adding missing else block\n"); + } + else { + warning( +"$lno: No else block (use -ame to add one)\n" + ); + } + } + } } } diff --git a/t/snippets/ame.in b/t/snippets/ame.in new file mode 100644 index 00000000..b73ff6c5 --- /dev/null +++ b/t/snippets/ame.in @@ -0,0 +1,2 @@ + if ( $level == 3 ) { $val = $global{'section'} } + elsif ( $level == 2 ) { $val = $global{'chapter'} } diff --git a/t/snippets/ame.par b/t/snippets/ame.par new file mode 100644 index 00000000..51c5b085 --- /dev/null +++ b/t/snippets/ame.par @@ -0,0 +1 @@ +--add-missing-else diff --git a/t/snippets/expect/ame.ame b/t/snippets/expect/ame.ame new file mode 100644 index 00000000..5d5e89ff --- /dev/null +++ b/t/snippets/expect/ame.ame @@ -0,0 +1,5 @@ + if ( $level == 3 ) { $val = $global{'section'} } + elsif ( $level == 2 ) { $val = $global{'chapter'} } + else { + ##FIXME - added with perltidy -ame + } diff --git a/t/snippets/expect/ame.def b/t/snippets/expect/ame.def new file mode 100644 index 00000000..b73ff6c5 --- /dev/null +++ b/t/snippets/expect/ame.def @@ -0,0 +1,2 @@ + if ( $level == 3 ) { $val = $global{'section'} } + elsif ( $level == 2 ) { $val = $global{'chapter'} } diff --git a/t/snippets/packing_list.txt b/t/snippets/packing_list.txt index 66fcffff..e3c562ad 100644 --- a/t/snippets/packing_list.txt +++ b/t/snippets/packing_list.txt @@ -537,3 +537,5 @@ ../snippets9.t rt98902.def ../snippets9.t rt98902.rt98902 ../snippets9.t rt99961.def +../snippets28.t ame.ame +../snippets28.t ame.def diff --git a/t/snippets28.t b/t/snippets28.t index bb9687a0..a2984c4c 100644 --- a/t/snippets28.t +++ b/t/snippets28.t @@ -14,6 +14,8 @@ #11 xbt.xbt3 #12 lrt.def #13 lrt.lrt +#14 ame.ame +#15 ame.def # To locate test #13 you can search for its name or the string '#13' @@ -31,6 +33,7 @@ BEGIN { # BEGIN SECTION 1: Parameter combinations # ########################################### $rparams = { + 'ame' => "--add-missing-else", 'def' => "", 'git116' => "-viu", 'lrt' => "--line-range-tidy=2:3", @@ -49,6 +52,11 @@ BEGIN { ############################ $rsources = { + 'ame' => <<'----------', + if ( $level == 3 ) { $val = $global{'section'} } + elsif ( $level == 2 ) { $val = $global{'chapter'} } +---------- + 'git116' => <<'----------', print "Tried to add: @ResolveRPM\n" if ( @ResolveRPM and !$Quiet ); print "Would need: @DepList\n" if ( @DepList and !$Quiet ); @@ -336,6 +344,27 @@ sub hello { =cut #13........... }, + + 'ame.ame' => { + source => "ame", + params => "ame", + expect => <<'#14...........', + if ( $level == 3 ) { $val = $global{'section'} } + elsif ( $level == 2 ) { $val = $global{'chapter'} } + else { + ##FIXME - added with perltidy -ame + } +#14........... + }, + + 'ame.def' => { + source => "ame", + params => "def", + expect => <<'#15...........', + if ( $level == 3 ) { $val = $global{'section'} } + elsif ( $level == 2 ) { $val = $global{'chapter'} } +#15........... + }, }; my $ntests = 0 + keys %{$rtests}; -- 2.39.5