Skip to content

feat(validation): guard public Rust examples against unwrap#132

Merged
acgetchell merged 2 commits into
mainfrom
feat/125-unwrap-expect-guardrails
Jun 3, 2026
Merged

feat(validation): guard public Rust examples against unwrap#132
acgetchell merged 2 commits into
mainfrom
feat/125-unwrap-expect-guardrails

Conversation

@acgetchell
Copy link
Copy Markdown
Owner

@acgetchell acgetchell commented Jun 3, 2026

  • Add repository-owned Semgrep rules for unwrap and expect usage in public doctests, examples, and benchmarks.
  • Add fixture-based Semgrep rule tests and include them in the lint workflow.
  • Update examples and benchmarks to model typed fallible flow or operation-labeled benchmark failures.

Closes #125

Summary by CodeRabbit

  • Refactor

    • Examples now use fallible error propagation instead of panicking for safer, clearer failures.
    • Benchmarks standardized error handling for more consistent failure messages during runs.
  • Tests

    • Added static analysis rules to forbid panicking patterns in docs/examples and benchmarks.
    • Added fixture-based tests to validate those rules.
  • Chores

    • Introduced a new validation script and expanded lint tasks to run fixture checks.

- Add repository-owned Semgrep rules for unwrap and expect usage in public doctests, examples, and benchmarks.
- Add fixture-based Semgrep rule tests and include them in the lint workflow.
- Update examples and benchmarks to model typed fallible flow or operation-labeled benchmark failures.

Closes #125
@acgetchell acgetchell self-assigned this Jun 3, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 3, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3f170e73-aa90-495e-a2a8-863069c4a0f8

📥 Commits

Reviewing files that changed from the base of the PR and between df1130a and ac44c07.

📒 Files selected for processing (1)
  • scripts/check_semgrep_fixtures.py

📝 Walkthrough

Walkthrough

This PR adds Semgrep guardrails to prevent .unwrap() and .expect() in Rust examples, benchmarks, and doctests; refactors examples and benchmarks to propagate errors or use standardized helpers; adds two Semgrep rules; provides fixture tests; and integrates a Python validator and justfile recipe into the lint workflow.

Changes

Semgrep Guardrails and Error Handling Compliance

Layer / File(s) Summary
Benchmark error handling helpers and refactoring
benches/exact.rs, benches/vs_linalg.rs
Adds require_ok/require_some helpers and replaces .expect()/.unwrap() uses in benchmarks; some benchmark iteration bodies are adjusted to directly bench LU results.
Example error handling migration to Result propagation
examples/const_det_4x4.rs, examples/exact_sign_3x3.rs, examples/ldlt_solve_3x3.rs
Converts example main functions to return Result<(), LaError>, replaces unchecked getters with get_checked(...)?, and adds explicit Ok(()) returns.
Semgrep rules for unwrap/expect guardrails
semgrep.yaml
Adds la-stack.rust.no-unwrap-expect-in-doctests and la-stack.rust.no-unwrap-expect-in-benches-examples to flag .unwrap()/.expect() in doctests, benchmarks, and examples.
Semgrep fixture validation infrastructure
scripts/check_semgrep_fixtures.py, tests/semgrep/doctests/unwrap_expect.txt, tests/semgrep/src/project_rules/bench_example_usage.rs
Adds a validator script that compares Semgrep JSON (via SEMGREP_JSON) against ruleid: annotations and new fixture files demonstrating expected matches and allowed patterns.
Build system wiring for Semgrep validation
justfile, pyproject.toml
Adds semgrep-test recipe, integrates it into lint-code, includes the validator in python-typecheck mypy checks, and registers check_semgrep_fixtures in pyproject.toml for packaging and isort recognition.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

rust, testing, documentation, enhancement

Poem

🐰 Hopped through benches, examples, and rhyme,
Replaced blunt panics one at a time.
Semgrep watches with careful eyes,
? and Result now win the prize.
Happy hops — clean docs, no surprise!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat(validation): guard public Rust examples against unwrap' accurately summarizes the main change—adding Semgrep rules and updates to guard public Rust examples against unwrap/expect usage.
Linked Issues check ✅ Passed The PR fully addresses issue #125 by adding Semgrep rules for doctests and benches/examples, updating existing code to use typed Result flows and the ? operator, and adding fixture-based tests in CI.
Out of Scope Changes check ✅ Passed All changes are in scope: Semgrep rule additions, fixture validation script, CI workflow updates via justfile, and refactoring examples/benches for error handling align with issue #125 objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/125-unwrap-expect-guardrails

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/check_semgrep_fixtures.py`:
- Line 60: The code assumes data["results"] and result["check_id"] exist and
will raise KeyError; update the aggregation in scripts/check_semgrep_fixtures.py
to guard the shape returned by _semgrep_results(): first verify
data.get("results") is a list (otherwise log to stderr and exit(1)), then build
actual using a safe lookup like collections.Counter(result.get("check_id",
"<missing>") for result in data.get("results", [])) or skip entries without
check_id and log which entries were malformed; ensure the script follows the
existing stderr-plus-exit-1 contract when encountering a non-list results or
missing check_id fields.
- Line 13: The RULE_ANNOTATION regex currently captures both "ruleid" and
"todoruleid" causing todoruleid to be counted in expected; update
RULE_ANNOTATION so it only matches the canonical "ruleid" annotation (or filter
the parsed annotations in expected to exclude any "todoruleid" entries) and
ensure the variable expected only contains real rule ids. Also harden
SEMGREP_JSON parsing: validate that data is a dict with a "results" list before
building actual, iterate results defensively and check each result contains
"check_id" (or skip/collect/report malformed entries), and raise or log a clear
error if the JSON shape is unexpected instead of letting a KeyError propagate;
reference variables/functions: RULE_ANNOTATION, expected, actual, data, results,
result, check_id.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e074295f-b077-48c2-97f8-4db23d148437

📥 Commits

Reviewing files that changed from the base of the PR and between 23bc7a4 and df1130a.

📒 Files selected for processing (11)
  • benches/exact.rs
  • benches/vs_linalg.rs
  • examples/const_det_4x4.rs
  • examples/exact_sign_3x3.rs
  • examples/ldlt_solve_3x3.rs
  • justfile
  • pyproject.toml
  • scripts/check_semgrep_fixtures.py
  • semgrep.yaml
  • tests/semgrep/doctests/unwrap_expect.txt
  • tests/semgrep/src/project_rules/bench_example_usage.rs

Comment thread scripts/check_semgrep_fixtures.py Outdated
Comment thread scripts/check_semgrep_fixtures.py Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.64%. Comparing base (23bc7a4) to head (ac44c07).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #132   +/-   ##
=======================================
  Coverage   99.64%   99.64%           
=======================================
  Files           5        5           
  Lines        2832     2832           
=======================================
  Hits         2822     2822           
  Misses         10       10           
Flag Coverage Δ
unittests 99.64% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- Ignore non-canonical todoruleid annotations when counting expected rule hits.
- Reject malformed Semgrep JSON results with clear stderr diagnostics instead of propagating KeyError.
@acgetchell acgetchell enabled auto-merge June 3, 2026 19:03
@acgetchell acgetchell merged commit eab56a6 into main Jun 3, 2026
18 checks passed
@acgetchell acgetchell deleted the feat/125-unwrap-expect-guardrails branch June 3, 2026 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Semgrep guardrail against unwrap/expect in examples, benches, and doctests

1 participant