checkpatch: More --fix insert/delete lines for BRACES

PATCH New Joe Perches 1 participant 5 messages Jul 10, 2014 Checkpatch

lore

Share & tools

lore

Joe Perches

author maintainer

2014-07-10 18:53 UTC #0 lore raw

cover checkpatch: More --fix insert/delete lines for BRACES

A bit of neatening and add a few patch/file corrections for misplaced braces

Joe Perches (4):
checkpatch: Add fix_insert_line and fix_delete_line helpers
checkpatch: Use the correct indentation for which()
checkpatch: Add --fix option for a couple OPEN_BRACE misuses
checkpatch: Fix brace style misuses of else and while

 scripts/checkpatch.pl | 148 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 93 insertions(+), 55 deletions(-)

Signature · 1 line

1.8.1.2.459.gbcd45b4.dirty

Joe Perches

author maintainer

2014-07-10 18:53 UTC #1 lore raw

PATCH 1/4 checkpatch: Add fix_insert_line and fix_delete_line helpers

Neaten the uses of patch/file line insertions or deletions. Hide the mechanism used.

Signed-off-by Joe Perches <[email protected]>

---
 scripts/checkpatch.pl | 65 +++++++++++++++++++++++----------------------------
 1 file changed, 29 insertions(+), 36 deletions(-)

scripts/checkpatch.pl +29 ~~-36~~

index f3d9a88..7c29069 100755
@@ -1593,6 +1593,27 @@ sub fix_inserted_deleted_lines {
 	return @lines;
 }
 
