336 lines
15 KiB
Python
Executable file
336 lines
15 KiB
Python
Executable file
#!/usr/bin/python3
|
|
# SPDX-FileCopyrightText: 2026 Méven Car <meven@kde.org>
|
|
# SPDX-License-Identifier: BSD-2-Clause
|
|
|
|
# Checks the commits of a merge request for mistakes that reviewers keep
|
|
# having to point out by hand.
|
|
#
|
|
# Every check is off unless the project asks for it in .kde-ci.yml:
|
|
#
|
|
# Options:
|
|
# commit-checks: ['bug-links', 'missing-since', 'since-version']
|
|
|
|
import argparse
|
|
import os
|
|
import re
|
|
import subprocess
|
|
import sys
|
|
from typing import Dict, List, Set, Tuple
|
|
|
|
import yaml
|
|
|
|
from components import CommonUtils
|
|
|
|
AVAILABLE_CHECKS = ['bug-links', 'missing-since', 'since-version']
|
|
|
|
# A commit that closes or mentions a bug says so with a keyword bugzilla reads.
|
|
BUG_LINK_RE = re.compile(r'https?://bugs\.kde\.org/(?:show_bug\.cgi\?id=)?(\d+)', re.IGNORECASE)
|
|
# The keyword lines the commit hooks act on, spelled as hooks/hooklib.py in
|
|
# sysadmin/repo-management spells them: leading space is allowed, the separator is a colon
|
|
# or an equals sign, the words are upper case, and several of them have a longer form.
|
|
GOOD_KEYWORD_RE = re.compile(r'^\s*(?:(?:BUGS?|CCBUGS?|FEATURE|ENHANCEMENT)[:=]\s*\S'
|
|
r'|FIXED[-_]?IN[:=]\s*\S'
|
|
r'|CC[-_]?MAIL[:=]\s*\S'
|
|
r'|C[Cc][:=]\s*\S'
|
|
r'|GUI:'
|
|
r'|(?:CVS|SVN|GIT|SCM).?SILENT)')
|
|
# A line that is meant to be one of those and nothing else, however it was spelled.
|
|
NEAR_KEYWORD_RE = re.compile(r'^\s*(?:(?:BUGS?|CCBUGS?|FEATURE|ENHANCEMENT)\b\s*[:=]?\s*#?\d+'
|
|
r'|FIXED[-_]?IN\b\s*[:=]?\s*[\w.]+)\s*$', re.IGNORECASE)
|
|
|
|
HEADER_SUFFIXES = ('.h', '.hh', '.hpp', '.hxx')
|
|
# Headers nobody outside the library includes, so they carry no promise of a version.
|
|
PRIVATE_PATH_RE = re.compile(r'(^|/)(autotests|tests|test|examples|src/kdeinit)/|_p\.(h|hpp)$')
|
|
|
|
SINCE_RE = re.compile(r'[\\@]since\b')
|
|
# The version a \since names, however it is dressed: a colon after the command, backticks
|
|
# around it, or a word such as KF in between.
|
|
SINCE_VERSION_RE = re.compile(r'[\\@]since\b[^0-9\n]{0,24}(\d+)\.(\d+)')
|
|
INTERNAL_RE = re.compile(r'[\\@]internal\b')
|
|
DOC_START_RE = re.compile(r'^\s*/\*[!*]')
|
|
MACRO_LINE_RE = re.compile(r'^\s*[A-Z][A-Z0-9_]*(\(.*\))?$')
|
|
ACCESS_RE = re.compile(r'^\s*(public|protected|private)\s*(Q_SLOTS|Q_SIGNALS|slots|signals)?\s*:')
|
|
SIGNALS_RE = re.compile(r'^\s*(Q_SIGNALS|signals)\s*:')
|
|
CLASS_RE = re.compile(r'^\s*(class|struct)\s+(?:[A-Z][A-Z0-9_]*_EXPORT\s+)?(\w+)\b(?!.*;\s*$)')
|
|
EXPORT_RE = re.compile(r'\b[A-Z][A-Z0-9_]*_EXPORT\b')
|
|
FUNCTION_RE = re.compile(r'^[\w:~<>,&*\[\]\s]*\w\s*\(.*\)\s*(?:const)?\s*(?:noexcept)?\s*(?:override|final)?\s*(?:=\s*(?:0|default|delete))?\s*;$')
|
|
TYPE_RE = re.compile(r'^\s*(?:enum(?:\s+class)?|class|struct)\s+\w+')
|
|
SKIP_STATEMENT_RE = re.compile(r'^\s*(?:friend|using|typedef|template|return|Q_DECLARE|Q_ENUM|Q_FLAG|Q_PROPERTY|Q_DISABLE|Q_OBJECT|Q_GADGET|#)')
|
|
DESTRUCTOR_RE = re.compile(r'^\s*(?:virtual\s+)?~\w+\s*\(')
|
|
# An override reimplements a method the base class already promised.
|
|
OVERRIDE_RE = re.compile(r'\b(?:override|final)\b\s*(?:=\s*(?:0|default|delete)\s*)?;$')
|
|
|
|
|
|
def gitOutput(arguments: List[str]) -> str:
|
|
return subprocess.run(['git'] + arguments, capture_output=True, text=True, check=True).stdout
|
|
|
|
|
|
def loadEnabledChecks() -> List[str]:
|
|
configuration = yaml.safe_load(open(os.path.join(CommonUtils.scriptsBaseDirectory(), 'config', 'global.yml')))
|
|
if os.path.exists('.kde-ci.yml'):
|
|
CommonUtils.recursiveUpdate(configuration, yaml.safe_load(open('.kde-ci.yml')))
|
|
return configuration['Options'].get('commit-checks', [])
|
|
|
|
|
|
def determineRange(arguments: argparse.Namespace) -> Tuple[str, str]:
|
|
base = arguments.base or os.environ.get('CI_MERGE_REQUEST_DIFF_BASE_SHA')
|
|
head = arguments.head or os.environ.get('CI_COMMIT_SHA') or 'HEAD'
|
|
if not base:
|
|
# Outside a merge request pipeline, look at the commit in front of us.
|
|
base = gitOutput(['rev-parse', head + '^']).strip()
|
|
return base, head
|
|
|
|
|
|
def addedLines(base: str, head: str, path: str) -> Set[int]:
|
|
# The line numbers the file has after the change, for the lines the change touches.
|
|
diff = gitOutput(['diff', '-U0', base, head, '--', path])
|
|
lines: Set[int] = set()
|
|
for hunk in re.finditer(r'^@@ -\S+ \+(\d+)(?:,(\d+))? @@', diff, re.MULTILINE):
|
|
start = int(hunk.group(1))
|
|
count = int(hunk.group(2)) if hunk.group(2) else 1
|
|
lines.update(range(start, start + count))
|
|
return lines
|
|
|
|
|
|
def changedFiles(base: str, head: str, suffixes: Tuple[str, ...]) -> List[str]:
|
|
output = gitOutput(['diff', '--name-only', '--diff-filter=d', base, head])
|
|
return [path for path in output.splitlines() if path.endswith(suffixes)]
|
|
|
|
|
|
def commitsIn(base: str, head: str) -> List[Tuple[str, str]]:
|
|
output = gitOutput(['rev-list', '--no-merges', '--reverse', base + '..' + head])
|
|
return [(sha, gitOutput(['show', '-s', '--format=%B', sha])) for sha in output.split()]
|
|
|
|
|
|
def checkBugLinks(commits: List[Tuple[str, str]]) -> List[str]:
|
|
problems = []
|
|
for sha, message in commits:
|
|
for number, line in enumerate(message.splitlines(), start=1):
|
|
link = BUG_LINK_RE.search(line)
|
|
if link:
|
|
problems.append(f'{sha[:12]}: line {number}: a link to bugs.kde.org tells bugzilla nothing. '
|
|
f'Write "BUG: {link.group(1)}" to close the bug, or "CCBUG: {link.group(1)}" to note it.')
|
|
if NEAR_KEYWORD_RE.match(line) and not GOOD_KEYWORD_RE.match(line.strip()):
|
|
problems.append(f'{sha[:12]}: line {number}: "{line.strip()}" is not a keyword the commit hooks read. '
|
|
'They read "BUG: 123456", "CCBUG: 123456" and "FIXED-IN: 6.30", in upper case.')
|
|
return problems
|
|
|
|
|
|
def commentLines(lines: List[str]) -> List[bool]:
|
|
# True for every line that is part of a comment, so prose is never read as code.
|
|
result = []
|
|
inBlock = False
|
|
for line in lines:
|
|
stripped = line.strip()
|
|
isComment = inBlock or stripped.startswith(('//', '/*'))
|
|
if '/*' in line and '*/' not in line.split('/*', 1)[1]:
|
|
inBlock = True
|
|
elif '*/' in line:
|
|
inBlock = False
|
|
result.append(isComment)
|
|
return result
|
|
|
|
|
|
def statementAt(lines: List[str], index: int) -> Tuple[int, int, str]:
|
|
# A declaration can be spread over several lines. Return where it begins, where it
|
|
# ends, and the whole of it on one line.
|
|
start = index
|
|
while start > 0:
|
|
above = lines[start - 1].strip()
|
|
if above == '' or above.endswith((';', '{', '}', '*/', ':')) or above.startswith(('//', '/*', '*')):
|
|
break
|
|
start -= 1
|
|
end = index
|
|
while end < len(lines) - 1 and not lines[end].rstrip().endswith((';', '{')):
|
|
end += 1
|
|
return start, end, ' '.join(line.strip() for line in lines[start:end + 1])
|
|
|
|
|
|
def documentationAbove(lines: List[str], start: int) -> str:
|
|
index = start - 1
|
|
while index >= 0 and (lines[index].strip() == '' or MACRO_LINE_RE.match(lines[index])):
|
|
index -= 1
|
|
if index < 0:
|
|
return ''
|
|
if lines[index].strip().startswith('//!'):
|
|
block = []
|
|
while index >= 0 and lines[index].strip().startswith('//!'):
|
|
block.append(lines[index])
|
|
index -= 1
|
|
return '\n'.join(block)
|
|
if not lines[index].strip().endswith('*/'):
|
|
return ''
|
|
block = []
|
|
while index >= 0:
|
|
block.append(lines[index])
|
|
if DOC_START_RE.match(lines[index]) or lines[index].strip().startswith('/*'):
|
|
break
|
|
index -= 1
|
|
return '\n'.join(block)
|
|
|
|
|
|
def declaredName(statement: str) -> str:
|
|
# The name a declaration introduces, ignoring the macros that may sit in front of it.
|
|
if TYPE_RE.match(statement):
|
|
name = re.search(r'(?:enum(?:\s+class)?|class|struct)\s+(?:[A-Z][A-Z0-9_]*_EXPORT\s+)?(\w+)', statement)
|
|
return name.group(1) if name else ''
|
|
names = [name for name in re.findall(r'(\w+)\s*\(', statement) if not name.isupper()]
|
|
return names[-1] if names else ''
|
|
|
|
|
|
def alreadyDeclared(name: str, previousContent: str) -> bool:
|
|
if not name or not previousContent:
|
|
return False
|
|
return re.search(r'\b' + re.escape(name) + r'\s*[({;:]', previousContent) is not None
|
|
|
|
|
|
def enclosingClass(lines: List[str], index: int) -> int:
|
|
for candidate in range(index - 1, -1, -1):
|
|
if CLASS_RE.match(lines[candidate]):
|
|
return candidate
|
|
return -1
|
|
|
|
|
|
def accessAbove(lines: List[str], index: int) -> str:
|
|
for candidate in range(index - 1, -1, -1):
|
|
if SIGNALS_RE.match(lines[candidate]):
|
|
return 'public'
|
|
access = ACCESS_RE.match(lines[candidate])
|
|
if access:
|
|
return access.group(1)
|
|
if CLASS_RE.match(lines[candidate]):
|
|
return 'struct' if lines[candidate].strip().startswith('struct') else 'private'
|
|
return ''
|
|
|
|
|
|
def newPublicDeclarations(base: str, head: str) -> List[Tuple[str, int, str, str]]:
|
|
# Every class or method the change adds to a header that other projects include,
|
|
# with the documentation block written above it.
|
|
declarations = []
|
|
for path in changedFiles(base, head, HEADER_SUFFIXES):
|
|
if PRIVATE_PATH_RE.search(path):
|
|
continue
|
|
added = addedLines(base, head, path)
|
|
if not added:
|
|
continue
|
|
lines = gitOutput(['show', f'{head}:{path}']).split('\n')
|
|
previousContent = subprocess.run(['git', 'show', f'{base}:{path}'], capture_output=True, text=True).stdout
|
|
isComment = commentLines(lines)
|
|
reported: Set[int] = set()
|
|
for number in sorted(added):
|
|
index = number - 1
|
|
if index >= len(lines) or isComment[index]:
|
|
continue
|
|
start, _, statement = statementAt(lines, index)
|
|
if isComment[start]:
|
|
continue
|
|
if start in reported or SKIP_STATEMENT_RE.match(statement) or DESTRUCTOR_RE.match(statement) or OVERRIDE_RE.search(statement):
|
|
continue
|
|
isType = bool(TYPE_RE.match(statement))
|
|
isFunction = bool(FUNCTION_RE.match(statement.strip()))
|
|
if not isType and not isFunction:
|
|
continue
|
|
indented = lines[start].startswith((' ', '\t'))
|
|
if indented and accessAbove(lines, start) not in ('public', 'protected', 'struct'):
|
|
continue
|
|
if not indented and not EXPORT_RE.search(statement):
|
|
continue
|
|
# A member of a class that this same change adds is covered by the version on the class.
|
|
classLine = enclosingClass(lines, start)
|
|
if indented and classLine >= 0 and (classLine + 1) in added and SINCE_RE.search(documentationAbove(lines, classLine)):
|
|
continue
|
|
documentation = documentationAbove(lines, start)
|
|
if INTERNAL_RE.search(documentation):
|
|
continue
|
|
# A declaration that was already in the file is being changed, not introduced.
|
|
if alreadyDeclared(declaredName(statement), previousContent):
|
|
continue
|
|
reported.add(start)
|
|
declarations.append((path, start + 1, statement, documentation))
|
|
return declarations
|
|
|
|
|
|
def checkMissingSince(declarations: List[Tuple[str, int, str, str]]) -> List[str]:
|
|
return [f'{path}:{line}: "{statement[:70]}" is new public API without a \\since version.'
|
|
for path, line, statement, documentation in declarations if not SINCE_RE.search(documentation)]
|
|
|
|
|
|
def developmentVersion(head: str) -> str:
|
|
# The version the branch will be released in, as major.minor.
|
|
text = subprocess.run(['git', 'show', f'{head}:CMakeLists.txt'], capture_output=True, text=True).stdout
|
|
for pattern in (r'set\s*\(\s*KF_VERSION\s+"?(\d+)\.(\d+)', r'project\s*\([^)]*\bVERSION\s+(\d+)\.(\d+)'):
|
|
found = re.search(pattern, text)
|
|
if found:
|
|
return f'{found.group(1)}.{found.group(2)}'
|
|
return ''
|
|
|
|
|
|
def checkSinceVersion(declarations: List[Tuple[str, int, str, str]], version: str) -> List[str]:
|
|
if not version:
|
|
return ['the version under development is not written as KF_VERSION or project(VERSION) in CMakeLists.txt, '
|
|
'so the \\since versions were not checked.']
|
|
problems = []
|
|
for path, line, statement, documentation in declarations:
|
|
written = SINCE_VERSION_RE.search(documentation)
|
|
if not written:
|
|
continue
|
|
said = f'{written.group(1)}.{written.group(2)}'
|
|
if said == version:
|
|
continue
|
|
older = (int(written.group(1)), int(written.group(2))) < tuple(int(part) for part in version.split('.'))
|
|
reason = 'a version that is already out' if older else 'a version that is not being developed yet'
|
|
problems.append(f'{path}:{line}: "{statement[:60]}" says \\since {said}, which is {reason}. '
|
|
f'This branch lands in {version}.')
|
|
return problems
|
|
|
|
|
|
def main() -> int:
|
|
parser = argparse.ArgumentParser(description='Check the commits of a merge request for common mistakes')
|
|
parser.add_argument('--base', help='Commit the branch started from, defaults to the merge request base')
|
|
parser.add_argument('--head', help='Last commit to check, defaults to the tip of the branch')
|
|
parser.add_argument('--check', action='append', choices=AVAILABLE_CHECKS,
|
|
help='Run this check whatever .kde-ci.yml says, can be given more than once')
|
|
arguments = parser.parse_args()
|
|
|
|
enabled = loadEnabledChecks()
|
|
if arguments.check:
|
|
enabled = arguments.check
|
|
unknown = [name for name in enabled if name not in AVAILABLE_CHECKS]
|
|
if unknown:
|
|
print(f'Unknown commit checks asked for in .kde-ci.yml: {", ".join(unknown)}')
|
|
return 1
|
|
if not enabled:
|
|
print('No commit checks are enabled for this project. To turn some on, add to .kde-ci.yml:')
|
|
print('')
|
|
print(' Options:')
|
|
print(f' commit-checks: {AVAILABLE_CHECKS}')
|
|
return 0
|
|
|
|
base, head = determineRange(arguments)
|
|
commits = commitsIn(base, head)
|
|
print(f'Checking {len(commits)} commit(s) between {base[:12]} and {head[:12]}: {", ".join(enabled)}')
|
|
|
|
problems: Dict[str, List[str]] = {}
|
|
if 'bug-links' in enabled:
|
|
problems['bug-links'] = checkBugLinks(commits)
|
|
if 'missing-since' in enabled or 'since-version' in enabled:
|
|
declarations = newPublicDeclarations(base, head)
|
|
if 'missing-since' in enabled:
|
|
problems['missing-since'] = checkMissingSince(declarations)
|
|
if 'since-version' in enabled:
|
|
problems['since-version'] = checkSinceVersion(declarations, developmentVersion(head))
|
|
|
|
found = 0
|
|
for check, messages in problems.items():
|
|
for message in messages:
|
|
print(f'{check}: {message}')
|
|
found += 1
|
|
if found:
|
|
print(f'\n{found} problem(s) found. Amend or rebase the branch to fix them.')
|
|
return 1
|
|
print('All enabled commit checks passed.')
|
|
return 0
|
|
|
|
|
|
if __name__ == '__main__':
|
|
sys.exit(main())
|