Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 103 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,109 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [1.3.0] - 2026-05-29

Minor release (one breaking removal — see below): **test-entry-point
recognition routed through one authoritative cfg-test file set** —
framework test attributes and per-crate integration-test directories are
now recognised everywhere, fixing a `DEAD_CODE` false-positive on
`tests/**` integration-test entry points. Package roots are identified by their crate-root file
(`src/lib.rs` / `src/main.rs` / autobinary `src/bin/<name>.rs`) derived
from the parsed `.rs` set — a `tests/` directory only counts when its
owning directory is such a root — and the DRY collectors consume the
shared set instead of their own path strings.
Failing-first regression tests in `src/adapters/shared/tests/cfg_test.rs`,
`src/adapters/shared/tests/cfg_test_files.rs`,
`src/adapters/analyzers/dry/tests/dead_code.rs`,
`src/adapters/analyzers/dry/tests/functions.rs`, and
`src/adapters/analyzers/dry/tests/wildcards.rs`.

### Fixed (test detection)

- **`DEAD_CODE` no longer false-flags integration-test entry points
under a crate's `tests/` directory.** Two narrow heuristics stacked
up: (1) `has_test_attr` only recognised the bare `#[test]`, so
`#[tokio::test]` / `#[rstest]` / `#[test_case]` functions were not
seen as test roots; (2) the cfg-test file detector classified only
workspace-root `tests/**`, missing per-crate
`crates/<name>/tests/**` integration binaries. A `#[tokio::test]`
entry point in `crates/sv-utility-retry/tests/integration.rs` was
reported as dead code while the same function in
`src/**/*_tests.rs` was not. Both narrownesses are removed.

### Changed (unified test recognition)

- **`has_test_attr` (`adapters/shared/cfg_test.rs`) now recognises
framework test attributes.** Matches any attribute whose path ends in
`test` (`#[test]`, `#[tokio::test]`, `#[async_std::test]`,
`#[googletest::test]`, …) plus the renamed macros `#[rstest]` and
`#[test_case]`. Shared by all seven dimensions, so the broadened
recognition applies uniformly (IOSP/complexity/DRY/SRP/coupling/TQ/
architecture leniency for test functions).
- **One authoritative cfg-test file set; the directory rule feeds it and
nothing else, and is derived from real package roots.**
`cfg_test_files` first computes the package roots present in the parsed
tree by their **crate-root file** — `src/lib.rs`, `src/main.rs`, or an
autobinary `src/bin/<name>.rs` (`""` for the analysis-root crate). This
is Cargo's defining marker of a package, taken from the `.rs` set with
no manifest/filesystem access. `cfg_test_files::is_integration_test_path`
accepts a `tests/` file when the owner of **any** `tests/` component in
its path is one of those roots (so a package nested under a directory
literally named `tests`, `fixtures/tests/retry/tests/it.rs`, is matched at
its real root). This matches `tests/**` and per-crate
`crates/<name>/tests/**`, but NOT a `tests/` directory whose owner has
no crate root (`fixtures/tests/**`, `tools/shared/tests/**`, a
coincidental `src/`+`tests/` pair) nor one nested under a package's
`src/` (`src/foo/tests/bar.rs`, a unit-test submodule reached via
`#[cfg(test)] mod`). All four DRY file collectors — `FunctionCollector`
(duplicate hashing, DRY-001), `FragmentCollector` (repeated fragments,
DRY-004), `MatchPatternCollector` (repeated matches, DRY-005), and
`WildcardCollector` (wildcard imports) — no longer apply their own
`/tests/` path strings; they consult the shared `cfg_test_files` set
(integration dirs + `#![cfg(test)]` + `#[cfg(test)] mod` chains), so a
production directory merely *named* `tests` is never blanket-classified
as test-only, and `ignore_tests` now skips package integration-test and
`#![cfg(test)]` companion files for every DRY check. Addresses review
findings: neither `contains("/tests/")` nor a "non-`src` `/tests/`"
guard actually identified package roots.
*Known limitation:* a crate using a non-default `[lib] path = …` /
`[[bin]] path = …` (no file at a conventional `src/` crate-root) is not
detected as a package root from paths alone — that would require parsing
`Cargo.toml`.
- **Test Quality coverage checks (TQ-004/TQ-005) key off the
analyzer-computed `is_test` flag, not a `test_` name prefix.**
`tq::coverage` previously skipped any function *named* `test_*` via
an isolated heuristic that both missed framework-attributed tests and
wrongly exempted production functions named like tests (e.g.
`test_connection`). It now uses `FunctionAnalysis::is_test`,
consistent with every other dimension.
- **All structural detectors (BTC, SLM, NMS, IET, OI, SIT, DEH) skip
whole test files.** They previously skipped only inline `#[cfg(test)]
mod` blocks (BTC/IET/DEH and the SLM/NMS inherent-method walk) or
nothing at all — so a stub mock impl, selfless/needless-`&mut`-self
helper, inconsistent error types, orphaned impl, single-impl trait, or
`downcast` used in a package integration-test file or `#![cfg(test)]`
companion was falsely flagged (SRP/Coupling). The visitor-based
detectors now skip files in `cfg_test_files`; OI/SIT are fixed at the
source — `collect_metadata` no longer records types/traits/impls from
test files. Brings the Structural dimension in line with the rest of
the tool's test-aware behaviour.