+sub fix_insert_line {
+	my ($linenr, $line) = @_;
+
+	my $inserted = {
+		LINENR => $linenr,
+		LINE => $line,
+	};
+	push(@fixed_inserted, $inserted);
+}
+
+sub fix_delete_line {
+	my ($linenr, $line) = @_;
+
+	my $deleted = {
+		LINENR => $linenr,
+		LINE => $line,
+	};
+
+	push(@fixed_deleted, $deleted);
+}
+
 sub ERROR {
 	my ($type, $msg) = @_;

@@ -2447,11 +2468,7 @@ sub process {
 			if (CHK("LINE_SPACING",
 				"Please use a blank line after function/struct/union/enum declarations\n" . $hereprev) &&
 			    $fix) {
-			my $inserted = {
-				LINENR => $fixlinenr,
-				LINE => "\+",
-			};
-			push(@fixed_inserted, $inserted);
+				fix_insert_line($fixlinenr, "\+");
 			}
 		}

@@ -2462,11 +2479,7 @@ sub process {
 			if (CHK("LINE_SPACING",
 				"Please don't use multiple blank lines\n" . $hereprev) &&
 			    $fix) {
-			my $deleted = {
-				LINENR => $fixlinenr,
-				LINE => $rawline,
-			};
-			push(@fixed_deleted, $deleted);
+				fix_delete_line($fixlinenr, $rawline);
 			}
 
 			$last_blank_line = $linenr;

@@ -2509,11 +2522,7 @@ sub process {
 			if (WARN("LINE_SPACING",
 				 "Missing a blank line after declarations\n" . $hereprev) &&
 			    $fix) {
-			my $inserted = {
-				LINENR => $fixlinenr,
-				LINE => "\+",
-			};
-			push(@fixed_inserted, $inserted);
+				fix_insert_line($fixlinenr, "\+");
 			}
 		}

@@ -2868,31 +2877,15 @@ sub process {
 		    $prevline =~ /(?:^|[^=])=\s*$/) {
 			if (ERROR("OPEN_BRACE",
 				  "that open brace { should be on the previous line\n" . $hereprev) &&
-			    $fix && $prevline =~ /^\+/) {
-				my $deleted = {
-					LINENR => $fixlinenr - 1,
-					LINE => $prevrawline,
-				};
-				push(@fixed_deleted, $deleted);
-				$deleted = {
-					LINENR => $fixlinenr,
-					LINE => $rawline,
-				};
-				push(@fixed_deleted, $deleted);
+			    $fix && $prevline =~ /^\+/ && $line =~ /^\+/) {
+				fix_delete_line($fixlinenr - 1, $prevrawline);
+				fix_delete_line($fixlinenr, $rawline);
 				my $fixedline = $prevrawline;
 				$fixedline =~ s/\s*=\s*$/ = {/;
-				my $inserted = {
-					LINENR => $fixlinenr,
-					LINE => $fixedline,
-				};
-				push(@fixed_inserted, $inserted);
+				fix_insert_line($fixlinenr, $fixedline);
 				$fixedline = $line;
 				$fixedline =~ s/^(.\s*){\s*/$1/;
-				$inserted = {
-					LINENR => $fixlinenr,
-					LINE => $fixedline,
-				};
-				push(@fixed_inserted, $inserted);
+				fix_insert_line($fixlinenr, $fixedline);
 			}
 		}

Signature · 1 line

1.8.1.2.459.gbcd45b4.dirty

Joe Perches

author maintainer

2014-07-10 18:53 UTC #2 lore raw

PATCH 2/4 checkpatch: Use the correct indentation for which()

In reply to Joe Perches

I copied the which subroutine from get_maintainer.pl.

Unfortunately, get_maintainer uses a 4 space indentation so use the proper tab indentation instead.

Signed-off-by Joe Perches <[email protected]>

---
 scripts/checkpatch.pl | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

scripts/checkpatch.pl +6 -6

index 7c29069..569ba31 100755
@@ -710,15 +710,15 @@ sub format_email {
 }
 
 sub which {
-    my ($bin) = @_;
+	my ($bin) = @_;
 
-    foreach my $path (split(/:/, $ENV{PATH})) {
-	if (-e "$path/$bin") {
-	    return "$path/$bin";
+	foreach my $path (split(/:/, $ENV{PATH})) {
+		if (-e "$path/$bin") {
+			return "$path/$bin";
+		}
 	}
-    }
 
-    return "";
+	return "";
 }
 
 sub which_conf {

Signature · 1 line

1.8.1.2.459.gbcd45b4.dirty

Joe Perches

author maintainer

2014-07-10 18:53 UTC #3 lore raw

PATCH 3/4 checkpatch: Add --fix option for a couple OPEN_BRACE misuses

In reply to Joe Perches

Style misuses of these types are corrected:

typedef struct foo

{
	int bar;
};

int foo(int bar) { return bar+1;
}

int foo(int bar) {
	return bar+1;
}

Signed-off-by Joe Perches <[email protected]>

---
 scripts/checkpatch.pl | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

scripts/checkpatch.pl +28 -5

index 569ba31..1e95953 100755
@@ -3164,17 +3164,40 @@ sub process {
 
 # function brace can't be on same line, except for #defines of do while,
 # or if closed on same line
-		if (($line=~/$Type\s*$Ident\(.*\).*\s{/) and
+		if (($line=~/$Type\s*$Ident\(.*\).*\s*{/) and
 		    !($line=~/\#\s*define.*do\s{/) and !($line=~/}/)) {
-			ERROR("OPEN_BRACE",
-			      "open brace '{' following function declarations go on the next line\n" . $herecurr);
+			if (ERROR("OPEN_BRACE",
+				  "open brace '{' following function declarations go on the next line\n" . $herecurr) &&
+			    $fix) {
+				fix_delete_line($fixlinenr, $rawline);
+				my $fixed_line = $rawline;
+				$fixed_line =~ /(^..*$Type\s*$Ident\(.*\)\s*){(.*)$/;
+				my $line1 = $1;
+				my $line2 = $2;
+				fix_insert_line($fixlinenr, ltrim($line1));
+				fix_insert_line($fixlinenr, "\+{");
+				if ($line2 !~ /^\s*$/) {
+					fix_insert_line($fixlinenr, "\+\t" . trim($line2));
+				}
+			}
 		}
 
 # open braces for enum, union and struct go on the same line.
 		if ($line =~ /^.\s*{/ &&
 		    $prevline =~ /^.\s*(?:typedef\s+)?(enum|union|struct)(?:\s+$Ident)?\s*$/) {
-			ERROR("OPEN_BRACE",
-			      "open brace '{' following $1 go on the same line\n" . $hereprev);
+			if (ERROR("OPEN_BRACE",
+				  "open brace '{' following $1 go on the same line\n" . $hereprev) &&
+			    $fix && $prevline =~ /^\+/ && $line =~ /^\+/) {
+				fix_delete_line($fixlinenr - 1, $prevrawline);
+				fix_delete_line($fixlinenr, $rawline);
+				my $fixedline = rtrim($prevrawline) . " {";
+				fix_insert_line($fixlinenr, $fixedline);
+				$fixedline = $rawline;
+				$fixedline =~ s/^(.\s*){\s*/$1\t/;
+				if ($fixedline !~ /^\+\s*$/) {
+					fix_insert_line($fixlinenr, $fixedline);
+				}
+			}
 		}
 
 # missing space after union, struct or enum definition

Signature · 1 line

1.8.1.2.459.gbcd45b4.dirty

Joe Perches

author maintainer

2014-07-10 18:54 UTC #4 lore raw

PATCH 4/4 checkpatch: Fix brace style misuses of else and while

In reply to Joe Perches

Add --fix corrections for ELSE_AFTER_BRACE and WHILE_AFTER_BRACE misuses.

	if (x) {
		...
	}
	else {
		...
	}
is corrected to
	if (x) {
		...
	} else {
		...
	}

and

	do {
		...
	}
	while (x);
is corrected to
	do {
		...
	} while (x);

Signed-off-by Joe Perches <[email protected]>

---
 scripts/checkpatch.pl | 38 ++++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

scripts/checkpatch.pl +30 -8

index 1e95953..404e3d6 100755
@@ -3885,14 +3885,26 @@ sub process {
 
 		# Check for }<nl>else {, these must be at the same
 		# indent level to be relevant to each other.
-		if ($prevline=~/}\s*$/ and $line=~/^.\s*else\s*/ and
-						$previndent == $indent) {
-			ERROR("ELSE_AFTER_BRACE",
-			      "else should follow close brace '}'\n" . $hereprev);
+		if ($prevline=~/}\s*$/ and $line=~/^.\s*else\s*/ &&
+		    $previndent == $indent) {
+			if (ERROR("ELSE_AFTER_BRACE",
+				  "else should follow close brace '}'\n" . $hereprev) &&
+			    $fix && $prevline =~ /^\+/ && $line =~ /^\+/) {
+				fix_delete_line($fixlinenr - 1, $prevrawline);
+				fix_delete_line($fixlinenr, $rawline);
+				my $fixedline = $prevrawline;
+				$fixedline =~ s/}\s*$//;
+				if ($fixedline !~ /^\+\s*$/) {
+					fix_insert_line($fixlinenr, $fixedline);
+				}
+				$fixedline = $rawline;
+				$fixedline =~ s/^(.\s*)else/$1} else/;
+				fix_insert_line($fixlinenr, $fixedline);
+			}
 		}
 
-		if ($prevline=~/}\s*$/ and $line=~/^.\s*while\s*/ and
-						$previndent == $indent) {
+		if ($prevline=~/}\s*$/ and $line=~/^.\s*while\s*/ &&
+		    $previndent == $indent) {
 			my ($s, $c) = ctx_statement_block($linenr, $realcnt, 0);
 
 			# Find out what is on the end of the line after the

@@ -3901,8 +3913,18 @@ sub process {
 			$s =~ s/\n.*//g;
 
 			if ($s =~ /^\s*;/) {
-				ERROR("WHILE_AFTER_BRACE",
-				      "while should follow close brace '}'\n" . $hereprev);
+				if (ERROR("WHILE_AFTER_BRACE",
+					  "while should follow close brace '}'\n" . $hereprev) &&
+				    $fix && $prevline =~ /^\+/ && $line =~ /^\+/) {
+					fix_delete_line($fixlinenr - 1, $prevrawline);
+					fix_delete_line($fixlinenr, $rawline);
+					my $fixedline = $prevrawline;
+					my $trailing = $rawline;
+					$trailing =~ s/^\+//;
+					$trailing = trim($trailing);
+					$fixedline =~ s/}\s*$/} $trailing/;
+					fix_insert_line($fixlinenr, $fixedline);
+				}
 			}
 		}

Signature · 1 line

1.8.1.2.459.gbcd45b4.dirty