Skip to content

Commit de01c8e

Browse files
fix: add robust error checking to pre-commit hooks (#6888)
- Add scripts/pre-commit-check.sh with pre-flight validation: - PHP version check (warns if not 8.1-8.4) - Merge conflict marker detection in staged PHP files - composer.lock staging prevention - Vendor dev-dep staging prevention - Autoload freshness check (composer.json vs vendor/autoload.php) - Tool availability + loadability check (catches stale autoload) - php -l fallback when phplint is broken or missing - Auto-fix suggestion when php-cs-fixer fails - Fix .pre-commit-config.yaml: - Change file filter from ^app/ to types: [php] - Route all hooks through wrapper script Gives clear actionable error messages instead of PHP fatal errors when dev dependencies are missing or out of sync. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net>
1 parent 0c281c9 commit de01c8e

File tree

2 files changed

+192
-3
lines changed

2 files changed

+192
-3
lines changed

.pre-commit-config.yaml

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,21 @@ repos:
33
hooks:
44
- id: lintcheck
55
name: PHP Lint Checking
6-
entry: composer run-script lint
6+
entry: sh scripts/pre-commit-check.sh lint
77
language: system
8+
types: [php]
89
always_run: true
910

1011
- id: phpcsfixer
1112
name: PHP CS Fixer Validation
12-
entry: composer run-script phpcsfixer
13+
entry: sh scripts/pre-commit-check.sh phpcsfixer
1314
language: system
15+
types: [php]
1416
always_run: true
1517

1618
- id: phpstan
1719
name: PHPStan Validation
18-
entry: composer run-script phpstan
20+
entry: sh scripts/pre-commit-check.sh phpstan
1921
language: system
22+
types: [php]
2023
always_run: true

scripts/pre-commit-check.sh

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
#!/bin/sh
2+
# +-------------------------------------------------------------------------+
3+
# | Pre-commit validation wrapper for Cacti |
4+
# | Checks environment and dev tools before running linters. |
5+
# +-------------------------------------------------------------------------+
6+
set -eu
7+
8+
VENDOR_BIN="include/vendor/bin"
9+
REQUIRED_PHP_MAJOR=8
10+
MAX_PHP_MINOR=4
11+
12+
# ---- Environment checks (run before any tool) ----
13+
14+
check_php_version() {
15+
php_ver=$(php -r 'echo PHP_MAJOR_VERSION . "." . PHP_MINOR_VERSION;' 2>/dev/null || echo "unknown")
16+
major=$(echo "$php_ver" | cut -d. -f1)
17+
minor=$(echo "$php_ver" | cut -d. -f2)
18+
19+
if [ "$major" != "$REQUIRED_PHP_MAJOR" ]; then
20+
echo "WARNING: PHP $php_ver detected. Cacti requires PHP 8.x"
21+
elif [ "$minor" -gt "$MAX_PHP_MINOR" ] 2>/dev/null; then
22+
echo "WARNING: PHP $php_ver detected. Cacti targets PHP 8.1-8.4."
23+
echo " Some type features (true|string) are not available on PHP 8.1."
24+
echo " Consider: export PATH=\"/opt/homebrew/opt/php@8.4/bin:\${PATH}\""
25+
fi
26+
}
27+
28+
check_merge_conflicts() {
29+
staged=$(git diff --cached --name-only --diff-filter=ACM -- '*.php')
30+
if [ -z "$staged" ]; then
31+
return 0
32+
fi
33+
conflicts=$(echo "$staged" | xargs grep -l '^<<<<<<<\|^=======$\|^>>>>>>>' 2>/dev/null || true)
34+
if [ -n "$conflicts" ]; then
35+
echo ""
36+
echo "ERROR: Merge conflict markers found in staged files:"
37+
echo "$conflicts"
38+
echo " Resolve conflicts before committing."
39+
echo ""
40+
exit 1
41+
fi
42+
}
43+
44+
check_composer_lock() {
45+
if git diff --cached --name-only | grep -q '^composer\.lock$'; then
46+
echo ""
47+
echo "ERROR: composer.lock is staged for commit."
48+
echo " Cacti supports multiple PHP versions; composer.lock must not be committed."
49+
echo " Run: git reset HEAD composer.lock"
50+
echo ""
51+
exit 1
52+
fi
53+
}
54+
55+
check_vendor_dev_deps() {
56+
staged_vendor=$(git diff --cached --name-only | grep '^include/vendor/' | head -5)
57+
if [ -n "$staged_vendor" ]; then
58+
echo ""
59+
echo "WARNING: Vendor files are staged for commit:"
60+
echo "$staged_vendor"
61+
echo " Dev dependencies should not be committed to include/vendor/."
62+
echo " Run: git reset HEAD include/vendor/"
63+
echo ""
64+
exit 1
65+
fi
66+
}
67+
68+
check_autoload_freshness() {
69+
if [ -f composer.json ] && [ -f include/vendor/autoload.php ]; then
70+
if [ composer.json -nt include/vendor/autoload.php ]; then
71+
echo "WARNING: composer.json is newer than vendor/autoload.php."
72+
echo " Run: composer install --ignore-platform-reqs"
73+
fi
74+
fi
75+
}
76+
77+
check_tool() {
78+
tool="$1"
79+
label="$2"
80+
if [ ! -x "$VENDOR_BIN/$tool" ]; then
81+
echo ""
82+
echo "ERROR: $label not found at $VENDOR_BIN/$tool"
83+
echo ""
84+
echo " Run: composer install --ignore-platform-reqs"
85+
echo ""
86+
echo " Dev dependencies must be installed for pre-commit hooks."
87+
echo " This is a one-time setup after cloning or switching branches."
88+
echo ""
89+
exit 1
90+
fi
91+
# Verify the tool can actually load (catches missing Symfony/autoload issues)
92+
if ! "$VENDOR_BIN/$tool" --version > /dev/null 2>&1; then
93+
echo ""
94+
echo "ERROR: $label exists but failed to load. Autoload may be stale."
95+
echo ""
96+
echo " Run: composer install --ignore-platform-reqs"
97+
echo ""
98+
exit 1
99+
fi
100+
}
101+
102+
# ---- Lint / analysis tools ----
103+
104+
run_lint() {
105+
if [ -x "$VENDOR_BIN/phplint" ] && "$VENDOR_BIN/phplint" --version > /dev/null 2>&1; then
106+
echo "Running PHP lint (phplint)..."
107+
composer run-script lint
108+
else
109+
echo "phplint not available or broken, falling back to php -l on staged files..."
110+
staged=$(git diff --cached --name-only --diff-filter=ACM -- '*.php')
111+
if [ -z "$staged" ]; then
112+
echo " No staged PHP files to check."
113+
return 0
114+
fi
115+
fail=0
116+
for f in $staged; do
117+
if ! php -l "$f" > /dev/null 2>&1; then
118+
php -l "$f"
119+
fail=1
120+
fi
121+
done
122+
if [ "$fail" -eq 1 ]; then
123+
exit 1
124+
fi
125+
echo " All staged PHP files pass syntax check."
126+
fi
127+
}
128+
129+
run_phpcsfixer() {
130+
check_tool "php-cs-fixer" "PHP CS Fixer"
131+
echo "Running PHP CS Fixer (dry-run)..."
132+
if ! composer run-script phpcsfixer; then
133+
echo ""
134+
echo "TIP: To auto-fix formatting issues, run:"
135+
echo " composer run-script phpcsfixit"
136+
echo ""
137+
exit 1
138+
fi
139+
}
140+
141+
run_phpstan() {
142+
check_tool "phpstan" "PHPStan"
143+
echo "Running PHPStan..."
144+
composer run-script phpstan
145+
}
146+
147+
# ---- Pre-flight checks (always run) ----
148+
149+
run_preflight() {
150+
check_php_version
151+
check_merge_conflicts
152+
check_composer_lock
153+
check_vendor_dev_deps
154+
check_autoload_freshness
155+
}
156+
157+
# ---- Main ----
158+
159+
case "${1:-all}" in
160+
lint)
161+
run_preflight
162+
run_lint
163+
;;
164+
phpcsfixer)
165+
run_preflight
166+
run_phpcsfixer
167+
;;
168+
phpstan)
169+
run_preflight
170+
run_phpstan
171+
;;
172+
all)
173+
run_preflight
174+
run_lint
175+
run_phpcsfixer
176+
run_phpstan
177+
;;
178+
preflight)
179+
run_preflight
180+
echo "Pre-flight checks passed."
181+
;;
182+
*)
183+
echo "Usage: $0 {lint|phpcsfixer|phpstan|all|preflight}"
184+
exit 1
185+
;;
186+
esac

0 commit comments

Comments
 (0)