refactor(xtask): split parity evidence and layers dependency checks (Sonar S3776) (#4406)

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>

xtask/src/layers.rs

@@ -182,6 +182,34 @@ impl std::fmt::Display for Violation {
     }
 }
 
+/// Layer and testkit rules for one dependency of `c` that is itself a workspace crate.
+fn check_workspace_dep(c: &Crate, class: Class, layer: u8, d: &Dep, out: &mut Vec<Violation>) {
+    let dep_class = classify(&d.name);
+    // Test support is a dev-dependency only (`Class::Testkit`) and so never reaches
+    // the product graph a standalone crate publishes. Checking it against the
+    // standalone allow-list would stop `cos` using the shared oracles in its own
+    // tests, which is the one thing those crates exist for.
+    let testkit_dev = dep_class == Some(Class::Testkit) && d.kind == DepKind::Dev;
+    if matches!(class, Class::Standalone(_)) && !testkit_dev && !standalone_allowed(&c.name, &d.name) {
+        out.push(Violation::StandaloneHasWorkspaceDep { krate: c.name.clone(), dep: d.name.clone() });
+        return;
+    }
+    match dep_class {
+        None => {}
+        Some(Class::Testkit) if d.kind != DepKind::Dev && class != Class::Testkit => {
+            out.push(Violation::TestkitAsNormalDep { krate: c.name.clone(), dep: d.name.clone() });
+        }
+        Some(Class::Testkit) => {}
+        Some(dc) => {
+            let to = dc.layer().unwrap_or(u8::MAX);
+            let ok = to < layer || (to == layer && sideways_allowed(&c.name, &d.name));
+            if !ok {
+                out.push(Violation::Upward { krate: c.name.clone(), dep: d.name.clone(), from: layer, to: to.min(8), kind: d.kind });
+            }
+        }
+    }
+}
+
 pub fn check(crates: &[Crate]) -> Vec<Violation> {
     let mut out = Vec::new();
     for c in crates {
@@ -198,30 +226,7 @@ pub fn check(crates: &[Crate]) -> Vec<Violation> {
                 continue;
             }
             if d.workspace {
-                let dep_class = classify(&d.name);
-                // Test support is a dev-dependency only (`Class::Testkit`) and so never reaches
-                // the product graph a standalone crate publishes. Checking it against the
-                // standalone allow-list would stop `cos` using the shared oracles in its own
-                // tests, which is the one thing those crates exist for.
-                let testkit_dev = dep_class == Some(Class::Testkit) && d.kind == DepKind::Dev;
-                if matches!(class, Class::Standalone(_)) && !testkit_dev && !standalone_allowed(&c.name, &d.name) {
-                    out.push(Violation::StandaloneHasWorkspaceDep { krate: c.name.clone(), dep: d.name.clone() });
-                    continue;
-                }
-                match dep_class {
-                    None => {}
-                    Some(Class::Testkit) if d.kind != DepKind::Dev && class != Class::Testkit => {
-                        out.push(Violation::TestkitAsNormalDep { krate: c.name.clone(), dep: d.name.clone() });
-                    }
-                    Some(Class::Testkit) => {}
-                    Some(dc) => {
-                        let to = dc.layer().unwrap_or(u8::MAX);
-                        let ok = to < layer || (to == layer && sideways_allowed(&c.name, &d.name));
-                        if !ok {
-                            out.push(Violation::Upward { krate: c.name.clone(), dep: d.name.clone(), from: layer, to: to.min(8), kind: d.kind });
-                        }
-                    }
-                }
+                check_workspace_dep(c, class, layer, d, &mut out);
             } else if layer < UI_MIN_LAYER && is_ui_crate(&d.name) && !(class == Class::Testkit || d.kind == DepKind::Dev && d.name == "egui_kittest")
             {
                 out.push(Violation::UiBelowL7 { krate: c.name.clone(), dep: d.name.clone(), layer });

xtask/src/parity.rs

@@ -104,6 +104,18 @@ fn valid_milestone(m: &str) -> bool {
     m == "none" || m.strip_prefix('M').and_then(|n| n.parse::<u32>().ok()).is_some_and(|n| n <= 14)
 }
 
+/// Why an `evidence` entry (`path.rs::test_fn`) does not resolve to a test in the source tree, if it doesn't.
+fn evidence_error(ev: &str, repo: &Path) -> Option<String> {
+    let Some((path, func)) = ev.split_once("::") else {
+        return Some(format!("evidence {ev:?} must be `path.rs::test_fn`"));
+    };
+    match std::fs::read_to_string(repo.join(path)) {
+        Ok(src) if src.contains(&format!("fn {func}(")) => None,
+        Ok(_) => Some(format!("evidence: no `fn {func}` in {path}")),
+        Err(_) => Some(format!("evidence: {path} does not exist")),
+    }
+}
+
 /// Validate features against the registry, tool table and source tree; returns the errors.
 pub fn validate(features: &[Feature], commands: &HashSet<String>, tools: &HashSet<String>, repo: &Path) -> Vec<String> {
     let mut errors = Vec::new();
@@ -143,14 +155,8 @@ pub fn validate(features: &[Feature], commands: &HashSet<String>, tools: &HashSe
             errors.push(e("shipped features must cite at least one test in `evidence`".into()));
         }
         for ev in &f.evidence {
-            let Some((path, func)) = ev.split_once("::") else {
-                errors.push(e(format!("evidence {ev:?} must be `path.rs::test_fn`")));
-                continue;
-            };
-            match std::fs::read_to_string(repo.join(path)) {
-                Ok(src) if src.contains(&format!("fn {func}(")) => {}
-                Ok(_) => errors.push(e(format!("evidence: no `fn {func}` in {path}"))),
-                Err(_) => errors.push(e(format!("evidence: {path} does not exist"))),
+            if let Some(m) = evidence_error(ev, repo) {
+                errors.push(e(m));
             }
         }
     }