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
This commit is contained in:
parent
c58e3f826e
commit
1e74bffd5c
3 changed files with 72 additions and 0 deletions
18
flake.nix
18
flake.nix
|
@ -157,6 +157,7 @@
|
||||||
nixUnstable = prev.nixUnstable;
|
nixUnstable = prev.nixUnstable;
|
||||||
|
|
||||||
build-release-notes = final.buildPackages.callPackage ./maintainers/build-release-notes.nix { };
|
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 { };
|
clangbuildanalyzer = final.buildPackages.callPackage ./misc/clangbuildanalyzer.nix { };
|
||||||
|
|
||||||
default-busybox-sandbox-shell = final.busybox.override {
|
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
|
${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
|
# TODO: Once the test suite is nicer, clean up and start
|
||||||
# enforcing trailing whitespace on tests that don't explicitly
|
# enforcing trailing whitespace on tests that don't explicitly
|
||||||
# check for it.
|
# check for it.
|
||||||
|
|
7
maintainers/check-headers.nix
Normal file
7
maintainers/check-headers.nix
Normal file
|
@ -0,0 +1,7 @@
|
||||||
|
{ writeShellApplication, gnugrep }:
|
||||||
|
writeShellApplication {
|
||||||
|
name = "check-headers";
|
||||||
|
|
||||||
|
runtimeInputs = [ gnugrep ];
|
||||||
|
text = builtins.readFile ./check-headers.sh;
|
||||||
|
}
|
47
maintainers/check-headers.sh
Executable file
47
maintainers/check-headers.sh
Executable file
|
@ -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
|
Loading…
Reference in a new issue