From d06ed87d49ca3d734fd1c2f1409280abb499c625 Mon Sep 17 00:00:00 2001
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: Mon, 12 Jun 2023 16:53:57 +0300
Subject: utils: checkstyle: Add trailers checker

The libcamera git history contains numerous examples of incorrect commit
message trailers due to invalid trailer types (e.g. Change-Id), typos
and other small issues. Those went unnoticed through reviews, which
shows that an automated checker is required.

Add a trailers checker to checkstyle.py to catch invalid or malformed
trailers, with a set of supported trailers that match libcamera's commit
message practices. New trailer keys can easily be added later as new
needs arise.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 utils/checkstyle.py | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 78 insertions(+), 2 deletions(-)

(limited to 'utils')

diff --git a/utils/checkstyle.py b/utils/checkstyle.py
index e68c8746..3558740d 100755
--- a/utils/checkstyle.py
+++ b/utils/checkstyle.py
@@ -210,13 +210,23 @@ class Commit:
 
     def _parse(self):
         # Get the commit title and list of files.
-        ret = subprocess.run(['git', 'show', '--format=%s', '--name-status',
+        ret = subprocess.run(['git', 'show', '--format=%s%n%(trailers:only,unfold)', '--name-status',
                               self.commit],
                              stdout=subprocess.PIPE).stdout.decode('utf-8')
         lines = ret.splitlines()
-        self._files = [CommitFile(f) for f in lines[1:] if f]
+
         self._title = lines[0]
 
+        self._trailers = []
+        for index in range(1, len(lines)):
+            line = lines[index]
+            if not line:
+                break
+
+            self._trailers.append(line)
+
+        self._files = [CommitFile(f) for f in lines[index:] if f]
+
     def files(self, filter='AMR'):
         return [f.filename for f in self._files if f.status in filter]
 
@@ -224,6 +234,10 @@ class Commit:
     def title(self):
         return self._title
 
+    @property
+    def trailers(self):
+        return self._trailers
+
     def get_diff(self, top_level, filename):
         diff = subprocess.run(['git', 'diff', '%s~..%s' % (self.commit, self.commit),
                                '--', '%s/%s' % (top_level, filename)],
@@ -424,6 +438,68 @@ class TitleChecker(CommitChecker):
                             'possible candidates are ' + candidates)]
 
 
+class TrailersChecker(CommitChecker):
+    commit_regex = re.compile(r'[0-9a-f]{12}[0-9a-f]* \(".*"\)')
+
+    coverity_regex = re.compile(r'Coverity CID=.*')
+
+    # Simple e-mail address validator regex, with an additional trailing
+    # comment. The complexity of a full RFC6531 validator isn't worth the
+    # additional invalid addresses it would reject.
+    email_regex = re.compile(r'[^<]+ <[^@>]+@[^>]+>( # .*)?')
+
+    link_regex = re.compile(r'https?://.*')
+
+    @staticmethod
+    def validate_reported_by(value):
+        if TrailersChecker.email_regex.fullmatch(value):
+            return True
+        if TrailersChecker.coverity_regex.fullmatch(value):
+            return True
+        return False
+
+    known_trailers = {
+        'Acked-by': email_regex,
+        'Bug': link_regex,
+        'Fixes': commit_regex,
+        'Link': link_regex,
+        'Reported-by': validate_reported_by,
+        'Reviewed-by': email_regex,
+        'Signed-off-by': email_regex,
+        'Suggested-by': email_regex,
+        'Tested-by': email_regex,
+    }
+
+    trailer_regex = re.compile(r'([A-Z][a-zA-Z-]*)\s*:\s*(.*)')
+
+    @classmethod
+    def check(cls, commit, top_level):
+        issues = []
+
+        for trailer in commit.trailers:
+            match = TrailersChecker.trailer_regex.fullmatch(trailer)
+            if not match:
+                raise RuntimeError(f"Malformed commit trailer '{trailer}'")
+
+            key, value = match.groups()
+
+            validator = TrailersChecker.known_trailers.get(key)
+            if not validator:
+                issues.append(CommitIssue(f"Invalid commit trailer key '{key}'"))
+                continue
+
+            if isinstance(validator, re.Pattern):
+                valid = bool(validator.fullmatch(value))
+            else:
+                valid = validator(value)
+
+            if not valid:
+                issues.append(CommitIssue(f"Malformed value '{value}' for commit trailer '{key}'"))
+                continue
+
+        return issues
+
+
 # ------------------------------------------------------------------------------
 # Style Checkers
 #
-- 
cgit v1.2.1