mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
ci: enforce no .unwrap(), .expect(), or assert!() in production code (#1087)
Add a diff-based CI job and pre-commit hook check that block panic-inducing calls (.unwrap(), .expect(), assert!, assert_eq!, assert_ne!) from entering production Rust code. debug_assert is excluded (compiled out in release). False positives can be suppressed with an inline `// safety: <reason>` comment. - pre-commit-safety.sh: add check 6 (PANIC) for staged diffs - code_style.yml: add `no-panics` job, wire into roll-up gate - check-boundaries.sh: extend check 2 to also catch assert!() Co-authored-by: Claude Opus 4.6 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
e805ec61aa
commit
7776d267f8
@@ -70,19 +70,21 @@ echo
|
||||
# This is a WARNING, not a hard violation.
|
||||
# --------------------------------------------------------------------------
|
||||
|
||||
echo "--- Check 2: .unwrap() / .expect() in production code ---"
|
||||
echo "--- Check 2: .unwrap() / .expect() / assert!() in production code ---"
|
||||
|
||||
# Collect raw matches excluding obvious test-only files and lines
|
||||
raw_results=$(grep -rn '\.unwrap()\|\.expect(' src/ \
|
||||
# Collect raw matches excluding obvious test-only files and lines.
|
||||
# Also catches assert!(), assert_eq!(), assert_ne!() but NOT debug_assert variants.
|
||||
raw_results=$(grep -rnE '\.(unwrap|expect)\(|[^_]assert(_eq|_ne)?!' src/ \
|
||||
--include='*.rs' \
|
||||
| grep -v 'src/main.rs' \
|
||||
| grep -v 'src/testing.rs' \
|
||||
| grep -v 'src/setup/' \
|
||||
| grep -Ev 'debug_assert|// safety:' \
|
||||
|| true)
|
||||
|
||||
if [ -n "$raw_results" ]; then
|
||||
total=$(echo "$raw_results" | wc -l | tr -d ' ')
|
||||
echo "WARNING: ~$total .unwrap()/.expect() calls found in src/ (excluding main/testing/setup)."
|
||||
echo "WARNING: ~$total .unwrap()/.expect()/assert!() calls found in src/ (excluding main/testing/setup)."
|
||||
echo "Many are in test modules; a per-file breakdown helps triage:"
|
||||
echo
|
||||
# Show per-file counts, sorted by count descending, top 15
|
||||
|
||||
@@ -10,6 +10,7 @@
|
||||
# 3. Hardcoded /tmp paths in tests (flaky in parallel runs)
|
||||
# 4. Tool parameters logged without redaction (secret leaks)
|
||||
# 5. Multi-step DB operations without transaction wrapping
|
||||
# 6. .unwrap(), .expect(), assert!() in production code (panics)
|
||||
#
|
||||
# Suppress individual lines with an inline "// safety: <reason>" comment.
|
||||
|
||||
@@ -128,6 +129,24 @@ if [ -n "$DIFF_W_OUTPUT" ]; then
|
||||
fi
|
||||
fi
|
||||
|
||||
# 6. .unwrap(), .expect(), assert!() in production code
|
||||
# Matches added lines containing panic-inducing calls.
|
||||
# Excludes test files, test modules, and debug_assert (compiled out in release).
|
||||
# Suppress with "// safety: <reason>".
|
||||
PROD_DIFF="$DIFF_OUTPUT"
|
||||
# Strip hunks from test-only files (tests/ directory, *_test.rs, test_*.rs)
|
||||
PROD_DIFF=$(echo "$PROD_DIFF" | grep -v '^+++ b/tests/' || true)
|
||||
if echo "$PROD_DIFF" | grep -nE '^\+' \
|
||||
| grep -E '\.(unwrap|expect)\(|[^_]assert(_eq|_ne)?!' \
|
||||
| grep -vE 'debug_assert|// safety:|#\[cfg\(test\)\]|#\[test\]|mod tests' \
|
||||
| head -5 | grep -q .; then
|
||||
warn "PANIC" "Production code must not use .unwrap(), .expect(), or assert!(). Use proper error handling."
|
||||
echo "$PROD_DIFF" | grep -nE '^\+' \
|
||||
| grep -E '\.(unwrap|expect)\(|[^_]assert(_eq|_ne)?!' \
|
||||
| grep -vE 'debug_assert|// safety:|#\[cfg\(test\)\]|#\[test\]|mod tests' \
|
||||
| head -5 | sed 's/^/ /'
|
||||
fi
|
||||
|
||||
if [ "$WARNINGS" -gt 0 ]; then
|
||||
echo ""
|
||||
echo "Found $WARNINGS potential issue(s). Fix them or add '// safety: <reason>' to suppress."
|
||||
|
||||
Reference in New Issue
Block a user