check_format.pl: Add checks for blank lines within/after local decls

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/17434)

diff --git a/util/check-format-test-negatives.c b/util/check-format-test-negatives.c
index 8149ff2b58a6f2c0c0a90420fa54d232b2e6065e..f4c0908e4c23b434d40af0b004818141eaa71ad4 100644 (file)
--- a/util/check-format-test-negatives.c
+++ b/util/check-format-test-negatives.c
@@ -1,7 +1,6 @@
 /*
- * Copyright 2007-2021 The OpenSSL Project Authors. All Rights Reserved.
- * Copyright Nokia 2007-2019
- * Copyright Siemens AG 2015-2019
+ * Copyright 2007-2022 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright Siemens AG 2015-2022
  *
  * Licensed under the Apache License 2.0 (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -14,6 +13,15 @@
  * There are some known false positives, though, which are marked below.
  */
 
+#define F                                       \
+    void f()                                    \
+    {                                           \
+        int i;                                  \
+        int j;                                  \
+                                                \
+        return;                                 \
+    }
+
 /*-
  * allow extra SPC in format-tagged multi-line comment
  */
@@ -21,6 +29,33 @@ int f(void) /*
              * trailing multi-line comment
              */
 {
+    typedef int INT;
+    void v;
+    short b;
+    char c;
+    signed s;
+    unsigned u;
+    int i;
+    long l;
+    float f;
+    double d;
+    enum {} enu;
+    struct {} stru;
+    union {} un;
+    auto a;
+    extern e;
+    static int stat;
+    const int con;
+    volatile int vola;
+    register int reg;
+    /*
+     * multi-line comment should not disturb detection of local decls
+     */
+    BIO1 ***b;
+    /* intra-line comment should not disturb detection of local decls */
+    unsigned k;
+
+    /* intra-line comment should not disturb detection of end of local decls */
     if (ctx == NULL) {    /* non-leading end-of-line comment */
         if (/* comment after '(' */ pem_name != NULL /* comment before ')' */)
             /* entire-line comment indent usually like for the following line */

diff --git a/util/check-format-test-positives.c b/util/check-format-test-positives.c
index 6281c5cbce3b15377e3a079d7c5c7cabbbfbb5ab..df1928848043f769ba1e7501bd36d4bbe8d73a53 100644 (file)
--- a/util/check-format-test-positives.c
+++ b/util/check-format-test-positives.c
@@ -1,7 +1,6 @@
 /*
- * Copyright 2007-2021 The OpenSSL Project Authors. All Rights Reserved.
- * Copyright Nokia 2007-2019
- * Copyright Siemens AG 2015-2019
+ * Copyright 2007-2022 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright Siemens AG 2015-2022
  *
  * Licensed under the Apache License 2.0 (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -85,8 +84,11 @@ int f (int a,       /*@ space after fn before '(', reported unless sloppy-spc */
         (xx         /*@0 unclosed parenthesis in expression */
          ? y        /*@0 unclosed '? (conditional expression) */
          [0;        /*@4 unclosed bracket in expression */
-   s_type s;        /*@ local variable declaration indent off by -1 */
-   somefunc(a,      /*@ statement indent off by -1 */
+    /*@ blank line within local decls */
+   s_type s;        /*@2 local variable declaration indent off by -1 */
+   t_type t;        /*@ local variable declaration indent again off by -1 */
+    /* */           /*@0 missing blank line after local decls */
+   somefunc(a,      /*@2 statement indent off by -1 */
           "aligned" /*@ expr indent off by -2 accepted if sloppy-hang */ "right"
            , b,     /*@ expr indent off by -1 */
            b,       /*@ expr indent as on line above, accepted if sloppy-hang */
@@ -338,11 +340,11 @@ void f_looong_body()
     ;
 
 
-    ;               /*@ 2 essentially empty lines before, if !sloppy-spc */
+    ;               /*@ 2 essentially blank lines before, if !sloppy-spc */
 }                   /*@ function body length > 200 lines */
 #if 0               /*@0 unclosed #if */
 struct t {          /*@0 unclosed brace at decl/block level */
     enum {          /*@0 unclosed brace at enum/expression level */
           v = (1    /*@0 unclosed parenthesis */
-               etyp /*@0 empty line follows just before EOF, if !sloppy-spc: */
+               etyp /*@0 blank line follows just before EOF, if !sloppy-spc: */
 

diff --git a/util/check-format.pl b/util/check-format.pl
index 8bc840b6fc25b53b5172f2c3be847d711720b199..aa2566e28e3f3c36526e668f943261f03a49786b 100755 (executable)
--- a/util/check-format.pl
+++ b/util/check-format.pl
@@ -1,7 +1,7 @@
 #! /usr/bin/env perl
 #
-# Copyright 2020-2021 The OpenSSL Project Authors. All Rights Reserved.
-# Copyright Siemens AG 2019-2020
+# Copyright 2020-2022 The OpenSSL Project Authors. All Rights Reserved.
+# Copyright Siemens AG 2019-2022
 #
 # Licensed under the Apache License 2.0 (the "License").
 # You may not use this file except in compliance with the License.
@@ -64,7 +64,7 @@
 #   after the current position (such that false positives would be reported)
 #   the tool by checks for this rule by default only for do/while/for bodies.
 #   Yet with the --1-stmt option false positives are preferred over negatives.
-#   False negatives occur if the braces are more than two non-empty lines apart.
+#   False negatives occur if the braces are more than two non-blank lines apart.
 #
 # * The presence of multiple consecutive spaces is regarded a coding style nit
 #   except when this is before end-of-line comments (unless the --eol-comment is given) and
@@ -73,7 +73,7 @@
 #   # define CDE 22
 #   # define F   3333
 #   This pattern is recognized - and consequently extra space not reported -
-#   for a given line if in the nonempty line before or after (if existing)
+#   for a given line if in the non-blank line before or after (if existing)
 #   for each occurrence of "  \S" (where \S means non-space) in the given line
 #   there is " \S" in the other line in the respective column position.
 #   This may lead to both false negatives (in case of coincidental " \S")
@@ -134,10 +134,11 @@ while ($ARGV[0] =~ m/^-(\w|-[\w\-]+)$/) {
 # status variables
 my $self_test;             # whether the current input file is regarded to contain (positive/negative) self-tests
 my $line;                  # current line number
-my $line_before;           # number of previous not essentially empty line (containing at most whitespace and '\')
-my $line_before2;          # number of not essentially empty line before previous not essentially empty line
+my $line_before;           # number of previous not essentially blank line (containing at most whitespace and '\')
+my $line_before2;          # number of not essentially blank line before previous not essentially blank line
 my $contents;              # contents of current line (without blinding)
 #  $_                      # current line, where comments etc. get blinded
+my $code_contents_before;  # contents of previous non-comment non-directive line (without blinding), initially ""
 my $contents_before;       # contents of $line_before (without blinding), if $line_before > 0
 my $contents_before_;      # contents of $line_before after blinding comments etc., if $line_before > 0
 my $contents_before2;      # contents of $line_before2  (without blinding), if $line_before2 > 0
@@ -168,6 +169,7 @@ my @nested_symbols;        # stack of hanging symbols '(', '{', '[', or '?', in
 my @nested_conds_indents;  # stack of hanging indents due to conditionals ('?' ... ':')
 my $expr_indent;           # resulting hanging indent within (multi-line) expressions including type exprs, else 0
 my $hanging_symbol;        # character ('(', '{', '[', not: '?') responsible for $expr_indent, if $expr_indent != 0
+my $in_block_decls;        # number of local declaration lines after block opening before normal statements
 my $in_expr;               # in expression after if/while/for/switch/return/enum/LHS of assignment
 my $in_paren_expr;         # in parenthesized if/while/for condition and switch expression, if $expr_indent != 0
 my $in_typedecl;           # nesting level of typedef/struct/union/enum
@@ -191,6 +193,7 @@ sub reset_file_state {
     $line = 0;
     $line_before = 0;
     $line_before2 = 0;
+    $code_contents_before = "";
     @nested_block_indents = ();
     @nested_hanging_offsets = ();
     @nested_in_typedecl = ();
@@ -198,8 +201,9 @@ sub reset_file_state {
     @nested_indents = ();
     @nested_conds_indents = ();
     $expr_indent = 0;
-    $in_paren_expr = 0;
+    $in_block_decls = -1;
     $in_expr = 0;
+    $in_paren_expr = 0;
     $hanging_offset = 0;
     @in_do_hanging_offsets = ();
     @in_if_hanging_offsets = ();
@@ -377,6 +381,7 @@ sub update_nested_indents { # may reset $in_paren_expr and in this case also res
         my $in_stmt = $in_expr || @nested_symbols != 0; # not: || $in_typedecl != 0
         if ($c =~ m/[{([?]/) { # $c is '{', '(', '[', or '?'
             if ($c eq "{") { # '{' in any context
+                $in_block_decls = 0 if !$in_expr && $in_typedecl == 0;
                 # cancel newly hanging_offset if opening brace '{' is after non-whitespace non-comment:
                 $hanging_offset -= INDENT_LEVEL if $hanging_offset > 0 && $head =~ m/[^\s\@]/;
                 push @nested_block_indents, $block_indent;
@@ -458,6 +463,7 @@ reset_file_state();
 
 while (<>) { # loop over all lines of all input files
     $self_test = $ARGV =~ m/check-format-test/;
+    $_ = "" if $self_test && m/ blank line within local decls /;
     $line++;
     s/\r$//; # strip any trailing CR '\r' (which are typical on Windows systems)
     $contents = $_;
@@ -563,7 +569,7 @@ while (<>) { # loop over all lines of all input files
             my $self_test_exception = $self_test ? "(@\d?)?" : "";
             report("text after '/*' in multi-line comment")
                 unless $tail =~ m/^$self_test_exception.?\s*$/;
-            # tail not essentially empty, first char already checked
+            # tail not essentially blank, first char already checked
             # adapt to actual indentation of first line
             $comment_indent = length($head) + 1;
             $_ = "$head@@".blind_nonspace($cmt_text);
@@ -605,7 +611,7 @@ while (<>) { # loop over all lines of all input files
 
     # at this point all non-space portions of any types of comments have been blinded as @
 
-    goto LINE_FINISHED if m/^\s*$/; # essentially empty line: just whitespace (and maybe a trailing '\')
+    goto LINE_FINISHED if m/^\s*$/; # essentially blank line: just whitespace (and maybe a trailing '\')
 
     # intra-line whitespace nits @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
 
@@ -701,7 +707,7 @@ while (<>) { # loop over all lines of all input files
         report("no space after '$2'")   if $intra_line =~ m/(^|\W)(return)[^\w\s;]/;  # return w/o SPC or ';'
         report("space after function/macro name")
                                       if $intra_line =~ m/(\w+)\s+\(/        # fn/macro name with space before '('
-       && !($1 =~ m/^(if|while|for|switch|return|typedef|void|char|unsigned|int|long|float|double)$/) # not keyword
+       && !($1 =~ m/^(sizeof|if|else|while|do|for|switch|case|default|break|continue|goto|return|void|char|signed|unsigned|int|short|long|float|double|typedef|enum|struct|union|auto|extern|static|const|volatile|register)$/) # not keyword
                                     && !(m/^\s*#\s*define\s/); # we skip macro definitions here because macros
                                     # without parameters but with body beginning with '(', e.g., '#define X (1)',
                                     # would lead to false positives - TODO also check for macros with parameters
@@ -738,7 +744,8 @@ while (<>) { # loop over all lines of all input files
     # update indents according to leading closing brace(s) '}' or label or switch case
     my $in_stmt = $in_expr || @nested_symbols != 0 || $in_typedecl != 0;
     if ($in_stmt) { # expr/stmt/type decl/var def/fn hdr, i.e., not at block level
-        if (m/^([\s@]*\})/) { # leading '}', any preceding blinded comment must not be matched
+        if (m/^([\s@]*\})/) { # leading '}' within stmt, any preceding blinded comment must not be matched
+            $in_block_decls = -1;
             my $head = $1;
             update_nested_indents($head);
             $nested_indents_position = length($head);
@@ -785,7 +792,8 @@ while (<>) { # loop over all lines of all input files
             }
             if ($before ne "") { # non-whitespace non-'{' before '}'
                 report("code before '}'");
-            } else { # leading '}', any preceding blinded comment must not be matched
+            } else { # leading '}' outside stmt, any preceding blinded comment must not be matched
+                $in_block_decls = -1;
                 $local_offset = $block_indent + $hanging_offset - INDENT_LEVEL;
                 update_nested_indents($head);
                 $nested_indents_position = length($head);
@@ -832,6 +840,25 @@ while (<>) { # loop over all lines of all input files
 
     check_indent() if $count >= 0; # not for #define and not if multi-line string literal is continued
 
+    # check for blank lines within/after local decls @@@@@@@@@@@@@@@@@@@@@@@@@@@
+
+    if ($in_block_decls >= 0 &&
+        $in_comment == 0 && !m/^\s*\*?@/ && # not multi-line or intra-line comment
+        !$in_expr && $in_typedecl == 0) {
+        my $blank_line_before = $line > 1 && $code_contents_before =~ m/^\s*(\\\s*)?$/;
+        # essentially blank line: just whitespace (and maybe a trailing '\')
+        if (m/^\s*(void|char|signed|unsigned|int|short|long|float|double|typedef|enum|struct|union|auto|extern|static|const|volatile|register)(\W|$)/ ||
+            (m/[\w)]\s+[*]*\w/ &&
+             !m/^\s*(\}|sizeof|if|else|while|do|for|switch|case|default|break|continue|goto|return)(\W|$)/)) {
+            report_flexibly($line - 1, "blank line within local decls, before", $contents) if $blank_line_before;
+            $in_block_decls++;
+        } elsif ($in_block_decls > 0) {
+            report_flexibly($line, "missing blank line after local decls", "\n$contents_before$contents")
+                unless $blank_line_before;
+            $in_block_decls = -1;
+        }
+    }
+
     $in_comment = 0 if $in_comment < 0; # multi-line comment has ended
 
     # do some further checks @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@@ -863,10 +890,6 @@ while (<>) { # loop over all lines of all input files
 
     # TODO report #if 0 and #if 1
 
-    # TODO report empty line within local variable definitions
-
-    # TODO report missing empty line after local variable definitions
-
     # TODO report needless use of parentheses, while
     #      macro parameters should always be in parens (except when passed on), e.g., '#define ID(x) (x)'
 
@@ -934,7 +957,7 @@ while (<>) { # loop over all lines of all input files
 
     # set $in_typedecl and potentially $hanging_offset for type declaration
     if (!$in_expr && @nested_indents == 0 # not in expression
-        && m/(^|^.*\W)(typedef|struct|union|enum)(\W.*|$)$/
+        && m/(^|^.*\W)(typedef|enum|struct|union)(\W.*|$)$/
         && parens_balance($1) == 0 # not in newly started expression or function arg list
         && ($2 eq "typedef" || !($3 =~ m/\s*\w++\s*(.)/ && $1 ne "{")) # 'struct'/'union'/'enum' <name> not followed by '{'
         # not needed: && $keyword_opening_brace = $2 if $3 =~ m/\{/;
@@ -1086,6 +1109,10 @@ while (<>) { # loop over all lines of all input files
     # post-processing at end of line @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
 
   LINE_FINISHED:
+    $code_contents_before = $contents if
+        !m/^\s*#(\s*)(\w+)/ && # not single-line directive
+        $in_comment == 0 && !m/^\s*\*?@/; # not multi-line or intra-line comment
+
     # on end of multi-line preprocessor directive, adapt indent
     if ($in_directive > 0 &&
         # need to use original line contents because trailing \ may have been stripped
@@ -1096,12 +1123,12 @@ while (<>) { # loop over all lines of all input files
         $hanging_offset = 0; # compensate for this in case macro ends, e.g., as 'while (0)'
     }
 
-    if (m/^\s*$/) { # at begin of file essentially empty line: just whitespace (and maybe a '\')
-            report("leading ".($1 eq "" ? "empty" :"whitespace")." line") if $line == 1 && !$sloppy_SPC;
+    if (m/^\s*$/) { # at begin of file essentially blank line: just whitespace (and maybe a '\')
+            report("leading ".($1 eq "" ? "blank" :"whitespace")." line") if $line == 1 && !$sloppy_SPC;
     } else {
         if ($line_before > 0) {
             my $linediff = $line - $line_before - 1;
-            report("$linediff empty lines before") if $linediff > 1 && !$sloppy_SPC;
+            report("$linediff blank lines before") if $linediff > 1 && !$sloppy_SPC;
         }
         $line_before2      = $line_before;
         $contents_before2  = $contents_before;
@@ -1123,8 +1150,8 @@ while (<>) { # loop over all lines of all input files
     # post-processing at end of file @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
 
     if (eof) {
-        # check for essentially empty line (which may include a '\') just before EOF
-        report(($1 eq "\n" ? "empty line" : $2 ne "" ? "'\\'" : "whitespace")." at EOF")
+        # check for essentially blank line (which may include a '\') just before EOF
+        report(($1 eq "\n" ? "blank line" : $2 ne "" ? "'\\'" : "whitespace")." at EOF")
             if $contents =~ m/^(\s*(\\?)\s*)$/ && !$sloppy_SPC;
 
         # report unclosed expression-level nesting