cordance-advise 0.1.1

Cordance advisory engine. Deterministic doctrine checks against project state.
Documentation
//! R-observe-1 — suggest structured logging for Rust binaries.

use camino::Utf8PathBuf;
use cordance_core::advise::{AdviseFinding, Severity};
use cordance_core::pack::CordancePack;

use super::AdviseRule;

pub struct RObserve1;

impl AdviseRule for RObserve1 {
    fn id(&self) -> &'static str {
        "R-observe-1"
    }

    fn doctrine_anchor(&self) -> &'static str {
        "doctrine/principles/observability.md"
    }

    fn check(&self, pack: &CordancePack) -> Vec<AdviseFinding> {
        // Collect every Cargo.toml in the pack — workspace root and per-crate.
        let cargo_tomls: Vec<&cordance_core::source::SourceRecord> = pack
            .sources
            .iter()
            .filter(|r| !r.blocked && r.path.as_str().ends_with("Cargo.toml"))
            .collect();

        if cargo_tomls.is_empty() {
            return vec![];
        }

        // Only fire when the project actually produces a binary. A pure-library
        // workspace doesn't need structured-log infrastructure.
        let has_main_rs = pack
            .sources
            .iter()
            .any(|r| r.path.as_str().ends_with("main.rs"));

        if !has_main_rs {
            return vec![];
        }

        // Bound the check by reading each Cargo.toml from disk (relative to
        // `pack.project.repo_root`) and looking for a `tracing` dependency
        // declaration. This is intentionally narrow — we only inspect content
        // when path-based metadata cannot answer the question.
        let repo_root = &pack.project.repo_root;
        let has_tracing = cargo_tomls.iter().any(|r| {
            let abs = repo_root.join(&r.path);
            std::fs::read_to_string(abs.as_std_path())
                .ok()
                .is_some_and(|content| cargo_toml_mentions_tracing(&content))
        });

        if has_tracing {
            return vec![];
        }

        vec![AdviseFinding {
            id: self.id().into(),
            severity: Severity::Info,
            summary: "Rust project: consider adding the tracing crate for structured logs.".into(),
            doctrine_anchor: Utf8PathBuf::from(self.doctrine_anchor()),
            project_paths: vec![".".into()],
            remediation: "Add tracing and tracing-subscriber to your Cargo.toml. \
                 Use tracing::info!() instead of println!()."
                .into(),
        }]
    }
}

/// Detect a `tracing` dependency in a Cargo.toml file's content.
///
/// Inspects every `tracing` key in any of `[dependencies]`, `[dev-dependencies]`,
/// `[build-dependencies]`, or their `[target.<cfg>.{dependencies,…}]` variants.
/// Returns `true` if any of them mentions the `tracing` crate.
///
/// Catches:
/// - `tracing = "0.1"`
/// - `tracing = { workspace = true }`
/// - `tracing = { version = "0.1", features = [...] }`
/// - `tracing.workspace = true`
/// - `[target.'cfg(unix)'.dependencies] tracing = "0.1"`
///
/// Round-3 codereview HIGH: replaces a hand-rolled line scanner that handled
/// the simple cases but ignored target-cfg dependency tables and was brittle
/// against unusual but valid TOML formatting. The `toml` crate gives uniform
/// semantics across every spec-compliant shape.
fn cargo_toml_mentions_tracing(content: &str) -> bool {
    #[derive(serde::Deserialize, Default)]
    struct TargetTable {
        #[serde(default)]
        dependencies: toml::Table,
        #[serde(default, rename = "dev-dependencies")]
        dev_dependencies: toml::Table,
        #[serde(default, rename = "build-dependencies")]
        build_dependencies: toml::Table,
    }
    #[derive(serde::Deserialize, Default)]
    struct CargoToml {
        #[serde(default)]
        dependencies: toml::Table,
        #[serde(default, rename = "dev-dependencies")]
        dev_dependencies: toml::Table,
        #[serde(default, rename = "build-dependencies")]
        build_dependencies: toml::Table,
        /// `[target.'<cfg>'.{dependencies,…}]` — a map of cfg-expr to its
        /// dependency tables. We aggregate across all entries.
        #[serde(default)]
        target: std::collections::BTreeMap<String, TargetTable>,
    }

    let Ok(parsed) = toml::from_str::<CargoToml>(content) else {
        return false;
    };

    let mentions = |t: &toml::Table| t.contains_key("tracing");

    if mentions(&parsed.dependencies)
        || mentions(&parsed.dev_dependencies)
        || mentions(&parsed.build_dependencies)
    {
        return true;
    }
    parsed.target.values().any(|t| {
        mentions(&t.dependencies)
            || mentions(&t.dev_dependencies)
            || mentions(&t.build_dependencies)
    })
}

