Re: [PATCH V3 2/2] checkpatch: throw error for malformed arrays

From: Charalampos Mitrodimas
Date: Thu Feb 20 2025 - 18:36:53 EST


Guilherme Giacomo Simoes <trintaeoitogc@xxxxxxxxx> writes:

> Implement a check to ensure these fields are correctly formatted. If a
> line contains one of these keys that should be of type Vec<String>, use
> a regex to verify whether the array holds multiple values.
> * If the array contains more than one value, enforce vertical formatting
> * If the array has only one value, it can remain on the same line
>
> Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@xxxxxxxxx>
> ---
> scripts/checkpatch.pl | 43 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7b28ad331742..1133fe68054b 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2775,6 +2775,9 @@ sub process {
> $realcnt = 0;
> $linenr = 0;
> $fixlinenr = -1;
> +
> + my %array_parse_module;
> +
> foreach my $line (@lines) {
> $linenr++;
> $fixlinenr++;
> @@ -3567,6 +3570,46 @@ sub process {
> # ignore non-hunk lines and lines being removed
> next if (!$hunk_line || $line =~ /^-/);
>
> +# check if arrays has more than one value in the same line
> + my $inline = 0;
> + my $key = "";
> + my $add_line = $line =~ /^\+/;
> +
> + if ($line =~ /\s*.*\b(author|alias|firmware)\s*:\s*\[/) {

Hi Guilherme,

Will this work now that the field is called "authors" and not "author"?
I think you forgot to update the regex here.

> + $inline = 1;
> + $array_parse_module{$1} = 1;
> + }
> +
> + my @keys = keys %array_parse_module;
> + if (@keys) {
> + $key = $keys[0];
> + }
> +
> + if ($add_line && $key) {
> + my $herevet = "$here\n" . cat_vet($rawline) . "\n";
> +
> + my $counter = () = $line =~ /"/g;
> + my $more_than_one = $counter > 2;
> + if ($more_than_one) {
> + WARN("ARRAY_MODULE_MACRO",
> + "Prefer a single-line value$herevet");

What do you think about this instead?
"Prefer each array element on a separate line$herevet"

> + } elsif ($inline && $line !~ /\]/ && $line !~ /,/ && $line =~ /"/) {
> + WARN("ARRAY_MODULE_MACRO",
> + "Prefer declare ] on the same line$herevet");
> + } elsif (!$inline && $line =~ /\]/ && $line =~ /\"/) {
> + WARN("ARRAY_MODULE_MACRO",
> + "Prefer a new line after the last value and before ]$herevet");
> + } elsif ($inline && $line =~ /,/ && $line !~ /\]/) {
> + WARN("ARRAY_MODULE_MACRO",
> + "Prefer a new line after [$herevet");
> + }
> + }
> +
> + #END OF ANALYZE FIELD
> + if ($line =~ /\]/) {
> + delete $array_parse_module{$key};
> + }
> +
> #trailing whitespace
> if ($line =~ /^\+.*\015/) {
> my $herevet = "$here\n" . cat_vet($rawline) . "\n";