aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJade Lovelace <lix@jade.fyi>2024-04-08 15:08:29 -0700
committerJade Lovelace <lix@jade.fyi>2024-04-08 16:10:57 -0700
commit1e74bffd5c37bed52ef8547ffa2b4e7f895e4e2a (patch)
tree7f228848a272e2883cf97705eae96f83200ef4b8
parentc58e3f826e891847bd454f4de8176e1fec74627a (diff)
pre-commit check for pragma once and ///@file
This is in our style guide, we can cheaply enforce it, let's do it. ``` $ pre-commit check-case-conflicts.....................................................Passed check-executables-have-shebangs..........................................Passed check-headers............................................................Failed - hook id: check-headers - exit code: 1 Missing pattern @file in file src/libexpr/value.hh We found some header files that don't conform to the style guide. The Lix style guide requests that header files: - Begin with `#pragma once` so they only get parsed once - Contain a doxygen comment (`/**` or `///`) containing `@file`, for example, `///@file`, which will make doxygen generate docs for them. When adding that, consider also adding a `@brief` with a sentence explaining what the header is for. For more details: https://wiki.lix.systems/link/3#bkmrk-header-files check-merge-conflicts....................................................Passed check-shebang-scripts-are-executable.....................................Passed check-symlinks.......................................(no files to check)Skipped end-of-file-fixer........................................................Passed mixed-line-endings.......................................................Passed no-commit-to-branch......................................................Passed release-notes........................................(no files to check)Skipped treefmt..................................................................Passed trim-trailing-whitespace.................................................Passed ``` Fixes: https://git.lix.systems/lix-project/lix/issues/233 Change-Id: I77150b9298c844ffedd0f85cc5250ae9208502e3
-rw-r--r--flake.nix18
-rw-r--r--maintainers/check-headers.nix7
-rwxr-xr-xmaintainers/check-headers.sh47
3 files changed, 72 insertions, 0 deletions
diff --git a/flake.nix b/flake.nix
index 56dd4bfc3..8eb5582b0 100644
--- a/flake.nix
+++ b/flake.nix
@@ -157,6 +157,7 @@
nixUnstable = prev.nixUnstable;
build-release-notes = final.buildPackages.callPackage ./maintainers/build-release-notes.nix { };
+ check-headers = final.buildPackages.callPackage ./maintainers/check-headers.nix { };
clangbuildanalyzer = final.buildPackages.callPackage ./misc/clangbuildanalyzer.nix { };
default-busybox-sandbox-shell = final.busybox.override {
@@ -353,6 +354,23 @@
${lib.getExe pkgs.build-release-notes} doc/manual/rl-next doc/manual/rl-next-dev
'';
};
+ check-headers = {
+ enable = true;
+ package = pkgs.check-headers;
+ files = "^src/";
+ types = [
+ "c++"
+ "file"
+ "header"
+ ];
+ # generated files; these will never actually be seen by this
+ # check, and are left here as documentation
+ excludes = [
+ "(parser|lexer)-tab\\.hh$"
+ "\\.gen\\.hh$"
+ ];
+ entry = lib.getExe pkgs.check-headers;
+ };
# TODO: Once the test suite is nicer, clean up and start
# enforcing trailing whitespace on tests that don't explicitly
# check for it.
diff --git a/maintainers/check-headers.nix b/maintainers/check-headers.nix
new file mode 100644
index 000000000..a184bb401
--- /dev/null
+++ b/maintainers/check-headers.nix
@@ -0,0 +1,7 @@
+{ writeShellApplication, gnugrep }:
+writeShellApplication {
+ name = "check-headers";
+
+ runtimeInputs = [ gnugrep ];
+ text = builtins.readFile ./check-headers.sh;
+}
diff --git a/maintainers/check-headers.sh b/maintainers/check-headers.sh
new file mode 100755
index 000000000..2b4dd248a
--- /dev/null
+++ b/maintainers/check-headers.sh
@@ -0,0 +1,47 @@
+#!/usr/bin/env bash
+
+set -eu
+
+# n.b. this might be printed multiple times if any violating header files are
+# in different parallelism groups inside pre-commit. We cannot do anything about
+# this.
+explanation=$(cat <<'EOF'
+
+We found some header files that don't conform to the style guide.
+
+The Lix style guide requests that header files:
+- Begin with `#pragma once` so they only get parsed once
+- Contain a doxygen comment (`/**` or `///`) containing `@file`, for
+ example, `///@file`, which will make doxygen generate docs for them.
+
+ When adding that, consider also adding a `@brief` with a sentence
+ explaining what the header is for.
+
+For more details: https://wiki.lix.systems/link/3#bkmrk-header-files
+EOF
+)
+
+check_file() {
+ grep -q "$1" "$2" || (echo "Missing pattern $1 in file $2" >&2; return 1)
+}
+
+patterns=(
+ # makes a file get included only once even if it is included multiple times
+ '^#pragma once$'
+ # as used in ///@file, makes the file appear to doxygen
+ '@file'
+)
+fail=0
+
+for pattern in "${patterns[@]}"; do
+ for file in "$@"; do
+ check_file "$pattern" "$file" || fail=1
+ done
+done
+
+if [[ $fail != 0 ]]; then
+ echo "$explanation" >&2
+ exit 1
+else
+ echo OK
+fi