feat(diffguard-core): add #[must_use] to fingerprint functions#1568
feat(diffguard-core): add #[must_use] to fingerprint functions#1568EffortlessSteven wants to merge 20 commits intomainfrom
Conversation
Work item: work-a140ddf6 Fixes clippy::unnecessary_wraps on cmd_doctor() which returns Result<i32> but never produces Err. The function always returns Ok(0) or Ok(1).
Add detailed docstring explaining what validate_config_rules checks: - Duplicate rule IDs - Empty pattern lists - Invalid regex patterns - Invalid multiline_window values - Unknown rule dependencies - Invalid path globs
Work item: work-3d8d9b32
- Remove unused doc comments inside proptest! blocks (clippy warning) - Fix formatting in property_test_string_syntax_invariant.rs - Add missing insta dependency to diffguard-analytics dev-dependencies These are pre-existing issues on the branch that block CI.
…duration_overflow test
Work item: work-5102a8b6 - Pin 7 GitHub Actions to verified SHA commits - Pin dtolnay/rust-toolchain to specific Rust version 1.85.0 - Scope: ci.yml, publish.yml, sarif-example.yml, diffguard.yml
…ntation Work item: work-5102a8b6
Work item: work-cd9db3ce
Fix copy-paste bug where detect_language() returns Some("xml")
instead of Some("html") for .html and .htm extensions.
- Split the combined match arm at line 223 so HTML extensions (html, htm)
return Some("html") instead of Some("xml")
- Updated unit tests in rules.rs to expect Some("html") for HTML extensions
- Updated property tests in properties.rs to expect "html" for HTML extensions
- Fixed CLAUDE.md API signature documentation
This is a copy-paste bug fix. The two-representation design maintains:
- detect_language() returns "html" for rule filtering (languages = ["html"])
- Language::Xml is used for preprocessing (HTML uses XML-style comments)
Fixes the inability to filter rules by HTML files via languages config.
Per work-5102a8b6: Pin all GitHub Actions to verified SHA commits to improve supply chain security and prevent tag manipulation attacks. Actions pinned: - actions/checkout@v4 -> 34e114876b0b11c390a56381ad16ebd13914f8d5 - dtolnay/rust-toolchain@stable -> 1.85.0 (specific version) - Swatinem/rust-cache@v2 -> 42dc69e1aa15d09112580998cf2ef0119e2e91ae - actions/upload-artifact@v4 -> ea165f8d65b6e75b540449e92b4886f43607fa02 - actions/download-artifact@v4 -> d3f86a106a0bac45b974a628896c90dbdf5c8093 - github/codeql-action/upload-sarif@v3 -> 865f5f5c36632f18690a3d569fa0a764f2da0c3e - softprops/action-gh-release@v2 -> 3bb12739c298aeb8a4eeaf626c5b8d85266b0e65 - actions/github-script@v7 -> f28e40c7f34bde8b3046d885e986cb6290c5673b Review quarterly for updated action versions.
Follow ADR-012 decision for issue #452: - Remove Language::Json from explicit match arm in string_syntax() - Keep Language::Yaml | Language::Toml explicit (not redundant) - JSON falls through to wildcard since it also uses C-style strings - Update comments to clarify JSON is handled by wildcard This aligns with comment_syntax() pattern where YAML/TOML are explicit and JSON falls through to CStyle wildcard. Refs: ADR-012, issue #452
Work item: work-e76dd272 Fix all 10 clippy::format_push_string warnings in diffguard-core across junit.rs (5), render.rs (4), and checkstyle.rs (1).
Work item: work-e76dd272 Fix all 10 clippy::format_push_string warnings in diffguard-core across junit.rs (5), render.rs (4), and checkstyle.rs (1). Includes task list for implementation.
… doc comment Fixes clippy::doc_markdown lint violation at line 532. The identifier rule_id in the doc comment was missing backticks, causing clippy to warn. Wrapped it in backticks per Rust doc conventions. See: GitHub issue #573
Adds a Rust doc comment with # Errors section to run_server() following Style B (categorized anyhow::Error without type links) per Rust API Guidelines C409. Error categories covered: - LSP protocol initialization or message handling failures - JSON parse/serialization errors - LSP message send failures Also adds run_server_doc_comment_test.rs which verifies the doc comment structure.
This test was added for work-3010cb68 but the corresponding fix was never merged. Remove to unblock CI for work-5102a8b6.
Adds #[must_use] attribute to both public fingerprint computation functions to prevent silent data loss when callers discard the return value. This enforces the module contract that fingerprints provide a 'stable identifier' for findings. Addresses GitHub issue #518.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (27)
📒 Files selected for processing (55)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| #[test] | ||
| fn language_from_str_yaml_toml_json_aliases() { | ||
| // YAML has 'yml' alias | ||
| assert_eq!("yml".parse::<Language>().unwrap(), Language::Yaml); |
| // YAML has 'yml' alias | ||
| assert_eq!("yml".parse::<Language>().unwrap(), Language::Yaml); | ||
| // JSON has 'jsonc' and 'json5' aliases (JSON with comments) | ||
| assert_eq!("jsonc".parse::<Language>().unwrap(), Language::Json); |
| assert_eq!("yml".parse::<Language>().unwrap(), Language::Yaml); | ||
| // JSON has 'jsonc' and 'json5' aliases (JSON with comments) | ||
| assert_eq!("jsonc".parse::<Language>().unwrap(), Language::Json); | ||
| assert_eq!("json5".parse::<Language>().unwrap(), Language::Json); |
| "Yaml", | ||
| "Yml", | ||
| ]) { | ||
| let lang: Language = alias.parse().unwrap(); |
| "Jsonc", | ||
| "Json5", | ||
| ]) { | ||
| let lang: Language = alias.parse().unwrap(); |
|
|
||
| #[test] | ||
| fn toml_parsed_returns_cstyle() { | ||
| let lang: Language = "toml".parse().unwrap(); |
BUILT Gate — Code Builder FindingsAdded
This is a compile-time lint hygiene change. The The Verification
Files Modified
Branch
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a80bc44f4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// Ignore comments when matching patterns. | ||
| /// When set to `true`, pattern matching skips comment lines. | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub ignore_comments: Option<bool>, |
There was a problem hiding this comment.
Wire defaults ignore flags into rule evaluation
Adding defaults.ignore_comments/defaults.ignore_strings here exposes new config knobs, but the check path never applies them to rules: cmd_check_inner only consumes defaults for scope/fail_on/max_findings/diff_context and then passes cfg.rule unchanged into run_check. In practice, a user setting these under [defaults] (and not per-rule) will still match inside comments/strings, so the new fields are silently ignored and produce unexpected findings.
Useful? React with 👍 / 👎.
Summary
Adds
#[must_use]attribute tocompute_fingerprint()andcompute_fingerprint_raw()incrates/diffguard-core/src/fingerprint.rsto prevent silent data loss when callers discard the return value.Changes
crates/diffguard-core/src/fingerprint.rs: Added#[must_use]to both public functionsMotivation
Addresses GitHub issue #518. Both functions return a SHA-256 fingerprint that serves as a stable identifier for findings across runs. If a caller discards this return value, the computation was performed but the result is silently lost — potentially leading to silent data loss in deduplication workflows.
Testing
cargo fmt -- --check: cleancargo clippy --workspace --lib --bins --tests -- -D warnings: cleancargo test -p diffguard-core --lib: 141/141 passingCloses #518