Feat/test quality execution

CHANGELOG.md

@@ -5,6 +5,52 @@ 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.4.2] - 2026-06-04
+
+Patch release: a false-positive fix found while applying 1.4.1. SIT stops
+flagging traits that have `#[cfg(test)]` test doubles. As a deliberate,
+documented trade-off it now counts test impls purely by name, so a rare name
+collision with an unrelated test-only trait can instead make SIT *under-report*
+a genuine single-impl production trait — the safe direction (a missed finding,
+never a wrong one). Details and rationale below.
+
+### Fixed
+- **SIT (single-impl trait) no longer flags traits with `#[cfg(test)]` test
+  doubles.** SIT counted only production impls (metadata collection skips test
+  code), so a non-pub trait with one production impl plus test-double impls —
+  the idiomatic dependency-injection / test-seam pattern — was wrongly reported
+  as an over-abstraction once the test impls became invisible. `#[cfg(test)]`
+  impls are now counted toward the trait's total implementor count — whether
+  they live in a whole test file, an inline `#[cfg(test)] mod`, or carry
+  `#[cfg(test)]` directly on the impl item — and SIT fires only when that total
+  is `1`. A genuine single-impl trait (one prod impl, no doubles) is still
+  flagged. Counting is deliberately *purely name-based* (the structural metadata
+  models no trait identity, on either the production or test side): it never
+  tries to resolve which trait a same-named impl targets, which guarantees a real
+  test double is never dropped — so a legitimate seam is never re-flagged (no
+  false positive, the original bug class). The accepted, documented trade-off is
+  a safe false *negative*: an unrelated test-only trait that happens to share a
+  production trait's last-segment name is counted toward it and can make SIT
+  under-report that production trait. Resolving that needs real trait identity
+  (the production side has the same name-collision limitation). Regressed when
+  v1.4.x improved
+  `#[cfg(test)] mod foo;`-chain test-file recognition (the better classification
+  correctly excluded the companion test files, shrinking SIT's denominator). The
+  sibling detector OI is unaffected — it matches impl locations, it does not
+  count.
+- **`#[cfg(test)]` on impls, methods and fns is now honoured by all structural
+  detectors.** Teaching SIT to count item-level `#[cfg(test)]` impls surfaced an
+  inconsistency: BTC (broken trait contract), SLM (selfless method), NMS
+  (needless `&mut self`), DEH (downcast escape hatch) and IET (inconsistent
+  error types) still treated a `#[cfg(test)] impl …`, a `#[cfg(test)]` method
+  inside a normal impl, or a `#[cfg(test)] pub fn` sitting in a *production* file
+  as production code — so e.g. `#[cfg(test)] impl Foo for Mock { fn m(&self) {
+  todo!() } }`, or `impl S { #[cfg(test)] fn helper(&self) -> i32 { 42 } }`,
+  could still trip BTC / SLM. The detectors now skip `#[cfg(test)]` at every
+  level (impl block, impl method, and free fn), consistent with whole test files
+  and `#[cfg(test)] mod` blocks (OI/SIT already do, via the shared metadata
+  walk).
+
 ## [1.4.1] - 2026-06-02
 
 Patch release: **test-suite effectiveness, proven by mutation testing** (Phases

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "rustqual"
-version = "1.4.1"
+version = "1.4.2"
 edition = "2021"
 description = "Comprehensive Rust code quality analyzer — seven dimensions: IOSP, Complexity, DRY, SRP, Coupling, Test Quality, Architecture"
 license = "MIT"

src/adapters/analyzers/structural/btc.rs

@@ -35,7 +35,9 @@ fn check_item(item: &syn::Item, path: &str, warnings: &mut Vec<StructuralWarning
         check_impl(imp, p, w);
     };
     match item {
-        syn::Item::Impl(imp) => impl_check(imp, path, warnings),
+        syn::Item::Impl(imp) if !super::has_cfg_test_attr(&imp.attrs) => {
+            impl_check(imp, path, warnings)
+        }
         syn::Item::Mod(m) if !super::has_cfg_test_attr(&m.attrs) => {
             m.content.iter().for_each(|(_, items)| {
                 items.iter().for_each(|i| check_item(i, path, warnings));
@@ -63,7 +65,7 @@ fn check_impl(imp: &syn::ItemImpl, path: &str, warnings: &mut Vec<StructuralWarn
         .items
         .iter()
         .filter_map(|i| match i {
-            syn::ImplItem::Fn(f) => Some(f),
+            syn::ImplItem::Fn(f) if !super::has_cfg_test_attr(&f.attrs) => Some(f),
             _ => None,
         })
         .collect();

src/adapters/analyzers/structural/deh.rs

@@ -66,6 +66,24 @@ impl<'ast, 'a> Visit<'ast> for DowncastVisitor<'a> {
         self.in_test = was_in_test;
     }
 
+    fn visit_item_impl(&mut self, node: &'ast syn::ItemImpl) {
+        let was_in_test = self.in_test;
+        if has_cfg_test_attr(&node.attrs) {
+            self.in_test = true;
+        }
+        syn::visit::visit_item_impl(self, node);
+        self.in_test = was_in_test;
+    }
+
+    fn visit_impl_item_fn(&mut self, node: &'ast syn::ImplItemFn) {
+        let was_in_test = self.in_test;
+        if has_cfg_test_attr(&node.attrs) {
+            self.in_test = true;
+        }
+        syn::visit::visit_impl_item_fn(self, node);
+        self.in_test = was_in_test;
+    }
+
     fn visit_expr_method_call(&mut self, node: &'ast syn::ExprMethodCall) {
         if !self.in_test {
             let method_name = node.method.to_string();

src/adapters/analyzers/structural/iet.rs

@@ -53,7 +53,7 @@ fn collect_pub_error_types(file: &syn::File) -> HashSet<String> {
 fn collect_item_errors(item: &syn::Item, error_types: &mut HashSet<String>) {
     let extract_error = |output: &syn::ReturnType| extract_result_error_type(output);
     match item {
-        syn::Item::Fn(f) => {
+        syn::Item::Fn(f) if !super::has_cfg_test_attr(&f.attrs) => {
             if matches!(f.vis, syn::Visibility::Public(_)) {
                 extract_error(&f.sig.output).iter().for_each(|e| {
                     error_types.insert(e.clone());

src/adapters/analyzers/structural/mod.rs

@@ -89,8 +89,15 @@ pub(crate) struct StructuralMetadata {
     pub type_defs: HashMap<String, String>,
     /// trait_name → TraitInfo
     pub trait_defs: HashMap<String, TraitInfo>,
-    /// trait_name → list of (impl_type, file)
+    /// trait_name → list of (impl_type, file) — production impls only
     pub trait_impls: HashMap<String, Vec<(String, String)>>,
+    /// trait_name → number of impls living in `#[cfg(test)]` code (a whole test
+    /// file, an inline `#[cfg(test)] mod` block, or an item-level `#[cfg(test)]`
+    /// on the impl). Not in `trait_impls` (metadata skips test code), but they
+    /// count toward a trait's TOTAL
+    /// implementor count so SIT does not flag a trait whose only extra
+    /// implementers are test doubles — the idiomatic DI / test-seam pattern.
+    pub cfg_test_trait_impl_counts: HashMap<String, usize>,
     /// (type_name, impl_file, impl_block_line) for inherent impls
     pub inherent_impls: Vec<(String, String, usize)>,
 }
@@ -117,6 +124,7 @@ pub(crate) fn collect_metadata(
         type_defs: HashMap::new(),
         trait_defs: HashMap::new(),
         trait_impls: HashMap::new(),
+        cfg_test_trait_impl_counts: collect_cfg_test_trait_impl_counts(parsed, cfg_test_files),
         inherent_impls: Vec::new(),
     };
     parsed.iter().for_each(|(path, _, syntax)| {
@@ -162,7 +170,7 @@ fn collect_item_metadata(item: &syn::Item, path: &str, meta: &mut StructuralMeta
                 },
             );
         }
-        syn::Item::Impl(imp) => {
+        syn::Item::Impl(imp) if !cfg_test(&imp.attrs) => {
             if let Some(ref type_name) = impl_type_name(imp) {
                 let line = imp.span().start().line;
                 if let Some((_, ref tp, _)) = imp.trait_ {
@@ -203,6 +211,72 @@ fn extract_impl_type_name(imp: &syn::ItemImpl) -> Option<String> {
 
 use crate::adapters::shared::cfg_test::has_cfg_test as has_cfg_test_attr;
 
+/// Count trait impls living in `#[cfg(test)]` code, keyed by trait name. The
+/// main metadata walk skips test code, so these production-invisible impls
+/// would otherwise vanish from a trait's implementor count — making a test
+/// seam (one prod impl + test doubles) look like a single-impl over-abstraction
+/// to SIT. "Test" means a whole `#![cfg(test)]` / integration-test file, an
+/// inline `#[cfg(test)] mod`, or an item-level `#[cfg(test)]` on the impl.
+/// Operation: iterates files, delegates per-scope collection via a closure.
+fn collect_cfg_test_trait_impl_counts(
+    parsed: &[(String, String, syn::File)],
+    cfg_test_files: &HashSet<String>,
+) -> HashMap<String, usize> {
+    let mut counts = HashMap::new();
+    parsed.iter().for_each(|(path, _, syntax)| {
+        let in_test = cfg_test_files.contains(path);
+        syntax
+            .items
+            .iter()
+            .for_each(|item| count_cfg_test_trait_impls(item, in_test, &mut counts));
+    });
+    counts
+}
+
+/// Recurse one item, counting `#[cfg(test)]` trait impls by last-path-segment
+/// trait name, descending into submodules. "In test" = a whole test file, an
+/// inline `#[cfg(test)] mod`, or an item-level `#[cfg(test)]` on the impl.
+///
+/// Counting is deliberately PURELY name-based: it makes no attempt to resolve
+/// which trait a same-named impl actually targets. That guarantees it never
+/// drops a real test double — so a legitimate DI seam is never re-flagged (no
+/// false positive, the original bug class). The accepted cost is a safe,
+/// documented false NEGATIVE: an unrelated test-only trait that happens to share
+/// a production trait's last-segment name is counted toward it and can make SIT
+/// under-report that production trait. (Earlier attempts to disambiguate by
+/// lexical scope / path form re-introduced false positives — e.g. a nested
+/// `super::Clock` double dropped because the nested module defined its own
+/// `Clock` — because correctly resolving `self::`/`super::`/glob/re-export/
+/// absolute paths IS trait identity, which the name-keyed metadata models on
+/// neither the production nor the test side. The production side has the same
+/// name-collision limitation.)
+/// Operation: match dispatch; own calls hidden in closures for IOSP.
+fn count_cfg_test_trait_impls(
+    item: &syn::Item,
+    in_test: bool,
+    counts: &mut HashMap<String, usize>,
+) {
+    let cfg_test = |attrs: &[syn::Attribute]| -> bool { has_cfg_test_attr(attrs) };
+    match item {
+        syn::Item::Impl(imp) if in_test || cfg_test(&imp.attrs) => {
+            if let Some((_, tp, _)) = &imp.trait_ {
+                if let Some(name) = tp.segments.last().map(|s| s.ident.to_string()) {
+                    *counts.entry(name).or_insert(0) += 1;
+                }
+            }
+        }
+        syn::Item::Mod(m) => {
+            let mod_in_test = in_test || cfg_test(&m.attrs);
+            m.content.iter().for_each(|(_, items)| {
+                items
+                    .iter()
+                    .for_each(|i| count_cfg_test_trait_impls(i, mod_in_test, counts));
+            });
+        }
+        _ => {}
+    }
+}
+
 /// Visit all inherent (non-trait) impl methods in parsed files, excluding test modules.
 /// Operation: iterates items, dispatches to callback via closure.
 pub(crate) fn visit_inherent_methods(
@@ -228,13 +302,15 @@ fn visit_item_methods(
     callback: &mut dyn FnMut(&syn::ImplItemFn, &str),
 ) {
     match item {
-        syn::Item::Impl(imp) => {
+        syn::Item::Impl(imp) if !has_cfg_test_attr(&imp.attrs) => {
             if imp.trait_.is_some() {
                 return;
             }
             imp.items.iter().for_each(|i| {
                 if let syn::ImplItem::Fn(method) = i {
-                    callback(method, path);
+                    if !has_cfg_test_attr(&method.attrs) {
+                        callback(method, path);
+                    }
                 }
             });
         }

src/adapters/analyzers/structural/sit.rs

@@ -3,8 +3,10 @@ use crate::findings::Dimension;
 
 use super::{StructuralMetadata, StructuralWarning, StructuralWarningKind};
 
-/// Detect single-implementation traits: non-pub trait with exactly 1 impl.
-/// Operation: compares trait definitions against impl counts.
+/// Detect single-implementation traits: non-pub trait with exactly 1 impl
+/// total, counting `#[cfg(test)]` impls (a test double is a real implementor,
+/// so a trait with prod impl + test doubles is a DI seam, not over-abstraction).
+/// Operation: compares trait definitions against total impl counts.
 pub(crate) fn detect_sit(
     warnings: &mut Vec<StructuralWarning>,
     meta: &StructuralMetadata,
@@ -32,6 +34,18 @@ pub(crate) fn detect_sit(
         let [(impl_type, _)] = impls.as_slice() else {
             return;
         };
+        // A `#[cfg(test)]` impl is a real implementor too: one prod impl plus any
+        // test doubles is the idiomatic DI / test-seam pattern, not an
+        // over-abstraction. Only fire when the TOTAL implementor count is 1.
+        if meta
+            .cfg_test_trait_impl_counts
+            .get(trait_name)
+            .copied()
+            .unwrap_or(0)
+            != 0
+        {
+            return;
+        }
         warnings.push(StructuralWarning {
             file: info.file.clone(),
             line: info.line,

src/adapters/analyzers/structural/tests/btc.rs

@@ -21,6 +21,35 @@ fn stub_impl_in_cfg_test_file_excluded() {
     );
 }
 
+#[test]
+fn cfg_test_attr_impl_stub_excluded() {
+    // An item-level `#[cfg(test)]` trait impl is test code (a mock) even in a
+    // production file — its stub bodies must not trip BTC, consistent with the
+    // SIT metadata treating the same impl as a test impl.
+    let w = detect_in(
+        "trait Foo { fn bar(&self); } struct M; #[cfg(test)] impl Foo for M { fn bar(&self) { todo!() } }",
+    );
+    assert!(
+        w.is_empty(),
+        "a #[cfg(test)] impl's stub must be excluded from BTC: {} warning(s)",
+        w.len()
+    );
+}
+
+#[test]
+fn cfg_test_attr_method_stub_excluded() {
+    // `#[cfg(test)]` directly on a trait-impl method marks it test code; its stub
+    // body must not trip BTC, consistent with the item-level `#[cfg(test)] impl`.
+    let w = detect_in(
+        "trait Foo { fn bar(&self); } struct M; impl Foo for M { #[cfg(test)] fn bar(&self) { todo!() } }",
+    );
+    assert!(
+        w.is_empty(),
+        "a #[cfg(test)] method stub must be excluded from BTC: {} warning(s)",
+        w.len()
+    );
+}
+
 #[test]
 fn test_all_stub_methods_flagged() {
     let w = detect_in("trait Foo { fn bar(&self); } impl Foo for MyType { fn bar(&self) { todo!() } } struct MyType;");

src/adapters/analyzers/structural/tests/deh.rs

@@ -16,6 +16,35 @@ fn test_downcast_ref_flagged() {
     ));
 }
 
+#[test]
+fn downcast_in_cfg_test_attr_impl_excluded() {
+    // A downcast inside a method of an item-level `#[cfg(test)]` impl is test
+    // code even in a production file — DEH must skip it, consistent with the
+    // function-level `#[test]`/`#[cfg(test)]` handling and the SIT metadata.
+    let w = detect_in(
+        "struct M; #[cfg(test)] impl M { fn foo(&self, a: &dyn std::any::Any) { a.downcast_ref::<i32>(); } }",
+    );
+    assert!(
+        w.is_empty(),
+        "a downcast in a #[cfg(test)] impl must be excluded from DEH: {} warning(s)",
+        w.len()
+    );
+}
+
+#[test]
+fn downcast_in_cfg_test_attr_method_excluded() {
+    // `#[cfg(test)]` directly on an impl METHOD makes its body test code even in
+    // a regular production impl — DEH must skip a downcast inside it.
+    let w = detect_in(
+        "struct M; impl M { #[cfg(test)] fn foo(&self, a: &dyn std::any::Any) { a.downcast_ref::<i32>(); } }",
+    );
+    assert!(
+        w.is_empty(),
+        "a downcast in a #[cfg(test)] method must be excluded from DEH: {} warning(s)",
+        w.len()
+    );
+}
+
 #[test]
 fn test_downcast_mut_flagged() {
     let w = detect_in("fn foo(a: &mut dyn std::any::Any) { a.downcast_mut::<i32>(); }");

src/adapters/analyzers/structural/tests/iet.rs

@@ -20,6 +20,21 @@ fn inconsistent_error_types_in_cfg_test_file_excluded() {
     );
 }
 
+#[test]
+fn cfg_test_attr_pub_fn_error_type_excluded() {
+    // A `#[cfg(test)]` pub fn is test code even in a production file — its error
+    // type must not count toward IET's inconsistent-error-type check, leaving a
+    // single production error type (no finding).
+    let w = detect_in(
+        "pub fn a() -> Result<(), String> { Ok(()) } #[cfg(test)] pub fn b() -> Result<(), u32> { Ok(()) }",
+    );
+    assert!(
+        w.is_empty(),
+        "a #[cfg(test)] pub fn's error type must be excluded from IET: {} warning(s)",
+        w.len()
+    );
+}
+
 #[test]
 fn test_consistent_error_types_not_flagged() {
     let w = detect_in(