#[cfg(test)]
mod tests {
    use super::cargo_toml_mentions_tracing;

    #[test]
    fn detects_workspace_inheritance() {
        // Round-3 codereview correctness pin: `tracing.workspace = true`
        // must be detected. The toml crate parses this as
        // `[dependencies.tracing] workspace = true` only when the line lives
        // inside a `[dependencies]` table.
        assert!(cargo_toml_mentions_tracing(
            "[dependencies]\ntracing = { workspace = true }\n"
        ));
    }

    #[test]
    fn detects_versioned_string() {
        assert!(cargo_toml_mentions_tracing(
            "[dependencies]\ntracing = \"0.1\"\n"
        ));
    }

    #[test]
    fn detects_dotted_inheritance() {
        // Dotted-key form: `tracing.workspace = true` inside `[dependencies]`.
        assert!(cargo_toml_mentions_tracing(
            "[dependencies]\ntracing.workspace = true\n"
        ));
    }

    #[test]
    fn detects_inline_table() {
        assert!(cargo_toml_mentions_tracing(
            "[dependencies]\ntracing = { version = \"0.1\", features = [\"std\"] }\n"
        ));
    }

    #[test]
    fn detects_dev_dependencies() {
        assert!(cargo_toml_mentions_tracing(
            "[dev-dependencies]\ntracing = \"0.1\"\n"
        ));
    }

    #[test]
    fn detects_build_dependencies() {
        assert!(cargo_toml_mentions_tracing(
            "[build-dependencies]\ntracing = \"0.1\"\n"
        ));
    }

    #[test]
    fn detects_target_cfg_dependencies() {
        assert!(cargo_toml_mentions_tracing(
            "[target.'cfg(unix)'.dependencies]\ntracing = \"0.1\"\n"
        ));
    }

    #[test]
    fn ignores_comments() {
        assert!(!cargo_toml_mentions_tracing("# tracing is a great crate"));
    }

    #[test]
    fn ignores_tracing_subscriber_alone() {
        // `tracing-subscriber` is a different crate key. The typed parser sees
        // the key as a literal table name, so the contains_key check is exact
        // (this is stronger than the line-prefix heuristic it replaces).
        assert!(!cargo_toml_mentions_tracing(
            "[dependencies]\ntracing-subscriber = \"0.3\"\n"
        ));
    }

    #[test]
    fn ignores_tracing_log() {
        // Underscore-suffixed variant — another crate that is not `tracing`.
        assert!(!cargo_toml_mentions_tracing(
            "[dependencies]\ntracing_log = \"0.1\"\n"
        ));
    }

    #[test]
    fn empty_returns_false() {
        assert!(!cargo_toml_mentions_tracing(""));
    }

    #[test]
    fn malformed_toml_returns_false() {
        // The function used to silently skip bad lines; we keep "return false
        // rather than panic" but now with a typed parse error path.
        assert!(!cargo_toml_mentions_tracing("not = = valid toml"));
    }

    #[test]
    fn realistic_workspace_root_with_tracing() {
        // A real cordance-style workspace root pins tracing via
        // [workspace.dependencies]. The R-observe-1 rule checks per-crate
        // Cargo.toml files (not the workspace root); this test pins the
        // expected behaviour: workspace.dependencies alone does NOT count
        // because the rule wants to know that the binary crate has a
        // tracing line. Pure-workspace roots fall through to per-crate checks.
        assert!(!cargo_toml_mentions_tracing(
            "[workspace.dependencies]\ntracing = \"0.1\"\n"
        ));
    }

    #[test]
    fn full_crate_cargo_toml_with_tracing() {
        // The typical per-crate Cargo.toml in this workspace. The parser
        // must walk the full document and still detect `tracing` in
        // `[dependencies]`.
        let content = r#"
[package]
name = "demo"
version = "0.1.0"
edition = "2021"

[dependencies]
serde = "1"
tracing = { workspace = true }
anyhow = "1"

[dev-dependencies]
tempfile = "3"
"#;
        assert!(cargo_toml_mentions_tracing(content));
    }
}