diff --git a/docs/reference/commands.md b/docs/reference/commands.md index 11c2011..87a5e4f 100644 --- a/docs/reference/commands.md +++ b/docs/reference/commands.md @@ -6,10 +6,14 @@ description: The surf CLI — init, new, suggest, lint, check, and verify, with - **`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 suggest [--format human|json]`** — scan source globs for top-level public - *functions* no hub anchors yet and print a copy-pasteable starter hub. Suggestions only — never - writes or stamps. (Suggestions are functions-only to avoid over-anchoring fatigue; anchoring - itself also supports constants, type aliases, and class attributes — see +- **`surf suggest [--format human|json]`** — scan source globs for public *callables* no + hub anchors yet — top-level functions plus **Python class methods and Go methods** (as + `file > Type > method` anchors) — and print a copy-pasteable starter hub. Suggestions only — + never writes or stamps. Coverage is keyed on the whole anchor path, so anchoring one method + doesn't suppress its siblings. A glob that matches **no files** is reported on stderr (so a typo + doesn't read as a clean "all anchored"); `suggest` exits non-zero only when *every* glob was + empty. (Suggestions are callables-only to avoid over-anchoring fatigue; anchoring itself also + supports constants, type aliases, and class attributes — see [Authoring hubs](../guides/authoring-hubs.md).) - **`surf lint [--format human|json]`** — validate frontmatter and that every `at:` resolves to exactly one symbol. Blocks on ambiguous or vanished anchors; **warns** (and suggests diff --git a/hubs/cli-suggest.md b/hubs/cli-suggest.md index 4ef3ef8..b8d4ebd 100644 --- a/hubs/cli-suggest.md +++ b/hubs/cli-suggest.md @@ -1,18 +1,22 @@ --- -summary: surf suggest — propose anchors for unanchored public functions; read-only, never stamps. +summary: surf suggest — propose anchors for unanchored public symbols; read-only, never stamps. anchors: - claim: > - surf suggest is read-only: run scans the given globs, lists each top-level public function - no hub already anchors, and prints them (a starter hub in human mode, or JSON). It never - writes a file and never computes or stamps a hash — the author edits the claims and verifies. + surf suggest is read-only: run scans the given globs, lists each public symbol (top-level + function, plus Python/Go methods) no hub already anchors, and prints them (a starter hub in + human mode, or JSON). It warns on stderr for any glob that matched no files, and exits + non-zero only when every glob was empty. It never writes a file and never computes or stamps + a hash — the author edits the claims and verifies. at: surf-cli/src/suggest.rs > run - hash: cf02ef9af242 + hash: 9f907f6299ff refs: [] --- # surf suggest Lowers the §8 authoring cost. `run` reads existing hub coverage, scans the requested source -globs via `scan` (which reuses `surf_core::public_fns` and skips already-anchored symbols), and -prints suggestions. Suggestions only: the author turns them into real claims and runs +globs via `scan` (which reuses `surf_core::public_symbols` and skips already-anchored symbols, +keyed on the full `file > seg > seg` anchor path so anchoring one method doesn't hide its +siblings), and prints suggestions. Per-glob match tallies let a typo'd glob read differently from +a clean "all anchored". Suggestions only: the author turns them into real claims and runs `surf verify`. diff --git a/surf-cli/src/suggest.rs b/surf-cli/src/suggest.rs index 52d3d8a..b8b06e3 100644 --- a/surf-cli/src/suggest.rs +++ b/surf-cli/src/suggest.rs @@ -1,7 +1,8 @@ -//! `surf suggest ` — propose anchors for public functions no hub covers yet (§8, #18). -//! Scans the given source files, lists each top-level public function that isn't already +//! `surf suggest ` — propose anchors for public symbols no hub covers yet (§8, #18). +//! Scans the given source files, lists each public function and method that isn't already //! anchored, and prints a copy-pasteable starter hub. Suggestions only: it never writes a file -//! and never stamps a hash — the author edits the claims and runs `surf verify`. +//! and never stamps a hash — the author edits the claims and runs `surf verify`. A glob that +//! matches no files is reported (and fails when every glob is empty) so typos don't look clean. use crate::format::Format; use crate::workspace::Workspace; @@ -9,7 +10,7 @@ use anyhow::{Context, Result}; use serde::Serialize; use std::collections::HashSet; use std::process::ExitCode; -use surf_core::{parse_anchor, parse_hub, public_fns, Lang}; +use surf_core::{parse_anchor, parse_hub, public_symbols, Lang}; #[derive(Debug, Clone, Serialize)] struct Suggestion { @@ -18,20 +19,51 @@ struct Suggestion { at: String, } +/// Per-glob tally so a typo'd glob (zero files) reads differently from a clean "all anchored". +struct GlobReport { + pattern: String, + files_matched: usize, + supported_matched: usize, +} + +struct ScanResult { + suggestions: Vec, + globs: Vec, +} + pub fn run(ws: &Workspace, globs: &[String], format: Format) -> Result { - let covered = covered_symbols(ws)?; - let suggestions = scan(ws, globs, &covered)?; + let covered = covered_anchors(ws)?; + let result = scan(ws, globs, &covered)?; match format { - Format::Json => println!("{}", serde_json::to_string_pretty(&suggestions)?), - Format::Human => print_human(&suggestions), + Format::Json => println!("{}", serde_json::to_string_pretty(&result.suggestions)?), + Format::Human => print_human(&result.suggestions), } - Ok(ExitCode::SUCCESS) + // Warnings go to stderr so JSON stdout stays machine-parseable. + for g in &result.globs { + if g.files_matched == 0 { + eprintln!("surf suggest: glob \"{}\" matched no files.", g.pattern); + } else if g.supported_matched == 0 { + eprintln!( + "surf suggest: glob \"{}\" matched files, but none in a supported language.", + g.pattern + ); + } + } + + // A typo'd glob should fail scripts/CI; but only when *every* glob matched nothing, so a + // partially-correct invocation still succeeds. + let all_empty = !result.globs.is_empty() && result.globs.iter().all(|g| g.files_matched == 0); + Ok(if all_empty { + ExitCode::FAILURE + } else { + ExitCode::SUCCESS + }) } -/// `(file, first-segment)` pairs already anchored by some hub — the same coverage notion lint's -/// under-coverage check uses. A public fn matching one of these is already documented. -fn covered_symbols(ws: &Workspace) -> Result> { +/// Full anchor paths already covered by some hub, normalized to `file > seg > seg` (positional +/// `@N` dropped). Keyed on the whole path so anchoring one method doesn't hide its siblings. +fn covered_anchors(ws: &Workspace) -> Result> { let mut covered = HashSet::new(); for hub_path in ws.hub_paths()? { let content = std::fs::read_to_string(&hub_path) @@ -42,9 +74,8 @@ fn covered_symbols(ws: &Workspace) -> Result> { for claim in &hub.frontmatter.anchors { for site in claim.at.sites() { if let Ok(anchor) = parse_anchor(site) { - if let Some(seg) = anchor.segments.first() { - covered.insert((anchor.file, seg.name.clone())); - } + let path: Vec<&str> = anchor.segments.iter().map(|s| s.name.as_str()).collect(); + covered.insert(format!("{} > {}", anchor.file, path.join(" > "))); } } } @@ -52,22 +83,22 @@ fn covered_symbols(ws: &Workspace) -> Result> { Ok(covered) } -fn scan( - ws: &Workspace, - globs: &[String], - covered: &HashSet<(String, String)>, -) -> Result> { +fn scan(ws: &Workspace, globs: &[String], covered: &HashSet) -> Result { let mut out = Vec::new(); + let mut reports = Vec::new(); for pattern in globs { let joined = ws.root.join(pattern); - let pattern = joined + let glob_str = joined .to_str() .with_context(|| format!("glob is not valid UTF-8: {}", joined.display()))?; - for entry in glob::glob(pattern).context("invalid glob pattern")? { + let mut files_matched = 0; + let mut supported_matched = 0; + for entry in glob::glob(glob_str).context("invalid glob pattern")? { let path = entry?; if !path.is_file() { continue; } + files_matched += 1; let rel = path .strip_prefix(&ws.root) .unwrap_or(&path) @@ -79,11 +110,13 @@ fn scan( let Ok(source) = std::fs::read_to_string(&path) else { continue; }; - for symbol in public_fns(&source, lang) { - if covered.contains(&(rel.clone(), symbol.clone())) { + supported_matched += 1; + for segments in public_symbols(&source, lang) { + let at = format!("{rel} > {}", segments.join(" > ")); + if covered.contains(&at) { continue; } - let at = format!("{rel} > {symbol}"); + let symbol = segments.last().cloned().unwrap_or_default(); out.push(Suggestion { file: rel.clone(), symbol, @@ -91,19 +124,27 @@ fn scan( }); } } + reports.push(GlobReport { + pattern: pattern.clone(), + files_matched, + supported_matched, + }); } - out.sort_by(|a, b| (&a.file, &a.symbol).cmp(&(&b.file, &b.symbol))); + out.sort_by(|a, b| a.at.cmp(&b.at)); out.dedup_by(|a, b| a.at == b.at); - Ok(out) + Ok(ScanResult { + suggestions: out, + globs: reports, + }) } fn print_human(suggestions: &[Suggestion]) { if suggestions.is_empty() { - println!("surf suggest: no unanchored public functions found."); + println!("surf suggest: no unanchored public symbols found."); return; } println!( - "# {} unanchored public function(s). Paste into a hub (or `surf new `), write the", + "# {} unanchored public symbol(s). Paste into a hub (or `surf new `), write the", suggestions.len() ); println!( @@ -153,8 +194,10 @@ mod tests { "---\nsummary: x\nanchors:\n - claim: a does\n at: src/m.rs > a\n---\n", ), ]); - let covered = covered_symbols(&ws).unwrap(); - let s = scan(&ws, &["src/*.rs".to_string()], &covered).unwrap(); + let covered = covered_anchors(&ws).unwrap(); + let s = scan(&ws, &["src/*.rs".to_string()], &covered) + .unwrap() + .suggestions; // `a` is anchored, `c` is private — only `b` remains. assert_eq!(s.len(), 1); assert_eq!(s[0].at, "src/m.rs > b"); @@ -164,16 +207,20 @@ mod tests { #[test] fn unsupported_files_are_skipped() { let (_t, ws) = ws_with(&[("notes.txt", "pub fn a() {}\n")]); - let covered = covered_symbols(&ws).unwrap(); - let s = scan(&ws, &["*.txt".to_string()], &covered).unwrap(); + let covered = covered_anchors(&ws).unwrap(); + let s = scan(&ws, &["*.txt".to_string()], &covered) + .unwrap() + .suggestions; assert!(s.is_empty()); } #[test] fn json_shape_has_no_hash() { let (_t, ws) = ws_with(&[("src/m.rs", "pub fn solo() {}\n")]); - let covered = covered_symbols(&ws).unwrap(); - let s = scan(&ws, &["src/*.rs".to_string()], &covered).unwrap(); + let covered = covered_anchors(&ws).unwrap(); + let s = scan(&ws, &["src/*.rs".to_string()], &covered) + .unwrap() + .suggestions; let json = serde_json::to_value(&s).unwrap(); let obj = json[0].as_object().unwrap(); for key in ["file", "symbol", "at"] { @@ -181,4 +228,78 @@ mod tests { } assert!(!obj.contains_key("hash")); } + + #[test] + fn proposes_python_methods_as_class_method_anchors() { + let (_t, ws) = ws_with(&[( + "src/api.py", + "class Client:\n def fetch(self):\n pass\n def send(self):\n pass\n", + )]); + let covered = covered_anchors(&ws).unwrap(); + let s = scan(&ws, &["src/*.py".to_string()], &covered) + .unwrap() + .suggestions; + let ats: Vec<&str> = s.iter().map(|x| x.at.as_str()).collect(); + assert_eq!( + ats, + vec!["src/api.py > Client > fetch", "src/api.py > Client > send"] + ); + } + + #[test] + fn anchoring_one_method_does_not_hide_siblings() { + let (_t, ws) = ws_with(&[ + ( + "src/api.py", + "class Client:\n def fetch(self):\n pass\n def send(self):\n pass\n", + ), + ( + "hubs/a.md", + "---\nsummary: x\nanchors:\n - claim: c\n at: src/api.py > Client > fetch\n---\n", + ), + ]); + let covered = covered_anchors(&ws).unwrap(); + let s = scan(&ws, &["src/*.py".to_string()], &covered) + .unwrap() + .suggestions; + // `fetch` is anchored; only `send` should remain. + assert_eq!(s.len(), 1); + assert_eq!(s[0].at, "src/api.py > Client > send"); + } + + #[test] + fn zero_match_glob_is_reported_and_fails_alone() { + let (_t, ws) = ws_with(&[("src/m.rs", "pub fn a() {}\n")]); + let r = scan( + &ws, + &["zzz/nope/**/*.go".to_string()], + &covered_anchors(&ws).unwrap(), + ) + .unwrap(); + assert_eq!(r.globs.len(), 1); + assert_eq!(r.globs[0].files_matched, 0); + assert!(r.suggestions.is_empty()); + let code = run(&ws, &["zzz/nope/**/*.go".to_string()], Format::Human).unwrap(); + assert_eq!(format!("{code:?}"), format!("{:?}", ExitCode::FAILURE)); + } + + #[test] + fn partial_match_still_succeeds() { + let (_t, ws) = ws_with(&[("src/m.rs", "pub fn a() {}\n")]); + let code = run( + &ws, + &["src/*.rs".to_string(), "zzz/nope/*.go".to_string()], + Format::Human, + ) + .unwrap(); + assert_eq!(format!("{code:?}"), format!("{:?}", ExitCode::SUCCESS)); + } + + #[test] + fn unsupported_language_match_is_distinguished() { + let (_t, ws) = ws_with(&[("notes.txt", "hello\n")]); + let r = scan(&ws, &["*.txt".to_string()], &covered_anchors(&ws).unwrap()).unwrap(); + assert_eq!(r.globs[0].files_matched, 1); + assert_eq!(r.globs[0].supported_matched, 0); + } } diff --git a/surf-core/src/lib.rs b/surf-core/src/lib.rs index 1eb0170..9cfc2fd 100644 --- a/surf-core/src/lib.rs +++ b/surf-core/src/lib.rs @@ -18,6 +18,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::{CheckReport, Divergence, DivergenceKind, REPORT_VERSION}; -pub use resolve::{public_fns, resolve, ResolveError, Span}; +pub use resolve::{public_fns, public_symbols, 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 b339ad7..17aa6e9 100644 --- a/surf-core/src/resolve.rs +++ b/surf-core/src/resolve.rs @@ -282,6 +282,112 @@ pub fn public_fns(source: &str, lang: Lang) -> Vec { out } +/// Public symbols on a file's surface as resolvable anchor segment paths: top-level functions +/// (`["foo"]`), and — unlike [`public_fns`] — the methods that make up most of a Python/Go API +/// (`["Builder", "Set"]`). Same behavior-not-data scope as `public_fns`, so pure types +/// (structs/enums/classes themselves) are never proposed — only their methods. Methods on Rust +/// `impl` blocks and TS classes are out of scope here; only top-level fns are enumerated for them. +pub fn public_symbols(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_symbol(child, src, family, &mut out); + } + out.sort(); + out.dedup(); + out +} + +fn collect_public_symbol(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(vec![name.to_string()]); + } + } + } + Family::TypeScript => { + if node.kind() == "export_statement" { + let mut names = Vec::new(); + let mut cursor = node.walk(); + for c in node.named_children(&mut cursor) { + ts_collect_export_fns(c, src, &mut names); + } + out.extend(names.into_iter().map(|n| vec![n])); + } + } + Family::Python => collect_python_symbol(node, src, None, out), + Family::Go => match 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(vec![name.to_string()]); + } + } + } + // Methods are flat in Go; pair the method with its receiver type so the anchor + // resolves as `file.go > Type > Method`. Both must be exported to be public surface. + "method_declaration" => { + if let (Some(name), Some(ty)) = + (field_text(node, "name", src), go_receiver_type(node, src)) + { + if name.chars().next().is_some_and(char::is_uppercase) + && ty.chars().next().is_some_and(char::is_uppercase) + { + out.push(vec![ty.to_string(), name.to_string()]); + } + } + } + _ => {} + }, + } +} + +/// `class` given: we're inside that class body, so non-underscore `def`s become +/// `[Class, method]`. `None`: top level, so functions become `[fn]` and we descend one level +/// into public classes to surface their methods (nested classes are not recursed into). +fn collect_python_symbol(node: Node, src: &[u8], class: Option<&str>, out: &mut Vec>) { + match node.kind() { + "function_definition" => { + if let Some(name) = python_def_name(node, src) { + if !name.starts_with('_') { + let mut path: Vec = class.into_iter().map(str::to_string).collect(); + path.push(name); + out.push(path); + } + } + } + "class_definition" if class.is_none() => { + let Some(name) = python_def_name(node, src) else { + return; + }; + if name.starts_with('_') { + return; + } + if let Some(body) = node.child_by_field_name("body") { + let mut cursor = body.walk(); + for c in body.named_children(&mut cursor) { + collect_python_symbol(c, src, Some(&name), out); + } + } + } + "decorated_definition" => { + let mut cursor = node.walk(); + for c in node.named_children(&mut cursor) { + collect_python_symbol(c, src, class, out); + } + } + _ => {} + } +} + fn collect_public_fn(node: Node, src: &[u8], family: Family, out: &mut Vec) { match family { Family::Rust => { @@ -517,4 +623,75 @@ mod tests { let src = "package p\nfunc Exported() {}\nfunc unexported() {}\ntype Foo struct{}\n"; assert_eq!(syms(src, Lang::Go), vec!["Exported".to_string()]); } + + #[test] + fn python_symbols_include_class_methods() { + let src = "\ +def top(): + pass +class C: + def visible(self): + pass + async def visible(self): # sync/async mirror dedupes + pass + def _private(self): + pass + def __init__(self): + pass +class _Hidden: + def m(self): + pass +"; + assert_eq!( + public_symbols(src, Lang::Python), + vec![ + vec!["C".to_string(), "visible".to_string()], + vec!["top".to_string()], + ] + ); + } + + #[test] + fn python_decorated_methods_are_enumerated() { + let src = "\ +class C: + @property + def value(self): + pass +"; + assert_eq!( + public_symbols(src, Lang::Python), + vec![vec!["C".to_string(), "value".to_string()]] + ); + } + + #[test] + fn go_symbols_include_methods_by_receiver() { + // Pointer and value receivers both yield the bare type name; lowercase method and + // method on an unexported type are excluded. + let src = "\ +package p +func Top() {} +type Builder struct{} +func (b *Builder) Set() {} +func (ls Labels) String() string { return \"\" } +func (b *Builder) internal() {} +type priv struct{} +func (p *priv) Exported() {} +"; + assert_eq!( + public_symbols(src, Lang::Go), + vec![ + vec!["Builder".to_string(), "Set".to_string()], + vec!["Labels".to_string(), "String".to_string()], + vec!["Top".to_string()], + ] + ); + } + + #[test] + fn rust_symbols_are_top_level_fns_only() { + let src = "pub fn a() {}\nfn b() {}\nimpl S { pub fn m(&self) {} }\n"; + assert_eq!(public_symbols(src, Lang::Rust), vec![vec!["a".to_string()]]); + } }