### Removed (breaking)

- **`--diff [REF]` changed-files mode removed.** It parsed only the
git-changed files, so every cross-file analysis ran on a partial set
and produced false positives/negatives: dead-code reachability (a fn
used only from an unchanged file looked uncalled), cross-file DRY
duplicates (one side unchanged → missed), coupling instability (partial
module graph), architecture call-parity (missing adapters/targets), and
the cfg-test/package-root detection above. The flag's promise of "the
same analysis, only reporting changed files" was not what it did — it
analysed only changed files. Rather than ship a mode that is unsound for
a whole-program analyzer, it is removed along with `get_git_changed_files`
/ `filter_to_changed`. Run rustqual over the full tree and use
`--format github` / `sarif` for PR-scoped annotations.

## [1.2.6] - 2026-05-27

Patch release: **dyn-compatibility precision in `must_be_object_safe`** —
Expand Down
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "rustqual"
version = "1.2.6"
version = "1.3.0"
edition = "2021"
description = "Comprehensive Rust code quality analyzer — seven dimensions: IOSP, Complexity, DRY, SRP, Coupling, Test Quality, Architecture"
license = "MIT"
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ With coverage and PR annotations:
```yaml
- run: cargo install rustqual cargo-llvm-cov
- run: cargo llvm-cov --lcov --output-path lcov.info
- run: rustqual --diff origin/main --coverage lcov.info --format github
- run: rustqual --coverage lcov.info --format github
```

