From 284ca193a908f223a2d4e5e7e4bc27f1389a7996 Mon Sep 17 00:00:00 2001 From: Tom de Vries Date: Thu, 16 Oct 2025 12:09:57 +0200 Subject: [PATCH] [pre-commit] Add check-gnu-style I got a review comment [1] because I forgot to do "space before paren". I realized I forgot to run check_GNU_style.py, a script from the GCC repo, which warns about things like this. [ The python script has been around since 2017 (and an earlier version written in shell script since 2010). ] So for this change in gdb/gdb.c: ... - return gdb_main (&args); + return gdb_main(&args); ... we get: ... $ ./contrib/check_GNU_style.py <(git diff) === ERROR type #1: there should be exactly one space between function name \ and parenthesis (1 error(s)) === gdb/gdb.c:38:17: return gdb_main(&args); ... Add a pre-commit hook to do this automatically. This copies two files from the GCC repo to root-level contrib, and adds a wrapper script gdb/contrib/check-gnu-style-pre-commit.sh (checked with shellcheck). The wrapper script is setup to not fail on violations, so the messages are informational at this point. I'm not sure all checks are 100% applicable to our coding style. The python script check_GNU_style.py has two dependencies: unidiff and termcolor, which users need to install themselves. The check is added at the pre-commit stage. I also considered post-commit, and I'm still not sure what is the better choice. As with all pre-commit checks, if the check is not to your liking, you can use SKIP=check-gnu-style to skip it. In summary, with the new pre-commit check we get: ... $ git commit -a -m "style error" black...............................................(no files to check)Skipped flake8..............................................(no files to check)Skipped isort...............................................(no files to check)Skipped codespell...........................................(no files to check)Skipped check-include-guards................................(no files to check)Skipped check-gnu-style.........................................................Passed - hook id: check-gnu-style - duration: 0.04s === ERROR type #1: there should be exactly one space between function name \ and parenthesis (1 error(s)) === gdb/gdb.c:38:17: return gdb_main(&args); tclint..............................................(no files to check)Skipped black...............................................(no files to check)Skipped flake8..............................................(no files to check)Skipped codespell...........................................(no files to check)Skipped check-include-guards................................(no files to check)Skipped codespell-log...........................................................Passed - hook id: codespell-log - duration: 0.19s tclint...............................................(no files to check)Skipped [master $hex] style error ... Approved-By: Simon Marchi [1] https://sourceware.org/pipermail/gdb-patches/2025-September/220983.html --- .pre-commit-config.yaml | 8 + contrib/check_GNU_style.py | 45 +++ contrib/check_GNU_style_lib.py | 326 ++++++++++++++++++++++ gdb/contrib/check-gnu-style-pre-commit.sh | 42 +++ 4 files changed, 421 insertions(+) create mode 100755 contrib/check_GNU_style.py create mode 100755 contrib/check_GNU_style_lib.py create mode 100755 gdb/contrib/check-gnu-style-pre-commit.sh diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b0964f52838..215a5e7c951 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -89,6 +89,14 @@ repos: verbose: true always_run: true stages: [commit-msg] + - id: check-gnu-style + name: check-gnu-style + language: script + entry: gdb/contrib/check-gnu-style-pre-commit.sh + files: '^(gdb(support|server)?)/.*\.(c|h|cc)$' + exclude: '.*/testsuite/.*' + verbose: true + stages: [pre-commit] - repo: https://github.com/nmoroze/tclint rev: v0.6.1 hooks: diff --git a/contrib/check_GNU_style.py b/contrib/check_GNU_style.py new file mode 100755 index 00000000000..b359bd8b4c0 --- /dev/null +++ b/contrib/check_GNU_style.py @@ -0,0 +1,45 @@ +#!/usr/bin/env python3 + +# Copyright (C) 2017-2025 Free Software Foundation, Inc. +# +# Checks some of the GNU style formatting rules in a set of patches. +# The script is a rewritten of the same bash script and should eventually +# replace the former script. +# +# This file is part of GCC. +# +# GCC is free software; you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation; either version 3, or (at your option) any later +# version. +# +# GCC is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +# for more details. +# +# You should have received a copy of the GNU General Public License +# along with GCC; see the file COPYING3. If not see +# . + +import argparse +import sys +from check_GNU_style_lib import check_GNU_style_file + +def main(): + parser = argparse.ArgumentParser(description='Check GNU coding style.') + parser.add_argument('file', help = 'File with a patch') + parser.add_argument('-f', '--format', default = 'stdio', + help = 'Display format', + choices = ['stdio', 'quickfix']) + args = parser.parse_args() + filename = args.file + format = args.format + + if filename == '-': + check_GNU_style_file(sys.stdin, format) + else: + with open(filename, newline='\n') as diff_file: + check_GNU_style_file(diff_file, format) + +main() diff --git a/contrib/check_GNU_style_lib.py b/contrib/check_GNU_style_lib.py new file mode 100755 index 00000000000..8b930ef6bdb --- /dev/null +++ b/contrib/check_GNU_style_lib.py @@ -0,0 +1,326 @@ +#!/usr/bin/env python3 + +# Copyright (C) 2017-2025 Free Software Foundation, Inc. +# +# Checks some of the GNU style formatting rules in a set of patches. +# The script is a rewritten of the same bash script and should eventually +# replace the former script. +# +# This file is part of GCC. +# +# GCC is free software; you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation; either version 3, or (at your option) any later +# version. +# +# GCC is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +# for more details. +# +# You should have received a copy of the GNU General Public License +# along with GCC; see the file COPYING3. If not see +# . +# +# The script requires python packages, which can be installed via pip3 +# like this: +# $ pip3 install unidiff termcolor + +import sys +import re +import unittest + +def import_pip3(*args): + missing=[] + for (module, names) in args: + try: + lib = __import__(module) + except ImportError: + missing.append(module) + continue + if not isinstance(names, list): + names=[names] + for name in names: + globals()[name]=getattr(lib, name) + if len(missing) > 0: + missing_and_sep = ' and '.join(missing) + missing_space_sep = ' '.join(missing) + print('%s %s missing (run: pip3 install %s)' + % (missing_and_sep, + ("module is" if len(missing) == 1 else "modules are"), + missing_space_sep)) + exit(3) + +import_pip3(('termcolor', 'colored'), + ('unidiff', 'PatchSet')) + +from itertools import * + +ws_char = '█' +ts = 8 + +def error_string(s): + return colored(s, 'red', attrs = ['bold']) + +class CheckError: + def __init__(self, filename, lineno, console_error, error_message, + column = -1): + self.filename = filename + self.lineno = lineno + self.console_error = console_error + self.error_message = error_message + self.column = column + + def error_location(self): + return '%s:%d:%d:' % (self.filename, self.lineno, + self.column if self.column != -1 else -1) + +class LineLengthCheck: + def __init__(self): + self.limit = 80 + self.expanded_tab = ' ' * ts + + def check(self, filename, lineno, line): + line_expanded = line.replace('\t', self.expanded_tab) + if len(line_expanded) > self.limit: + return CheckError(filename, lineno, + line_expanded[:self.limit] + + error_string(line_expanded[self.limit:]), + 'lines should not exceed 80 characters', self.limit) + + return None + +class SpacesCheck: + def __init__(self): + self.expanded_tab = ' ' * ts + + def check(self, filename, lineno, line): + i = line.find(self.expanded_tab) + if i != -1: + return CheckError(filename, lineno, + line.replace(self.expanded_tab, error_string(ws_char * ts)), + 'blocks of 8 spaces should be replaced with tabs', i) + +class SpacesAndTabsMixedCheck: + def __init__(self): + self.re = re.compile(r'\ \t') + + def check(self, filename, lineno, line): + stripped = line.lstrip() + start = line[:len(line) - len(stripped)] + if self.re.search(line): + return CheckError(filename, lineno, + error_string(start.replace('\t', ws_char * ts)) + line[len(start):], + 'a space should not precede a tab', 0) + +class TrailingWhitespaceCheck: + def __init__(self): + self.re = re.compile(r'(\s+)$') + + def check(self, filename, lineno, line): + assert(len(line) == 0 or line[-1] != '\n') + m = self.re.search(line) + if m != None: + return CheckError(filename, lineno, + line[:m.start(1)] + error_string(ws_char * len(m.group(1))) + + line[m.end(1):], + 'trailing whitespace', m.start(1)) + +class SentenceSeparatorCheck: + def __init__(self): + self.re = re.compile(r'\w\.(\s|\s{3,})\w') + + def check(self, filename, lineno, line): + m = self.re.search(line) + if m != None: + return CheckError(filename, lineno, + line[:m.start(1)] + error_string(ws_char * len(m.group(1))) + + line[m.end(1):], + 'dot, space, space, new sentence', m.start(1)) + +class SentenceEndOfCommentCheck: + def __init__(self): + self.re = re.compile(r'\w\.(\s{0,1}|\s{3,})\*/') + + def check(self, filename, lineno, line): + m = self.re.search(line) + if m != None: + return CheckError(filename, lineno, + line[:m.start(1)] + error_string(ws_char * len(m.group(1))) + + line[m.end(1):], + 'dot, space, space, end of comment', m.start(1)) + +class SentenceDotEndCheck: + def __init__(self): + self.re = re.compile(r'\w(\s*\*/)') + + def check(self, filename, lineno, line): + m = self.re.search(line) + if m != None: + return CheckError(filename, lineno, + line[:m.start(1)] + error_string(m.group(1)) + line[m.end(1):], + 'dot, space, space, end of comment', m.start(1)) + +class FunctionParenthesisCheck: + # TODO: filter out GTY stuff + def __init__(self): + self.re = re.compile(r'\w(\s{2,})?(\()') + + def check(self, filename, lineno, line): + if '#define' in line: + return None + + m = self.re.search(line) + if m != None: + return CheckError(filename, lineno, + line[:m.start(2)] + error_string(m.group(2)) + line[m.end(2):], + 'there should be exactly one space between function name ' \ + 'and parenthesis', m.start(2)) + +class SquareBracketCheck: + def __init__(self): + self.re = re.compile(r'\w\s+(\[)') + + def check(self, filename, lineno, line): + if filename.endswith('.md'): + return None + + m = self.re.search(line) + if m != None: + return CheckError(filename, lineno, + line[:m.start(1)] + error_string(m.group(1)) + line[m.end(1):], + 'there should be no space before a left square bracket', + m.start(1)) + +class ClosingParenthesisCheck: + def __init__(self): + self.re = re.compile(r'\S\s+(\))') + + def check(self, filename, lineno, line): + m = self.re.search(line) + if m != None: + return CheckError(filename, lineno, + line[:m.start(1)] + error_string(m.group(1)) + line[m.end(1):], + 'there should be no space before closing parenthesis', + m.start(1)) + +class BracesOnSeparateLineCheck: + # This will give false positives for C99 compound literals. + + def __init__(self): + self.re = re.compile(r'(\)|else)\s*({)') + + def check(self, filename, lineno, line): + m = self.re.search(line) + if m != None: + return CheckError(filename, lineno, + line[:m.start(2)] + error_string(m.group(2)) + line[m.end(2):], + 'braces should be on a separate line', m.start(2)) + +class TrailinigOperatorCheck: + def __init__(self): + regex = r'^\s.*(([^a-zA-Z_]\*)|([-%<=&|^?])|([^*]/)|([^:][+]))$' + self.re = re.compile(regex) + + def check(self, filename, lineno, line): + m = self.re.search(line) + if m != None: + return CheckError(filename, lineno, + line[:m.start(1)] + error_string(m.group(1)) + line[m.end(1):], + 'trailing operator', m.start(1)) + +class LineLengthTest(unittest.TestCase): + def setUp(self): + self.check = LineLengthCheck() + + def test_line_length_check_basic(self): + r = self.check.check('foo', 123, self.check.limit * 'a' + ' = 123;') + self.assertIsNotNone(r) + self.assertEqual('foo', r.filename) + self.assertEqual(80, r.column) + self.assertEqual(r.console_error, + self.check.limit * 'a' + error_string(' = 123;')) + +class TrailingWhitespaceTest(unittest.TestCase): + def setUp(self): + self.check = TrailingWhitespaceCheck() + + def test_trailing_whitespace_check_basic(self): + r = self.check.check('foo', 123, 'a = 123;') + self.assertIsNone(r) + r = self.check.check('foo', 123, 'a = 123; ') + self.assertIsNotNone(r) + r = self.check.check('foo', 123, 'a = 123;\t') + self.assertIsNotNone(r) + +class SpacesAndTabsMixedTest(unittest.TestCase): + def setUp(self): + self.check = SpacesAndTabsMixedCheck() + + def test_trailing_whitespace_check_basic(self): + r = self.check.check('foo', 123, ' \ta = 123;') + self.assertEqual('foo', r.filename) + self.assertEqual(0, r.column) + self.assertIsNotNone(r.console_error) + r = self.check.check('foo', 123, ' \t a = 123;') + self.assertIsNotNone(r.console_error) + r = self.check.check('foo', 123, '\t a = 123;') + self.assertIsNone(r) + +def check_GNU_style_file(file, format): + checks = [LineLengthCheck(), SpacesCheck(), TrailingWhitespaceCheck(), + SentenceSeparatorCheck(), SentenceEndOfCommentCheck(), + SentenceDotEndCheck(), FunctionParenthesisCheck(), + SquareBracketCheck(), ClosingParenthesisCheck(), + BracesOnSeparateLineCheck(), TrailinigOperatorCheck(), + SpacesAndTabsMixedCheck()] + errors = [] + + patch = PatchSet(file) + + for pfile in patch.added_files + patch.modified_files: + t = pfile.target_file + if t.startswith('b/'): + t = t[2:] + # Skip testsuite files + if 'testsuite' in t or t.endswith('.py'): + continue + + for hunk in pfile: + delta = 0 + for line in hunk: + if line.is_added and line.target_line_no != None: + for check in checks: + line_chomp = line.value.replace('\n', '') + e = check.check(t, line.target_line_no, line_chomp) + if e != None: + errors.append(e) + + if format == 'stdio': + fn = lambda x: x.error_message + i = 1 + for (k, errors) in groupby(sorted(errors, key = fn), fn): + errors = list(errors) + print('=== ERROR type #%d: %s (%d error(s)) ===' + % (i, k, len(errors))) + i += 1 + for e in errors: + print(e.error_location () + e.console_error) + print() + + exit(0 if len(errors) == 0 else 1) + elif format == 'quickfix': + f = 'errors.err' + with open(f, 'w+') as qf: + for e in errors: + qf.write('%s%s\n' % (e.error_location(), e.error_message)) + if len(errors) == 0: + exit(0) + else: + print('%d error(s) written to %s file.' % (len(errors), f)) + exit(1) + else: + assert False + +if __name__ == '__main__': + unittest.main() diff --git a/gdb/contrib/check-gnu-style-pre-commit.sh b/gdb/contrib/check-gnu-style-pre-commit.sh new file mode 100755 index 00000000000..ab4b5e7196a --- /dev/null +++ b/gdb/contrib/check-gnu-style-pre-commit.sh @@ -0,0 +1,42 @@ +#!/bin/sh + +# Copyright (C) 2025 Free Software Foundation, Inc. +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +set -e + +scriptdir=$(cd "$(dirname "$0")" || exit 1; pwd -P) + +tmp="" + +cleanup() +{ + if [ "$tmp" != "" ]; then + rm -f "$tmp" + fi +} + +# Schedule cleanup. +trap cleanup EXIT + +# Get temporary file. +tmp=$(mktemp) + +# Generate patch. +git diff --staged "$@" \ + > "$tmp" + +# Verify patch. Ignore exit status. +"$scriptdir"/../../contrib/check_GNU_style.py "$tmp" \ + || true -- 2.47.3