2021-02-21 12:00:58

by Dwaipayan Ray

[permalink] [raw]
Subject: [PATCH v6 1/2] checkpatch: add verbose mode

Add a new verbose mode to checkpatch.pl to emit additional verbose
test descriptions. The verbose mode is optional and can be enabled
by the flag -v or --verbose.

The test descriptions are parsed from the checkpatch documentation
file at `Documentation/dev-tools/checkpatch.rst`. The test
descriptions in the docs are kept in a fixed format grouped by
usage. Some examples of this format are:

**LINE_SPACING**
Vertical space is wasted given the limited number of lines an
editor window can display when multiple blank lines are used.

**MISSING_SIGN_OFF**
The patch is missing a Signed-off-by line. A signed-off-by
line should be added according to Developer's certificate of
Origin.

To avoid lengthy output, the verbose description is printed only
for the first instance of a particular message type.

The --verbose option cannot be used along with the --terse option.

Verbose mode can also be used with the --list-types option, which
will then output the verbose descriptions along with the checkpatch
message types.

Signed-off-by: Dwaipayan Ray <[email protected]>
---
scripts/checkpatch.pl | 68 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 65 insertions(+), 3 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9a549b009d2f..b44fcf13dec1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -23,6 +23,9 @@ my $V = '0.32';
use Getopt::Long qw(:config no_auto_abbrev);

my $quiet = 0;
+my $verbose = 0;
+my %verbose_messages = ();
+my %verbose_emitted = ();
my $tree = 1;
my $chk_signoff = 1;
my $chk_patch = 1;
@@ -61,6 +64,7 @@ my $spelling_file = "$D/spelling.txt";
my $codespell = 0;
my $codespellfile = "/usr/share/codespell/dictionary.txt";
my $conststructsfile = "$D/const_structs.checkpatch";
+my $docsfile = "$D/../Documentation/dev-tools/checkpatch.rst";
my $typedefsfile;
my $color = "auto";
my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANCE
@@ -78,6 +82,7 @@ Version: $V

