Re: [PATCH 3/3] checkpatch: throw error in malformed arrays

[Joe Perches <joe@perches.com>]

On Wed, 2025-02-12 at 16:48 -0300, Guilherme Giacomo Simoes wrote:
> In module! macro, some fields have a string array type, and we need a
> check for guarantee a good formatation.

Isn't this is a style desire not a parsing limitation?

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -2775,6 +2775,9 @@ sub process {
>  	$realcnt = 0;
>  	$linenr = 0;
>  	$fixlinenr = -1;
> +
> +	my %parse_module_arr;

Probably not useful to add hungarian style naming.

> @@ -3567,6 +3570,71 @@ 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;
> +		if ($line =~ /^\+\s*.*\b(author|alias|firmware)\s*:\s*\[/) {

\s*.* could be just .*

> +			$inline = 1;
> +
> +			if ($line =~ /author/) {
> +				$parse_module_arr{'author'} = 1;
> +			}
> +
> +			if ($line =~ /alias/) {
> +				$parse_module_arr{'alias'} = 1;
> +			}
> +
> +			if ($line =~ /firmware/) {
> +				$parse_module_arr{'firmware'} = 1;
> +			}

$parse_module_arr{$1} = 1;

> +		}
> +
> +		if (keys %parse_module_arr) {
> +			my @keys = keys %parse_module_arr;
> +			my $key = $keys[0];
> +
> +			my $more_than_one_vl = qr/".+",\s*".+"/;
> +
> +			if ($line =~ $more_than_one_vl) {
> +					ERROR("ERR_ARRAY_MODULE_MACRO",
> +						"the key $key have more than one values in same line\n$here\n". cat_vet($rawline));

"Prefer one value per line\n"

ERR_ error prefixes are not useful.
These seem more like a WARN than an ERROR
Bad indentation
And please use a $herevet temporary


> +			}
> +			elsif ($inline && $line !~ /\]/ && $line =~ /\["/) {
> +					ERROR("ERR_ARRAY_MODULE_MACRO",
> +						qq~the key $key need a new line after declare the key\n$here\n~ . cat_vet($rawline) . qq~\nFor example:

Use } elsif

"Prefer declaring keys on separate lines"

Are quote characters allowed in keys?
If not, this seems unnecessarily complicated.

$herevet

I think the qq is unnecessary and the for example
should refer to some style documentation and not
be explicit in checkpatch.