For codebases that aren't yet at 100% but want to prevent regression:
Expand Down
4 changes: 2 additions & 2 deletions book/ai-coding-workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ jobs:
- uses: dtolnay/rust-toolchain@stable
- run: cargo install rustqual cargo-llvm-cov
- run: cargo llvm-cov --lcov --output-path lcov.info
- run: rustqual --diff HEAD~1 --coverage lcov.info --format github
- run: rustqual --coverage lcov.info --format github
```

`--format github` produces inline annotations on the PR diff — exactly where the issue is, what rule fired, why it matters. `--diff HEAD~1` restricts analysis to the changed files so PRs stay fast even on large codebases.
`--format github` produces inline annotations on the PR diff — exactly where the issue is, what rule fired, why it matters.

## Pattern 4: baseline tracking for AI-velocity codebases

Expand Down
13 changes: 1 addition & 12 deletions book/ci-integration.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,6 @@ That's all. If any dimension flags a finding, the job fails.

`--format github` emits `::error::` and `::warning::` annotations that GitHub renders inline on the PR diff. `--min-quality-score 90` enforces a hard floor on the overall quality score.

## GitHub Actions — changed files only

For large codebases, restrict analysis to files changed in the PR:

```yaml
- run: rustqual --diff origin/main --format github
```

`--diff <REF>` runs the same analysis but only reports findings in files that differ from `<REF>`. Cuts CI time for large codebases without losing PR-level signal.

## GitHub Actions — with coverage

The Test Quality dimension can use LCOV coverage data to detect untested logic:
Expand Down Expand Up @@ -123,15 +113,14 @@ The flags compose. A typical "production-grade" CI step:
```yaml
- run: |
rustqual \
--diff origin/main \
--coverage lcov.info \
--min-quality-score 90 \
--fail-on-warnings \
--format github
```

This:
- analyses only files changed vs `main`,
- analyses the full source tree,
- includes coverage-based test-quality checks,
- requires the overall quality score to be at least 90%,
- treats suppression-ratio overruns as hard errors,
Expand Down
2 changes: 1 addition & 1 deletion book/code-reuse.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ pub fn assert_in_range(actual: f64, expected: f64, tol: f64) { /* … */ }

`qual:api` and `qual:test_helper` exclude the function from `DRY-002` *and* from `TQ-003` (untested), without counting against `max_suppression_ratio`. Use them on functions that are exported to consumers outside the crate or used only by integration tests.

By default, the dead-code analysis treats workspace-root `tests/**` files as call-sites — so a function used only from integration tests is not dead.
By default, the dead-code analysis treats a package's `tests/**` files as call-sites — both the analysis-root crate's `tests/**` and each member's `crates/*/tests/**` — so a function used only from integration tests is not dead.

## Code fragments and repeated matches

Expand Down
2 changes: 1 addition & 1 deletion book/function-quality.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ allow_expect = false # set true to permit .expect() but still flag .unwrap()

### Test-aware

`A20` and `CX-004` skip `#[test]` functions and files under workspace-root `tests/**`. Asserting on `.unwrap()` in a test is fine; it's panicking in production that matters.
`A20` and `CX-004` skip test functions (`#[test]`, `#[tokio::test]`, `#[rstest]`, `#[test_case]`, any `…::test`) and files in a package's `tests/**` directory — the analysis-root crate's `tests/**` and each member's `crates/*/tests/**` alike. Asserting on `.unwrap()` in a test is fine; it's panicking in production that matters.

## Method-shape checks

Expand Down
1 change: 0 additions & 1 deletion book/getting-started.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ This writes a `rustqual.toml` next to `Cargo.toml`, with thresholds calibrated t
| `--no-fail` | Local exploration; don't exit non-zero |
| `--verbose` | Show every function, not just findings |
| `--findings` | One finding per line: `file:line category in fn_name` |
| `--diff [REF]` | Only analyse files changed vs a git ref |
| `--coverage <LCOV>` | Include coverage-based test-quality checks |
| `--init` | Generate a config tailored to your codebase |
| `--watch` | Re-analyse on file changes |
Expand Down
4 changes: 0 additions & 4 deletions book/reference-cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ rustqual [OPTIONS] [PATH]
| Flag | Description |
|---|---|
| `-c`, `--config <FILE>` | Path to config. Default: `rustqual.toml` in the target directory. |
| `--diff [REF]` | Only analyse files changed vs a git ref (default: `HEAD`). Conflicts with `--watch`. |
| `--coverage <LCOV>` | Path to LCOV coverage file. Enables TQ-004 / TQ-005. |
| `--explain <FILE>` | Diagnostic mode: explain architecture-rule classification for one file. |
| `--watch` | Watch for file changes and re-analyse continuously. |
Expand Down Expand Up @@ -91,9 +90,6 @@ rustqual --no-fail --verbose
# CI hard gate with coverage and PR annotations
rustqual --coverage lcov.info --min-quality-score 90 --fail-on-warnings --format github

# PR-only analysis
rustqual --diff origin/main --format github

# Baseline-based regression gate
rustqual --compare baseline.json --fail-on-regression --format github

Expand Down
6 changes: 0 additions & 6 deletions book/reference-output-formats.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,6 @@ summary annotation (`::error::Quality analysis: N finding(s)…` or
of the run. For per-rule filtering in CI, use `--format sarif` and
upload to Code Scanning instead.

Combine with `--diff origin/main` for PR-only analysis:

```yaml
- run: rustqual --diff origin/main --format github
```

## `sarif`

SARIF v2.1.0. Designed for GitHub Code Scanning, but consumed by Azure DevOps, Sonatype, and any SARIF tool.
Expand Down
2 changes: 1 addition & 1 deletion book/reference-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ For dimension intent and refactor patterns, see the use-case guides linked at th
| `CX-006` | Unsafe block detected | `detect_unsafe = true` |
| `A20` | Error-handling issue (`unwrap`/`expect`/`panic!`/`todo!`) | `detect_error_handling = true` |

`A20` and `CX-004` skip `#[test]` functions and workspace-root `tests/**` files.
`A20` and `CX-004` skip test functions (`#[test]`, `#[tokio::test]`, `#[rstest]`, `#[test_case]`, any `…::test`) and files in a package's `tests/**` directory (the analysis-root crate and each `crates/*/tests/**`).

## DRY

Expand Down
10 changes: 8 additions & 2 deletions src/adapters/analyzers/dry/fragments.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};

use syn::spanned::Spanned;
use syn::visit::Visit;
Expand Down Expand Up @@ -75,8 +75,11 @@ fn collect_all_windows(
parsed: &[(String, String, syn::File)],
config: &DuplicatesConfig,
) -> (Vec<FnInfo>, Vec<WindowEntry>) {
let cfg_test_files =
crate::adapters::shared::cfg_test_files::collect_cfg_test_file_paths(parsed);
let mut collector = FragmentCollector {
config,
cfg_test_files: &cfg_test_files,
file: String::new(),
fn_infos: Vec::new(),
windows: Vec::new(),
Expand Down Expand Up @@ -221,6 +224,7 @@ pub(crate) fn merge_into_fragments(
/// AST visitor that collects statement windows from all function bodies.
struct FragmentCollector<'a> {
config: &'a DuplicatesConfig,
cfg_test_files: &'a HashSet<String>,
file: String,
fn_infos: Vec<FnInfo>,
windows: Vec<WindowEntry>,
Expand All @@ -232,7 +236,9 @@ struct FragmentCollector<'a> {
impl super::FileVisitor for FragmentCollector<'_> {
fn reset_for_file(&mut self, file_path: &str) {
self.file = file_path.to_string();
self.in_test = false;
// Whole-file test classification from the authoritative cfg-test
// set; inline `#[cfg(test)] mod` is still handled per-item below.
self.in_test = self.cfg_test_files.contains(file_path);
self.parent_type = None;
self.is_trait_impl = false;
}
Expand Down
15 changes: 9 additions & 6 deletions src/adapters/analyzers/dry/functions.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};

use syn::spanned::Spanned;
use syn::visit::Visit;
Expand All @@ -11,6 +11,7 @@ use crate::config::sections::DuplicatesConfig;
/// AST visitor that collects function bodies and computes their normalized hashes.
pub(crate) struct FunctionCollector<'a> {
pub(crate) config: &'a DuplicatesConfig,
pub(crate) cfg_test_files: &'a HashSet<String>,
pub(crate) file: String,
pub(crate) entries: Vec<FunctionHashEntry>,
in_test: bool,
Expand All @@ -19,9 +20,10 @@ pub(crate) struct FunctionCollector<'a> {
}

impl<'a> FunctionCollector<'a> {
pub(crate) fn new(config: &'a DuplicatesConfig) -> Self {
pub(crate) fn new(config: &'a DuplicatesConfig, cfg_test_files: &'a HashSet<String>) -> Self {
Self {
config,
cfg_test_files,
file: String::new(),
entries: Vec::new(),
in_test: false,
Expand All @@ -34,10 +36,11 @@ impl<'a> FunctionCollector<'a> {
impl FileVisitor for FunctionCollector<'_> {
fn reset_for_file(&mut self, file_path: &str) {
self.file = file_path.to_string();
// Files under a `tests/` subdir are test companions loaded via a
// parent's `#[cfg(test)] mod tests;` — treat them as test code even
// though their AST has no visible #[cfg(test)] attribute.
self.in_test = file_path.contains("/tests/");
// Whole-file test classification comes from the authoritative
// cfg-test file set (integration-test dirs + `#![cfg(test)]` +
// `#[cfg(test)] mod` chains), not a path heuristic. Inline
// `#[cfg(test)] mod` blocks are still handled per-item below.
self.in_test = self.cfg_test_files.contains(file_path);
self.parent_type = None;
self.is_trait_impl = false;
}
Expand Down
11 changes: 9 additions & 2 deletions src/adapters/analyzers/dry/match_patterns.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};

use syn::visit::Visit;

Expand Down Expand Up @@ -39,6 +39,7 @@ pub(crate) struct CollectedMatch {
/// AST visitor that collects match expressions for repeated pattern detection.
struct MatchPatternCollector<'a> {
config: &'a crate::config::sections::DuplicatesConfig,
cfg_test_files: &'a HashSet<String>,
file: String,
collected: Vec<CollectedMatch>,
in_test: bool,
Expand All @@ -57,10 +58,13 @@ struct MatchPatternCollector<'a> {

impl FileVisitor for MatchPatternCollector<'_> {
fn reset_for_file(&mut self, file_path: &str) {
// Whole-file test classification from the authoritative cfg-test
// set (checked against the raw path, matching its keys). Inline
// `#[cfg(test)] mod` is still handled per-item below.
self.in_test = self.cfg_test_files.contains(file_path);
// Normalise separators for deterministic findings across OSes,
// matching the other DRY collectors (e.g. wildcards.rs).
self.file = file_path.replace('\\', "/");
self.in_test = false;
self.current_fn = String::new();
self.current_impl_type = String::new();
self.module_stack.clear();
Expand Down Expand Up @@ -177,8 +181,11 @@ pub fn detect_repeated_matches(
parsed: &[(String, String, syn::File)],
config: &crate::config::sections::DuplicatesConfig,
) -> Vec<RepeatedMatchGroup> {
let cfg_test_files =
crate::adapters::shared::cfg_test_files::collect_cfg_test_file_paths(parsed);
let mut collector = MatchPatternCollector {
config,
cfg_test_files: &cfg_test_files,
file: String::new(),
collected: Vec::new(),
in_test: false,
Expand Down
4 changes: 3 additions & 1 deletion src/adapters/analyzers/dry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ pub(crate) fn collect_function_hashes(
parsed: &[(String, String, syn::File)],
config: &crate::config::sections::DuplicatesConfig,
) -> Vec<FunctionHashEntry> {
let mut collector = functions::FunctionCollector::new(config);
let cfg_test_files =
crate::adapters::shared::cfg_test_files::collect_cfg_test_file_paths(parsed);
let mut collector = functions::FunctionCollector::new(config, &cfg_test_files);
visit_all_files(parsed, &mut collector);
collector.entries
}
Expand Down
Loading
Loading