Options:
-q, --quiet quiet
+ -v, --verbose verbose mode
--no-tree run without a kernel tree
--no-signoff do not check for 'Signed-off-by' line
--patch treat FILE as patchfile (default)
@@ -167,6 +172,11 @@ sub list_types {
print("#\tMessage type\n\n");
foreach my $type (@types) {
print(++$count . "\t" . $type . "\n");
+ if ($verbose && exists($verbose_messages{$type})) {
+ my $message = $verbose_messages{$type};
+ $message =~ s/\n/\n\t/g;
+ print("\t" . $message . "\n\n");
+ }
}

exit($exitcode);
@@ -198,6 +208,46 @@ if (-f $conf) {
unshift(@ARGV, @conf_args) if @conf_args;
}

+sub load_docs {
+ open(my $docs, '<', "$docsfile")
+ or warn "$P: Can't read the documentation file $docsfile $!\n";
+
+ my $type = '';
+ my $desc = '';
+ my $in_desc = 0;
+
+ while (<$docs>) {
+ chomp;
+ my $line = $_;
+ $line =~ s/\s+$//;
+
+ if ($line =~ /^\s*\*\*(.+)\*\*$/) {
+ if ($desc ne '') {
+ $verbose_messages{$type} = trim($desc);
+ }
+ $type = $1;
+ $desc = '';
+ $in_desc = 1;
+ } elsif ($in_desc) {
+ if ($line =~ /^(?:\s{4,}|$)/) {
+ $line =~ s/^\s{4}//;
+ $desc .= $line;
+ $desc .= "\n";
+ } else {
+ $verbose_messages{$type} = trim($desc);
+ $type = '';
+ $desc = '';
+ $in_desc = 0;
+ }
+ }
+ }
+
+ if ($desc ne '') {
+ $verbose_messages{$type} = trim($desc);
+ }
+ close($docs);
+}
+
# Perl's Getopt::Long allows options to take optional arguments after a space.
# Prevent --color by itself from consuming other arguments
foreach (@ARGV) {
@@ -208,6 +258,7 @@ foreach (@ARGV) {

GetOptions(
'q|quiet+' => \$quiet,
+ 'v|verbose!' => \$verbose,
'tree!' => \$tree,
'signoff!' => \$chk_signoff,
'patch!' => \$chk_patch,
@@ -247,13 +298,15 @@ GetOptions(

help(0) if ($help);

+die "$P: --git cannot be used with --file or --fix\n" if ($git && ($file || $fix));
+die "$P: --verbose cannot be used with --terse\n" if ($verbose && $terse);
+
+load_docs() if ($verbose);
list_types(0) if ($list_types);

$fix = 1 if ($fix_inplace);
$check_orig = $check;

-die "$P: --git cannot be used with --file or --fix\n" if ($git && ($file || $fix));
-
my $exit = 0;

my $perl_version_ok = 1;
@@ -2209,7 +2262,16 @@ sub report {
splice(@lines, 1, 1);
$output = join("\n", @lines);
}
- $output = (split('\n', $output))[0] . "\n" if ($terse);
+
+ if ($terse) {
+ $output = (split('\n', $output))[0] . "\n";
+ }
+
+ if ($verbose && exists($verbose_messages{$type}) &&
+ !exists($verbose_emitted{$type})) {
+ $output .= $verbose_messages{$type} . "\n\n";
+ $verbose_emitted{$type} = 1;
+ }

push(our @report, $output);

--
2.30.0


2021-02-21 18:11:51

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v6 1/2] checkpatch: add verbose mode

On Sun, 2021-02-21 at 17:28 +0530, Dwaipayan Ray wrote:
> Add a new verbose mode to checkpatch.pl to emit additional verbose
> test descriptions. The verbose mode is optional and can be enabled
> by the flag -v or --verbose.

OK, maybe add color coding to the list_types output.
Something like:
---
scripts/checkpatch.pl | 61 ++++++++++++++++++++++++++++++++++++---------------
1 file changed, 43 insertions(+), 18 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index bbff13e0ca7e..ccd4a1bd73cb 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -163,14 +163,39 @@ sub list_types {
my $text = <$script>;
close($script);

- my @types = ();
+ my %types = ();
# Also catch when type or level is passed through a variable
- for ($text =~ /(?:(?:\bCHK|\bWARN|\bERROR|&\{\$msg_level})\s*\(|\$msg_type\s*=)\s*"([^"]+)"/g) {
- push (@types, $_);
+ while ($text =~ /\b(CHK|WARN|ERROR|&\{\$msg_level})\s*\(\s*"([^"]+)"/g) {
+ if (exists($types{$2})) {
+ $types{$2} .= ",$1" if ($types{$2} ne $1);
+ } else {
+ $types{$2} = $1;
+ }
+ }
+ print("#\tMessage type");
+ if ($color) {
+ print(" (color coding: ");
+ print(RED . "ERROR" . RESET);
+ print("|");
+ print(YELLOW . "WARNING" . RESET);
+ print("|");
+ print(GREEN . "CHECK" . RESET);
+ print("|");
+ print("Multiple levels" . RESET);
+ print(")");
}
- @types = sort(uniq(@types));
- print("#\tMessage type\n\n");
- foreach my $type (@types) {
+ print("\n\n");
+ foreach my $type (sort keys %types) {
+ if ($color) {
+ my $level = $types{$type};
+ if ($level eq 'ERROR') {
+ $type = RED . $type . RESET;
+ } elsif ($level eq 'WARN') {
+ $type = YELLOW . $type . RESET;
+ } elsif ($level eq 'CHK') {
+ $type = GREEN . $type . RESET;
+ }
+ }
print(++$count . "\t" . $type . "\n");
if ($verbose && exists($verbose_messages{$type})) {
my $message = $verbose_messages{$type};
@@ -301,6 +326,18 @@ help(0) if ($help);
die "$P: --git cannot be used with --file or --fix\n" if ($git && ($file || $fix));
die "$P: --verbose cannot be used with --terse\n" if ($verbose && $terse);

+if ($color =~ /^[01]$/) {
+ $color = !$color;
+} elsif ($color =~ /^always$/i) {
+ $color = 1;
+} elsif ($color =~ /^never$/i) {
+ $color = 0;
+} elsif ($color =~ /^auto$/i) {
+ $color = (-t STDOUT);
+} else {
+ die "$P: Invalid color mode: $color\n";
+}
+
load_docs() if ($verbose);
list_types(0) if ($list_types);

@@ -321,18 +358,6 @@ if ($#ARGV < 0) {
push(@ARGV, '-');
}

-if ($color =~ /^[01]$/) {
- $color = !$color;
-} elsif ($color =~ /^always$/i) {
- $color = 1;
-} elsif ($color =~ /^never$/i) {
- $color = 0;
-} elsif ($color =~ /^auto$/i) {
- $color = (-t STDOUT);
-} else {
- die "$P: Invalid color mode: $color\n";
-}
-
# skip TAB size 1 to avoid additional checks on $tabsize - 1
die "$P: Invalid TAB size: $tabsize\n" if ($tabsize < 2);


2021-02-21 18:38:08

by Dwaipayan Ray

[permalink] [raw]
Subject: Re: [PATCH v6 1/2] checkpatch: add verbose mode

On Sun, Feb 21, 2021 at 11:36 PM Joe Perches <[email protected]> wrote:
>
> On Sun, 2021-02-21 at 17:28 +0530, Dwaipayan Ray wrote:
> > Add a new verbose mode to checkpatch.pl to emit additional verbose
> > test descriptions. The verbose mode is optional and can be enabled
> > by the flag -v or --verbose.
>
> OK, maybe add color coding to the list_types output.

Okay, nice idea!
> Something like:
> ---
> scripts/checkpatch.pl | 61 ++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 43 insertions(+), 18 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index bbff13e0ca7e..ccd4a1bd73cb 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -163,14 +163,39 @@ sub list_types {
> my $text = <$script>;
> close($script);
>
> - my @types = ();
> + my %types = ();
> # Also catch when type or level is passed through a variable
> - for ($text =~ /(?:(?:\bCHK|\bWARN|\bERROR|&\{\$msg_level})\s*\(|\$msg_type\s*=)\s*"([^"]+)"/g) {
> - push (@types, $_);
> + while ($text =~ /\b(CHK|WARN|ERROR|&\{\$msg_level})\s*\(\s*"([^"]+)"/g) {
> + if (exists($types{$2})) {
> + $types{$2} .= ",$1" if ($types{$2} ne $1);
> + } else {
> + $types{$2} = $1;
> + }
> + }
> + print("#\tMessage type");
> + if ($color) {
> + print(" (color coding: ");
> + print(RED . "ERROR" . RESET);
> + print("|");
> + print(YELLOW . "WARNING" . RESET);
> + print("|");
> + print(GREEN . "CHECK" . RESET);
> + print("|");
> + print("Multiple levels" . RESET);
> + print(")");
> }
> - @types = sort(uniq(@types));
> - print("#\tMessage type\n\n");
> - foreach my $type (@types) {
> + print("\n\n");
> + foreach my $type (sort keys %types) {
> + if ($color) {
> + my $level = $types{$type};
> + if ($level eq 'ERROR') {
> + $type = RED . $type . RESET;
> + } elsif ($level eq 'WARN') {
> + $type = YELLOW . $type . RESET;
> + } elsif ($level eq 'CHK') {
> + $type = GREEN . $type . RESET;
> + }
> + }
> print(++$count . "\t" . $type . "\n");
> if ($verbose && exists($verbose_messages{$type})) {
> my $message = $verbose_messages{$type};
> @@ -301,6 +326,18 @@ help(0) if ($help);
> die "$P: --git cannot be used with --file or --fix\n" if ($git && ($file || $fix));
> die "$P: --verbose cannot be used with --terse\n" if ($verbose && $terse);
>
> +if ($color =~ /^[01]$/) {
> + $color = !$color;
> +} elsif ($color =~ /^always$/i) {
> + $color = 1;
> +} elsif ($color =~ /^never$/i) {
> + $color = 0;
> +} elsif ($color =~ /^auto$/i) {
> + $color = (-t STDOUT);
> +} else {
> + die "$P: Invalid color mode: $color\n";
> +}
> +
> load_docs() if ($verbose);
> list_types(0) if ($list_types);
>
> @@ -321,18 +358,6 @@ if ($#ARGV < 0) {
> push(@ARGV, '-');
> }
>
> -if ($color =~ /^[01]$/) {
> - $color = !$color;
> -} elsif ($color =~ /^always$/i) {
> - $color = 1;
> -} elsif ($color =~ /^never$/i) {
> - $color = 0;
> -} elsif ($color =~ /^auto$/i) {
> - $color = (-t STDOUT);
> -} else {
> - die "$P: Invalid color mode: $color\n";
> -}
> -
> # skip TAB size 1 to avoid additional checks on $tabsize - 1
> die "$P: Invalid TAB size: $tabsize\n" if ($tabsize < 2);
>
>

Sure, I will do something like this.
Are there any other improvements you can see right now
or will the coloring thing suffice?

Thanks,
Dwaipayan

2021-02-22 02:46:17

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v6 1/2] checkpatch: add verbose mode

On Mon, 2021-02-22 at 00:05 +0530, Dwaipayan Ray wrote:
> On Sun, Feb 21, 2021 at 11:36 PM Joe Perches <[email protected]> wrote:
> >
> > On Sun, 2021-02-21 at 17:28 +0530, Dwaipayan Ray wrote:
> > > Add a new verbose mode to checkpatch.pl to emit additional verbose
> > > test descriptions. The verbose mode is optional and can be enabled
> > > by the flag -v or --verbose.
> >
> > OK, maybe add color coding to the list_types output.
> Okay, nice idea!
[]
> Sure, I will do something like this.
> Are there any other improvements you can see right now
> or will the coloring thing suffice?

A lot more descriptive output for the .rst file and
of course a lot more of the types documented...

2021-02-22 04:51:00

by Dwaipayan Ray

[permalink] [raw]
Subject: Re: [PATCH v6 1/2] checkpatch: add verbose mode

On Mon, Feb 22, 2021 at 8:14 AM Joe Perches <[email protected]> wrote:
>
> On Mon, 2021-02-22 at 00:05 +0530, Dwaipayan Ray wrote:
> > On Sun, Feb 21, 2021 at 11:36 PM Joe Perches <[email protected]> wrote:
> > >
> > > On Sun, 2021-02-21 at 17:28 +0530, Dwaipayan Ray wrote:
> > > > Add a new verbose mode to checkpatch.pl to emit additional verbose
> > > > test descriptions. The verbose mode is optional and can be enabled
> > > > by the flag -v or --verbose.
> > >
> > > OK, maybe add color coding to the list_types output.
> > Okay, nice idea!
> []
> > Sure, I will do something like this.
> > Are there any other improvements you can see right now
> > or will the coloring thing suffice?
>
> A lot more descriptive output for the .rst file and
> of course a lot more of the types documented...
>

Sure I was hoping to do that over time after getting
this series in...

Nevertheless I will send the v7 in right away with the color
code changes.

Thanks,
Dwaipayan.