From b0a0cb6c703477281400b5697a7a9c0921158151 Mon Sep 17 00:00:00 2001 From: Connor McDonald Date: Sun, 7 Jun 2026 09:09:29 +0200 Subject: [PATCH] feat(cli): scope check, JSON for lint/verify, lint granularity warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements issues #1, #2, and #6 — no version bump / release. #1 check scoping: --files and diff-scoping via an explicit --base (merge-base..working-tree). Omitting --base keeps a full check with HEAD enrichment; a bad ref / non-repo falls back to full check. #2 --format json for lint and verify, mirroring check. Extracts the shared Format enum into format.rs; lint findings serialize directly; verify gains a structured VerifyReport while keeping its file-stamping side effects. #6 advisory lint granularity warnings (never block, §8): near-whole-file anchor spans, hubs with too many anchors, and public functions in an anchored file that no claim covers. Adds surf_core::public_fns (top-level public functions only — pub(crate) and data types excluded to avoid over-anchoring noise). Hubs updated to document the new behavior and re-sealed. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 6 +- action.yml | 4 +- hubs/cli-check.md | 10 +- hubs/cli-lint.md | 14 +- hubs/cli-verify.md | 5 +- surf-cli/src/check.rs | 211 +++++++++++++++++++++++--- surf-cli/src/format.rs | 9 ++ surf-cli/src/lint.rs | 310 ++++++++++++++++++++++++++++++++++----- surf-cli/src/main.rs | 37 ++++- surf-cli/src/verify.rs | 171 ++++++++++++++++----- surf-core/src/lib.rs | 2 +- surf-core/src/resolve.rs | 139 ++++++++++++++++++ 12 files changed, 804 insertions(+), 114 deletions(-) create mode 100644 surf-cli/src/format.rs diff --git a/README.md b/README.md index b1b2f99..f33d8ef 100644 --- a/README.md +++ b/README.md @@ -235,9 +235,9 @@ surf check --format json - `surf init` — bootstrap a workspace: write `surf.toml` and create the hubs directory (idempotent). - `surf new ` — scaffold a new empty hub under your hubs directory. -- `surf lint` — validate frontmatter and that every `at:` resolves to exactly one symbol. Warns on a renamed symbol (suggests `verify --follow`); blocks on ambiguous or vanished anchors. -- `surf check [--format human|json] [--base ]` — the gate. AST-canonical-hash each anchored span and compare to the stored hash; non-zero exit on any divergence. `--base` (default `HEAD`) is the git ref used to recover the advisory `old_code` / `magnitude` fields. -- `surf verify [] [--follow]` — re-seal after you've confirmed the prose still holds; writes the hash into the frontmatter. `` limits to one anchor; `--follow` re-points a renamed single-symbol anchor and re-hashes in one step. +- `surf lint [--format human|json]` — validate frontmatter and that every `at:` resolves to exactly one symbol. Warns on a renamed symbol (suggests `verify --follow`); blocks on ambiguous or vanished anchors. Also emits advisory granularity warnings (never blocking, §8): a near-whole-file anchor span, a hub with too many anchors, and public functions in an anchored file that no claim covers. +- `surf check [--format human|json] [--base ] [--files ]` — the gate. AST-canonical-hash each anchored span and compare to the stored hash; non-zero exit on any divergence. By default every claim is checked. `--base ` scopes to claims whose anchored files changed since the merge base **and** recovers the advisory `old_code` / `magnitude` fields from that ref (omit it for a full check with enrichment against `HEAD`). `--files ` scopes to claims whose anchored file(s) match a comma-separated glob (e.g. `surf-core/**`). +- `surf verify [] [--follow] [--format human|json]` — re-seal after you've confirmed the prose still holds; writes the hash into the frontmatter. `` limits to one anchor; `--follow` re-points a renamed single-symbol anchor and re-hashes in one step. ## Configuration diff --git a/action.yml b/action.yml index 94d729b..61d245f 100644 --- a/action.yml +++ b/action.yml @@ -6,7 +6,9 @@ branding: # Consumers: a plain `actions/checkout@v4` is enough — do NOT set `fetch-depth: 0`. # The verdict only needs the working tree; git history is used solely for the advisory -# old_code/magnitude enrichment in the JSON report. +# old_code/magnitude enrichment in the JSON report. If you pass `check --base ` to +# diff-scope the gate to changed files, fetch enough history to reach the merge base (a +# shallow `git fetch ` is plenty) — still not `fetch-depth: 0`. inputs: args: diff --git a/hubs/cli-check.md b/hubs/cli-check.md index 18f8eb7..f76719a 100644 --- a/hubs/cli-check.md +++ b/hubs/cli-check.md @@ -1,5 +1,5 @@ --- -summary: surf check — the gate. Hash each anchored span, compare to the stored hash, block on divergence. +summary: surf check — the gate. Hash each anchored span, compare to the stored hash, block on divergence. Optionally scope to changed files. anchors: - claim: > Per claim: resolve and hash every site, combine into one hash, compare to the stored @@ -12,6 +12,13 @@ anchors: `git show :`; absent git the verdict is unchanged and those fields are omitted. at: surf-cli/src/check.rs > git_show hash: cd1f35beb1ec + - claim: > + Scoping is opt-in and intersective: with neither --base nor --files every claim is checked. + A claim is in scope when any of its anchored files matches each active filter — the --base + changed-files set (merge-base..working-tree) and/or the --files globs. A bad ref or non-repo + yields no changed set, falling back to a full check rather than checking nothing. + at: surf-cli/src/check.rs > Scope > includes + hash: 2e21db33542d refs: [] --- @@ -19,3 +26,4 @@ refs: [] `check_claim` is the verdict; `git_show` only feeds the advisory `old_code`/`magnitude` in the `--format json` report. Any divergence makes `run` exit non-zero (the CI-blocking signal). +`Scope` narrows which claims `check_workspace` evaluates when `--base`/`--files` are given. diff --git a/hubs/cli-lint.md b/hubs/cli-lint.md index 6af21a5..ad40485 100644 --- a/hubs/cli-lint.md +++ b/hubs/cli-lint.md @@ -1,16 +1,24 @@ --- -summary: surf lint — every anchor resolves to exactly one symbol; renames warn, not block. +summary: surf lint — anchors must resolve to one symbol (renames warn); plus advisory granularity warnings. anchors: - claim: > lint produces a Finding per anchor site: ambiguous or vanished anchors block, while a renamed-but-present symbol (stored-hash match) only warns and points at verify --follow. Block-level findings set a non-zero exit; warnings alone keep exit 0. at: surf-cli/src/lint.rs > lint_site - hash: 840a2d93cf22 + hash: 51bb818e87ae + - claim: > + Advisory granularity guidance (§8), never blocking: lint_under_coverage flags public + functions in a hub's already-anchored files that no claim covers — but only for files + whose anchors all resolved cleanly, so coverage nags never pile onto broken anchors. + at: surf-cli/src/lint.rs > lint_under_coverage + hash: dad7767e0594 refs: [] --- # surf lint `lint_workspace` loads every hub and runs `lint_site` over each anchor; `run` prints the -findings and chooses the exit code. +findings and chooses the exit code. Beyond resolution, lint emits advisory warnings (§8) that +nudge granularity: a near-whole-file span (`lint_coarse_span`), too many anchors in one hub, +and public functions with no covering claim (`lint_under_coverage`). diff --git a/hubs/cli-verify.md b/hubs/cli-verify.md index ed06b15..b353246 100644 --- a/hubs/cli-verify.md +++ b/hubs/cli-verify.md @@ -13,5 +13,6 @@ refs: [] # surf verify -The human escape hatch. `run` applies each `plan_claim` result through the surgical hub editor -and only rewrites a file when something actually changed. +The human escape hatch. `verify_all` applies each `plan_claim` result through the surgical hub +editor and only rewrites a file when something actually changed; `run` then renders the +collected report as human text or JSON. diff --git a/surf-cli/src/check.rs b/surf-cli/src/check.rs index 1f248c9..c2ee1f1 100644 --- a/surf-cli/src/check.rs +++ b/surf-cli/src/check.rs @@ -4,21 +4,23 @@ //! deterministic and needs no git. `old_code`/`magnitude` are recovered best-effort from the //! previous source via `git show :` and are advisory only. +use crate::format::Format; use crate::workspace::{read_site, Workspace}; use anyhow::{Context, Result}; -use clap::ValueEnum; +use std::collections::HashSet; use std::path::Path; use std::process::{Command, ExitCode}; -use surf_core::{diff_magnitude, hash_anchor, parse_hub, resolve, Divergence, DivergenceKind}; +use surf_core::{ + diff_magnitude, hash_anchor, parse_anchor, parse_hub, resolve, Divergence, DivergenceKind, +}; -#[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum)] -pub enum Format { - Human, - Json, -} - -pub fn run(ws: &Workspace, format: Format, base: &str) -> Result { - let divergences = check_workspace(ws, base)?; +pub fn run( + ws: &Workspace, + format: Format, + base: Option<&str>, + files: &[String], +) -> Result { + let divergences = check_workspace(ws, base, files)?; match format { Format::Json => { @@ -34,7 +36,15 @@ pub fn run(ws: &Workspace, format: Format, base: &str) -> Result { }) } -fn check_workspace(ws: &Workspace, base: &str) -> Result> { +fn check_workspace( + ws: &Workspace, + base: Option<&str>, + files: &[String], +) -> Result> { + let scope = Scope::build(ws, base, files); + // Enrichment always needs a ref; an explicit --base doubles as the diff base, else HEAD. + let enrich_base = base.unwrap_or("HEAD"); + let mut out = Vec::new(); for hub_path in ws.hub_paths()? { let rel = hub_path @@ -50,7 +60,10 @@ fn check_workspace(ws: &Workspace, base: &str) -> Result> { }; for claim in &hub.frontmatter.anchors { - if let Some(d) = check_claim(ws, &rel, claim, base) { + if !scope.includes(claim) { + continue; + } + if let Some(d) = check_claim(ws, &rel, claim, enrich_base) { out.push(d); } } @@ -58,6 +71,49 @@ fn check_workspace(ws: &Workspace, base: &str) -> Result> { Ok(out) } +/// Which claims `check` evaluates. Each active filter narrows the set; a claim must satisfy +/// every active filter (intersection). With neither filter active, every claim is in scope. +struct Scope { + changed: Option>, + globs: Vec, +} + +impl Scope { + fn build(ws: &Workspace, base: Option<&str>, files: &[String]) -> Scope { + // A bad ref / non-repo yields None — we fall back to a full check rather than + // silently checking nothing. + let changed = base.and_then(|b| git_changed_files(&ws.root, b)); + let globs = files + .iter() + .filter_map(|p| glob::Pattern::new(p).ok()) + .collect(); + Scope { changed, globs } + } + + fn includes(&self, claim: &surf_core::Claim) -> bool { + let anchor_files: Vec = claim + .at + .sites() + .iter() + .filter_map(|s| parse_anchor(s).ok().map(|a| a.file)) + .collect(); + + if let Some(changed) = &self.changed { + if !anchor_files.iter().any(|f| changed.contains(f)) { + return false; + } + } + if !self.globs.is_empty() + && !anchor_files + .iter() + .any(|f| self.globs.iter().any(|g| g.matches(f))) + { + return false; + } + true + } +} + fn check_claim( ws: &Workspace, hub: &str, @@ -168,6 +224,33 @@ fn enrich_from_git( (old_code, magnitude) } +/// Files changed between the merge base of `base`..HEAD and the working tree. Paths are +/// repo-root-relative; they match `Anchor.file` (workspace-root-relative) when the +/// workspace root is the repo root, the normal case. `None` if git can't answer. +fn git_changed_files(root: &Path, base: &str) -> Option> { + let merge_base = Command::new("git") + .current_dir(root) + .args(["merge-base", base, "HEAD"]) + .output() + .ok() + .filter(|o| o.status.success()) + .map(|o| String::from_utf8_lossy(&o.stdout).trim().to_string()) + // Shallow clones may lack the merge base; diff against the ref directly. + .unwrap_or_else(|| base.to_string()); + + let output = Command::new("git") + .current_dir(root) + .args(["diff", "--name-only", &merge_base]) + .output() + .ok()?; + output.status.success().then(|| { + String::from_utf8_lossy(&output.stdout) + .lines() + .map(str::to_string) + .collect() + }) +} + fn git_show(root: &Path, base: &str, rel_file: &str) -> Option { let output = Command::new("git") .current_dir(root) @@ -257,7 +340,7 @@ mod tests { &format!("---\nsummary: x\nanchors:\n - claim: add sums\n at: src/m.rs > add\n hash: {h}\n---\n"), ); - assert!(check_workspace(&ws_at(root.to_path_buf()), "HEAD") + assert!(check_workspace(&ws_at(root.to_path_buf()), None, &[]) .unwrap() .is_empty()); } @@ -281,7 +364,7 @@ mod tests { &format!("---\nsummary: x\nanchors:\n - claim: add sums\n at: src/m.rs > add\n hash: {h}\n---\n"), ); - assert!(check_workspace(&ws_at(root.to_path_buf()), "HEAD") + assert!(check_workspace(&ws_at(root.to_path_buf()), None, &[]) .unwrap() .is_empty()); } @@ -298,7 +381,7 @@ mod tests { "---\nsummary: x\nanchors:\n - claim: c\n at: src/m.rs > add\n---\n", ); - let d = check_workspace(&ws_at(root.to_path_buf()), "HEAD").unwrap(); + let d = check_workspace(&ws_at(root.to_path_buf()), None, &[]).unwrap(); assert_eq!(d.len(), 1); assert_eq!(d[0].kind, DivergenceKind::Unverified); } @@ -343,7 +426,7 @@ mod tests { "pub fn add(a: i64, b: i64) -> i64 { a - b }\n", ); - let d = check_workspace(&ws_at(root.to_path_buf()), "HEAD").unwrap(); + let d = check_workspace(&ws_at(root.to_path_buf()), None, &[]).unwrap(); assert_eq!(d.len(), 1); let d = &d[0]; assert_eq!(d.kind, DivergenceKind::Changed); @@ -366,7 +449,7 @@ mod tests { "---\nsummary: x\nanchors:\n - claim: c\n at: schema.sql > users\n---\n", ); - let d = check_workspace(&ws_at(root.to_path_buf()), "HEAD").unwrap(); + let d = check_workspace(&ws_at(root.to_path_buf()), None, &[]).unwrap(); assert_eq!(d.len(), 1); assert_eq!(d[0].kind, DivergenceKind::Unresolvable); assert_eq!( @@ -393,7 +476,7 @@ mod tests { &format!("---\nsummary: x\nanchors:\n - claim: add sums\n at: src/m.rs > add\n hash: {h}\n---\n"), ); - let d = check_workspace(&ws_at(root.to_path_buf()), "HEAD").unwrap(); + let d = check_workspace(&ws_at(root.to_path_buf()), None, &[]).unwrap(); let json = serde_json::to_value(&d).unwrap(); let obj = json[0].as_object().unwrap(); for key in [ @@ -402,4 +485,96 @@ mod tests { assert!(obj.contains_key(key), "missing key `{key}` in {obj:?}"); } } + + /// Two diverged claims in different files; both surface with no scope, but a `--files` + /// glob narrows the result to the matching file. + fn two_diverged_files(root: &Path) { + let a = "pub fn add(a: i64, b: i64) -> i64 { a + b }\n"; + let s = "pub fn sub(a: i64, b: i64) -> i64 { a - b }\n"; + let ha = stored_hash(a, "src/a.rs > add"); + let hs = stored_hash(s, "src/b.rs > sub"); + write(root, "surf.toml", ""); + // Working tree diverges from the stored hashes. + write( + root, + "src/a.rs", + "pub fn add(a: i64, b: i64) -> i64 { a - b }\n", + ); + write( + root, + "src/b.rs", + "pub fn sub(a: i64, b: i64) -> i64 { a + b }\n", + ); + write( + root, + "hubs/a.md", + &format!( + "---\nsummary: x\nanchors:\n - claim: add\n at: src/a.rs > add\n hash: {ha}\n - claim: sub\n at: src/b.rs > sub\n hash: {hs}\n---\n" + ), + ); + } + + #[test] + fn files_scope_limits_claims() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + two_diverged_files(root); + let ws = ws_at(root.to_path_buf()); + + assert_eq!(check_workspace(&ws, None, &[]).unwrap().len(), 2); + + let scoped = check_workspace(&ws, None, &["src/a.rs".to_string()]).unwrap(); + assert_eq!(scoped.len(), 1); + assert_eq!(scoped[0].at, "src/a.rs > add"); + + let globbed = check_workspace(&ws, None, &["src/b*.rs".to_string()]).unwrap(); + assert_eq!(globbed.len(), 1); + assert_eq!(globbed[0].at, "src/b.rs > sub"); + } + + #[test] + fn base_scope_limits_to_changed() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + two_diverged_files(root); + + // Commit the diverged working tree as v0, then change only src/a.rs. + git(root, &["init", "-q"]); + git( + root, + &["-c", "user.email=t@t", "-c", "user.name=t", "add", "."], + ); + git( + root, + &[ + "-c", + "user.email=t@t", + "-c", + "user.name=t", + "commit", + "-q", + "-m", + "v0", + ], + ); + write( + root, + "src/a.rs", + "pub fn add(a: i64, b: i64) -> i64 { a * b }\n", + ); + + let ws = ws_at(root.to_path_buf()); + let scoped = check_workspace(&ws, Some("HEAD"), &[]).unwrap(); + assert_eq!(scoped.len(), 1); + assert_eq!(scoped[0].at, "src/a.rs > add"); + } + + #[test] + fn no_flags_checks_everything() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + two_diverged_files(root); + let ws = ws_at(root.to_path_buf()); + assert_eq!(check_workspace(&ws, None, &[]).unwrap().len(), 2); + } } diff --git a/surf-cli/src/format.rs b/surf-cli/src/format.rs new file mode 100644 index 0000000..64a80c0 --- /dev/null +++ b/surf-cli/src/format.rs @@ -0,0 +1,9 @@ +//! Output format shared by `check`, `lint`, and `verify`. + +use clap::ValueEnum; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum)] +pub enum Format { + Human, + Json, +} diff --git a/surf-cli/src/lint.rs b/surf-cli/src/lint.rs index b5941bf..0f42562 100644 --- a/surf-cli/src/lint.rs +++ b/surf-cli/src/lint.rs @@ -1,29 +1,42 @@ //! `surf lint` (§9.1.2): every anchor must resolve to exactly one symbol. Ambiguous or //! vanished anchors block; a symbol that was merely renamed (detected via stored-hash -//! match, §6.4) only warns and points at `surf verify --follow`. +//! match, §6.4) only warns and points at `surf verify --follow`. It also emits advisory +//! granularity warnings (§8): anchors that span (nearly) a whole file, hubs with too many +//! anchors, and exported symbols in an anchored file that no claim covers. +use crate::format::Format; use crate::workspace::Workspace; use anyhow::{Context, Result}; +use serde::Serialize; +use std::collections::{HashMap, HashSet}; use std::path::PathBuf; use std::process::ExitCode; -use surf_core::{find_renamed, parse_anchor, parse_hub, resolve, Lang, ResolveError}; +use surf_core::{find_renamed, parse_anchor, parse_hub, public_fns, resolve, Lang, ResolveError}; -#[derive(Debug, PartialEq, Eq)] +/// Over an anchored span this fraction of its file, the anchor is "whole-file-ish" and any +/// edit re-triggers verification — the over-anchoring tension of §8. +const COARSE_SPAN_FRACTION_PCT: usize = 75; +const COARSE_MIN_FILE_LINES: usize = 15; +/// Past this many anchors a hub invites rubber-stamping during a bulk `verify` (§8). +const MAX_ANCHORS_PER_HUB: usize = 12; + +#[derive(Debug, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] pub enum Severity { Block, Warn, } -#[derive(Debug)] +#[derive(Debug, Serialize)] pub struct Finding { pub severity: Severity, pub hub: String, - pub claim: String, pub at: String, pub message: String, + pub claim: String, } -pub fn run(ws: &Workspace) -> Result { +pub fn run(ws: &Workspace, format: Format) -> Result { let findings = lint_workspace(ws)?; let blocks = findings .iter() @@ -31,7 +44,20 @@ pub fn run(ws: &Workspace) -> Result { .count(); let warns = findings.len() - blocks; - for f in &findings { + match format { + Format::Json => println!("{}", serde_json::to_string_pretty(&findings)?), + Format::Human => print_human(&findings, blocks, warns), + } + + Ok(if blocks > 0 { + ExitCode::FAILURE + } else { + ExitCode::SUCCESS + }) +} + +fn print_human(findings: &[Finding], blocks: usize, warns: usize) { + for f in findings { let tag = match f.severity { Severity::Block => "error", Severity::Warn => "warning", @@ -46,12 +72,6 @@ pub fn run(ws: &Workspace) -> Result { } else { println!("surf lint: {blocks} error(s), {warns} warning(s)."); } - - Ok(if blocks > 0 { - ExitCode::FAILURE - } else { - ExitCode::SUCCESS - }) } fn lint_workspace(ws: &Workspace) -> Result> { @@ -79,9 +99,15 @@ fn lint_workspace(ws: &Workspace) -> Result> { } }; + // Per anchored file: the first-segment names a hub's claims cover, and whether every + // anchor in it resolved cleanly. Under-coverage only runs on healthy files — piling + // coverage nags onto a hub with broken anchors would just be noise. + let mut covered: HashMap> = HashMap::new(); + let mut unhealthy: HashSet = HashSet::new(); + for claim in &hub.frontmatter.anchors { for site in claim.at.sites() { - lint_site( + let outcome = lint_site( ws, &rel, &claim.claim, @@ -89,12 +115,47 @@ fn lint_workspace(ws: &Workspace) -> Result> { claim.hash.as_deref(), &mut findings, ); + if let Some(info) = outcome { + if info.resolved { + covered.entry(info.file).or_default().insert(info.first_seg); + } else { + unhealthy.insert(info.file); + } + } + } + } + + if hub.frontmatter.anchors.len() > MAX_ANCHORS_PER_HUB { + findings.push(Finding { + severity: Severity::Warn, + hub: rel.clone(), + claim: String::new(), + at: String::new(), + message: format!( + "{} anchors in one hub (> {MAX_ANCHORS_PER_HUB}) — consider splitting; bulk verify of a long list invites rubber-stamping", + hub.frontmatter.anchors.len() + ), + }); + } + + for (file, covered_names) in &covered { + if unhealthy.contains(file) { + continue; } + lint_under_coverage(ws, &rel, file, covered_names, &mut findings); } } Ok(findings) } +/// What `lint_site` learned about one anchor site: which file/symbol it names and whether it +/// resolved cleanly. `None` when the site can't even be attributed to a file (unparseable). +struct SiteInfo { + file: String, + first_seg: String, + resolved: bool, +} + fn lint_site( ws: &Workspace, hub: &str, @@ -102,7 +163,7 @@ fn lint_site( site: &str, stored_hash: Option<&str>, findings: &mut Vec, -) { +) -> Option { let mut block = |message: String| { findings.push(Finding { severity: Severity::Block, @@ -115,44 +176,129 @@ fn lint_site( let anchor = match parse_anchor(site) { Ok(a) => a, - Err(e) => return block(format!("invalid anchor: {e}")), + Err(e) => { + block(format!("invalid anchor: {e}")); + return None; + } }; + let first_seg = anchor + .segments + .first() + .map(|s| s.name.clone()) + .unwrap_or_default(); + let unresolved = |resolved: bool| { + Some(SiteInfo { + file: anchor.file.clone(), + first_seg: first_seg.clone(), + resolved, + }) + }; + let Some(lang) = Lang::from_path(&anchor.file) else { - return block(format!("unsupported file type: {}", anchor.file)); + block(format!("unsupported file type: {}", anchor.file)); + return unresolved(false); }; let path: PathBuf = ws.root.join(&anchor.file); let source = match std::fs::read_to_string(&path) { Ok(s) => s, Err(_) => { - return block(format!( + block(format!( "cannot read `{}` (file moved or removed?)", anchor.file - )) + )); + return unresolved(false); } }; match resolve(&source, lang, &anchor) { - Ok(_) => {} + Ok(span) => { + lint_coarse_span(hub, claim, site, &anchor.file, &source, span, findings); + unresolved(true) + } Err(ResolveError::Ambiguous { segment, count }) => { - block(format!("`{segment}` is ambiguous ({count} matches); disambiguate with `@N`")); + block(format!( + "`{segment}` is ambiguous ({count} matches); disambiguate with `@N`" + )); + unresolved(false) + } + Err(ResolveError::Parse) => { + block(format!("could not parse `{}`", anchor.file)); + unresolved(false) + } + Err(ResolveError::NotFound { segment }) => { + match stored_hash { + Some(h) => match find_renamed(&source, lang, h) { + Ok(Some(new_name)) => findings.push(Finding { + severity: Severity::Warn, + hub: hub.to_string(), + claim: claim.to_string(), + at: site.to_string(), + message: format!( + "`{segment}` not found, but its code appears to live under `{new_name}` now — run `surf verify --follow`" + ), + }), + Ok(None) => block(format!("`{segment}` not found and no current symbol matches the stored hash — the claim points at nothing")), + Err(e) => block(format!("`{segment}` not found; rename check failed: {e}")), + }, + None => block(format!("`{segment}` not found (claim has no stored hash to match against)")), + } + unresolved(false) + } + } +} + +fn lint_coarse_span( + hub: &str, + claim: &str, + site: &str, + file: &str, + source: &str, + span: surf_core::Span, + findings: &mut Vec, +) { + let span_lines = span.end_line.saturating_sub(span.start_line) + 1; + let file_lines = source.lines().count().max(1); + if file_lines >= COARSE_MIN_FILE_LINES + && span_lines * 100 >= file_lines * COARSE_SPAN_FRACTION_PCT + { + let pct = span_lines * 100 / file_lines; + findings.push(Finding { + severity: Severity::Warn, + hub: hub.to_string(), + claim: claim.to_string(), + at: site.to_string(), + message: format!( + "anchored span covers {pct}% of {file} ({span_lines}/{file_lines} lines) — a near-whole-file anchor re-triggers verification on any edit; point at a narrower symbol" + ), + }); + } +} + +fn lint_under_coverage( + ws: &Workspace, + hub: &str, + file: &str, + covered: &HashSet, + findings: &mut Vec, +) { + let Some(lang) = Lang::from_path(file) else { + return; + }; + let Ok(source) = std::fs::read_to_string(ws.root.join(file)) else { + return; + }; + for sym in public_fns(&source, lang) { + if !covered.contains(&sym) { + findings.push(Finding { + severity: Severity::Warn, + hub: hub.to_string(), + claim: String::new(), + at: format!("{file} > {sym}"), + message: format!( + "public function `{sym}` in {file} has no claim in this hub — add an anchor or accept it as intentionally undocumented" + ), + }); } - Err(ResolveError::Parse) => block(format!("could not parse `{}`", anchor.file)), - Err(ResolveError::NotFound { segment }) => match stored_hash { - Some(h) => match find_renamed(&source, lang, h) { - Ok(Some(new_name)) => findings.push(Finding { - severity: Severity::Warn, - hub: hub.to_string(), - claim: claim.to_string(), - at: site.to_string(), - message: format!( - "`{segment}` not found, but its code appears to live under `{new_name}` now — run `surf verify --follow`" - ), - }), - Ok(None) => block(format!("`{segment}` not found and no current symbol matches the stored hash — the claim points at nothing")), - Err(e) => block(format!("`{segment}` not found; rename check failed: {e}")), - }, - None => block(format!("`{segment}` not found (claim has no stored hash to match against)")), - }, } } @@ -237,6 +383,24 @@ mod tests { assert_eq!(f[0].severity, Severity::Block); } + #[test] + fn findings_serialize_with_expected_keys() { + let (_t, ws) = ws_with(&[ + ("src/auth.rs", "pub fn greet() {}\n"), + ( + "hubs/a.md", + "---\nsummary: x\nanchors:\n - claim: ghost\n at: src/auth.rs > ghost\n---\n", + ), + ]); + let findings = lint_workspace(&ws).unwrap(); + let json = serde_json::to_value(&findings).unwrap(); + let obj = json[0].as_object().unwrap(); + for key in ["severity", "hub", "at", "message", "claim"] { + assert!(obj.contains_key(key), "missing key `{key}` in {obj:?}"); + } + assert_eq!(obj["severity"], "block"); + } + #[test] fn renamed_symbol_warns_and_suggests_follow() { let new_src = "pub fn rotate_token(t: &str) -> String { t.to_string() }\n"; @@ -252,4 +416,74 @@ mod tests { assert!(f[0].message.contains("rotate_token")); assert!(f[0].message.contains("--follow")); } + + #[test] + fn under_coverage_warns_for_unanchored_export() { + let (_t, ws) = ws_with(&[ + ( + "src/m.rs", + "pub fn a() {}\npub fn b() {}\nfn private() {}\n", + ), + ( + "hubs/a.md", + "---\nsummary: x\nanchors:\n - claim: a does\n at: src/m.rs > a\n---\n", + ), + ]); + let f = lint_workspace(&ws).unwrap(); + // Only the exported-but-unanchored `b`; the private fn and the covered `a` are silent. + assert_eq!(f.len(), 1); + assert_eq!(f[0].severity, Severity::Warn); + assert!(f[0].message.contains("`b`"), "{}", f[0].message); + } + + #[test] + fn broken_anchor_suppresses_under_coverage() { + // `ghost` blocks, so the file is unhealthy and `b` is NOT additionally flagged. + let (_t, ws) = ws_with(&[ + ("src/m.rs", "pub fn a() {}\npub fn b() {}\n"), + ( + "hubs/a.md", + "---\nsummary: x\nanchors:\n - claim: c\n at: src/m.rs > ghost\n---\n", + ), + ]); + let f = lint_workspace(&ws).unwrap(); + assert_eq!(f.len(), 1); + assert_eq!(f[0].severity, Severity::Block); + } + + #[test] + fn coarse_span_warns_on_whole_file_anchor() { + let body: String = (0..40).map(|i| format!(" let x{i} = {i};\n")).collect(); + let src = format!("pub fn big() {{\n{body}}}\n"); + let (_t, ws) = ws_with(&[ + ("src/m.rs", src.as_str()), + ( + "hubs/a.md", + "---\nsummary: x\nanchors:\n - claim: big does\n at: src/m.rs > big\n---\n", + ), + ]); + let f = lint_workspace(&ws).unwrap(); + assert_eq!(f.len(), 1); + assert_eq!(f[0].severity, Severity::Warn); + assert!(f[0].message.contains("whole-file"), "{}", f[0].message); + } + + #[test] + fn too_many_anchors_warns() { + let mut src = String::new(); + let mut anchors = String::new(); + for i in 0..=MAX_ANCHORS_PER_HUB { + src.push_str(&format!("pub fn f{i}() {{}}\n")); + anchors.push_str(&format!(" - claim: c{i}\n at: src/m.rs > f{i}\n")); + } + let hub = format!("---\nsummary: x\nanchors:\n{anchors}---\n"); + let (_t, ws) = ws_with(&[("src/m.rs", src.as_str()), ("hubs/a.md", hub.as_str())]); + + let f = lint_workspace(&ws).unwrap(); + assert!( + f.iter() + .any(|x| x.severity == Severity::Warn && x.message.contains("anchors in one hub")), + "expected a too-many-anchors warning, got {f:?}" + ); + } } diff --git a/surf-cli/src/main.rs b/surf-cli/src/main.rs index 059ff41..edb5c81 100644 --- a/surf-cli/src/main.rs +++ b/surf-cli/src/main.rs @@ -1,13 +1,14 @@ use clap::{Parser, Subcommand}; mod check; +mod format; mod init; mod lint; mod new; mod verify; mod workspace; -use check::Format; +use format::Format; use workspace::Workspace; const SCOPE_DISCLAIMER: &str = "\ @@ -37,15 +38,24 @@ enum Command { name: String, }, /// Validate hub frontmatter and that every anchor resolves to exactly one symbol. - Lint, + Lint { + /// Output format for the findings. + #[arg(long, value_enum, default_value_t = Format::Human)] + format: Format, + }, /// The gate: hash each anchored span and block on any documented span that diverged. Check { /// Output format for the divergence report. #[arg(long, value_enum, default_value_t = Format::Human)] format: Format, - /// Git ref to recover previous code from for advisory old_code/magnitude. - #[arg(long, default_value = "HEAD")] - base: String, + /// Git ref to diff against: scopes the check to claims whose files changed since the + /// merge base, and recovers previous code for advisory old_code/magnitude. Omit for a + /// full check (enrichment falls back to HEAD). + #[arg(long)] + base: Option, + /// Only evaluate claims whose anchored file(s) match one of these globs. + #[arg(long, value_delimiter = ',')] + files: Vec, }, /// Re-hash an anchor after a human confirms the prose still holds. Verify { @@ -54,6 +64,9 @@ enum Command { /// Re-point a renamed single-segment anchor to its new symbol, then re-hash. #[arg(long)] follow: bool, + /// Output format for the verify report. + #[arg(long, value_enum, default_value_t = Format::Human)] + format: Format, }, } @@ -80,8 +93,16 @@ fn run() -> anyhow::Result { match cli.command { Command::Init => unreachable!("handled before discovery"), Command::New { name } => new::run(&ws, &name), - Command::Lint => lint::run(&ws), - Command::Check { format, base } => check::run(&ws, format, &base), - Command::Verify { target, follow } => verify::run(&ws, target.as_deref(), follow), + Command::Lint { format } => lint::run(&ws, format), + Command::Check { + format, + base, + files, + } => check::run(&ws, format, base.as_deref(), &files), + Command::Verify { + target, + follow, + format, + } => verify::run(&ws, target.as_deref(), follow, format), } } diff --git a/surf-cli/src/verify.rs b/surf-cli/src/verify.rs index 7d02ecd..638efb3 100644 --- a/surf-cli/src/verify.rs +++ b/surf-cli/src/verify.rs @@ -4,8 +4,10 @@ //! step (§6.4). Writes are surgical (only the touched line changes) and skipped entirely //! when nothing changed, so a no-op verify leaves the file byte-identical. +use crate::format::Format; use crate::workspace::{read_site, Workspace}; use anyhow::{Context, Result}; +use serde::Serialize; use std::process::ExitCode; use surf_core::{ combine_site_hashes, find_renamed, hash_anchor, parse_anchor, parse_hub, set_anchor_at, @@ -19,35 +21,71 @@ enum Plan { Skip(String), } -#[derive(Default)] -struct Summary { +#[derive(Debug, Clone, Serialize)] +#[serde(tag = "outcome", rename_all = "snake_case")] +enum VerifyOutcome { + Stamped, + Followed { new_at: String }, + Unchanged, + Skipped { reason: String }, +} + +#[derive(Debug, Clone, Serialize)] +struct AnchorResult { + hub: String, + at: String, + #[serde(flatten)] + outcome: VerifyOutcome, +} + +#[derive(Debug, Default, Serialize)] +struct VerifyReport { stamped: usize, unchanged: usize, - errors: Vec, + errors: usize, + anchors: Vec, + #[serde(skip)] + updated_files: Vec, } -pub fn run(ws: &Workspace, target: Option<&str>, follow: bool) -> Result { - let summary = verify_all(ws, target, follow)?; +pub fn run(ws: &Workspace, target: Option<&str>, follow: bool, format: Format) -> Result { + let report = verify_all(ws, target, follow)?; - for e in &summary.errors { - println!("error {e}"); + match format { + Format::Json => println!("{}", serde_json::to_string_pretty(&report)?), + Format::Human => print_human(&report), } - println!( - "surf verify: stamped {} anchor(s), {} skipped, {} error(s).", - summary.stamped, - summary.unchanged, - summary.errors.len() - ); - Ok(if summary.errors.is_empty() { + Ok(if report.errors == 0 { ExitCode::SUCCESS } else { ExitCode::FAILURE }) } -fn verify_all(ws: &Workspace, target: Option<&str>, follow: bool) -> Result { - let mut summary = Summary::default(); +fn print_human(report: &VerifyReport) { + for a in &report.anchors { + match &a.outcome { + VerifyOutcome::Followed { new_at } => { + println!("followed {} :: {} → {new_at}", a.hub, a.at) + } + VerifyOutcome::Skipped { reason } => { + println!("error {} :: {} ({reason})", a.hub, a.at) + } + _ => {} + } + } + for f in &report.updated_files { + println!("updated {f}"); + } + println!( + "surf verify: stamped {} anchor(s), {} skipped, {} error(s).", + report.stamped, report.unchanged, report.errors + ); +} + +fn verify_all(ws: &Workspace, target: Option<&str>, follow: bool) -> Result { + let mut report = VerifyReport::default(); let mut matched_any = false; for hub_path in ws.hub_paths()? { @@ -71,17 +109,21 @@ fn verify_all(ws: &Workspace, target: Option<&str>, follow: bool) -> Result match set_anchor_hash(&text, idx, &new_hash) { Some(updated) => { text = updated; - summary.stamped += 1; + report.stamped += 1; + VerifyOutcome::Stamped + } + None => { + report.errors += 1; + VerifyOutcome::Skipped { + reason: "could not write hash".into(), + } } - None => summary - .errors - .push(format!("{label} (could not write hash)")), }, Plan::Follow { new_at, new_hash } => { match set_anchor_at(&text, idx, &new_at) @@ -89,23 +131,38 @@ fn verify_all(ws: &Workspace, target: Option<&str>, follow: bool) -> Result { text = updated; - summary.stamped += 1; - println!("followed {label} → {new_at}"); + report.stamped += 1; + VerifyOutcome::Followed { new_at } + } + None => { + report.errors += 1; + VerifyOutcome::Skipped { + reason: "could not rewrite at:".into(), + } } - None => summary - .errors - .push(format!("{label} (could not rewrite at:)")), } } - Plan::Unchanged => summary.unchanged += 1, - Plan::Skip(reason) => summary.errors.push(format!("{label} ({reason})")), - } + Plan::Unchanged => { + report.unchanged += 1; + VerifyOutcome::Unchanged + } + Plan::Skip(reason) => { + report.errors += 1; + VerifyOutcome::Skipped { reason } + } + }; + + report.anchors.push(AnchorResult { + hub: rel.clone(), + at, + outcome, + }); } if text != original { std::fs::write(&hub_path, &text) .with_context(|| format!("writing {}", hub_path.display()))?; - println!("updated {rel}"); + report.updated_files.push(rel); } } @@ -115,7 +172,7 @@ fn verify_all(ws: &Workspace, target: Option<&str>, follow: bool) -> Result Plan { @@ -215,7 +272,7 @@ mod tests { ); let ws = Workspace::discover(root).unwrap(); - run(&ws, None, false).unwrap(); + run(&ws, None, false, Format::Human).unwrap(); // Hash now present and equals the canonical hash of the symbol. let after = fs::read_to_string(root.join("hubs/a.md")).unwrap(); @@ -232,10 +289,11 @@ mod tests { ); // Second verify is a no-op: byte-identical, and reported as skipped not stamped. - let summary = verify_all(&ws, None, false).unwrap(); - assert_eq!(summary.stamped, 0); - assert_eq!(summary.unchanged, 1); - assert!(summary.errors.is_empty()); + let report = verify_all(&ws, None, false).unwrap(); + assert_eq!(report.stamped, 0); + assert_eq!(report.unchanged, 1); + assert_eq!(report.errors, 0); + assert!(report.updated_files.is_empty()); assert_eq!(fs::read_to_string(root.join("hubs/a.md")).unwrap(), after); } @@ -259,7 +317,7 @@ mod tests { ); let ws = Workspace::discover(root).unwrap(); - let code = run(&ws, None, true).unwrap(); + let code = run(&ws, None, true, Format::Human).unwrap(); assert_eq!(code, ExitCode::SUCCESS); let hub = parse_hub(&fs::read_to_string(root.join("hubs/a.md")).unwrap()).unwrap(); @@ -286,10 +344,45 @@ mod tests { ); let ws = Workspace::discover(root).unwrap(); - let code = run(&ws, None, false).unwrap(); + let code = run(&ws, None, false, Format::Human).unwrap(); assert_eq!(code, ExitCode::FAILURE); // Unchanged: no hash written. let hub = parse_hub(&fs::read_to_string(root.join("hubs/a.md")).unwrap()).unwrap(); assert_eq!(hub.frontmatter.anchors[0].hash, None); } + + #[test] + fn report_serializes_and_stamps_side_effect() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + write(root, "surf.toml", ""); + write( + root, + "src/m.rs", + "pub fn add(a: i64, b: i64) -> i64 { a + b }\n", + ); + write( + root, + "hubs/a.md", + "---\nsummary: s\nanchors:\n - claim: add sums\n at: src/m.rs > add\n---\n", + ); + + let ws = Workspace::discover(root).unwrap(); + let report = verify_all(&ws, None, false).unwrap(); + + let json = serde_json::to_value(&report).unwrap(); + assert_eq!(json["stamped"], 1); + let anchor = json["anchors"][0].as_object().unwrap(); + for key in ["hub", "at", "outcome"] { + assert!( + anchor.contains_key(key), + "missing key `{key}` in {anchor:?}" + ); + } + assert_eq!(anchor["outcome"], "stamped"); + + // Side effect intact: the hub file was stamped. + let hub = parse_hub(&fs::read_to_string(root.join("hubs/a.md")).unwrap()).unwrap(); + assert!(hub.frontmatter.anchors[0].hash.is_some()); + } } diff --git a/surf-core/src/lib.rs b/surf-core/src/lib.rs index a6b9f9c..3588d94 100644 --- a/surf-core/src/lib.rs +++ b/surf-core/src/lib.rs @@ -16,6 +16,6 @@ pub use hub::{parse_hub, set_anchor_at, set_anchor_hash, At, Claim, Frontmatter, pub use lang::Lang; pub use rename::find_renamed; pub use report::{Divergence, DivergenceKind}; -pub use resolve::{resolve, ResolveError, Span}; +pub use resolve::{public_fns, resolve, ResolveError, Span}; pub const VERSION: &str = env!("CARGO_PKG_VERSION"); diff --git a/surf-core/src/resolve.rs b/surf-core/src/resolve.rs index f635aa9..c062b1d 100644 --- a/surf-core/src/resolve.rs +++ b/surf-core/src/resolve.rs @@ -244,6 +244,108 @@ pub(crate) fn collect_all_defs<'a>( } } +/// Top-level public *functions* on a file's surface: unrestricted `pub fn` in Rust, +/// `export`ed functions in TS/JS, non-underscore `def`s in Python, capitalized `func`s in Go. +/// Advisory input to lint's under-coverage warning. Deliberately functions only — a claim +/// documents *behavior* that can drift, so pure data types (structs/enums/consts) are out of +/// scope to avoid the over-anchoring fatigue of §8. Shallow (top-level) and best-effort. +pub fn public_fns(source: &str, lang: Lang) -> Vec { + let Some(tree) = parse_tree(source, lang) else { + return Vec::new(); + }; + let src = source.as_bytes(); + let family = lang.family(); + let mut out = Vec::new(); + let root = tree.root_node(); + let mut cursor = root.walk(); + for child in root.named_children(&mut cursor) { + collect_public_fn(child, src, family, &mut out); + } + out.sort(); + out.dedup(); + out +} + +fn collect_public_fn(node: Node, src: &[u8], family: Family, out: &mut Vec) { + match family { + Family::Rust => { + if node.kind() == "function_item" && is_rust_pub(node, src) { + if let Some(name) = field_text(node, "name", src) { + out.push(name.to_string()); + } + } + } + Family::TypeScript => { + if node.kind() == "export_statement" { + let mut cursor = node.walk(); + for c in node.named_children(&mut cursor) { + ts_collect_export_fns(c, src, out); + } + } + } + Family::Python => collect_python_fn(node, src, out), + Family::Go => { + if node.kind() == "function_declaration" { + if let Some(name) = field_text(node, "name", src) { + if name.chars().next().is_some_and(char::is_uppercase) { + out.push(name.to_string()); + } + } + } + } + } +} + +/// True only for unrestricted `pub` — `pub(crate)`/`pub(super)`/`pub(in …)` are internal and +/// not part of the file's outward surface. +fn is_rust_pub(node: Node, src: &[u8]) -> bool { + let mut cursor = node.walk(); + let is_pub = node + .children(&mut cursor) + .filter(|c| c.kind() == "visibility_modifier") + .any(|c| c.utf8_text(src).map(str::trim) == Ok("pub")); + is_pub +} + +fn ts_collect_export_fns(node: Node, src: &[u8], out: &mut Vec) { + match node.kind() { + "function_declaration" | "generator_function_declaration" | "function_signature" => { + if let Some(name) = field_text(node, "name", src) { + out.push(name.to_string()); + } + } + // `export const f = () => {}` — only the function-valued declarators. + "lexical_declaration" | "variable_declaration" => { + let mut cursor = node.walk(); + for c in node.named_children(&mut cursor) { + if let Some(name) = ts_def_name(c, src) { + out.push(name); + } + } + } + _ => {} + } +} + +fn collect_python_fn(node: Node, src: &[u8], out: &mut Vec) { + match node.kind() { + "function_definition" => { + if let Some(name) = python_def_name(node, src) { + if !name.starts_with('_') { + out.push(name); + } + } + } + "decorated_definition" => { + let mut cursor = node.walk(); + for c in node.named_children(&mut cursor) { + collect_python_fn(c, src, out); + } + } + _ => {} + } +} + fn field_text<'a>(node: Node, field: &str, src: &'a [u8]) -> Option<&'a str> { node.child_by_field_name(field)?.utf8_text(src).ok() } @@ -344,3 +446,40 @@ fn scope_of(node: Node, family: Family) -> Node { _ => node.child_by_field_name("body").unwrap_or(node), } } + +#[cfg(test)] +mod tests { + use super::*; + + fn syms(source: &str, lang: Lang) -> Vec { + public_fns(source, lang) + } + + #[test] + fn rust_only_unrestricted_pub_fns() { + // Functions only; private, `pub(crate)`, and non-fn items (struct/enum/impl) excluded. + let src = "pub fn a() {}\nfn b() {}\npub(crate) fn c() {}\npub struct S;\nenum E {}\nimpl S { pub fn m(&self) {} }\n"; + assert_eq!(syms(src, Lang::Rust), vec!["a".to_string()]); + } + + #[test] + fn ts_only_exported_fns() { + let src = "export function a() {}\nfunction b() {}\nexport const c = () => {};\nexport class D {}\nexport const e = 1;\n"; + assert_eq!( + syms(src, Lang::TypeScript), + vec!["a".to_string(), "c".to_string()] + ); + } + + #[test] + fn python_only_nonunderscore_defs() { + let src = "def a():\n pass\ndef _b():\n pass\nclass C:\n pass\n"; + assert_eq!(syms(src, Lang::Python), vec!["a".to_string()]); + } + + #[test] + fn go_only_capitalized_funcs() { + let src = "package p\nfunc Exported() {}\nfunc unexported() {}\ntype Foo struct{}\n"; + assert_eq!(syms(src, Lang::Go), vec!["Exported".to_string()]); + } +}