[PATCH V7 2/2] checkpatch: check format of Vec<String> in modules

Guilherme Giacomo Simoes posted 2 patches 11 months, 1 week ago
There is a newer version of this series
[PATCH V7 2/2] checkpatch: check format of Vec<String> in modules
Posted by Guilherme Giacomo Simoes 11 months, 1 week ago
Implement a check to ensure that the author, firmware, and alias fields
of the module! macro are properly formatted.

* If the array contains more than one value, enforce vertical
  formatting.
* If the array contains only one value, it may be formatted on a single
  line

Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@gmail.com>
---
 scripts/checkpatch.pl | 67 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7b28ad331742..7c42c1a0ea6b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2775,6 +2775,12 @@ sub process {
 	$realcnt = 0;
 	$linenr = 0;
 	$fixlinenr = -1;
+
+	my %array_parse_module;
+	my $expected_spaces;
+	my $spaces;
+	my $herevet_space_add;
+
 	foreach my $line (@lines) {
 		$linenr++;
 		$fixlinenr++;
@@ -3567,6 +3573,67 @@ sub process {
 # ignore non-hunk lines and lines being removed
 		next if (!$hunk_line || $line =~ /^-/);
 
+# check if the field is about author, firmware or alias from module! macro and find malformed arrays
+		my $inline = 0;
+		my $key = "";
+		my $add_line = $line =~ /^\+/;
+
+		if ($line =~ /\b(authors|alias|firmware)\s*:\s*\[/) {
+			$inline = 1;
+			$array_parse_module{$1} = 1;
+		}
+
+		my @keys = keys %array_parse_module;
+		if (@keys) {
+			$key = $keys[0];
+		}
+
+		if (!$expected_spaces && (!$add_line && $key && !$inline)) {
+			if ($line =~ /^([\t ]+)(\s)/) {
+				$expected_spaces = $1;
+			}
+		}
+
+		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 each array element on a separate line\n". $herevet);
+			} elsif ($inline && $line !~ /\]/ && $line !~ /,/ && $line =~ /"/) {
+				WARN("ARRAY_MODULE_MACRO",
+				     "Prefer declare ] on the same line\n" . $herevet);
+			} elsif (!$inline && $line =~ /\]/ && $line =~ /\"/) {
+				WARN("ARRAY_MODULE_MACRO",
+				     "Prefer a new line after the last value and before ]\n" . $herevet);
+			} elsif ($inline && $line =~ /,/ && $line !~ /\]/) {
+				WARN("ARRAY_MODULE_MACRO",
+				     "Prefer a new line after [\n" . $herevet);
+			}
+
+			if ($line =~ /^\+\s*([\t ]+)(\S)/) {
+				$spaces = $1;
+				$herevet_space_add = $herevet;
+			}
+		}
+
+		if ($expected_spaces && $spaces) {
+			if (length($spaces) != length($expected_spaces)) {
+				WARN("ARRAY_MODULE_MACRO",
+					 "Prefer a align parameters\n" . $herevet_space_add);
+			}
+
+			$spaces = undef;
+		}
+
+		#END OF ANALYZE FIELD
+		if ($line =~ /\]/) {
+			delete $array_parse_module{$key};
+			$expected_spaces = undef;
+		}
+
 #trailing whitespace
 		if ($line =~ /^\+.*\015/) {
 			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
-- 
2.34.1
Re: [PATCH V7 2/2] checkpatch: check format of Vec<String> in modules
Posted by Daniel Sedlak 11 months ago

On 3/7/25 2:14 PM, Guilherme Giacomo Simoes wrote:
> Implement a check to ensure that the author, firmware, and alias fields
> of the module! macro are properly formatted.
> 
> * If the array contains more than one value, enforce vertical
>    formatting.
> * If the array contains only one value, it may be formatted on a single
>    line
> 
> Signed-off-by: Guilherme Giacomo Simoes <trintaeoitogc@gmail.com>
> ---
>   scripts/checkpatch.pl | 67 +++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 67 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7b28ad331742..7c42c1a0ea6b 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2775,6 +2775,12 @@ sub process {
>   	$realcnt = 0;
>   	$linenr = 0;
>   	$fixlinenr = -1;
> +
> +	my %array_parse_module;
> +	my $expected_spaces;
> +	my $spaces;
> +	my $herevet_space_add;
> +
>   	foreach my $line (@lines) {
>   		$linenr++;
>   		$fixlinenr++;
> @@ -3567,6 +3573,67 @@ sub process {
>   # ignore non-hunk lines and lines being removed
>   		next if (!$hunk_line || $line =~ /^-/);
>   
> +# check if the field is about author, firmware or alias from module! macro and find malformed arrays
> +		my $inline = 0;
> +		my $key = "";
> +		my $add_line = $line =~ /^\+/;
> +
> +		if ($line =~ /\b(authors|alias|firmware)\s*:\s*\[/) {
> +			$inline = 1;
> +			$array_parse_module{$1} = 1;
> +		}
> +
> +		my @keys = keys %array_parse_module;
> +		if (@keys) {
> +			$key = $keys[0];
> +		}
> +
> +		if (!$expected_spaces && (!$add_line && $key && !$inline)) {
> +			if ($line =~ /^([\t ]+)(\s)/) {
> +				$expected_spaces = $1;
> +			}
> +		}
> +
> +		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 each array element on a separate line\n". $herevet);
> +			} elsif ($inline && $line !~ /\]/ && $line !~ /,/ && $line =~ /"/) {
> +				WARN("ARRAY_MODULE_MACRO",
> +				     "Prefer declare ] on the same line\n" . $herevet);

Small grammar nit. "Prefer to declare…"

> +			} elsif (!$inline && $line =~ /\]/ && $line =~ /\"/) {
> +				WARN("ARRAY_MODULE_MACRO",
> +				     "Prefer a new line after the last value and before ]\n" . $herevet);
> +			} elsif ($inline && $line =~ /,/ && $line !~ /\]/) {
> +				WARN("ARRAY_MODULE_MACRO",
> +				     "Prefer a new line after [\n" . $herevet);
> +			}
> +
> +			if ($line =~ /^\+\s*([\t ]+)(\S)/) {
> +				$spaces = $1;
> +				$herevet_space_add = $herevet;
> +			}
> +		}
> +
> +		if ($expected_spaces && $spaces) {
> +			if (length($spaces) != length($expected_spaces)) {
> +				WARN("ARRAY_MODULE_MACRO",
> +					 "Prefer a align parameters\n" . $herevet_space_add);

Small grammar nit. Shouldn't this be rather: "Prefer to align 
parameters" or "Prefer aligned parameters"?

> +			}
> +
> +			$spaces = undef;
> +		}
> +
> +		#END OF ANALYZE FIELD
> +		if ($line =~ /\]/) {
> +			delete $array_parse_module{$key};
> +			$expected_spaces = undef;
> +		}
> +
>   #trailing whitespace
>   		if ($line =~ /^\+.*\015/) {
>   			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
Re: [PATCH V7 2/2] checkpatch: check format of Vec<String> in modules
Posted by Guilherme Giacomo Simoes 11 months ago
Daniel Sedlak <daniel@sedlak.dev> wrote:
> ThanksSmall grammar nit. "Prefer to declare…"
Okay, I will fix this and send a v9

> Small grammar nit. Shouldn't this be rather: "Prefer to align 
> parameters" or "Prefer aligned parameters"?
"Prefere aligned parameters" seems better to me

Thanks,
Guilherme