diff --git a/CHANGELOG.md b/CHANGELOG.md index fbd6f71..4faf9eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,19 @@ All notable changes to srcwalk are documented here. +## [1.7.0] - 2026-08-13 + +### Added +- Shared typed `path:symbol` selector grammar with a single resolution seam, consumed by `show`, `context`, and `trace`, so canonical selectors round-trip between command output and command input. +- `discover` emits the canonical owning-container selector for resolvable hits, including per-term sections of batch (comma-separated) queries, derived from outline primitives. + +### Fixed +- Exact selector evidence is preserved through display instead of being rewritten into non-resolvable forms. +- Copyable path displays are classified consistently across discover/show/callers output. + +### Known limitations +- Dotted bare terms inside batch discover queries (e.g. `discover 'A.b,C'`) still resolve as free-text for the dotted term; the canonical selector for that term is not emitted. Single-term dotted queries are unaffected. + ## [1.6.0] - 2026-08-12 ### Added diff --git a/Cargo.lock b/Cargo.lock index 77d292c..74420ef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -678,7 +678,7 @@ checksum = "67b1b7a3b5fe4f1376887184045fcf45c69e92af734b7aaddc05fb777b6fbd03" [[package]] name = "srcwalk" -version = "1.6.0" +version = "1.7.0" dependencies = [ "aho-corasick", "clap", diff --git a/Cargo.toml b/Cargo.toml index 4fabdf2..d2f7f25 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "srcwalk" -version = "1.6.0" +version = "1.7.0" edition = "2021" description = "Tree-sitter indexed lookups — smart code reading for AI agents" license = "MIT" diff --git a/npm/package.json b/npm/package.json index 145a02c..6a9d85c 100644 --- a/npm/package.json +++ b/npm/package.json @@ -1,6 +1,6 @@ { "name": "srcwalk", - "version": "1.6.0", + "version": "1.7.0", "description": "Code-intelligence CLI for AI agents — tree-sitter outlines, symbol search, caller/callee graphs, deps, overview", "bin": { "srcwalk": "run.js" diff --git a/skills/srcwalk/GUIDE.md b/skills/srcwalk/GUIDE.md index 5952d27..877dc12 100644 --- a/skills/srcwalk/GUIDE.md +++ b/skills/srcwalk/GUIDE.md @@ -28,7 +28,7 @@ Turn the request's explicit evidence questions into a short coverage list. In ea - Multi-root symbol discovery may repeat the flag: `srcwalk discover 'foo,bar' --as symbol --scope src --scope tests`. - Other routes and `discover --as text|file|access` accept one scope; use a common ancestor or run independent commands in the same model/tool round. - Keep scope as small as the evidence question allows; narrow scopes can hide definitions. -- Symbol batches accept 2-5 comma-separated symbols: `srcwalk discover 'foo,bar,baz' --as symbol --scope src`. Split larger symbol sets. One dot qualifies a method by receiver/container: `'Batch.Set'`. +- Symbol batches accept 2-5 comma-separated symbols: `srcwalk discover 'foo,bar,baz' --as symbol --scope src`. Split larger symbol sets. One dot qualifies a method by receiver/container in a single-symbol query (`'Batch.Set'`); dotted terms inside a batch are a known matching limitation, so run that dotted term separately when exact qualification is required. - Text OR is separate: `srcwalk discover 'alloc,copy' --match any --as text --scope src` is literal text evidence, not a symbol batch. - Do not infer definitions, usages, callers, deps, or code paths from shell path lists, broad grep, or converted identifier paths. @@ -66,7 +66,7 @@ Use auto overview depth first; explicit `--depth N` is strict. Narrow `overview Intent inference: path-like globs infer file discovery; punctuation/path comma lists infer literal Text OR; symbol globs stay symbol search. Add `--as symbol|file|text|access` when ambiguous. After a first pass, use `--expand=3`, `--filter kind:fn`, or `--exclude 'tests/**'` only when output is too broad. Regex-style queries are translated, not executed as regex: `foo\(` de-escapes to literal + symbol search, `a.*b` runs same-line ordered co-occurrence, `models\.json` behaves like `models.json`, and an unresolved path fragment like `packages/ai` lists matching relative paths (≤20). Each translation is labeled `interpreted as`; zero-match branches print a `> Try:` recovery line. Windows drive paths and `./`/`../` paths are never treated as regex. Text Search and Text OR may add a conditional `> Note:` when a term has at least 400 matches across at least 150 files and reaches at least 1.5% of eligible files; treat it as measured spread, then consider `overview`, a narrower term or scope, or a structural route when that spread was not intentional. -If `discover` prints `## Confirmed structural targets`, run the printed `> Next:` command verbatim. It is symbol-addressed (`show --section `) when the selector round-trips to the exact body; a numeric `:` command is the safe fallback when ambiguity prevents a unique symbol selector. A numeric range shown beside a stable symbol is evidence metadata (a bounded preview), not the preferred body address. Use `srcwalk context ` only when you need a Flow Map, scoped occurrences, or call neighborhood; do not run `context` for each hop just to read source. +If `discover` prints `## Confirmed structural targets`, run the printed `> Next:` command verbatim. The target is a canonical `:` built from parser outline primitives and proven to resolve to that one body; a numeric `:` command is the safe fallback when ambiguity prevents a unique symbol selector. A numeric range shown beside a stable symbol is evidence metadata (a bounded preview), not the preferred body address. Use `srcwalk context ` only when you need a Flow Map, scoped occurrences, or call neighborhood; do not run `context` for each hop just to read source. Reuse that `:` string unchanged across `show`, `context`, `trace callers`, and `trace callees`, keeping any `--scope` printed with it; never rebuild it from a displayed qualified name, because a namespace/module display prefix (`System.Text.Json.GetTypeInfoInternal`) is not a selector and the emitted form uses the owning container (`JsonSerializerOptions.GetTypeInfoInternal`). The path identifies the requested definition; it does not upgrade caller evidence — `trace callers` stays a direct by-name search and may include same-name definitions elsewhere. Unresolved or ambiguous targets fail explicitly instead of falling back to a bare-name search. Symbol discovery separates parser-backed definitions from text-matched name occurrences. Repeated same-name definitions receive an ambiguity caveat. Text discovery remains literal evidence; `--match all` is same-file co-occurrence, not semantic relation proof. @@ -81,12 +81,12 @@ For Go `discover --as text --match any`, matched lines may carry a structural ow Use `show` for known bodies and citations. Use `context` when the task needs a rich local packet such as Flow Map, scoped occurrences, or call neighborhood. Exact path/range contexts may include bounded `Source Evidence`; `show` `Source frame` lines orient exact numeric reads and numeric `--section` blocks, not relation proof. ```bash -srcwalk context : -srcwalk context : +srcwalk context : # or : srcwalk context --scope srcwalk show :123 -C 10 srcwalk show 'a.rs:12,b.rs:40-55' srcwalk show :12,40-55 # same-file ranges route to --section +srcwalk show : # canonical exact target, reusable across all four commands srcwalk show --section srcwalk show README.md --section '# Install' ``` @@ -98,8 +98,8 @@ Do not pass a bare file to `context`; use `show` or root reads. `context` accept Use `trace callers` for upstream call sites and `trace callees` for downstream calls. Do not grep `foo(` for relation claims. Long result lists collapse to the top entries plus a `+N more → ` pointer, and wide evidence ranges may appear as an anchor plus an `expand: ` line. These pointer lines are exact commands: run them verbatim when you need the remaining items or the full range. Nothing is dropped — collapsed items are always one command away. Drill down with exact call-site reads or `context` on a caller/callee. ```bash -srcwalk trace callers --scope -srcwalk trace callees --detailed --scope +srcwalk trace callers --scope # exact root; call sites still matched by name +srcwalk trace callees --detailed --scope srcwalk trace callees --depth 2 --scope ``` diff --git a/src/commands/callees.rs b/src/commands/callees.rs index 21defe7..6f888d2 100644 --- a/src/commands/callees.rs +++ b/src/commands/callees.rs @@ -1,5 +1,5 @@ use std::fmt::Write as _; -use std::path::Path; +use std::path::{Path, PathBuf}; use crate::cache::OutlineCache; use crate::commands::context::ArtifactMode; @@ -12,6 +12,7 @@ use crate::commands::call_format::{ format_call_site, format_direct_call_edge, format_direct_call_unknown, }; use crate::commands::find::symbol_or_file_suggestion; +use crate::commands::pathsymbol::{self, PathSymbolOutcome}; /// Show what a symbol calls (forward call graph). pub(crate) fn run_callees( @@ -46,7 +47,6 @@ pub(crate) fn run_callees_with_artifact( filter: Option<&str>, artifact: ArtifactMode, ) -> Result { - use std::fmt::Write; if artifact.enabled() && matches!(depth, Some(d) if d >= 2) { return Err(SrcwalkError::InvalidQuery { query: target.to_string(), @@ -54,90 +54,93 @@ pub(crate) fn run_callees_with_artifact( .to_string(), }); } - let bloom = index::bloom::BloomFilterCache::new(); - let path_target = crate::read::resolve_path_symbol_target(target, scope) - .filter(|resolved| resolved.range.is_some()); - let lookup_target = path_target - .as_ref() - .map_or(target, |resolved| resolved.symbol.as_str()); - // A path-qualified target is only rewritten when its symbol is unique in the - // named file and in the whole scope; otherwise do not suggest a risky command. - let raw = search::search_symbol_raw_with_artifact(lookup_target, scope, None, artifact)?; - let definitions: Vec<_> = raw - .matches - .iter() - .filter(|m| m.is_definition && m.def_range.is_some()) - .collect(); - if let Some(resolved) = &path_target { - if definitions.len() == 1 && definitions[0].path == resolved.path { - let symbol = - format::shell_quote_arg(&resolved.symbol).unwrap_or_else(|| "".to_string()); - let scope_arg = format::shell_quote_arg(&format::display_path(scope)) - .unwrap_or_else(|| "".to_string()); - let mut command = format!("srcwalk trace callees {symbol}"); - if detailed { - command.push_str(" --detailed"); - } - if let Some(depth) = depth { - let _ = write!(command, " --depth {depth}"); - } - if let Some(filter) = filter { - let filter = - format::shell_quote_arg(filter).unwrap_or_else(|| "".to_string()); - let _ = write!(command, " --filter {filter}"); - } - let _ = write!(command, " --scope {scope_arg}"); - return Err(SrcwalkError::NoMatches { - query: target.to_string(), - scope: scope.to_path_buf(), - suggestion: Some(resolved.symbol.clone()), - guidance: Some(format!( - "`path:symbol` targets are accepted by `context`, not `trace`. For this symbol: {command}" - )), - }); + // A canonical `path:symbol` root names its own definition, so use that exact + // path/range directly. Global uniqueness is NOT required: a same-name definition + // in another file no longer blocks the exact root. Missing / unreadable / + // unresolved / ambiguous / `::` roots surface the shared explicit intent instead + // of silently broadening to a bare-name search. + let exact_root = match pathsymbol::resolve_path_symbol_outcome(target, scope) { + PathSymbolOutcome::Error(err) | PathSymbolOutcome::UnresolvedInNamedFile(err) => { + return Err(err) } + PathSymbolOutcome::Unique { + path, + symbol, + start_line, + end_line, + } => Some((path, symbol, (start_line as u32, end_line as u32))), + PathSymbolOutcome::FallThrough => None, + }; - let candidates = definitions - .iter() - .take(5) - .map(|m| { - let range = m - .def_range - .map(|(start, end)| format!(":{start}-{end}")) - .unwrap_or_default(); - format!("{}{}", crate::format::display_path(&m.path), range) - }) - .collect::>() - .join(", "); - return Err(SrcwalkError::NoMatches { - query: target.to_string(), - scope: scope.to_path_buf(), - suggestion: None, - guidance: Some(format!( - "`path:symbol` targets are accepted by `context`, not `trace`.{}", - if candidates.is_empty() { - String::new() - } else { - format!(" Candidates: {candidates}") - } - )), + // The path only IDENTIFIES the root, so it may name a definition outside + // --scope. Callee resolution and traversal stay bounded by --scope, so label + // the boundary rather than implicitly widening it. Applying the note at this + // single exit is what stops any render shape (detailed, no-call, direct, or + // transitive) from dropping it on an early return. + let outside_scope_note = exact_root + .as_ref() + .filter(|(path, _, _)| !path.starts_with(scope)) + .map(|(path, _, _)| { + format!( + "\n> Note: the exact root {} lies outside --scope {}; callee resolution and traversal stay inside --scope only.", + format::display_path(path), + format::display_path(scope) + ) }); + + let body = render_callees( + target, scope, cache, depth, detailed, filter, artifact, exact_root, + )?; + let output = match outside_scope_note { + Some(note) => format!("{body}{note}"), + None => body, + }; + match budget_tokens { + Some(b) => Ok(budget::apply_preserving_footer(&output, b)), + None => Ok(output), } +} - let def_match = definitions.first().ok_or_else(|| SrcwalkError::NoMatches { - query: target.to_string(), - scope: scope.to_path_buf(), - suggestion: symbol_or_file_suggestion(scope, target, None), - guidance: None, - })?; +/// Render the callee report body for an already-resolved root. +#[allow(clippy::too_many_arguments)] +fn render_callees( + target: &str, + scope: &Path, + cache: &OutlineCache, + depth: Option, + detailed: bool, + filter: Option<&str>, + artifact: ArtifactMode, + exact_root: Option<(PathBuf, String, (u32, u32))>, +) -> Result { + use std::fmt::Write; + let bloom = index::bloom::BloomFilterCache::new(); + let from_exact_path = exact_root.is_some(); + + let (def_path, def_range, lookup_target) = if let Some((path, symbol, range)) = exact_root { + (path, Some(range), symbol) + } else { + let raw = search::search_symbol_raw_with_artifact(target, scope, None, artifact)?; + let def_match = raw + .matches + .into_iter() + .find(|m| m.is_definition && m.def_range.is_some()) + .ok_or_else(|| SrcwalkError::NoMatches { + query: target.to_string(), + scope: scope.to_path_buf(), + suggestion: symbol_or_file_suggestion(scope, target, None), + guidance: None, + })?; + (def_match.path, def_match.def_range, target.to_string()) + }; - let content = std::fs::read_to_string(&def_match.path).map_err(|e| SrcwalkError::IoError { - path: def_match.path.clone(), + let content = std::fs::read_to_string(&def_path).map_err(|e| SrcwalkError::IoError { + path: def_path.clone(), source: e, })?; - let file_type = lang::detect_file_type(&def_match.path); + let file_type = lang::detect_file_type(&def_path); let types::FileType::Code(lang) = file_type else { return Ok(format!("# Callees: {target}\n\n(not a code file)")); }; @@ -155,14 +158,14 @@ pub(crate) fn run_callees_with_artifact( None } else { Some(crate::read::js_alias::classify_js_imports( - &def_match.path, + &def_path, &logical_sources, scope, &config_cache, )) }; - let rel = format::rel_nonempty(&def_match.path, scope); + let rel = format::rel_nonempty(&def_path, scope); // Detailed mode: ordered call sites with args + assignment context. if detailed { @@ -170,21 +173,17 @@ pub(crate) fn run_callees_with_artifact( search::callees::extract_call_sites_for_artifact_target( &content, lang, - target, - def_match.def_range, + &lookup_target, + def_range, ) } else { - search::callees::extract_call_sites(&content, lang, def_match.def_range) + search::callees::extract_call_sites(&content, lang, def_range) }; let total_sites = sites.len(); let sites = search::callees::filter_call_sites(sites, filter)?; let direct_calls = (!artifact.enabled()).then(|| { crate::evidence::direct_call::build_direct_call_evidence_index( - &def_match.path, - &content, - lang, - def_match.def_range, - &sites, + &def_path, &content, lang, def_range, &sites, ) }); if sites.is_empty() { @@ -256,11 +255,7 @@ pub(crate) fn run_callees_with_artifact( out.push_str("\n> "); out.push_str(note); } - let output = match budget_tokens { - Some(b) => budget::apply_preserving_footer(&out, b), - None => out, - }; - return Ok(output); + return Ok(out); } // Default mode: resolved callees with transitive expansion. @@ -268,11 +263,11 @@ pub(crate) fn run_callees_with_artifact( search::callees::extract_callee_names_for_artifact_target( &content, lang, - target, - def_match.def_range, + &lookup_target, + def_range, ) } else { - search::callees::extract_callee_names(&content, lang, def_match.def_range) + search::callees::extract_callee_names(&content, lang, def_range) }; if callee_names.is_empty() { return Ok(format!( @@ -283,19 +278,14 @@ pub(crate) fn run_callees_with_artifact( let depth_limit = depth.map_or(1, |d| d.min(5) as u32); let nodes = if artifact.enabled() { search::callees::resolve_callees_same_file_artifact( - target, - &def_match.path, + &lookup_target, + &def_path, &content, lang, &callee_names, ) .unwrap_or_else(|| { - search::callees::resolve_callees_same_file( - &callee_names, - &def_match.path, - &content, - lang, - ) + search::callees::resolve_callees_same_file(&callee_names, &def_path, &content, lang) }) .into_iter() .map(|callee| search::callees::ResolvedCalleeNode { @@ -306,7 +296,7 @@ pub(crate) fn run_callees_with_artifact( } else { search::callees::resolve_callees_transitive_with_stream( &callee_names, - &def_match.path, + &def_path, &content, &logical_sources, decisions.as_deref(), @@ -320,6 +310,14 @@ pub(crate) fn run_callees_with_artifact( }; let mut out = format!("# Callees: {target} (in {rel})\n"); + // Only the root body came from the named file; every deeper hop is resolved + // from a callee name, so say that before rendering transitive children. + if from_exact_path && depth_limit >= 2 { + let _ = writeln!( + out, + "> Caveat: only the root is exact (`{lookup_target}` in the named file); every later hop resolves by name." + ); + } // Unresolved callees let resolved_names: std::collections::HashSet<&str> = @@ -359,11 +357,11 @@ pub(crate) fn run_callees_with_artifact( search::callees::extract_call_sites_for_artifact_target( &content, lang, - target, - def_match.def_range, + &lookup_target, + def_range, ) } else { - search::callees::extract_call_sites(&content, lang, def_match.def_range) + search::callees::extract_call_sites(&content, lang, def_range) }; append_unresolved_call_site_evidence(&mut out, &unresolved, &sites); } @@ -381,11 +379,7 @@ pub(crate) fn run_callees_with_artifact( out.push_str(note); } - let output = match budget_tokens { - Some(b) => budget::apply_preserving_footer(&out, b), - None => out, - }; - Ok(output) + Ok(out) } fn append_unresolved_call_site_evidence( diff --git a/src/commands/callers.rs b/src/commands/callers.rs index f1261e3..460af81 100644 --- a/src/commands/callers.rs +++ b/src/commands/callers.rs @@ -2,6 +2,7 @@ use std::path::Path; use crate::cache::OutlineCache; use crate::commands::context::ArtifactMode; +use crate::commands::pathsymbol::{self, PathSymbolOutcome}; use crate::error::SrcwalkError; use crate::{budget, index, search, session}; @@ -79,10 +80,42 @@ pub(crate) fn run_callers_with_artifact( let session = session::Session::new(); let bloom = index::bloom::BloomFilterCache::new(); + // Resolve an exact `path:symbol` root first. The path pins WHICH definition + // the agent asked about; the terminal callable key is what a direct by-name + // search can actually match. An unresolved / ambiguous / missing / `::` root + // is an explicit error, never a silent bare-name search. + // The root file may sit outside --scope (it is only being identified); the + // relation search itself stays bounded by --scope. + let exact_root = match pathsymbol::resolve_path_symbol_outcome(target, scope) { + PathSymbolOutcome::Error(err) | PathSymbolOutcome::UnresolvedInNamedFile(err) => { + return Err(err) + } + PathSymbolOutcome::Unique { path, symbol, .. } => Some((path, symbol)), + PathSymbolOutcome::FallThrough => None, + }; + let lookup = exact_root.as_ref().map_or(target, |(_, symbol)| { + crate::lang::qualified::terminal_callable_key(symbol) + }); + let root = search::callers::CallerRoot { + lookup, + display: target, + from_exact_path: exact_root.is_some(), + }; + let outside_scope_note = exact_root + .as_ref() + .filter(|(path, _)| !path.starts_with(scope)) + .map(|(path, _)| { + format!( + "\n> Note: the exact root {} lies outside --scope {}; call sites are searched inside --scope only.", + crate::format::display_path(path), + crate::format::display_path(scope) + ) + }); + // BFS path when --depth N (N >= 2). Otherwise use compact direct-call rows by default. let output = match depth { Some(d) if d >= 2 => search::callers::search_callers_bfs( - target, + root, scope, cache, &bloom, @@ -93,10 +126,14 @@ pub(crate) fn run_callers_with_artifact( skip_hubs, )?, _ => search::callers::search_callers_expanded_with_artifact( - target, scope, cache, &session, &bloom, expand, None, limit, offset, glob, filter, + root, scope, cache, &session, &bloom, expand, None, limit, offset, glob, filter, count_by, artifact, )?, }; + let output = match outside_scope_note { + Some(note) => format!("{output}{note}"), + None => output, + }; match budget_tokens { Some(b) => Ok(budget::apply_preserving_footer(&output, b)), None => Ok(output), diff --git a/src/commands/decision_flow.rs b/src/commands/decision_flow.rs index 3a94b5a..95741d5 100644 --- a/src/commands/decision_flow.rs +++ b/src/commands/decision_flow.rs @@ -2,6 +2,7 @@ use std::path::{Path, PathBuf}; use crate::cache::OutlineCache; use crate::commands::find::symbol_or_file_suggestion; +use crate::commands::pathsymbol::{self, PathSymbolOutcome}; use crate::error::SrcwalkError; use crate::lang::{self, decision_flow, decision_flow::FlowTarget, decision_flow::TargetSelector}; use crate::search; @@ -53,6 +54,34 @@ pub(crate) fn resolve_decision_flow_target( } fn resolve_path_target(target: &str, scope: &Path) -> Result, SrcwalkError> { + // Consume the shared `path:symbol` grammar first so context reports the same + // explicit intent as show/callees/callers for a raw `::` selector, and refuses + // to silently first-pick one of several same-name definitions. + // A unique selector becomes its exact owning range. Other states fall through + // to the named-file semantics below (file-level), never to a bare-name search. + match pathsymbol::resolve_path_symbol_outcome(target, scope) { + PathSymbolOutcome::Unique { + path, + symbol, + start_line, + end_line, + } => { + return Ok(Some(FlowTarget { + path, + display_target: target.to_string(), + resolved_symbol: Some(symbol), + selector: TargetSelector::LineRange { + start: start_line as u32, + end: end_line as u32, + }, + })); + } + PathSymbolOutcome::Error(err) => return Err(err), + // Named file exists but has no such selector: keep the existing + // named-file (file-level) semantics rather than abstaining hard. + PathSymbolOutcome::UnresolvedInNamedFile(_) | PathSymbolOutcome::FallThrough => {} + } + if let Some((path_part, selector)) = target.rsplit_once(':') { if path_part.is_empty() { return Ok(None); diff --git a/src/commands/find.rs b/src/commands/find.rs index caf65f9..498de89 100644 --- a/src/commands/find.rs +++ b/src/commands/find.rs @@ -1330,6 +1330,41 @@ fn run_inner( return Err(err); } + // US-071 Step 4 (amendment): pre-parse comma-separated multi-symbol + // candidates BEFORE automatic classification. A dotted part can make the + // whole string look like a filename (QueryType::Glob), which the old + // classify-first gate then wrongly excluded. `parse_multi_symbol_query` + // already rejects real globs (`*?{[`), regexes, paths, and section syntax + // because it requires every part to be a bare/dotted identifier, so a real + // file/glob/regex/path/section query falls through to `classify` unchanged. + // Explicit `--as text|glob|regex|path` modes are dispatched before + // `run_inner` and never reach this route. + if let Some(parts) = parse_multi_symbol_query(query)? { + let session = session::Session::new(); + let sym_index = index::SymbolIndex::new(); + let bloom = index::bloom::BloomFilterCache::new(); + let expand = if expand > 0 { expand } else { 2 }; + let output = search::search_multi_symbol_expanded( + &parts, + scope, + cache, + &session, + &sym_index, + &bloom, + expand, + None, + limit, + offset, + glob, + filter, + budget_tokens, + )?; + return match budget_tokens { + Some(b) => Ok(budget::apply_preserving_footer(&output, b)), + None => Ok(output), + }; + } + let config_cache = crate::lang::tsconfig::ConfigCache::new(); let query_type = classify(query, scope); @@ -1363,45 +1398,6 @@ fn run_inner( | QueryType::Glob(_) ); - // Multi-symbol: comma-separated identifiers, 2..=5 items. - // Check before main dispatch. Only activate when all parts look like identifiers - // to avoid hijacking regex (/foo,bar/) or glob (*.{rs,ts}) queries. - if !matches!( - query_type, - QueryType::Glob(_) - | QueryType::SymbolGlob(_) - | QueryType::FilePath(_) - | QueryType::FilePathLine(_, _) - | QueryType::FilePathSection(_, _) - ) { - if let Some(parts) = parse_multi_symbol_query(query)? { - let session = session::Session::new(); - let sym_index = index::SymbolIndex::new(); - let bloom = index::bloom::BloomFilterCache::new(); - let expand = if expand > 0 { expand } else { 2 }; - let output = search::search_multi_symbol_expanded( - &parts, - scope, - cache, - &session, - &sym_index, - &bloom, - expand, - None, - limit, - offset, - glob, - filter, - budget_tokens, - )?; - return match budget_tokens { - Some(b) => Ok(budget::apply_preserving_footer(&output, b)), - None => Ok(output), - }; - } - } - - // FilePath and Glob are read operations, not search — handle before expanded dispatch let output_result = match query_type { QueryType::FilePath(_) | QueryType::FilePathLine(_, _) | QueryType::Glob(_) if filter.is_some() => diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 7f23357..e61524a 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -11,5 +11,6 @@ pub(crate) mod flow; pub(crate) mod impact; pub(crate) mod multi_scope; pub(crate) mod path; +pub(crate) mod pathsymbol; pub(crate) mod review; pub(crate) mod section_disambiguation; diff --git a/src/commands/path.rs b/src/commands/path.rs index 3ab31d3..cb885e1 100644 --- a/src/commands/path.rs +++ b/src/commands/path.rs @@ -2,6 +2,7 @@ use std::path::Path; use crate::cache::OutlineCache; use crate::commands::context::{with_artifact_note, with_artifact_read_label, ArtifactMode}; +use crate::commands::pathsymbol::{self, PathSymbolOutcome}; use crate::error::SrcwalkError; use crate::{artifact, budget, read}; @@ -129,27 +130,34 @@ pub(crate) fn run_path_exact_with_artifact_and_context( context_lines: Option, cache: &OutlineCache, ) -> Result { - if section.is_none() { - if let Some(target) = read::resolve_path_symbol_target(query, scope) { - let suggestion = target.range.map(|(start, end)| { - format!( - "{}:{start}-{end}", - crate::format::rel_nonempty(&target.path, scope) - ) - }); - return Err(SrcwalkError::NotFound { - path: scope.join(query), - suggestion, - guidance: Some( - "`show` takes line ranges; `path:symbol` is `context` grammar.".to_string(), - ), - }); + // `path:symbol` is the canonical exact-body selector: resolve it through the + // shared consumer and read the owning range, i.e. `show --section `. + // Missing / unreadable / unresolved / ambiguous / `::` targets surface the shared + // explicit intent instead of silently falling back to a whole-file read. + let exact_body = if section.is_none() { + match pathsymbol::resolve_path_symbol_outcome(query, scope) { + PathSymbolOutcome::FallThrough => None, + PathSymbolOutcome::Error(err) | PathSymbolOutcome::UnresolvedInNamedFile(err) => { + return Err(err) + } + PathSymbolOutcome::Unique { + path, + start_line, + end_line, + .. + } => Some((path, format!("{start_line}-{end_line}"))), } - } + } else { + None + }; - let (query, inline_section) = split_inline_section(query, scope, section); + let (path, inline_section) = if let Some((path, range)) = exact_body { + (path, Some(range)) + } else { + let (query, inline_section) = split_inline_section(query, scope, section); + (resolve_exact_path(&query, scope)?, inline_section) + }; let section = inline_section.as_deref(); - let path = resolve_exact_path(&query, scope)?; let artifact_mode = ArtifactMode::from(artifact || artifact::should_auto_artifact_file(&path)); let output = if artifact_mode.enabled() && context_lines.is_none() { if let Some(symbol) = section { diff --git a/src/commands/pathsymbol.rs b/src/commands/pathsymbol.rs new file mode 100644 index 0000000..e388825 --- /dev/null +++ b/src/commands/pathsymbol.rs @@ -0,0 +1,135 @@ +use std::path::{Path, PathBuf}; + +use crate::error::SrcwalkError; +use crate::read::PathSymbolResolution; + +/// Bounded candidate list size for an ambiguous same-file selector. +const MAX_AMBIGUOUS_CANDIDATES: usize = 5; + +/// How a `:`-shaped target is handled by an exact-target command +/// (`show`, `context`, `trace callees`, `trace callers`). +/// +/// One shared consumer keeps the four commands honest: the same explicit intent +/// for a raw `::` selector, and no silent broadening to a bare-name search when +/// the target is missing, unreadable, unresolved, or ambiguous. +#[derive(Debug)] +pub(crate) enum PathSymbolOutcome { + /// Not a `:` form (bare symbol, `path:range`, drive-only). + /// The command falls through to its existing behavior unchanged. + FallThrough, + /// A unique exact named-file target: use this path/range directly. + Unique { + path: PathBuf, + symbol: String, + start_line: usize, + end_line: usize, + }, + /// The named file exists and was readable, but defines no such selector. + /// Exact-body commands turn this into an error; `context` may still fall + /// back to named-file (never bare-name) semantics. + UnresolvedInNamedFile(SrcwalkError), + /// Surface verbatim and stop: raw `::` selector, missing/unreadable file, + /// or ambiguous same-file definition. + Error(SrcwalkError), +} + +/// Resolve a `:` target into the shared command outcome. +pub(crate) fn resolve_path_symbol_outcome(target: &str, scope: &Path) -> PathSymbolOutcome { + match crate::read::resolve_path_symbol_resolution(target, scope) { + PathSymbolResolution::NotForm => PathSymbolOutcome::FallThrough, + PathSymbolResolution::Unique { + path, + symbol, + start_line, + end_line, + } => PathSymbolOutcome::Unique { + path, + symbol, + start_line, + end_line, + }, + PathSymbolResolution::UnsupportedColonSymbol { symbol } => { + PathSymbolOutcome::Error(unsupported_colon_symbol_error(target, &symbol)) + } + PathSymbolResolution::NamedPathMissing { path, symbol } => { + PathSymbolOutcome::Error(SrcwalkError::NotFound { + path, + suggestion: None, + guidance: Some(format!( + "`path:symbol` needs an existing named file; `{symbol}` was not resolved." + )), + }) + } + PathSymbolResolution::NamedFileUnreadable { path, symbol } => { + PathSymbolOutcome::Error(named_file_error( + &path, + &symbol, + "could not be read, so the exact body was not resolved", + )) + } + PathSymbolResolution::NamedFileUnresolvable { path, symbol } => { + PathSymbolOutcome::Error(named_file_error( + &path, + &symbol, + "has no structural outline, so the exact body was not resolved", + )) + } + PathSymbolResolution::NamedFileUnresolved { path, symbol } => { + PathSymbolOutcome::UnresolvedInNamedFile(named_file_error( + &path, + &symbol, + "does not define that selector", + )) + } + PathSymbolResolution::Ambiguous { + path, + symbol, + ranges, + } => PathSymbolOutcome::Error(ambiguous_error(&path, &symbol, &ranges)), + } +} + +/// The single explicit intent every exact-target command surfaces for a raw +/// colon-bearing selector, so agents get one consistent recovery. +fn unsupported_colon_symbol_error(target: &str, symbol: &str) -> SrcwalkError { + SrcwalkError::InvalidQuery { + query: target.to_string(), + reason: format!( + "`path:symbol` takes a plain selector; `{symbol}` contains `::`. \ + Use the last segment as the selector, or a `path:start-end` range." + ), + } +} + +fn named_file_error(path: &Path, symbol: &str, why: &str) -> SrcwalkError { + SrcwalkError::NotFound { + path: path.to_path_buf(), + suggestion: None, + guidance: Some(format!( + "`{}` {why}; `{symbol}` was not resolved. Use a `path:start-end` range, or `srcwalk discover {symbol}`.", + crate::format::display_path(path) + )), + } +} + +fn ambiguous_error(path: &Path, symbol: &str, ranges: &[(usize, usize)]) -> SrcwalkError { + let display = crate::format::display_path(path); + let shown = ranges.len().min(MAX_AMBIGUOUS_CANDIDATES); + let mut candidates = ranges + .iter() + .take(shown) + .map(|(start, end)| format!("{display}:{start}-{end}")) + .collect::>() + .join(", "); + if ranges.len() > shown { + use std::fmt::Write as _; + let _ = write!(candidates, ", (+{} more)", ranges.len() - shown); + } + SrcwalkError::InvalidQuery { + query: format!("{display}:{symbol}"), + reason: format!( + "`{symbol}` has {} definitions in `{display}`; pick one exact body. Candidates: {candidates}", + ranges.len() + ), + } +} diff --git a/src/format.rs b/src/format.rs index 9de577f..19d6bcf 100644 --- a/src/format.rs +++ b/src/format.rs @@ -95,15 +95,50 @@ pub(crate) fn display_path(path: &Path) -> String { normalize_display_path(cwd_relative(path).unwrap_or_else(|| path.display().to_string())) } +/// What a displayed path is relative to. A printed command can only be copied +/// verbatim when the display addresses the file on its own; otherwise it must +/// carry the same `--scope`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum DisplayBasis { + /// CWD-relative or a full path: the display addresses the file by itself. + SelfContained, + /// Scope-relative: the display only addresses the file given that `--scope`. + Scope, + /// The path is the scope itself, so the stripped display is empty and + /// `rel_nonempty` shortens it into a label that addresses no file. + Unaddressable, +} + +/// Path for human result rows plus what that display is relative to. +/// +/// `rel` is this function's string half. Keeping the basis on the same result +/// is what stops a caller from re-deriving it with a second predicate that can +/// answer a subtly different question. +pub(crate) fn rel_with_basis(path: &Path, scope: &Path) -> (String, DisplayBasis) { + if let Some(rel) = cwd_relative(path) { + return (rel, DisplayBasis::SelfContained); + } + let Ok(stripped) = path.strip_prefix(scope) else { + return ( + normalize_display_path(path.display().to_string()), + DisplayBasis::SelfContained, + ); + }; + let basis = if stripped.as_os_str().is_empty() { + DisplayBasis::Unaddressable + } else { + DisplayBasis::Scope + }; + ( + normalize_display_path(stripped.display().to_string()), + basis, + ) +} + /// Path for human result rows. Prefer cwd-relative copy-pasteable paths, then /// fall back to scope-relative legacy display when the scope lives elsewhere. pub(crate) fn rel(path: &Path, scope: &Path) -> String { - normalize_display_path(cwd_relative(path).unwrap_or_else(|| { - path.strip_prefix(scope) - .unwrap_or(path) - .display() - .to_string() - })) + rel_with_basis(path, scope).0 } fn cwd_relative(path: &Path) -> Option { diff --git a/src/lang/qualified.rs b/src/lang/qualified.rs index d4f804c..b66d48c 100644 --- a/src/lang/qualified.rs +++ b/src/lang/qualified.rs @@ -20,6 +20,42 @@ pub(crate) fn split_dot_symbol_query(query: &str) -> Option<(&str, &str)> { Some((qualifier, plain)) } +/// The terminal bare callable name from a possibly-qualified callable label +/// (`Class.method`, `mod::func`, or bare `method`). This is the single shared +/// source used by direct callers, BFS frontiers, and recovery for relation +/// lookup keys. `Class.method` -> `method`, `mod::func` -> `func`, bare name +/// unchanged. Commands must use this helper and never re-split on `.` or `::`. +pub(crate) fn terminal_callable_key(qualified: &str) -> &str { + // Terminal key = the segment after the RIGHTMOST supported separator of + // either kind (`.` or `::`), so `A.B::method` -> `method` and + // `A::B.method` -> `method` regardless of separator order/subtype/width. + // `(start_byte, width)` of the rightmost separator. + let mut sep: Option<(usize, usize)> = None; + let bytes = qualified.as_bytes(); + let mut idx = 0; + while idx < bytes.len() { + if bytes[idx] == b'.' { + sep = Some((idx, 1)); + idx += 1; + } else if bytes[idx] == b':' && idx + 1 < bytes.len() && bytes[idx + 1] == b':' { + sep = Some((idx, 2)); + idx += 2; + } else { + idx += 1; + } + } + match sep { + // Rightmost separator is not trailing: terminal = suffix after it. + Some((start, width)) if start + width < qualified.len() => &qualified[start + width..], + // Trailing separator (`method.` / `method::`): the key is the prefix up + // to the separator, so it is never an empty lookup string. + Some((start, _)) if start > 0 => &qualified[..start], + // `.` / `::` alone -> explicitly empty (no invented value). + Some(_) => "", + None => qualified, + } +} + /// Normalize a Go receiver type — strip leading `*` and generic params: /// `*Batch` -> `Batch`, `syncQueue[T]` -> `syncQueue`. Single source of truth /// shared by search and the `--section` reader. @@ -80,41 +116,132 @@ pub(crate) fn resolve_selector_first( lang: Option, selector: &str, ) -> Option<(u32, u32)> { - if let Some((start, end)) = find_exact_in_entries(entries, selector) { - return Some((start, end)); + resolve_selector_matches(entries, lang, selector) + .into_iter() + .next() +} + +/// Resolve a symbol selector against a file's outline, returning every distinct +/// matching `(start_line, end_line)` range in deterministic order. This is the +/// shared cardinality primitive for canonical `path:symbol` targets. +/// +/// Precedence mirrors US-064 exact selection first (distinct sorted), then +/// `Q.N` qualifier + plain resolution (distinct sorted). Duplicate outline rows +/// covering the same `(start,end)` range collapse to one entry, so N>1 here +/// means genuinely distinct same-file definitions (e.g. overloads). +pub(crate) fn resolve_selector_matches( + entries: &[OutlineEntry], + lang: Option, + selector: &str, +) -> Vec<(u32, u32)> { + let exact = find_exact_ranges(entries, selector); + if !exact.is_empty() { + return exact; } if let Some((qualifier, plain)) = split_dot_symbol_query(selector) { - if let Some((start, end)) = qualified_outline_ranges(entries, lang, qualifier, plain) - .into_iter() - .next() - { - return Some((start, end)); - } + return qualified_distinct_ranges(entries, lang, qualifier, plain); } - None + Vec::new() } -/// Recursively find an exact entry.name / signature / CSS / document match. -fn find_exact_in_entries(entries: &[OutlineEntry], symbol: &str) -> Option<(u32, u32)> { +/// Collect every distinct exact-match range (name/signature/CSS/document +/// dotted name) in sorted order, collapsing duplicate outline rows. +fn find_exact_ranges(entries: &[OutlineEntry], symbol: &str) -> Vec<(u32, u32)> { + let mut ranges = Vec::new(); + collect_exact_ranges(entries, symbol, &mut ranges); + ranges.sort_unstable(); + ranges.dedup(); + ranges +} + +fn collect_exact_ranges(entries: &[OutlineEntry], symbol: &str, out: &mut Vec<(u32, u32)>) { for entry in entries { if entry.name == symbol || entry.signature.as_deref() == Some(symbol) || crate::lang::css::outline_name_matches(entry.kind, &entry.name, symbol) || crate::lang::document::outline_name_matches(entry.kind, &entry.name, symbol) { - return Some((entry.start_line, entry.end_line)); + out.push((entry.start_line, entry.end_line)); } - if let Some((start, end)) = find_exact_in_entries(&entry.children, symbol) { - return Some((start, end)); + collect_exact_ranges(&entry.children, symbol, out); + } +} + +/// Build the canonical `Container.Member` selector for the definition occupying +/// `(start_line, end_line)` named `member`, from outline primitives only (never +/// the display-qualified name). The owning container is the deepest container +/// threaded on the path to the member, using the SAME threading rule as +/// `collect_qualified`, so the selector the resolver accepts is the one we +/// emit. A class/struct/interface deeper than a namespace naturally wins, +/// dropping the namespace prefix (C# `System.Text.Json` -> +/// `JsonSerializerOptions.GetTypeInfoInternal`). Go methods are top-level but +/// their receiver type is the owning container, so `Batch.Set` is emitted. +/// Returns `None` only when a body has no resolvable owning container (a +/// top-level non-Go function). +pub(crate) fn selector_from_outline( + entries: &[OutlineEntry], + lang: Option, + start_line: u32, + end_line: u32, + member: &str, +) -> Option { + let mut out = None; + build_outline_selector(entries, lang, start_line, end_line, member, None, &mut out); + out +} + +fn build_outline_selector( + entries: &[OutlineEntry], + lang: Option, + start_line: u32, + end_line: u32, + member: &str, + container: Option<&str>, + out: &mut Option, +) { + for entry in entries { + if entry.start_line == start_line + && entry.end_line == end_line + && entry.name == member + && entry.kind == OutlineKind::Function + { + let qualifier = if lang == Some(Lang::Go) { + entry + .signature + .as_deref() + .and_then(go_receiver_from_signature) + } else { + container.map(str::to_string) + }; + if let Some(q) = qualifier { + *out = Some(format!("{q}.{member}")); + } + return; + } + let child_container = if outline_container_kind(entry.kind) { + Some(normalize_outline_container(&entry.name)) + } else { + container.map(str::to_string) + }; + build_outline_selector( + &entry.children, + lang, + start_line, + end_line, + member, + child_container.as_deref(), + out, + ); + if out.is_some() { + return; } } - None } -/// Resolve `Q.N` against outline entries, threading container names down into -/// children (class/struct/module/interface/enum) and matching Go methods by -/// receiver type parsed from the signature. Returns every matching range. -fn qualified_outline_ranges( +/// Resolve `Q.N` against outline entries, returning distinct sorted ranges. +/// Same container/receiver threading as `resolve_selector_first`, but collapses +/// duplicate outline rows and sorts deterministically for cardinality checks. +fn qualified_distinct_ranges( entries: &[OutlineEntry], lang: Option, qualifier: &str, @@ -122,6 +249,8 @@ fn qualified_outline_ranges( ) -> Vec<(u32, u32)> { let mut out = Vec::new(); collect_qualified(entries, lang, qualifier, plain, None, &mut out); + out.sort_unstable(); + out.dedup(); out } @@ -178,6 +307,39 @@ mod tests { assert_eq!(normalize_receiver_type(" *Batch "), "Batch"); } + #[test] + fn terminal_callable_key_returns_bare_terminal_name() { + assert_eq!(terminal_callable_key("Class.method"), "method"); + assert_eq!(terminal_callable_key("mod::func"), "func"); + assert_eq!(terminal_callable_key("plain"), "plain"); + assert_eq!(terminal_callable_key("A::b::c"), "c"); + assert_eq!(terminal_callable_key(""), ""); + } + + #[test] + fn terminal_callable_key_uses_rightmost_separator_across_both_forms() { + // BLOCKER 2 regression: any separator must not win over a later one of + // the other kind. Terminal is after the RIGHTMOST `.` or `::`. + assert_eq!(terminal_callable_key("A.B::method"), "method"); + assert_eq!(terminal_callable_key("A::B.method"), "method"); + assert_eq!(terminal_callable_key("A.B.C"), "C"); + assert_eq!(terminal_callable_key("A::B::C"), "C"); + assert_eq!(terminal_callable_key("a.b::c.d"), "d"); + assert_eq!(terminal_callable_key("a::b.c::d"), "d"); + // Leading separators: suffix is non-empty. + assert_eq!(terminal_callable_key(".method"), "method"); + assert_eq!(terminal_callable_key("::method"), "method"); + // Trailing separators yield the prefix, never an empty key. + assert_eq!(terminal_callable_key("method."), "method"); + assert_eq!(terminal_callable_key("method::"), "method"); + assert_eq!(terminal_callable_key("A.B."), "A.B"); + // Bare / degenerate. + assert_eq!(terminal_callable_key("plain"), "plain"); + assert_eq!(terminal_callable_key("A...B"), "B"); + // Empty input stays empty (explicit, not invented). + assert_eq!(terminal_callable_key(""), ""); + } + #[test] fn go_receiver_from_signature_distinguishes_method_from_function() { assert_eq!( @@ -195,4 +357,126 @@ mod tests { // A plain function has no receiver group right after `func`. assert_eq!(go_receiver_from_signature("func Set(x int) int"), None); } + + fn fn_entry(name: &str, start: u32, end: u32, signature: Option<&str>) -> OutlineEntry { + OutlineEntry { + kind: OutlineKind::Function, + name: name.to_string(), + start_line: start, + end_line: end, + signature: signature.map(str::to_string), + children: Vec::new(), + doc: None, + } + } + + fn container( + kind: OutlineKind, + name: &str, + start: u32, + end: u32, + children: Vec, + ) -> OutlineEntry { + OutlineEntry { + kind, + name: name.to_string(), + start_line: start, + end_line: end, + signature: None, + children, + doc: None, + } + } + + // C#: namespace is a Module wrapper; the owning container is the class. + // Emit must drop the namespace and produce `JsonSerializerOptions.GetTypeInfoInternal`. + #[test] + fn selector_from_outline_drops_namespace_uses_owning_class() { + let entries = vec![container( + OutlineKind::Module, + "System.Text.Json", + 1, + 120, + vec![container( + OutlineKind::Class, + "JsonSerializerOptions", + 2, + 118, + vec![fn_entry("GetTypeInfoInternal", 55, 65, None)], + )], + )]; + let lang = Some(Lang::CSharp); + let selector = selector_from_outline(&entries, lang, 55, 65, "GetTypeInfoInternal"); + assert_eq!( + selector.as_deref(), + Some("JsonSerializerOptions.GetTypeInfoInternal") + ); + // And the emitted selector resolves UNIQUELY to exactly this body through + // the production resolver. + let matches = resolve_selector_matches(&entries, lang, selector.as_deref().unwrap()); + assert_eq!(matches, vec![(55, 65)]); + } + + // N>1 same-file distinct definitions (e.g. overloads / multiple impl blocks) + // => Ambiguous, never canonical emit (symbol_backed false). + #[test] + fn same_file_distinct_matches_report_cardinality_no_canonical_emit() { + let entries = vec![container( + OutlineKind::Class, + "JsonSerializerContext", + 1, + 120, + vec![ + fn_entry("GetTypeInfo", 108, 108, None), + fn_entry("GetTypeInfo", 110, 118, None), + ], + )]; + let lang = Some(Lang::CSharp); + let selector = selector_from_outline(&entries, lang, 108, 108, "GetTypeInfo"); + assert_eq!( + selector.as_deref(), + Some("JsonSerializerContext.GetTypeInfo") + ); + // Two distinct bodies => cardinality 2: this selector must NOT be treated + // as uniquely resolving to either body. + let matches = resolve_selector_matches(&entries, lang, selector.as_deref().unwrap()); + assert_eq!(matches, vec![(108, 108), (110, 118)]); + } + + // Duplicate outline rows covering the SAME (start,end) collapse to one entry; + // N>1 means genuinely distinct ranges only. + #[test] + fn duplicate_rows_same_range_are_deduped_not_false_overload() { + let entries = vec![container( + OutlineKind::Struct, + "Thing", + 1, + 50, + vec![fn_entry("Run", 10, 20, None), fn_entry("Run", 10, 20, None)], + )]; + let lang = Some(Lang::CSharp); + let selector = selector_from_outline(&entries, lang, 10, 20, "Run"); + assert_eq!(selector.as_deref(), Some("Thing.Run")); + let matches = resolve_selector_matches(&entries, lang, selector.as_deref().unwrap()); + assert_eq!(matches, vec![(10, 20)]); + } + + // Go: method receiver is the owning container; emit `Batch.Set`. + #[test] + fn selector_from_outline_go_uses_receiver_type() { + let entries = vec![fn_entry("Set", 10, 15, Some("func (b *Batch) Set(v int)"))]; + let lang = Some(Lang::Go); + let selector = selector_from_outline(&entries, lang, 10, 15, "Set"); + assert_eq!(selector.as_deref(), Some("Batch.Set")); + let matches = resolve_selector_matches(&entries, lang, selector.as_deref().unwrap()); + assert_eq!(matches, vec![(10, 15)]); + } + + // A top-level non-Go function has no owning container => no canonical selector. + #[test] + fn top_level_non_go_function_has_no_canonical_selector() { + let entries = vec![fn_entry("helper", 3, 8, None)]; + let selector = selector_from_outline(&entries, Some(Lang::Rust), 3, 8, "helper"); + assert_eq!(selector, None); + } } diff --git a/src/read/mod.rs b/src/read/mod.rs index 722e474..4d39ad5 100644 --- a/src/read/mod.rs +++ b/src/read/mod.rs @@ -25,7 +25,10 @@ use crate::lang::detect_file_type; use crate::types::{FileType, ViewMode}; pub use full::{read_file_with_budget, read_file_with_budget_and_context}; -pub(crate) use section::{resolve_path_symbol_target, PathSymbolTarget}; +pub(crate) use section::{ + resolve_path_symbol_resolution, resolve_path_symbol_target, PathSymbolResolution, + PathSymbolTarget, +}; pub(crate) use suggest::edit_distance; pub use suggest::suggest_similar_file; diff --git a/src/read/section.rs b/src/read/section.rs index 510187f..3209eb3 100644 --- a/src/read/section.rs +++ b/src/read/section.rs @@ -988,7 +988,141 @@ fn parse_range(s: &str) -> Option<(usize, usize, Option)> { Some((start, end, None)) } +/// Resolved state of a `:` target, distinguished by the shared +/// grammar parser and the AST-outline resolver. Every exact-body command +/// (show/context/callees) and callers share one result shape so ambiguity and +/// unresolved states are handled identically everywhere. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum PathSymbolResolution { + /// Not a `:` form: no single separating colon, the right side + /// is a line/range (`path:1-3`), or the drive colon of a naked `C:\` path. + NotForm, + /// The right side is a colon-bearing selector (`A::target`). Raw `::` input + /// is unsupported as `path:symbol` — the canonical dotted `.` form is what + /// commands accept. Explicit, consistent error contract. + UnsupportedColonSymbol { symbol: String }, + /// The named file does not exist / could not be resolved to a real file. + /// Carries the raw path and symbol so callers can report the missing path + /// honestly without pretending an existing named file. + NamedPathMissing { path: PathBuf, symbol: String }, + /// The named file exists and is readable, and the selector resolves uniquely + /// to one definition. + Unique { + path: PathBuf, + symbol: String, + start_line: usize, + end_line: usize, + }, + /// The named file exists and is readable, and the selector has no exact + /// definition match (zero-cardinality against the AST outline). + NamedFileUnresolved { path: PathBuf, symbol: String }, + /// The named file exists but could not be read; resolution never ran. + NamedFileUnreadable { path: PathBuf, symbol: String }, + /// The named file exists and was read but has no parseable structural + /// outline (or non-UTF-8 content), so exact resolution was not attempted. + NamedFileUnresolvable { path: PathBuf, symbol: String }, + /// The named file exists and the selector matches N>1 distinct definitions. + Ambiguous { + path: PathBuf, + symbol: String, + ranges: Vec<(usize, usize)>, + }, +} + +/// Split a `:` target on its SINGLE separating colon, preserving a +/// leading Windows drive colon (`C:\\repo` / `C:/repo`) and treating a `::` +/// colon-bearing selector as its own state. Returns `(path_part, symbol)` only +/// for a clean, well-formed split; `None` otherwise. +fn split_path_symbol(target: &str) -> Option<(&str, &str)> { + // Last colon that is not part of a "::" run (so `A::target` keeps its colons + // in the symbol side, while `C:\\...` / `C:/...` keep the drive colon in + // the path side). + let mut split_at = None; + for (i, b) in target.bytes().enumerate() { + if b == b':' { + let prev_double = i >= 1 && target.as_bytes()[i - 1] == b':'; + let next_double = target.as_bytes().get(i + 1) == Some(&b':'); + if !prev_double && !next_double { + split_at = Some(i); + } + } + } + let at = split_at?; + let path_part = &target[..at]; + let symbol = &target[at + 1..]; + if path_part.is_empty() || symbol.is_empty() { + return None; + } + Some((path_part, symbol)) +} + +pub(crate) fn resolve_path_symbol_resolution(target: &str, scope: &Path) -> PathSymbolResolution { + let Some((path_part, symbol)) = split_path_symbol(target) else { + return PathSymbolResolution::NotForm; + }; + + // A naked Windows drive path (`C:\\repo` / `C:/repo`) with no symbol after + // the drive colon: if the symbol side begins with a root slash, the "colon" + // was actually the drive colon, not a path-symbol separator. + if path_part.len() == 1 + && path_part.as_bytes()[0].is_ascii_alphabetic() + && (symbol.starts_with('/') || symbol.starts_with('\\')) + { + return PathSymbolResolution::NotForm; + } + + // Colon-bearing selector (`A::target`) is not the canonical dotted form. + if symbol.contains(':') { + return PathSymbolResolution::UnsupportedColonSymbol { + symbol: symbol.to_string(), + }; + } + + if parse_range(symbol).is_some() { + return PathSymbolResolution::NotForm; + } + + let Some(path) = resolve_existing_file(path_part, scope) else { + return PathSymbolResolution::NamedPathMissing { + path: PathBuf::from(path_part), + symbol: symbol.to_string(), + }; + }; + let Some(buf) = fs::read(&path).ok() else { + return PathSymbolResolution::NamedFileUnreadable { + path, + symbol: symbol.to_string(), + }; + }; + match resolve_symbol_ranges(&buf, &path, symbol) { + Some(ranges) if ranges.len() == 1 => { + let (start_line, end_line) = ranges[0]; + PathSymbolResolution::Unique { + path, + symbol: symbol.to_string(), + start_line, + end_line, + } + } + Some(ranges) if ranges.is_empty() => PathSymbolResolution::NamedFileUnresolved { + path, + symbol: symbol.to_string(), + }, + Some(ranges) => PathSymbolResolution::Ambiguous { + path, + symbol: symbol.to_string(), + ranges, + }, + None => PathSymbolResolution::NamedFileUnresolvable { + path, + symbol: symbol.to_string(), + }, + } +} + /// A path-qualified symbol resolved from the same AST outline used by `--section`. +/// Retains only the resolved exact target (for the reader/plumbing); ambiguity +/// and unresolved states are surfaced via `resolve_path_symbol_resolution`. #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct PathSymbolTarget { pub(crate) path: PathBuf, @@ -996,25 +1130,48 @@ pub(crate) struct PathSymbolTarget { pub(crate) range: Option<(usize, usize)>, } +/// Resolve a `:` target, returning `Some` only for well-formed, +/// uniquely-resolved targets (used by the reader and best-effort plumbing). +/// Non-canonical, unresolved, and ambiguous cases return `None`; callers that +/// need to distinguish those states use `resolve_path_symbol_resolution`. pub(crate) fn resolve_path_symbol_target(target: &str, scope: &Path) -> Option { - let (path_part, symbol) = target.rsplit_once(':')?; - if path_part.is_empty() - || symbol.is_empty() - || parse_range(symbol).is_some() - || (path_part.len() == 1 - && path_part.as_bytes()[0].is_ascii_alphabetic() - && (symbol.starts_with('/') || symbol.starts_with('\\'))) - { - return None; + match resolve_path_symbol_resolution(target, scope) { + PathSymbolResolution::Unique { + path, + symbol, + start_line, + end_line, + } => Some(PathSymbolTarget { + path, + symbol, + range: Some((start_line, end_line)), + }), + // The historical signature returned Some with range=None for a resolvable + // file whose symbol didn't match, so callers (e.g. `show` suggestion) + // treat it as a known path with no exact body. Preserve that behavior for + // the named-file-unresolved state. + PathSymbolResolution::NamedFileUnresolved { path, symbol, .. } => Some(PathSymbolTarget { + path, + symbol, + range: None, + }), + // A readable real file that has no structural outline (e.g. plain text) + // is still a recognized `path:symbol` on a real file: the prior wrapper + // returned Some(range=None) whenever resolve_existing_file+read + // succeeded. Preserve that for the resolvable-but-no-outline state. + PathSymbolResolution::NamedFileUnresolvable { path, symbol, .. } => { + Some(PathSymbolTarget { + path, + symbol, + range: None, + }) + } + // Missing path, unreadable file, ambiguity, and colon-bearing selectors + // return None (the prior `resolve_existing_file(...)?` / + // `fs::read(...).ok()?` early returns produced None for a missing or + // unreadable path). They surface through resolve_path_symbol_resolution. + _ => None, } - let path = resolve_existing_file(path_part, scope)?; - let buf = std::fs::read(&path).ok()?; - let range = resolve_symbol(&buf, &path, symbol); - Some(PathSymbolTarget { - path, - symbol: symbol.to_string(), - range, - }) } fn resolve_existing_file(raw: &str, scope: &Path) -> Option { @@ -1038,8 +1195,10 @@ fn resolve_existing_file(raw: &str, scope: &Path) -> Option { }) } -/// Resolve a symbol name to its line range using AST outline. -/// Returns (`start_line`, `end_line`) if found. +/// Resolve a symbol name to its first matching `(start_line, end_line)` against +/// the AST outline. Retained for the `--section` reader, which intentionally +/// preserves first-match navigation for pre-existing exact-section behavior. +/// New exact-body plumbing uses `resolve_symbol_ranges` for cardinality. fn resolve_symbol(buf: &[u8], path: &Path, symbol: &str) -> Option<(usize, usize)> { let content = std::str::from_utf8(buf).ok()?; let lang = detect_file_type(path).structural_lang()?; @@ -1050,6 +1209,24 @@ fn resolve_symbol(buf: &[u8], path: &Path, symbol: &str) -> Option<(usize, usize .map(|(start, end)| (start as usize, end as usize)) } +/// Resolve a symbol name against the AST outline, returning every distinct +/// `(start_line, end_line)` match in deterministic order. Uses the shared +/// cardinality primitive `resolve_selector_matches` so N>1 same-file definitions +/// (e.g. overloads) are surfaced explicitly rather than silently selecting the +/// first. Returns `Some(vec![...])` for a resolvable file; `None` when the file +/// has no structural language or the outline cannot be read. +fn resolve_symbol_ranges(buf: &[u8], path: &Path, symbol: &str) -> Option> { + let content = std::str::from_utf8(buf).ok()?; + let lang = detect_file_type(path).structural_lang()?; + let entries = lang_get_outline_entries(content, lang); + Some( + crate::lang::qualified::resolve_selector_matches(&entries, Some(lang), symbol) + .into_iter() + .map(|(start, end)| (start as usize, end as usize)) + .collect::>(), + ) +} + /// Collect symbol names from outline entries (recursively) with their line ranges, /// then rank by prefix match + edit distance, returning top `top_n` suggestions. pub(super) fn suggest_symbols(buf: &[u8], path: &Path, query: &str, top_n: usize) -> Vec { diff --git a/src/read/tests.rs b/src/read/tests.rs index e458979..6ac9b8c 100644 --- a/src/read/tests.rs +++ b/src/read/tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::read::section::{resolve_path_symbol_resolution, PathSymbolResolution}; use std::fmt::Write as _; #[test] @@ -1034,3 +1035,145 @@ fn path_symbol_target_reuses_section_resolution_and_rejects_drive_prefix() { let _ = std::fs::remove_dir_all(&dir); } + +#[test] +fn typed_path_symbol_resolution_states() { + let dir = std::env::temp_dir().join(format!( + "srcwalk_psr_unit_{}_{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_or(0, |duration| duration.as_nanos()) + )); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("lib.rs"), "fn target() {\n let x = 1;\n}\n").unwrap(); + std::fs::write(dir.join("C"), "fn a() {}\n").unwrap(); + + // NotForm: no separating colon, range form, naked drive path. + assert_eq!( + resolve_path_symbol_resolution("target", &dir), + PathSymbolResolution::NotForm + ); + assert_eq!( + resolve_path_symbol_resolution("lib.rs:1-3", &dir), + PathSymbolResolution::NotForm + ); + assert_eq!( + resolve_path_symbol_resolution(r"C:\a", &dir), + PathSymbolResolution::NotForm + ); + + // Unique. + match resolve_path_symbol_resolution("lib.rs:target", &dir) { + PathSymbolResolution::Unique { symbol, .. } => { + assert_eq!(symbol, "target"); + } + other => panic!("expected Unique, got {other:?}"), + } + + // NamedFileUnresolved (file exists, zero matches). + assert!(matches!( + resolve_path_symbol_resolution("lib.rs:missing", &dir), + PathSymbolResolution::NamedFileUnresolved { .. } + )); + // Legacy wrapper preserves Some(range=None) for the existing-file, zero-match + // case only. + assert_eq!( + resolve_path_symbol_target("lib.rs:missing", &dir) + .unwrap() + .range, + None + ); + + // NamedPathMissing: non-existent file is NOT NamedFileUnresolved and the + // legacy wrapper returns None (prior behavior from resolve_existing_file()?). + assert!(matches!( + resolve_path_symbol_resolution("missing.rs:target", &dir), + PathSymbolResolution::NamedPathMissing { .. } + )); + assert_eq!(resolve_path_symbol_target("missing.rs:target", &dir), None); + + // Colon-bearing Rust selector => explicit unsupported, not NotForm/dotted. + assert_eq!( + resolve_path_symbol_resolution("lib.rs:A::target", &dir), + PathSymbolResolution::UnsupportedColonSymbol { + symbol: "A::target".to_string() + } + ); + + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn typed_path_symbol_ambiguous_cardinality() { + let dir = std::env::temp_dir().join(format!( + "srcwalk_psr_amb_{}_{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_or(0, |duration| duration.as_nanos()) + )); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write( + dir.join("ctx.rs"), + "fn Get() -> i32 { 1 }\nfn Get() -> i32 { 2 }\n", + ) + .unwrap(); + + match resolve_path_symbol_resolution("ctx.rs:Get", &dir) { + PathSymbolResolution::Ambiguous { symbol, ranges, .. } => { + assert_eq!(symbol, "Get"); + assert_eq!(ranges.len(), 2); + // Distinct (start,end) ranges, deduped + sorted. + assert_eq!(ranges, vec![(1, 1), (2, 2)]); + } + other => panic!("expected Ambiguous, got {other:?}"), + } + + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn windows_drive_path_symbol_splits_after_path_not_drive() { + // Drive colon must not be consumed as the path-symbol separator; these are + // ideal inputs (file does not exist on this host) so the split still lands + // on the canonical dotted symbol, and an existing-file miss is unresolved, + // NOT NotForm (drive-only) — proving the drive colon stayed in the path. + let dir = std::env::temp_dir().join(format!( + "srcwalk_winpsr_{}_{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_or(0, |duration| duration.as_nanos()) + )); + std::fs::create_dir_all(&dir).unwrap(); + + match resolve_path_symbol_resolution( + r"C:\repo\file.cs:JsonSerializerOptions.GetTypeInfoInternal", + &dir, + ) { + PathSymbolResolution::NamedPathMissing { path, symbol } => { + // Confirm the drive colon stayed in the path, not split at "C:". + assert!( + path.as_os_str() + .to_string_lossy() + .contains(r"C:\repo\file.cs"), + "{path:?}" + ); + assert_eq!(symbol, "JsonSerializerOptions.GetTypeInfoInternal"); + } + other => panic!("expected NamedPathMissing, got {other:?}"), + } + + // Same for the forward-slash drive spelling. + assert!(matches!( + resolve_path_symbol_resolution( + "C:/repo/file.cs:JsonSerializerOptions.GetTypeInfoInternal", + &dir, + ), + PathSymbolResolution::NamedPathMissing { symbol, .. } + if symbol == "JsonSerializerOptions.GetTypeInfoInternal" + )); + + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/src/search/callers/bfs.rs b/src/search/callers/bfs.rs index d18729a..7951423 100644 --- a/src/search/callers/bfs.rs +++ b/src/search/callers/bfs.rs @@ -136,7 +136,7 @@ fn parse_hubs(skip_hubs: Option<&str>) -> HashSet { /// - Unsupported-lang files become leaf nodes (inherited from `find_callers_batch`). #[allow(clippy::too_many_arguments)] pub fn search_callers_bfs( - target: &str, + root: crate::search::callers::CallerRoot<'_>, scope: &Path, cache: &OutlineCache, bloom: &crate::index::bloom::BloomFilterCache, @@ -159,7 +159,9 @@ pub fn search_callers_bfs( let mut visited: HashSet<(String, PathBuf)> = HashSet::new(); // Frontier at hop k: set of symbol names whose callers we want next. - let mut frontier: HashSet = HashSet::from([target.to_string()]); + // Only the root hop is exact: it is the terminal key of the resolved + // definition. Every later hop is a plain by-name expansion. + let mut frontier: HashSet = HashSet::from([root.lookup.to_string()]); 'outer: for hop in 1..=max_depth { if frontier.is_empty() { @@ -228,12 +230,10 @@ pub fn search_callers_bfs( } else if visited.insert(key) { // calling_function is qualified ("Class.method" / "mod::func"). // Frontier needs the bare name since find_callers_batch matches - // by-name call sites, not qualified references. - let bare = m - .calling_function - .rsplit_once('.') - .or_else(|| m.calling_function.rsplit_once("::")) - .map_or(m.calling_function.as_str(), |(_, name)| name); + // by-name call sites, not qualified references. US-071 Step 2: + // the terminal key comes from the shared helper, never a local + // ad-hoc split. + let bare = crate::lang::qualified::terminal_callable_key(&m.calling_function); next_frontier.insert(bare.to_string()); } @@ -290,12 +290,12 @@ pub fn search_callers_bfs( .then_with(|| a.from.cmp(&b.from)) }); - Ok(format_bfs(target, scope, &edges, &stats, max_depth)) + Ok(format_bfs(root, scope, &edges, &stats, max_depth)) } /// Deterministic sort + pretty-print BFS edges grouped by hop. fn format_bfs( - target: &str, + root: crate::search::callers::CallerRoot<'_>, scope: &Path, edges: &[BfsEdge], stats: &BfsStats, @@ -305,7 +305,7 @@ fn format_bfs( let _ = writeln!( out, "# BFS callers of \"{}\" in {} — depth={}/{}, {} edge{}, {} ms", - target, + root.display, crate::format::display_path(scope), stats.depth_reached, max_depth, @@ -313,6 +313,13 @@ fn format_bfs( if stats.edges_total == 1 { "" } else { "s" }, stats.elapsed_ms ); + if root.from_exact_path { + let _ = writeln!( + out, + "> Caveat: only the root is exact (`{}` in the named file); every later hop expands by name.", + root.lookup + ); + } if edges.is_empty() { let _ = writeln!( @@ -432,7 +439,7 @@ fn format_bfs( s.total_edges, s.related_edges, s.hop - 1, - target + root.lookup )); } let has_budget_notes = !notes.is_empty(); diff --git a/src/search/callers/mod.rs b/src/search/callers/mod.rs index 18ca58f..69fa4be 100644 --- a/src/search/callers/mod.rs +++ b/src/search/callers/mod.rs @@ -16,5 +16,5 @@ pub(crate) use single::find_callers_batch; #[allow(unused_imports)] pub use single::{ find_callers, find_callers_with_artifact, search_callers_expanded, - search_callers_expanded_with_artifact, CallerMatch, + search_callers_expanded_with_artifact, CallerMatch, CallerRoot, }; diff --git a/src/search/callers/single.rs b/src/search/callers/single.rs index 0b0286d..d403aa8 100644 --- a/src/search/callers/single.rs +++ b/src/search/callers/single.rs @@ -942,10 +942,44 @@ fn find_enclosing_function( ("".to_string(), None) } +/// The by-name key actually searched, plus the canonical target string echoed +/// back to the agent. +/// +/// These differ when the root came from an exact `path:symbol` selector: the +/// path pins *which* definition the agent asked about, while the terminal +/// callable key is what call sites can actually be matched against. Keeping +/// them separate stops the report from claiming path-scoped caller evidence +/// that a by-name search cannot support. +#[derive(Debug, Clone, Copy)] +pub struct CallerRoot<'a> { + /// Terminal callable key used for the direct by-name search. + pub lookup: &'a str, + /// Canonical root string preserved in headers, counts, and recovery. + pub display: &'a str, + /// True when `display` came from an exact `path:symbol` selector, so the + /// report must state that matches remain by-name. + pub from_exact_path: bool, +} + +/// Caveat emitted when the root came from an exact `path:symbol` selector. +/// +/// The path pins which definition was requested, but the call sites below are +/// still found by a direct by-name search, so same-name definitions elsewhere +/// may appear. Say so rather than implying path-scoped caller evidence. +fn exact_root_caveat(root: CallerRoot<'_>) -> String { + if !root.from_exact_path { + return String::new(); + } + format!( + "> Caveat: the path identifies the requested definition of `{}`; call sites are still matched by name, so same-name definitions elsewhere may be included.\n", + root.lookup + ) +} + /// Format and rank caller search results with optional expand. #[allow(dead_code)] pub fn search_callers_expanded( - target: &str, + root: CallerRoot<'_>, scope: &Path, cache: &OutlineCache, session: &Session, @@ -959,7 +993,7 @@ pub fn search_callers_expanded( count_by: Option<&str>, ) -> Result { search_callers_expanded_with_artifact( - target, + root, scope, cache, session, @@ -977,7 +1011,7 @@ pub fn search_callers_expanded( #[allow(clippy::too_many_arguments)] pub fn search_callers_expanded_with_artifact( - target: &str, + root: CallerRoot<'_>, scope: &Path, cache: &OutlineCache, _session: &Session, @@ -991,10 +1025,13 @@ pub fn search_callers_expanded_with_artifact( count_by: Option<&str>, artifact: ArtifactMode, ) -> Result { + // `lookup` drives the by-name search; `target` stays the canonical root that + // headers, counts, pagination, and recovery echo back unchanged. + let target = root.display; let max_matches = limit.unwrap_or(usize::MAX); let group_limit = limit.unwrap_or(50); let mut callers = - find_callers_with_artifact(target, scope, bloom, glob, Some(cache), artifact)?; + find_callers_with_artifact(root.lookup, scope, bloom, glob, Some(cache), artifact)?; let filters = parse_callsite_filters(filter)?; let unfiltered_total = callers.len(); if !filters.is_empty() { @@ -1002,30 +1039,26 @@ pub fn search_callers_expanded_with_artifact( } if callers.is_empty() { - let recovery = crate::read::resolve_path_symbol_target(target, scope) - .filter(|resolved| resolved.range.is_some()) - .map_or_else( - || format!("use `srcwalk discover {target}` or search interface/trait/implementor names."), - |resolved| { - format!( - "use `srcwalk discover {}` or search interface/trait/implementor names; trace accepts bare symbols; `path:symbol` is `context` grammar.", - resolved.symbol - ) - }, - ); + // Recovery searches the key that was actually looked up, and keeps the + // canonical root in the header so the agent can retry the same target. + let recovery = format!( + "use `srcwalk discover {}` or search interface/trait/implementor names.", + root.lookup + ); let next = render_next_actions(&[NextAction::guidance(recovery, "caller miss recovery", 40)]); return Ok(format!( "# Callers of \"{}\" in {} — no call sites found\n\n\ > Caveat: direct by-name search only; misses dynamic dispatch, reflection, macros.\n\ - {next}", + {}{next}", target, crate::format::display_path(scope), + exact_root_caveat(root), )); } if let Some(field) = count_by { - return format_callsite_counts(target, scope, &callers, field, filter, group_limit, offset); + return format_callsite_counts(root, scope, &callers, field, filter, group_limit, offset); } // Sort by relevance (context file first, then by proximity) @@ -1055,8 +1088,9 @@ pub fn search_callers_expanded_with_artifact( .any(|caller| crate::artifact::is_artifact_js_ts_file(&caller.path)); // Format the output as semantic-compact call edges. let mut output = format!( - "# Trace callers: {target} — {total} call site{}\n\n[symbol] {target}\n<- calls\n", - if total == 1 { "" } else { "s" } + "# Trace callers: {target} — {total} call site{}\n{}\n[symbol] {target}\n<- calls\n", + if total == 1 { "" } else { "s" }, + exact_root_caveat(root), ); if js_ts_artifact_callers { @@ -1515,8 +1549,11 @@ fn ceil_char_boundary(text: &str, mut idx: usize) -> usize { idx } +/// Grouped counts are still a by-name caller view, so they carry the same +/// canonical display root and the same exact-root caveat as the row output. +/// Both live in the header, which every `--offset`/`--limit` page reprints. fn format_callsite_counts( - target: &str, + root: CallerRoot<'_>, scope: &Path, callers: &[CallerMatch], field: &str, @@ -1533,10 +1570,12 @@ fn format_callsite_counts( let total = callers.len(); let filter_suffix = filter.map_or(String::new(), |f| format!(" matching `{f}`")); + let target = root.display; let mut output = format!( - "# Trace callers: {target} — {total} call site{} grouped by {field}{}\n\n[symbol] {target}\n<- calls\n", + "# Trace callers: {target} — {total} call site{} grouped by {field}{}\n{}\n[symbol] {target}\n<- calls\n", if total == 1 { "" } else { "s" }, filter_suffix, + exact_root_caveat(root), ); let mut rows: Vec<_> = counts.into_iter().collect(); diff --git a/src/search/display/expand.rs b/src/search/display/expand.rs index 8a0ae30..20a43ae 100644 --- a/src/search/display/expand.rs +++ b/src/search/display/expand.rs @@ -52,7 +52,7 @@ pub(crate) fn append_expand_budget_note(out: &mut String, budget: &ExpandBudget) let used = budget.cap_tokens.saturating_sub(budget.remaining_tokens); let cap = budget.cap_tokens; let rendered = render_next_actions(&[NextAction::guidance( - "read omitted hits with `srcwalk show : -C 10` or `srcwalk show --section `.", + "read omitted hits with `srcwalk show : -C 10` or `srcwalk show :`.", "expanded omitted-hit drilldown", 40, )]); diff --git a/src/search/display/mod.rs b/src/search/display/mod.rs index a208dd3..68f0846 100644 --- a/src/search/display/mod.rs +++ b/src/search/display/mod.rs @@ -27,7 +27,9 @@ mod semantic; mod structural_targets; pub(super) use expand::{append_expand_budget_note, ExpandBudget}; -pub(super) use structural_targets::has_confirmed_structural_targets; +pub(super) use structural_targets::{ + append_structural_next_targets, has_confirmed_structural_targets, +}; #[derive(Default)] pub(super) struct RenderedSourceLines { @@ -1222,7 +1224,7 @@ pub(super) fn format_search_result_with_header( append_next_action( &mut footer, NextAction::guidance( - "read omitted hits with `srcwalk show : -C 10` or `srcwalk show --section `.", + "read omitted hits with `srcwalk show : -C 10` or `srcwalk show :`.", "expanded hits omitted by budget", 70, ), diff --git a/src/search/display/semantic.rs b/src/search/display/semantic.rs index 1228405..f13fab4 100644 --- a/src/search/display/semantic.rs +++ b/src/search/display/semantic.rs @@ -74,25 +74,24 @@ pub(super) fn context_target_for_match(m: &Match, cache: &OutlineCache) -> Optio let range = m.def_range.unwrap_or((m.line, m.line)); if let Some(candidate) = best_semantic_candidate(&entries, m) { if candidate.kind == OutlineKind::Function { - // US-064: `m.def_name` retains the parser-backed `Q.N` form for - // qualified matches, so the footer carries the same selector - // the discover query used instead of reconstructing it. - let selector = m.def_name.clone().unwrap_or_else(|| candidate.name.clone()); + // US-071 Step 1: build the selector from the outline member name, + // not the display-qualified `m.def_name`, so a C# method unter a + // namespace never emits `System.Text.Json.GetTypeInfoInternal`. + let member = candidate.name.clone(); return Some(build_target( &m.path, &entries, candidate.start_line, candidate.end_line, - selector, + &member, )); } } - if let Some((start_line, end_line)) = - find_function_definition_range(&entries, range, m.def_name.as_deref()) - { - let selector = m.def_name.clone().unwrap_or_else(|| start_line.to_string()); + if let Some(member) = find_function_definition_member(&entries, range) { + let start_line = range.0; + let end_line = range.1; return Some(build_target( - &m.path, &entries, start_line, end_line, selector, + &m.path, &entries, start_line, end_line, &member, )); } return None; @@ -103,38 +102,58 @@ pub(super) fn context_target_for_match(m: &Match, cache: &OutlineCache) -> Optio } let candidate = best_enclosing_function(&entries, m.line)?; - let selector = candidate.name.clone(); + let member = candidate.name.clone(); Some(build_target( &m.path, &entries, candidate.start_line, candidate.end_line, - selector, + &member, )) } -/// Build a `ContextTarget` from a candidate range and parser-backed selector. -/// `symbol_backed` is true only when the selector resolves back to exactly this -/// range (round-trip check), so the footer never emits a symbol command that -/// would read a different body. +/// Build a `ContextTarget` from a candidate range and the member name. The +/// selector is constructed from OUTLINE PRIMITIVES via `selector_from_outline` +/// (never the display-qualified name), and `symbol_backed` requires that +/// selector to resolve to EXACTLY this one range (unique cardinality through +/// the shared resolver). Any ambiguity or mismatch falls back to a numeric +/// body address (`symbol_backed` = false). fn build_target( path: &Path, entries: &[OutlineEntry], start_line: u32, end_line: u32, - selector: String, + member: &str, ) -> ContextTarget { let lang = crate::lang::detect_file_type(path).structural_lang(); - let symbol_backed = crate::lang::qualified::resolve_selector_first(entries, lang, &selector) - == Some((start_line, end_line)); + // Canonical owning-container selector from outline primitives (drops the + // namespace). `None` means no owning container resolves (a top-level + // non-Go function) and the bare member name is the display selector. + let qualified = + crate::lang::qualified::selector_from_outline(entries, lang, start_line, end_line, member); + // symbol_backed ALWAYS requires unique cardinality through the shared + // resolver: the candidate selector must resolve to EXACTLY this one range. + // A top-level bare member with N>1 distinct ranges (two same-name + // definitions) is ambiguous for every body, and no body is symbol-backed; + // all fall back to numeric. There is no first-match escape hatch for a + // target advertised canonical/exact. + let symbol_backed = match &qualified { + Some(q) => { + crate::lang::qualified::resolve_selector_matches(entries, lang, q) + == vec![(start_line, end_line)] + } + None => { + crate::lang::qualified::resolve_selector_matches(entries, lang, member) + == vec![(start_line, end_line)] + } + }; ContextTarget { start_line, end_line, - selector, + selector: qualified.unwrap_or_else(|| member.to_string()), symbol_backed, } } - pub(super) fn format_definition_semantic_match( m: &Match, scope: &Path, @@ -520,23 +539,35 @@ fn collect_semantic_candidates( } } -fn find_function_definition_range( - entries: &[OutlineEntry], - range: (u32, u32), - wanted: Option<&str>, -) -> Option<(u32, u32)> { +/// Recover the ACTUAL outline member name of the function definition occupying +/// `range`. This is RANGE-OWNED: it reads the outline tree at the exact range and +/// never derives or blocks on a display-qualified name (`wanted`). A display +/// string like `System.Text.Json.GetTypeInfoInternal` therefore cannot cause the +/// fallback to miss the real member `GetTypeInfoInternal`, and a simple +/// `def_name="foo"` no longer falls through to a line-number member. +/// +/// If multiple distinct Function names share the exact same range (a +/// contradictory fixture), the recovery is ambiguous and returns `None` rather +/// than silently picking the first; duplicate rows carrying the same name at the +/// same range are deduped and fine. +fn find_function_definition_member(entries: &[OutlineEntry], range: (u32, u32)) -> Option { + let mut distinct: Vec<&str> = Vec::new(); + collect_range_names(entries, range, &mut distinct); + distinct.sort_unstable(); + distinct.dedup(); + match distinct.as_slice() { + [only] => Some((*only).to_string()), + _ => None, // zero, or more than one distinct name at the range + } +} + +fn collect_range_names<'a>(entries: &'a [OutlineEntry], range: (u32, u32), out: &mut Vec<&'a str>) { for entry in entries { - if entry.kind == OutlineKind::Function - && (entry.start_line, entry.end_line) == range - && wanted.is_none_or(|name| entry.name == name) - { - return Some(range); - } - if let Some(found) = find_function_definition_range(&entry.children, range, wanted) { - return Some(found); + if entry.kind == OutlineKind::Function && (entry.start_line, entry.end_line) == range { + out.push(&entry.name); } + collect_range_names(&entry.children, range, out); } - None } fn best_enclosing_function(entries: &[OutlineEntry], line: u32) -> Option { @@ -620,3 +651,228 @@ pub(super) fn outline_kind_label(kind: OutlineKind) -> &'static str { OutlineKind::TestCase => "test_case", } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::types::{OutlineEntry, OutlineKind}; + + fn path(name: &str) -> std::path::PathBuf { + std::path::PathBuf::from(name) + } + + fn fn_entry(name: &str, start: u32, end: u32) -> OutlineEntry { + OutlineEntry { + kind: OutlineKind::Function, + name: name.to_string(), + start_line: start, + end_line: end, + signature: None, + children: Vec::new(), + doc: None, + } + } + + fn container( + kind: OutlineKind, + name: &str, + start: u32, + end: u32, + children: Vec, + ) -> OutlineEntry { + OutlineEntry { + kind, + name: name.to_string(), + start_line: start, + end_line: end, + signature: None, + children, + doc: None, + } + } + + // BLOCKER 1 test: two same-file top-level same-name functions -> NEITHER is + // symbol-backed; a unique top-level function IS symbol-backed. + #[test] + fn ambiguous_top_level_member_never_symbol_backed() { + let entries = vec![ + fn_entry("helper", 1, 1), + fn_entry("other", 2, 2), + fn_entry("helper", 3, 3), + ]; + let first = build_target(&path("lib.rs"), &entries, 1, 1, "helper"); + let second = build_target(&path("lib.rs"), &entries, 3, 3, "helper"); + assert!( + !first.symbol_backed, + "first same-name body must not be symbol-backed" + ); + assert!( + !second.symbol_backed, + "second same-name body must not be symbol-backed" + ); + assert_eq!(first.selector, "helper"); + assert_eq!(second.selector, "helper"); + + let unique = build_target(&path("lib.rs"), &entries, 2, 2, "other"); + assert!( + unique.symbol_backed, + "unique top-level member must be symbol-backed" + ); + assert_eq!(unique.selector, "other"); + } + + // BLOCKER 2: the fallback is range-owned. It recovers the ACTUAL outline + // member at the exact range and never derives/blocks on a display name. A + // simple `def_name=Some("foo")` rebuilds `foo` (not a line number), and a + // multi-dot `Some("System.Text.Json.GetTypeInfoInternal")` no longer trips + // the T21 trap — the real member `GetTypeInfoInternal` is recovered. + // + // These go through `context_target_for_match` with a real Match + a real + // outline file on disk (via OutlineCache), so the production fallback path + // (not just the helper) is exercised. + fn temp_outline(ext: &str, content: &str) -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!( + "srcwalk_us071_semval_{}_{}", + ext, + std::process::id() + )); + std::fs::create_dir_all(&dir).unwrap(); + let file = dir.join(format!("file.{ext}")); + std::fs::write(&file, content).unwrap(); + file + } + + fn definition_match(file: &std::path::Path, def_name: Option<&str>, line: u32) -> Match { + Match { + path: file.to_path_buf(), + line, + text: String::new(), + is_definition: true, + exact: true, + file_lines: 1000, + mtime: std::time::SystemTime::now(), + def_range: Some((line, line)), + def_name: def_name.map(str::to_string), + def_weight: 0, + impl_target: None, + base_target: None, + in_comment: false, + } + } + + // Simple `def_name=Some("foo")` -> rebuilds the real member "foo" and is + // symbol-backed. The matched body is the single `fn foo`. + #[test] + fn fallback_recovers_simple_def_name_member() { + let file = temp_outline( + "rs", + "fn helper() -> i32 { 0 } +fn foo() -> i32 { 7 } +", + ); + let m = definition_match(&file, Some("foo"), 2); + let cache = OutlineCache::default(); + let target = context_target_for_match(&m, &cache); + assert_eq!(target.as_ref().map(|t| t.selector.as_str()), Some("foo")); + assert_eq!(target.as_ref().map(|t| t.symbol_backed), Some(true)); + std::fs::remove_dir_all(file.parent().unwrap()).ok(); + } + + // Multi-dot display name is the T21 trap: `best_semantic_candidate`'s wanted + // filter (whole multi-dot) rejects, so the FALLBACK runs and must recover the + // real member GetTypeInfoInternal, NOT manufacture from the display string. + #[test] + fn fallback_recovers_actual_member_for_multidot_def_name() { + let file = temp_outline( + "cs", + "namespace System.Text.Json { + class JsonSerializerOptions { + void GetTypeInfoInternal() { } + } +} +", + ); + let m = definition_match(&file, Some("System.Text.Json.GetTypeInfoInternal"), 3); + let cache = OutlineCache::default(); + let target = context_target_for_match(&m, &cache); + let target = target.expect("fallback must produce a context target"); + // Owning class selector, never the display-qualified namespace chain. + assert_eq!(target.selector, "JsonSerializerOptions.GetTypeInfoInternal"); + assert!(target.symbol_backed); + std::fs::remove_dir_all(file.parent().unwrap()).ok(); + } + + // Two distinct Function names at the SAME exact range -> ambiguous, None. + fn only_function_entry(name: &str, start: u32, end: u32) -> OutlineEntry { + fn_entry(name, start, end) + } + #[test] + fn contradictory_same_range_different_names_is_ambiguous() { + let entries = vec![container( + OutlineKind::Class, + "Thing", + 1, + 10, + vec![ + only_function_entry("Run", 5, 5), + only_function_entry("Other", 5, 5), + ], + )]; + let member = find_function_definition_member(&entries, (5, 5)); + assert_eq!( + member, None, + "two distinct names at same range must be ambiguous" + ); + } + + // Qualified member: C# class under a namespace -> selector from owning + // class, drop namespace, symbol-backed when unique. + #[test] + fn qualified_container_selector_symbol_backed_when_unique() { + let entries = vec![container( + OutlineKind::Module, + "System.Text.Json", + 1, + 120, + vec![container( + OutlineKind::Class, + "JsonSerializerOptions", + 2, + 118, + vec![fn_entry("GetTypeInfoInternal", 55, 65)], + )], + )]; + let target = build_target( + &path("JsonSerializerOptions.cs"), + &entries, + 55, + 65, + "GetTypeInfoInternal", + ); + assert_eq!(target.selector, "JsonSerializerOptions.GetTypeInfoInternal"); + assert!( + target.symbol_backed, + "unique owning-class member must be symbol-backed" + ); + } + + // Qualified member with N>1 distinct same-file ranges -> not symbol-backed. + #[test] + fn qualified_ambiguous_member_never_symbol_backed() { + let entries = vec![container( + OutlineKind::Class, + "JsonSerializerContext", + 1, + 120, + vec![ + fn_entry("GetTypeInfo", 108, 108), + fn_entry("GetTypeInfo", 110, 118), + ], + )]; + let a = build_target(&path("ctx.cs"), &entries, 108, 108, "GetTypeInfo"); + let b = build_target(&path("ctx.cs"), &entries, 110, 118, "GetTypeInfo"); + assert_eq!(a.selector, "JsonSerializerContext.GetTypeInfo"); + assert!(!a.symbol_backed); + assert!(!b.symbol_backed); + } +} diff --git a/src/search/display/structural_targets.rs b/src/search/display/structural_targets.rs index d71cfd3..50840cc 100644 --- a/src/search/display/structural_targets.rs +++ b/src/search/display/structural_targets.rs @@ -15,10 +15,11 @@ use super::semantic; use super::RenderedSourceLines; /// Header line for the confirmed-target block. It teaches the classification -/// (run the printed `Next` command; symbol when stable, numeric as a safe -/// fallback) instead of claiming every target is symbol-addressed. +/// (run the printed `Next` command; the canonical `:` target when +/// stable, numeric as a safe fallback) instead of claiming every target is +/// symbol-addressed. const CONFIRMED_TARGETS_HEADER: &str = - "## Confirmed structural targets - run the printed Next command (symbol when stable; numeric as fallback)"; + "## Confirmed structural targets - run the printed Next command (exact : when stable; numeric range as fallback)"; /// Single line emitted after any numeric-fallback targets. Kept at block level /// so several fallbacks in one output share one explanation instead of @@ -26,6 +27,13 @@ const CONFIRMED_TARGETS_HEADER: &str = const NUMERIC_FALLBACK_NOTE: &str = "> Note: numeric fallback - no unique symbol selector resolves to the exact body, so the printed range is the safe read."; +/// Single block-level line emitted when at least one canonical `:` +/// target was printed. It states the reuse contract (the same string is +/// accepted unchanged by all four exact-target commands) and keeps the caller +/// evidence bound honest. +const CANONICAL_TARGET_NOTE: &str = + "> Note: : is the exact target; the same string is accepted unchanged by show, context, trace callers, and trace callees (keep any --scope printed with it). Caller matches remain by-name, so same-name definitions elsewhere may be included."; + struct StructuralTarget { path: PathBuf, start_line: u32, @@ -58,6 +66,25 @@ impl StructuralTarget { fn requires_section_form(&self, scope: &Path) -> bool { self.path_arg_relative_to(scope).contains(',') } + + /// Canonical exact target `:` — the one string every + /// exact-target command accepts unchanged. Emitted only when: + /// - the selector is parser-backed; + /// - the displayed path holds no comma, because `show`/`context` read a + /// comma as a multi-target separator, so the inline form could not be + /// copied as a single target; + /// - the display addresses a file at all (`Unaddressable` is the shortened + /// `dir/file` label `rel_nonempty` falls back to). + fn canonical_target(&self, scope: &Path) -> Option { + if !self.symbol_backed || self.requires_section_form(scope) { + return None; + } + let (displayed, basis) = format::rel_with_basis(&self.path, scope); + if basis == format::DisplayBasis::Unaddressable { + return None; + } + Some(format!("{displayed}:{}", self.selector)) + } } pub(in crate::search) fn has_confirmed_structural_targets( @@ -70,7 +97,7 @@ pub(in crate::search) fn has_confirmed_structural_targets( .any(|m| semantic::context_target_for_match(m, cache).is_some()) } -pub(super) fn append_structural_next_targets( +pub(in crate::search) fn append_structural_next_targets( out: &mut String, result: &SearchResult, cache: &OutlineCache, @@ -81,8 +108,8 @@ pub(super) fn append_structural_next_targets( return false; } - let (actions, fallback_count) = - show_actions_for_targets(&targets, &result.scope, rendered_lines); + let plan = show_actions_for_targets(&targets, &result.scope, rendered_lines); + let actions = plan.actions; let rendered = render_next_actions(&actions); if rendered.is_empty() { let target = &targets[0]; @@ -110,7 +137,11 @@ pub(super) fn append_structural_next_targets( out.push_str(CONFIRMED_TARGETS_HEADER); out.push('\n'); out.push_str(&rendered); - if fallback_count > 0 { + if plan.canonical_count > 0 { + out.push('\n'); + out.push_str(CANONICAL_TARGET_NOTE); + } + if plan.fallback_count > 0 { out.push('\n'); out.push_str(NUMERIC_FALLBACK_NOTE); } @@ -146,13 +177,21 @@ fn collect_structural_targets( targets } -/// Build the confirmed-target actions plus how many of them are numeric -/// fallbacks (used to emit a single block-level explanation). +/// Confirmed-target actions plus the block-level counts that decide which +/// single explanation lines follow them. +struct TargetActions { + actions: Vec, + /// Targets printed as a numeric `:` fallback. + fallback_count: usize, + /// Targets printed as a canonical `:` exact target. + canonical_count: usize, +} + fn show_actions_for_targets( targets: &[StructuralTarget], scope: &Path, rendered: &RenderedSourceLines, -) -> (Vec, usize) { +) -> TargetActions { let ranges = targets .iter() .map(|target| (target.start_line, target.end_line)) @@ -160,14 +199,24 @@ fn show_actions_for_targets( let kept_targets = bounded_line_range_indices(&ranges) .into_iter() .map(|index| &targets[index]) - // US-063: drop any target whose full range was already rendered - // verbatim in this packet — offering it again is pure redundancy. The - // whole range must be covered; a partial render keeps the offer. - .filter(|target| !rendered.contains_range(&target.path, target.start_line, target.end_line)) + // US-063: a fully-rendered NUMERIC read is pure redundancy (the body is + // already inline), so it stays suppressed. A fully-rendered CANONICAL + // `:` is kept: the same exact target remains reusable for + // context, callers, and callees even though its body is already shown. + // A partial render always keeps the offer. + .filter(|target| { + let fully_rendered = + rendered.contains_range(&target.path, target.start_line, target.end_line); + !fully_rendered || target.canonical_target(scope).is_some() + }) .collect::>(); if kept_targets.is_empty() { - return (Vec::new(), 0); + return TargetActions { + actions: Vec::new(), + fallback_count: 0, + canonical_count: 0, + }; } // US-060b: split wide (>W-line) targets out so their numeric range anchors @@ -204,8 +253,8 @@ fn show_actions_for_targets( } // Each narrow target is offered as its own read command. Symbol-backed - // targets use `show --section `; the rest fall back to a - // numeric range and are labeled at block level. + // targets use the canonical `:` exact target; the rest fall + // back to a numeric range and are labeled at block level. for (index, target) in narrow.iter().enumerate() { if !target.symbol_backed { fallback_count += 1; @@ -218,7 +267,19 @@ fn show_actions_for_targets( target.anchor(), )); } - (actions, fallback_count) + + // The reuse note only holds for targets actually printed in canonical form. + let canonical_count = wide + .iter() + .chain(narrow.iter()) + .filter(|target| target.canonical_target(scope).is_some()) + .count(); + + TargetActions { + actions, + fallback_count, + canonical_count, + } } /// US-060b: keep the numeric range of a wide target visible as evidence while @@ -234,12 +295,23 @@ fn anchor_line(target: &StructuralTarget, scope: &Path) -> String { } fn show_command_for_target(target: &StructuralTarget, scope: &Path) -> String { - // Symbol form: `show --section ` for a round-tripping - // parser-backed selector. The path is quoted separately so a comma inside - // a quoted path never becomes a multi-target command. + let scope_suffix = scope_suffix(target, scope); + + // Canonical exact target: one `:` string that show, context, + // trace callers, and trace callees all accept unchanged. + if let Some(canonical) = target.canonical_target(scope) { + return format!( + "srcwalk show {}{scope_suffix}", + quote_or_placeholder(&canonical) + ); + } + + // Parser-backed selector inside a comma-bearing path: the inline form would + // be split into multiple targets, so keep the separately quoted `--section` + // command. It reads the same body but is not the canonical reusable target. if target.symbol_backed { return format!( - "srcwalk show {} --section {}", + "srcwalk show {} --section {}{scope_suffix}", quote_or_placeholder(&target.path_arg_relative_to(scope)), quote_or_placeholder(&target.selector) ); @@ -248,18 +320,35 @@ fn show_command_for_target(target: &StructuralTarget, scope: &Path) -> String { // Numeric fallback for exceptional targets with no stable symbol selector. if target.requires_section_form(scope) { return format!( - "srcwalk show {} --section {}", + "srcwalk show {} --section {}{scope_suffix}", quote_or_placeholder(&target.path_arg_relative_to(scope)), quote_or_placeholder(&target.range_arg()) ); } format!( - "srcwalk show {}", + "srcwalk show {}{scope_suffix}", quote_or_placeholder(&target.location_arg_relative_to(scope)) ) } +/// The `--scope ` a printed command needs to resolve its target. +/// +/// Driven by the SAME `rel_with_basis` result that produced the displayed path, +/// so the suffix can never disagree with what the display is relative to. Only +/// a scope-relative display needs it; a CWD-relative or full path runs verbatim. +fn scope_suffix(target: &StructuralTarget, scope: &Path) -> String { + if format::rel_with_basis(&target.path, scope).1 != format::DisplayBasis::Scope { + return String::new(); + } + let scope = format::display_path(scope); + // An empty scope display cannot be turned into a usable flag. + if scope.is_empty() { + return String::new(); + } + format!(" --scope {}", quote_or_placeholder(&scope)) +} + fn quote_or_placeholder(value: &str) -> String { format::shell_quote_arg(value).unwrap_or_else(|| "".to_string()) } @@ -310,7 +399,7 @@ mod tests { #[test] fn batched_show_quotes_combined_space_targets() { let scope = Path::new(""); - let (actions, _) = show_actions_for_targets( + let TargetActions { actions, .. } = show_actions_for_targets( &[target("a file.rs", 1, 1), target("b file.rs", 2, 2)], scope, &RenderedSourceLines::default(), @@ -322,24 +411,24 @@ mod tests { } #[test] - fn symbol_target_renders_symbol_section_command() { + fn symbol_target_renders_canonical_path_symbol_command() { let scope = Path::new(""); let command = show_command_for_target(&symbol_target("lib.rs", 1, 3, "target"), scope); - assert_eq!(command, "srcwalk show lib.rs --section target"); + assert_eq!(command, "srcwalk show lib.rs:target"); } #[test] fn symbol_target_preserves_qualified_selector() { let scope = Path::new(""); let command = show_command_for_target(&symbol_target("batch.go", 3, 3, "Batch.Set"), scope); - assert_eq!(command, "srcwalk show batch.go --section Batch.Set"); + assert_eq!(command, "srcwalk show batch.go:Batch.Set"); } #[test] fn symbol_target_quotes_space_path_and_selector() { let scope = Path::new(""); let command = show_command_for_target(&symbol_target("a file.rs", 1, 3, "my fn"), scope); - assert_eq!(command, "srcwalk show 'a file.rs' --section 'my fn'"); + assert_eq!(command, "srcwalk show 'a file.rs:my fn'"); } #[test] @@ -352,7 +441,7 @@ mod tests { #[test] fn batched_show_splits_comma_paths() { let scope = Path::new(""); - let (actions, _) = show_actions_for_targets( + let TargetActions { actions, .. } = show_actions_for_targets( &[target("a,file.rs", 1, 1), target("b.rs", 2, 2)], scope, &RenderedSourceLines::default(), @@ -367,7 +456,7 @@ mod tests { #[test] fn over_cap_singleton_emits_no_action() { - let (actions, _) = show_actions_for_targets( + let TargetActions { actions, .. } = show_actions_for_targets( &[target("large.rs", 1, 201)], Path::new(""), &RenderedSourceLines::default(), @@ -378,7 +467,11 @@ mod tests { #[test] fn over_cap_ranges_skip_and_keep_contiguous_priorities() { let scope = Path::new(""); - let (actions, fallbacks) = show_actions_for_targets( + let TargetActions { + actions, + fallback_count: fallbacks, + .. + } = show_actions_for_targets( &[ target("a,file.rs", 1, 150), target("b,file.rs", 1, 51), @@ -409,7 +502,11 @@ mod tests { fn wide_boundary_stays_plain_offer() { let scope = Path::new(""); // Width W (40) is the boundary: exactly 40 lines stays a plain offer. - let (actions, fallbacks) = show_actions_for_targets( + let TargetActions { + actions, + fallback_count: fallbacks, + .. + } = show_actions_for_targets( &[target("p.rs", 1, 40)], scope, &RenderedSourceLines::default(), @@ -424,7 +521,11 @@ mod tests { let scope = Path::new(""); // Width W+1 (41) >W, non-symbol-backed: numeric command is the primary // action (no separate anchor, no `> expand:`). - let (actions, fallbacks) = show_actions_for_targets( + let TargetActions { + actions, + fallback_count: fallbacks, + .. + } = show_actions_for_targets( &[target("p.rs", 1, 41)], scope, &RenderedSourceLines::default(), @@ -444,7 +545,11 @@ mod tests { let scope = Path::new(""); // Wide symbol-backed target: the symbol command is the primary action // and the numeric range is non-action evidence metadata. - let (actions, fallbacks) = show_actions_for_targets( + let TargetActions { + actions, + fallback_count: fallbacks, + .. + } = show_actions_for_targets( &[symbol_target( "semantic.rs", 156, @@ -458,7 +563,7 @@ mod tests { assert_eq!(fallbacks, 0); assert_eq!( actions[0].command(), - "srcwalk show semantic.rs --section format_definition_semantic_match_with_path" + "srcwalk show semantic.rs:format_definition_semantic_match_with_path" ); assert_eq!( actions[0].reason(), @@ -469,7 +574,7 @@ mod tests { #[test] fn wide_symbol_backed_anchor_is_non_action_metadata() { let scope = Path::new(""); - let (actions, _) = show_actions_for_targets( + let TargetActions { actions, .. } = show_actions_for_targets( &[symbol_target("semantic.rs", 156, 272, "Type.method")], scope, &RenderedSourceLines::default(), @@ -478,14 +583,11 @@ mod tests { // The action's command is the symbol command; the anchor is plain // metadata using the START line only, never a competing action or a // repeat of the full body range. - assert_eq!( - preamble.command(), - "srcwalk show semantic.rs --section Type.method" - ); + assert_eq!(preamble.command(), "srcwalk show semantic.rs:Type.method"); let rendered = render_next_actions(std::slice::from_ref(preamble)); assert_eq!( rendered, - " evidence anchor: semantic.rs:156 (bounded preview; not the body address)\n> Next: srcwalk show semantic.rs --section Type.method" + " evidence anchor: semantic.rs:156 (bounded preview; not the body address)\n> Next: srcwalk show semantic.rs:Type.method" ); assert!(!rendered.contains("expand:"), "{rendered}"); assert!(!rendered.contains("> Next: semantic.rs"), "{rendered}"); @@ -502,7 +604,11 @@ mod tests { #[test] fn narrow_symbol_backed_has_exactly_one_symbol_next() { let scope = Path::new(""); - let (actions, fallbacks) = show_actions_for_targets( + let TargetActions { + actions, + fallback_count: fallbacks, + .. + } = show_actions_for_targets( &[symbol_target("lib.rs", 1, 3, "helper")], scope, &RenderedSourceLines::default(), @@ -510,7 +616,7 @@ mod tests { assert_eq!(actions.len(), 1); assert_eq!(fallbacks, 0); let rendered = render_next_actions(&actions); - assert_eq!(rendered, "> Next: srcwalk show lib.rs --section helper"); + assert_eq!(rendered, "> Next: srcwalk show lib.rs:helper"); assert!(!rendered.contains("expand:"), "{rendered}"); assert!(!rendered.contains(":1-3"), "{rendered}"); } @@ -518,15 +624,95 @@ mod tests { #[test] fn header_teaches_classification() { assert!(CONFIRMED_TARGETS_HEADER.contains("run the printed Next command")); - assert!(CONFIRMED_TARGETS_HEADER.contains("symbol when stable")); - assert!(CONFIRMED_TARGETS_HEADER.contains("numeric as fallback")); + assert!(CONFIRMED_TARGETS_HEADER.contains("exact : when stable")); + assert!(CONFIRMED_TARGETS_HEADER.contains("numeric range as fallback")); + } + + #[test] + fn canonical_note_names_all_four_commands_and_keeps_caller_bound() { + for command in ["show", "context", "trace callers", "trace callees"] { + assert!(CANONICAL_TARGET_NOTE.contains(command), "{command}"); + } + // The reuse claim must not upgrade caller evidence. + assert!(CANONICAL_TARGET_NOTE.contains("by-name")); + assert!(CANONICAL_TARGET_NOTE.contains("same-name definitions")); + } + + #[test] + fn canonical_count_covers_only_targets_printed_as_path_symbol() { + let scope = Path::new(""); + let plan = show_actions_for_targets( + &[ + // canonical: parser-backed, comma-free path + symbol_target("a.rs", 1, 3, "Type.method"), + // parser-backed but comma path -> `--section`, not canonical + symbol_target("b,file.rs", 1, 3, "helper"), + // not parser-backed -> numeric fallback + target("c.rs", 1, 3), + ], + scope, + &RenderedSourceLines::default(), + ); + assert_eq!(plan.canonical_count, 1); + assert_eq!(plan.fallback_count, 1); + assert_eq!(plan.actions[0].command(), "srcwalk show a.rs:Type.method"); + assert_eq!( + plan.actions[1].command(), + "srcwalk show 'b,file.rs' --section helper" + ); + assert_eq!(plan.actions[2].command(), "srcwalk show c.rs:1-3"); + } + + #[test] + fn scope_suffix_is_printed_only_when_the_display_needs_it() { + // Search results carry canonicalized paths, so the CWD case must be + // built the same way the path cache builds it (a raw `current_dir()` + // join is a representation production never produces, and on Windows a + // non-verbatim path never matches the canonical CWD prefix). + let cwd = std::env::current_dir().unwrap(); + let cwd = cwd.canonicalize().unwrap_or(cwd); + let under_cwd = symbol_target( + cwd.join("src").join("lib.rs").to_str().unwrap(), + 1, + 3, + "helper", + ); + assert_eq!( + format::rel_with_basis(&under_cwd.path, &cwd).1, + format::DisplayBasis::SelfContained + ); + assert_eq!(scope_suffix(&under_cwd, &cwd), ""); + assert_eq!( + show_command_for_target(&under_cwd, &cwd), + "srcwalk show src/lib.rs:helper" + ); + + // A scope outside the CWD is displayed scope-relative, so the command + // must carry the scope that makes the copied target resolve. Use a + // host-native absolute path rather than Unix-only `/elsewhere`. + let scope = std::env::temp_dir().join("us071_scope_suffix_pkg"); + let outside = symbol_target(scope.join("lib.rs").to_str().unwrap(), 1, 3, "helper"); + assert_eq!( + format::rel_with_basis(&outside.path, &scope).1, + format::DisplayBasis::Scope + ); + let suffix = scope_suffix(&outside, &scope); + assert!(suffix.starts_with(" --scope "), "{suffix}"); + assert_eq!( + show_command_for_target(&outside, &scope), + format!("srcwalk show lib.rs:helper{suffix}") + ); } #[test] fn numeric_fallback_note_is_single_block_level() { // Two non-symbol-backed wide targets -> one shared fallback note at // block level, not repeated per target. - let (actions, fallbacks) = show_actions_for_targets( + let TargetActions { + actions, + fallback_count: fallbacks, + .. + } = show_actions_for_targets( &[target("a.rs", 1, 41), target("b.rs", 1, 41)], Path::new(""), &RenderedSourceLines::default(), @@ -548,14 +734,17 @@ mod tests { ); // Full containment (1-6) -> offer suppressed. - let (full, _) = show_actions_for_targets(&[target("p.rs", 1, 6)], scope, &rendered); + let TargetActions { actions: full, .. } = + show_actions_for_targets(&[target("p.rs", 1, 6)], scope, &rendered); assert!( full.is_empty(), "fully-rendered target must be suppressed: {full:?}" ); // Partial overlap (1-3 all rendered, 4-8 not) -> offer kept. - let (partial, _) = show_actions_for_targets(&[target("p.rs", 1, 8)], scope, &rendered); + let TargetActions { + actions: partial, .. + } = show_actions_for_targets(&[target("p.rs", 1, 8)], scope, &rendered); assert_eq!(partial.len(), 1, "partial overlap must keep the offer"); assert!( partial[0].command().contains('8'), @@ -564,14 +753,62 @@ mod tests { ); // No overlap (20-25) -> offer kept. - let (none, _) = show_actions_for_targets(&[target("p.rs", 20, 25)], scope, &rendered); + let TargetActions { actions: none, .. } = + show_actions_for_targets(&[target("p.rs", 20, 25)], scope, &rendered); assert_eq!(none.len(), 1); } + #[test] + fn fully_rendered_canonical_target_is_kept_as_reusable_evidence() { + let scope = Path::new(""); + let mut rendered = RenderedSourceLines::default(); + rendered.record_code_block( + Path::new("p.rs"), + " 1 │ a\n 2 │ b\n 3 │ c\n 4 │ d\n 5 │ e\n 6 │ f\n", + ); + + // A canonical : target whose body is already shown inline + // is retained: the same exact string remains reusable for context, + // callers, and callees even though the body is already visible. + let TargetActions { + actions, + canonical_count, + fallback_count, + } = show_actions_for_targets(&[symbol_target("p.rs", 1, 6, "helper")], scope, &rendered); + assert_eq!(actions.len(), 1); + assert_eq!(actions[0].command(), "srcwalk show p.rs:helper"); + assert_eq!(canonical_count, 1); + assert_eq!(fallback_count, 0); + } + + #[test] + fn fully_rendered_comma_path_symbol_target_is_suppressed() { + let scope = Path::new(""); + let mut rendered = RenderedSourceLines::default(); + rendered.record_code_block(Path::new("p,file.rs"), " 1 │ a\n 2 │ b\n 3 │ c\n"); + + // A symbol-backed target whose path holds a comma renders via + // `--section` (never canonical), so a fully-rendered body is still a + // redundant read and stays suppressed. + let TargetActions { actions, .. } = show_actions_for_targets( + &[symbol_target("p,file.rs", 1, 3, "helper")], + scope, + &rendered, + ); + assert!( + actions.is_empty(), + "comma-path section read must be suppressed: {actions:?}" + ); + } + #[test] fn mixed_symbol_and_fallback_emit_one_primary_action_each() { let scope = Path::new(""); - let (actions, fallbacks) = show_actions_for_targets( + let TargetActions { + actions, + fallback_count: fallbacks, + .. + } = show_actions_for_targets( &[ symbol_target("a.rs", 1, 3, "stableOne"), target("b.rs", 1, 41), diff --git a/src/search/mod.rs b/src/search/mod.rs index b85c65c..1712967 100644 --- a/src/search/mod.rs +++ b/src/search/mod.rs @@ -47,9 +47,9 @@ pub(crate) use self::low_signal::{ use self::pagination::paginate; use self::display::{ - append_expand_budget_note, append_symbol_ambiguity_caveat, format_matches, - format_search_result, format_search_result_with_header, has_confirmed_structural_targets, - ExpandBudget, RenderedSourceLines, + append_expand_budget_note, append_structural_next_targets, append_symbol_ambiguity_caveat, + format_matches, format_search_result, format_search_result_with_header, + has_confirmed_structural_targets, ExpandBudget, RenderedSourceLines, }; const DEFAULT_MULTI_SYMBOL_MATCH_LIMIT: usize = 3; @@ -430,6 +430,12 @@ pub fn search_multi_symbol_expanded( &mut smart_truncated, &mut out, ); + // US-071 Step 4: invoke the same structural target planner the + // single-symbol formatter uses, once per paginated per-query section, + // after format_matches and before pagination/truncation footer actions. + // Targets derive only from this query's retained matches. + append_structural_next_targets(&mut out, &result, cache, &rendered_source_lines); + let next_offset = result.offset + result.matches.len(); let omitted = result.total_found.saturating_sub(next_offset); if omitted > 0 { diff --git a/tests/intent_first_cli.rs b/tests/intent_first_cli.rs index 273572b..0612272 100644 --- a/tests/intent_first_cli.rs +++ b/tests/intent_first_cli.rs @@ -2196,17 +2196,15 @@ fn caller() -> i32 { "{stdout}" ); assert!( - stdout.contains("> Next: srcwalk show lib.rs --section target"), + stdout.contains("> Next: srcwalk show lib.rs:target"), "{stdout}" ); assert!( - stdout.contains("> Next: srcwalk show lib.rs --section caller"), + stdout.contains("> Next: srcwalk show lib.rs:caller"), "{stdout}" ); assert_eq!( - stdout - .matches("> Next: srcwalk show lib.rs --section target") - .count(), + stdout.matches("> Next: srcwalk show lib.rs:target").count(), 1, "confirmed structural next action should be deduplicated:\n{stdout}" ); @@ -2292,16 +2290,16 @@ fn discover_multiple_structural_matches_suggest_batched_show_target() { "{stdout}" ); assert!( - stdout.contains("> Next: srcwalk show a/lib.rs --section target"), + stdout.contains("> Next: srcwalk show a/lib.rs:target"), "expected symbol show next action:\n{stdout}" ); assert!( - stdout.contains("> Next: srcwalk show b/lib.rs --section target"), + stdout.contains("> Next: srcwalk show b/lib.rs:target"), "expected symbol show next action:\n{stdout}" ); assert_eq!( stdout - .matches("> Next: srcwalk show a/lib.rs --section target") + .matches("> Next: srcwalk show a/lib.rs:target") .count(), 1, "symbol show next action should be deduplicated:\n{stdout}" @@ -2328,7 +2326,7 @@ fn discover_structural_show_targets_quote_spaces_and_comma_paths() { assert!(output.status.success()); let stdout = String::from_utf8_lossy(&output.stdout); assert!( - stdout.contains("> Next: srcwalk show 'a file.rs' --section target"), + stdout.contains("> Next: srcwalk show 'a file.rs:target'"), "space-bearing target must be shell-quoted:\n{stdout}" ); @@ -2393,11 +2391,11 @@ export function startServer() { "{stdout}" ); assert!( - stdout.contains("> Next: srcwalk show server.ts --section createGatewayServer"), + stdout.contains("> Next: srcwalk show server.ts:createGatewayServer"), "{stdout}" ); assert!( - stdout.contains("> Next: srcwalk show server.ts --section startServer"), + stdout.contains("> Next: srcwalk show server.ts:startServer"), "{stdout}" ); assert!( @@ -2795,93 +2793,156 @@ func Beta() {} } #[test] -fn path_symbol_rejections_preserve_command_intent() { - let dir = temp_repo("path_symbol_rejections"); +fn path_symbol_exact_target_is_consumed_by_show_and_trace() { + let dir = temp_repo("path_symbol_exact_target"); fs::write( dir.join("lib.rs"), "fn target() {\n helper();\n}\nfn helper() {}\n", ) .unwrap(); + // `trace callees` roots on the exact definition named by the path. let trace = srcwalk() .args(["trace", "callees", "lib.rs:target", "--detailed", "--scope"]) .arg(&dir) .output() .unwrap(); - assert!(!trace.status.success()); let trace_stderr = String::from_utf8_lossy(&trace.stderr); assert!( - trace_stderr.contains("> Did you mean: target"), - "{trace_stderr}" - ); - assert!( - trace_stderr.contains("`path:symbol` targets are accepted by `context`, not `trace`."), - "{trace_stderr}" + trace.status.success(), + "exact path:symbol root should be consumed, stderr:\n{trace_stderr}" ); + let trace_stdout = String::from_utf8_lossy(&trace.stdout); + assert!(trace_stdout.contains("helper"), "{trace_stdout}"); assert!( - trace_stderr.contains("srcwalk trace callees target --detailed --scope"), - "{trace_stderr}" + !trace_stderr.contains("`path:symbol` targets are accepted by `context`, not `trace`."), + "path:symbol must no longer be rejected by trace:\n{trace_stderr}" ); + // `show` reads the exact owning body, i.e. `show --section `. let show = srcwalk() .args(["show", "lib.rs:target", "--scope"]) .arg(&dir) .output() .unwrap(); - assert!(!show.status.success()); let show_stderr = String::from_utf8_lossy(&show.stderr); assert!( - show_stderr.contains("did you mean: lib.rs:1-3"), - "{show_stderr}" + show.status.success(), + "exact path:symbol should read the owning body, stderr:\n{show_stderr}" ); + let show_stdout = String::from_utf8_lossy(&show.stdout); + assert!(show_stdout.contains("fn target()"), "{show_stdout}"); + assert!(show_stdout.contains("helper();"), "{show_stdout}"); assert!( - show_stderr.contains("`show` takes line ranges; `path:symbol` is `context` grammar."), - "{show_stderr}" + !show_stdout.contains("fn helper() {}"), + "show must read only the owning range, not the whole file:\n{show_stdout}" ); - assert!(!show_stderr.contains("trace callees"), "{show_stderr}"); let _ = fs::remove_dir_all(&dir); } #[test] -fn ambiguous_path_symbol_trace_names_candidates_without_runnable_command() { - let dir = temp_repo("path_symbol_trace_ambiguous"); - fs::write(dir.join("a.rs"), "fn target() {}\n").unwrap(); - fs::write(dir.join("b.rs"), "fn target() {}\n").unwrap(); +fn path_symbol_states_never_broaden_to_a_bare_search() { + let dir = temp_repo("path_symbol_states"); + fs::write(dir.join("lib.rs"), "fn target() {}\n").unwrap(); + fs::write(dir.join("plain.txt"), "target text\n").unwrap(); + // Two same-name definitions in ONE file: ambiguity is per named file. + fs::write( + dir.join("dup.rs"), + "fn dup() {}\nfn other() {}\nfn dup() {}\n", + ) + .unwrap(); - let output = srcwalk() - .args(["trace", "callees", "a.rs:target", "--scope"]) + // Missing named file. + for cmd in [ + vec!["show"], + vec!["context"], + vec!["trace", "callers"], + vec!["trace", "callees"], + ] { + let out = srcwalk() + .args(&cmd) + .args(["missing.rs:target", "--scope"]) + .arg(&dir) + .output() + .unwrap(); + assert!( + !out.status.success(), + "{cmd:?} should abstain on missing path" + ); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("needs an existing named file"), + "{cmd:?}:\n{stderr}" + ); + } + + // Ambiguous within the named file: bounded candidates, no silent first-pick. + let ambiguous = srcwalk() + .args(["show", "dup.rs:dup", "--scope"]) .arg(&dir) .output() .unwrap(); - assert!(!output.status.success()); - let stderr = String::from_utf8_lossy(&output.stderr); + assert!(!ambiguous.status.success()); + let stderr = String::from_utf8_lossy(&ambiguous.stderr); assert!(stderr.contains("Candidates:"), "{stderr}"); - assert!(stderr.contains("a.rs:1-1"), "{stderr}"); - assert!(stderr.contains("b.rs:1-1"), "{stderr}"); - assert!(!stderr.contains("For this symbol:"), "{stderr}"); - assert!(!stderr.contains("Did you mean: target"), "{stderr}"); + assert!(stderr.contains("dup.rs:1-1"), "{stderr}"); + assert!(stderr.contains("dup.rs:3-3"), "{stderr}"); + + // Existing file with no structural outline: honest abstention, not a bare read. + let unresolvable = srcwalk() + .args(["show", "plain.txt:target", "--scope"]) + .arg(&dir) + .output() + .unwrap(); + assert!(!unresolvable.status.success()); + let stderr = String::from_utf8_lossy(&unresolvable.stderr); + assert!(stderr.contains("was not resolved"), "{stderr}"); + assert!(!stderr.contains("did you mean:"), "{stderr}"); let _ = fs::remove_dir_all(&dir); } #[test] -fn unsupported_path_symbol_show_explains_grammar_without_guessing_range() { - let dir = temp_repo("path_symbol_unsupported"); - fs::write(dir.join("plain.txt"), "target text\n").unwrap(); +fn raw_colon_selector_reports_one_shared_intent_across_all_four_commands() { + let dir = temp_repo("path_symbol_colon_intent"); + fs::write(dir.join("lib.rs"), "fn target() {}\n").unwrap(); - let output = srcwalk() - .args(["show", "plain.txt:target", "--scope"]) - .arg(&dir) - .output() - .unwrap(); - assert!(!output.status.success()); - let stderr = String::from_utf8_lossy(&output.stderr); + let mut intents = Vec::new(); + for cmd in [ + vec!["show"], + vec!["context"], + vec!["trace", "callers"], + vec!["trace", "callees"], + ] { + let out = srcwalk() + .args(&cmd) + .args(["lib.rs:A::target", "--scope"]) + .arg(&dir) + .output() + .unwrap(); + assert!( + !out.status.success(), + "{cmd:?} should reject a raw `::` selector" + ); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("`path:symbol` takes a plain selector") + && stderr.contains("contains `::`"), + "{cmd:?}:\n{stderr}" + ); + intents.push( + stderr + .split("takes a plain selector") + .nth(1) + .unwrap_or_default() + .to_string(), + ); + } assert!( - stderr.contains("`show` takes line ranges; `path:symbol` is `context` grammar."), - "{stderr}" + intents.windows(2).all(|w| w[0] == w[1]), + "all four commands must surface the SAME colon intent: {intents:?}" ); - assert!(!stderr.contains("did you mean:"), "{stderr}"); let _ = fs::remove_dir_all(&dir); } @@ -3765,3 +3826,725 @@ fn discover_low_signal_budget_survives_and_replays() { let _ = fs::remove_dir_all(&dir); } + +#[test] +fn path_symbol_exact_root_resolves_qualified_language_forms() { + let dir = temp_repo("path_symbol_qualified_langs"); + // Every file pairs a qualified definition with a same-named sibling that must + // never leak into the exact root: a C# namespaced method, a Go receiver method + // shadowed by a top-level func, and a Rust impl method shadowed by a free fn. + fs::write( + dir.join("Svc.cs"), + "namespace N {\n class Svc {\n public void Run() { Helper(); }\n public void Helper() { Leaf(); }\n public void Leaf() {}\n }\n}\n", + ) + .unwrap(); + fs::write( + dir.join("batch.go"), + "package main\n\ntype Batch struct{}\n\nfunc (b *Batch) Set() {\n\tb.flush()\n}\n\nfunc (b *Batch) flush() {}\n\nfunc Set() {\n\tunrelatedTopLevel()\n}\n\nfunc unrelatedTopLevel() {}\n", + ) + .unwrap(); + fs::write( + dir.join("svc.rs"), + "struct Svc;\nimpl Svc {\n fn run(&self) {\n self.helper();\n }\n fn helper(&self) {}\n}\nfn run() {\n bare_only();\n}\nfn bare_only() {}\n", + ) + .unwrap(); + + for (target, frame, want, unwanted) in [ + ( + "Svc.cs:Svc.Run", + "requested 3; displayed 3", + "Helper", + "Leaf", + ), + ( + "batch.go:Batch.Set", + "requested 5-7; displayed 5-7", + "flush", + "unrelatedTopLevel", + ), + ( + "svc.rs:Svc.run", + "requested 3-5; displayed 3-5", + "helper", + "bare_only", + ), + ] { + // The qualified selector must pin the exact range, not the sibling body. + let shown = srcwalk() + .args(["show", target, "--scope"]) + .arg(&dir) + .arg("--no-budget") + .output() + .unwrap(); + let shown_err = String::from_utf8_lossy(&shown.stderr); + assert!(shown.status.success(), "{target} should show:\n{shown_err}"); + let shown_out = String::from_utf8_lossy(&shown.stdout); + assert!( + shown_out.contains(frame), + "{target} must resolve the exact body frame `{frame}`:\n{shown_out}" + ); + + // ... and the callee view must be built from that body alone. + let out = srcwalk() + .args(["trace", "callees", target, "--scope"]) + .arg(&dir) + .arg("--no-budget") + .output() + .unwrap(); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(out.status.success(), "{target} should resolve:\n{stderr}"); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(stdout.contains(want), "{target} callees:\n{stdout}"); + assert!( + !stdout.contains(unwanted), + "{target} leaked a sibling body's callee:\n{stdout}" + ); + } + + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn path_symbol_exact_root_still_accepts_bare_selectors() { + let dir = temp_repo("path_symbol_adjacent_langs"); + fs::write( + dir.join("Svc.cs"), + "namespace N {\n class Svc {\n public void Run() { Helper(); }\n public void Helper() {}\n }\n}\n", + ) + .unwrap(); + fs::write( + dir.join("svc.go"), + "package main\n\nfunc Run() {\n\tHelper()\n}\n\nfunc Helper() {}\n", + ) + .unwrap(); + fs::write( + dir.join("svc.rs"), + "impl Svc {\n fn run(&self) {\n self.helper();\n }\n fn helper(&self) {}\n}\n", + ) + .unwrap(); + + for (target, want) in [ + ("Svc.cs:Run", "Helper"), + ("svc.go:Run", "Helper"), + ("svc.rs:run", "helper"), + ] { + let out = srcwalk() + .args(["trace", "callees", target, "--scope"]) + .arg(&dir) + .output() + .unwrap(); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(out.status.success(), "{target} should resolve:\n{stderr}"); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(stdout.contains(want), "{target} callees:\n{stdout}"); + } + + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn callers_exact_root_outside_scope_is_readable_but_search_stays_scoped() { + let dir = temp_repo("path_symbol_outside_scope"); + let inside = dir.join("inside"); + fs::create_dir_all(&inside).unwrap(); + // Root definition lives OUTSIDE the --scope directory. + fs::write(dir.join("root.rs"), "fn target() {}\n").unwrap(); + fs::write(inside.join("use.rs"), "fn caller() { target(); }\n").unwrap(); + + let out = srcwalk() + .args(["trace", "callers"]) + .arg(dir.join("root.rs:target")) + .args(["--scope"]) + .arg(&inside) + .output() + .unwrap(); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + out.status.success(), + "an exact root outside --scope must still be readable:\n{stderr}" + ); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout.contains("caller"), + "scoped call site missing:\n{stdout}" + ); + assert!( + stdout.contains("lies outside --scope") && stdout.contains("searched inside --scope only"), + "outside-scope root must be labeled:\n{stdout}" + ); + + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn callees_exact_root_does_not_require_global_uniqueness() { + let dir = temp_repo("path_symbol_same_name_defs"); + fs::write( + dir.join("a.rs"), + "fn target() {\n from_a();\n}\nfn from_a() {}\n", + ) + .unwrap(); + fs::write( + dir.join("b.rs"), + "fn target() {\n from_b();\n}\nfn from_b() {}\n", + ) + .unwrap(); + + for (target, want, unwanted) in [ + ("a.rs:target", "from_a", "from_b"), + ("b.rs:target", "from_b", "from_a"), + ] { + let out = srcwalk() + .args(["trace", "callees", target, "--scope"]) + .arg(&dir) + .output() + .unwrap(); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + out.status.success(), + "a same-name definition elsewhere must not block {target}:\n{stderr}" + ); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(stdout.contains(want), "{target}:\n{stdout}"); + assert!( + !stdout.contains(unwanted), + "{target} leaked the other def:\n{stdout}" + ); + } + + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn callers_depth_two_marks_only_the_root_as_exact() { + let dir = temp_repo("path_symbol_depth_two"); + fs::write( + dir.join("lib.rs"), + "fn target() {}\nfn mid() { target(); }\nfn top() { mid(); }\n", + ) + .unwrap(); + + let out = srcwalk() + .args([ + "trace", + "callers", + "lib.rs:target", + "--depth", + "2", + "--scope", + ]) + .arg(&dir) + .output() + .unwrap(); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(out.status.success(), "{stderr}"); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout.starts_with("# BFS callers of \"lib.rs:target\""), + "canonical root must be preserved in the BFS header:\n{stdout}" + ); + assert!( + stdout.contains("only the root is exact") && stdout.contains("later hop expands by name"), + "depth>=2 must caveat that later hops are by-name:\n{stdout}" + ); + + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn callers_exact_count_by_keeps_canonical_root_and_by_name_caveat() { + let dir = temp_repo("path_symbol_count_by_caveat"); + fs::write( + dir.join("lib.rs"), + "fn target() {}\nfn mid() { target(); }\nfn other() { target(); }\n", + ) + .unwrap(); + + // Counts are a grouped view of the SAME by-name search, so the canonical root + // and the exact-root caveat must survive --count-by, --filter, and every page. + for extra in [ + vec!["--count-by", "caller"], + vec!["--count-by", "caller", "--filter", "args:0"], + vec![ + "--count-by", + "caller", + "--filter", + "args:0", + "--limit", + "1", + "--offset", + "1", + ], + ] { + let out = srcwalk() + .args(["trace", "callers", "lib.rs:target"]) + .args(&extra) + .arg("--scope") + .arg(&dir) + .arg("--no-budget") + .output() + .unwrap(); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(out.status.success(), "{extra:?}:\n{stderr}"); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout.contains("[symbol] lib.rs:target"), + "{extra:?} must echo the canonical root:\n{stdout}" + ); + assert!( + stdout.contains("the path identifies the requested definition of `target`") + && stdout.contains("call sites are still matched by name"), + "{extra:?} must keep the exact-root by-name caveat:\n{stdout}" + ); + } + + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn callees_exact_depth_two_marks_only_the_root_as_exact() { + let dir = temp_repo("path_symbol_callee_depth_two"); + fs::write( + dir.join("lib.rs"), + "fn leaf() {}\nfn mid() { leaf(); }\nfn top() { mid(); }\n", + ) + .unwrap(); + + let out = srcwalk() + .args(["trace", "callees", "lib.rs:top", "--depth", "2", "--scope"]) + .arg(&dir) + .arg("--no-budget") + .output() + .unwrap(); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(out.status.success(), "{stderr}"); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout.contains("leaf"), + "depth 2 should expand a transitive hop:\n{stdout}" + ); + assert!( + stdout.contains("only the root is exact") && stdout.contains("later hop resolves by name"), + "depth>=2 callees must caveat that later hops are by-name:\n{stdout}" + ); + + // Depth 1 renders no later hop, so it must not claim one. + let direct = srcwalk() + .args(["trace", "callees", "lib.rs:top", "--scope"]) + .arg(&dir) + .arg("--no-budget") + .output() + .unwrap(); + let direct_out = String::from_utf8_lossy(&direct.stdout); + assert!( + !direct_out.contains("only the root is exact"), + "direct callees have no later hop to caveat:\n{direct_out}" + ); + + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn callees_exact_root_outside_scope_is_labeled_on_every_output_shape() { + let dir = temp_repo("path_symbol_callee_outside_scope"); + let inside = dir.join("inside"); + fs::create_dir_all(&inside).unwrap(); + // Both roots live OUTSIDE --scope; only the boundary label is shared. + fs::write(dir.join("root.rs"), "fn seed() { inside_helper(); }\n").unwrap(); + fs::write(dir.join("quiet.rs"), "fn quiet() { let _x = 1; }\n").unwrap(); + fs::write(inside.join("use.rs"), "fn inside_helper() {}\n").unwrap(); + + // Transitive, detailed, and no-call returns are separate exits; each must + // still say the root sits outside --scope. + for (file, symbol, extra) in [ + ("root.rs", "seed", vec!["--depth", "2"]), + ("root.rs", "seed", vec!["--detailed"]), + ("quiet.rs", "quiet", vec![]), + ] { + let out = srcwalk() + .args(["trace", "callees"]) + .arg(dir.join(format!("{file}:{symbol}"))) + .args(&extra) + .arg("--scope") + .arg(&inside) + .arg("--no-budget") + .output() + .unwrap(); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + out.status.success(), + "an exact root outside --scope must still be readable ({file} {extra:?}):\n{stderr}" + ); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout.contains("lies outside --scope") + && stdout.contains("traversal stay inside --scope only"), + "{file} {extra:?} must label the scope boundary:\n{stdout}" + ); + } + + let _ = fs::remove_dir_all(&dir); +} + +// ------------------------------------------------------------------ US-071 Step 4 + +/// Multi-symbol Discover emits the same confirmed structural targets the +/// single-symbol route would, once per per-query section, without touching the +/// numeric fallback for ambiguous names or deduplicating across sections. +#[test] +fn multi_symbol_discover_emits_canonical_targets_per_section() { + let dir = temp_repo("multi_symbol_emit"); + fs::create_dir_all(dir.join("src")).unwrap(); + // One-unique `run` (canonical), plus two same-name `helper` bodies (ambiguous). + fs::write( + dir.join("src/alpha.rs"), + "pub struct Alpha;\nimpl Alpha {\n pub fn run(&self) {}\n}\n", + ) + .unwrap(); + fs::write( + dir.join("src/helper.rs"), + "fn helper() -> i32 {\n let x = 1;\n let y = 2;\n x + y\n}\nfn other() -> i32 { 2 }\nfn helper() -> i32 {\n let a = 3;\n let b = 4;\n a + b\n}\n", + ) + .unwrap(); + + let out = srcwalk() + .current_dir(&dir) + .args([ + "discover", + "run,helper", + "--as", + "symbol", + "--scope", + "src", + "--expand=0", + ]) + .output() + .unwrap(); + assert!( + out.status.success(), + "{}", + String::from_utf8_lossy(&out.stderr) + ); + let stdout = String::from_utf8_lossy(&out.stdout); + + // Canonical target in the `run` section, and it addresses the unique method. + assert!( + stdout.contains("> Next: srcwalk show src/alpha.rs:Alpha.run"), + "multi-symbol run section must emit the canonical target:\n{stdout}" + ); + + // The ambiguous `helper` section never advertises a canonical `helper` + // selector. With the body rendered inline it abstains ("already shown" + // caveat); without inline rendering it would fall back to a numeric range. + // Either way it must never claim a canonical symbol. + assert!( + !stdout.contains("srcwalk show src/helper.rs:helper"), + "ambiguous bare name must not be advertised canonical:\n{stdout}" + ); + assert!( + stdout.contains("already shown in full above") || stdout.contains("numeric fallback"), + "ambiguous multi-symbol term must abstain or fall back honestly:\n{stdout}" + ); + + let _ = fs::remove_dir_all(&dir); +} + +/// A canonical target resolved by two different query terms is kept once in each +/// section — sections stay independently auditable against their own query. +#[test] +fn multi_symbol_duplicate_selector_kept_once_per_section() { + let dir = temp_repo("multi_symbol_duplicate"); + fs::create_dir_all(dir.join("src")).unwrap(); + fs::write( + dir.join("src/a.rs"), + "pub struct Alpha;\nimpl Alpha {\n pub fn run(&self) {}\n}\n", + ) + .unwrap(); + + // `run` and `Alpha` are different terms, but only `run` is a function name; + // to force a real cross-section duplicate, query the same bare name twice. + let out = srcwalk() + .current_dir(&dir) + .args(["discover", "run,run", "--as", "symbol", "--scope", "src"]) + .output() + .unwrap(); + assert!( + out.status.success(), + "{}", + String::from_utf8_lossy(&out.stderr) + ); + let stdout = String::from_utf8_lossy(&out.stdout); + + let occurrences = stdout.matches("srcwalk show src/a.rs:Alpha.run").count(); + assert_eq!( + occurrences, 2, + "a canonical selector must appear once per section, not be globally deduped:\n{stdout}" + ); + + let _ = fs::remove_dir_all(&dir); +} + +// -------------------------------------------------- US-071 Step 4 dispatch (amendment) + +/// Helper: run `discover` in a fresh repo with two uniquely-named Rust methods, +/// returning stdout so the caller can assert routing (multi-symbol section vs glob). +fn dotted_multi_symbol_repo(name: &str) -> PathBuf { + let dir = temp_repo(name); + fs::create_dir_all(dir.join("src")).unwrap(); + fs::write( + dir.join("src/alpha.rs"), + "pub struct Alpha;\nimpl Alpha {\n pub fn run(&self) {}\n}\n", + ) + .unwrap(); + fs::write( + dir.join("src/beta.rs"), + "pub struct Beta;\nimpl Beta {\n pub fn stop(&self) {}\n}\n", + ) + .unwrap(); + dir +} + +#[test] +fn dotted_multi_symbol_dispatches_before_auto_classify() { + let dir = dotted_multi_symbol_repo("dotted_routes"); + let out = srcwalk() + .current_dir(&dir) + .args([ + "discover", + "Alpha.run,Beta.stop", + "--as", + "symbol", + "--scope", + "src", + ]) + .output() + .unwrap(); + assert!( + out.status.success(), + "{}", + String::from_utf8_lossy(&out.stderr) + ); + let stdout = String::from_utf8_lossy(&out.stdout); + // A dotted part must not turn the whole query into a `**/...` file glob. + assert!( + !stdout.contains("# Files:") && !stdout.contains("**/Alpha.run"), + "dotted multi-symbol must not be a file glob:\n{stdout}" + ); + // Each term is its own section. + assert!(stdout.contains("# Search: \"Alpha.run\""), "{stdout}"); + assert!(stdout.contains("# Search: \"Beta.stop\""), "{stdout}"); + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn bare_multi_symbol_still_dispatches() { + let dir = dotted_multi_symbol_repo("bare_still"); + let out = srcwalk() + .current_dir(&dir) + .args(["discover", "run,stop", "--as", "symbol", "--scope", "src"]) + .output() + .unwrap(); + assert!( + out.status.success(), + "{}", + String::from_utf8_lossy(&out.stderr) + ); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(stdout.contains("# Search: \"run\""), "{stdout}"); + assert!(stdout.contains("# Search: \"stop\""), "{stdout}"); + assert!( + stdout.contains("srcwalk show src/alpha.rs:Alpha.run"), + "{stdout}" + ); + assert!( + stdout.contains("srcwalk show src/beta.rs:Beta.stop"), + "{stdout}" + ); + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn comma_filename_shapes_are_multi_symbol_in_auto_mode() { + let dir = dotted_multi_symbol_repo("comma_filename"); + // `a,b.txt` and `foo.rs,bar.rs` become symbols `a`/`b.txt` per the pinned + // amendment decision; they route multi-symbol (ID sections), never a glob. + for query in ["a,b.txt", "foo.rs,bar.rs"] { + let out = srcwalk() + .current_dir(&dir) + .args(["discover", query, "--as", "symbol", "--scope", "src"]) + .output() + .unwrap(); + assert!( + out.status.success(), + "{}", + String::from_utf8_lossy(&out.stderr) + ); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + !stdout.contains("# Files:") && !stdout.contains("**/"), + "`{query}` must be multi-symbol, not a glob:\n{stdout}" + ); + assert!( + stdout.contains("# Search:"), + "`{query}` must emit sections:\n{stdout}" + ); + } + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn multi_symbol_whitespace_and_degenerate_shapes() { + let dir = dotted_multi_symbol_repo("ws_degenerate"); + // `foo, bar` trims and dispatches two symbols. + let out = srcwalk() + .current_dir(&dir) + .args(["discover", "run, stop", "--as", "symbol", "--scope", "src"]) + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(stdout.contains("# Search: \"run\""), "{stdout}"); + assert!(stdout.contains("# Search: \"stop\""), "{stdout}"); + + // `,a,b` drops the empty leading slot. + let out = srcwalk() + .current_dir(&dir) + .args(["discover", ",run,stop", "--as", "symbol", "--scope", "src"]) + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(stdout.contains("# Search: \"run\""), "{stdout}"); + assert!(stdout.contains("# Search: \"stop\""), "{stdout}"); + + // `foo,` drops the trailing slot, leaves one part -> None -> old flow + // (single term, not multi-symbol sections). + let out = srcwalk() + .current_dir(&dir) + .args(["discover", "run,", "--as", "symbol", "--scope", "src"]) + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + !stdout.contains("# Search: \"run\"\n"), + "`run,` must fall back to the old single-term flow, not multi-symbol:\n{stdout}" + ); + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn multi_symbol_over_five_keeps_explicit_error() { + let dir = dotted_multi_symbol_repo("over_five"); + let out = srcwalk() + .current_dir(&dir) + .args([ + "discover", + "a,b,c,d,e,f", + "--as", + "symbol", + "--scope", + "src", + ]) + .output() + .unwrap(); + assert!(!out.status.success(), "6 symbols must error"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("multi-symbol search supports 2-5 symbols"), + "{stderr}" + ); + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn multi_symbol_does_not_hijack_glob_regex_path_or_section() { + let dir = dotted_multi_symbol_repo("no_hijack"); + // Real glob: `*?{[` must keep the glob/file route, never multi-symbol sections. + let out = srcwalk() + .current_dir(&dir) + .args(["discover", "*.{rs,ts}", "--as", "symbol", "--scope", "src"]) + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + !stdout.contains("# Search: \"*."), + "glob must not become multi-symbol:\n{stdout}" + ); + + // Regex delimiters (leading `/`) must not be split into symbols. + let out = srcwalk() + .current_dir(&dir) + .args(["discover", "/foo,bar/", "--as", "symbol", "--scope", "src"]) + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + !stdout.contains("# Search: \"/foo\""), + "regex must not be hijacked to multi-symbol:\n{stdout}" + ); + + // Path separator must keep the path/file route. + let out = srcwalk() + .current_dir(&dir) + .args(["discover", "src/alpha.rs,beta.rs", "--scope", "."]) + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + !stdout.contains("# Search: \"src\""), + "path-like query must not be hijacked:\n{stdout}" + ); + + // Section colon must keep the path/section route, not multi-symbol. + let out = srcwalk() + .current_dir(&dir) + .args([ + "discover", + "alpha.rs:1,2", + "--as", + "symbol", + "--scope", + "src", + ]) + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + !stdout.contains("# Search: \"alpha.rs:1\""), + "section query must not be hijacked to multi-symbol:\n{stdout}" + ); + + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn explicit_as_text_and_file_are_not_intercepted() { + let dir = dotted_multi_symbol_repo("explicit_as"); + // `--as text` treats the whole comma string as ONE literal text query. + let out = srcwalk() + .current_dir(&dir) + .args(["discover", "run,stop", "--as", "text", "--scope", "src"]) + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout.contains("# Search: \"run,stop\""), + "--as text must not split the comma into symbols:\n{stdout}" + ); + assert!( + !stdout.contains("# Search: \"run\"\n# Search: \"stop\""), + "--as text must stay one literal query:\n{stdout}" + ); + + // `--as file` keeps the file route. + let out = srcwalk() + .current_dir(&dir) + .args(["discover", "a.rs,b.rs", "--as", "file", "--scope", "src"]) + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout.contains("# Files:") && !stdout.contains("# Search:"), + "--as file must stay the file route, not multi-symbol:\n{stdout}" + ); + + let _ = fs::remove_dir_all(&dir); +} diff --git a/tests/semantic_compact.rs b/tests/semantic_compact.rs index 737c5e2..221329b 100644 --- a/tests/semantic_compact.rs +++ b/tests/semantic_compact.rs @@ -828,7 +828,7 @@ fn callers_count_by_zero_matches_uses_no_callers_diagnostic() { } #[test] -fn callers_path_symbol_zero_matches_recovers_with_bare_symbol() { +fn callers_path_symbol_zero_matches_keeps_canonical_root_and_lookup_key() { let dir = tempfile::tempdir().unwrap(); std::fs::write(dir.path().join("lib.rs"), "fn target() {}\n").unwrap(); @@ -843,17 +843,20 @@ fn callers_path_symbol_zero_matches_recovers_with_bare_symbol() { assert!( stdout.starts_with("# Callers of \"lib.rs:target\"") && stdout.contains("> Caveat: direct by-name search only;"), - "header and caveat must remain present:\n{stdout}" + "canonical root and caveat must remain present:\n{stdout}" + ); + assert!( + stdout.contains("the path identifies the requested definition of `target`") + && stdout.contains("matched by name"), + "exact root must be labeled as by-name evidence:\n{stdout}" ); let next = stdout .lines() .find(|line| line.starts_with("> Next:")) .expect("no-callers output must retain recovery guidance"); assert!( - next.contains("use `srcwalk discover target`") - && next.contains("trace accepts bare symbols") - && next.contains("`path:symbol` is `context` grammar"), - "resolved path-symbol recovery must name the bare symbol:\n{stdout}" + next.contains("use `srcwalk discover target`"), + "recovery must name the looked-up terminal key:\n{stdout}" ); assert!( !next.contains("`srcwalk discover lib.rs:target`"), @@ -862,7 +865,7 @@ fn callers_path_symbol_zero_matches_recovers_with_bare_symbol() { } #[test] -fn callers_unresolved_path_symbol_zero_matches_recovery_is_unchanged() { +fn callers_missing_named_file_abstains_instead_of_broadening_to_bare() { let dir = tempfile::tempdir().unwrap(); let out = srcwalk() @@ -870,15 +873,15 @@ fn callers_unresolved_path_symbol_zero_matches_recovery_is_unchanged() { .arg(dir.path()) .output() .unwrap(); - let stdout = String::from_utf8_lossy(&out.stdout); - assert!(out.status.success(), "unresolved miss must remain exit 0"); - assert_eq!( - stdout - .lines() - .find(|line| line.starts_with("> Next:")), - Some("> Next: use `srcwalk discover nosuch.go:Nothing` or search interface/trait/implementor names."), - "unresolved path-symbol recovery must remain unchanged:\n{stdout}" + assert!( + !out.status.success(), + "a missing named file must abstain, not run a bare-name search" + ); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("needs an existing named file") && stderr.contains("Nothing"), + "abstention must name the missing file and selector:\n{stderr}" ); } diff --git a/tests/us060b_anchor_seam.rs b/tests/us060b_anchor_seam.rs index 7fcfc66..049c009 100644 --- a/tests/us060b_anchor_seam.rs +++ b/tests/us060b_anchor_seam.rs @@ -88,7 +88,7 @@ fn discover_offers_no_bare_range_wider_than_forty() { // metadata (a bounded START-line preview), never a competing `> expand:` // or an action-shaped `> anchor:` line repeating the full body range. assert!( - stdout.contains("> Next: srcwalk show target.js --section target"), + stdout.contains("> Next: srcwalk show target.js:target"), "wide range should be symbol-addressed as the primary next action, got:\n{stdout}" ); assert!( diff --git a/tests/us063_offer_dedupe.rs b/tests/us063_offer_dedupe.rs index dc688c9..32b84a8 100644 --- a/tests/us063_offer_dedupe.rs +++ b/tests/us063_offer_dedupe.rs @@ -27,13 +27,34 @@ fn fixture(name: &str) -> PathBuf { dir } +/// A single file with two same-name top-level functions, so the bare selector is +/// ambiguous for both bodies and neither is symbol-backed (numeric fallback). +fn ambiguous_fixture(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!( + "us063_{name}_{}_{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_or(0, |d| d.as_nanos()) + )); + fs::create_dir_all(&dir).unwrap(); + fs::write( + dir.join("lib.rs"), + "fn helper() -> i32 { 1 }\nfn other() -> i32 { 2 }\nfn helper() -> i32 { 3 }\n", + ) + .unwrap(); + dir +} + #[test] fn discover_suppresses_offer_for_fully_rendered_target() { - let dir = fixture("suppressed"); - // `--expand` renders the definition's code block verbatim. + let dir = ambiguous_fixture("suppressed"); + // Two same-name top-level functions make the selector ambiguous, so the + // target is a NUMERIC fallback (never canonical). `--expand` renders the + // bodies verbatim, so the redundant numeric read stays suppressed. let out = srcwalk() .current_dir(&dir) - .args(["discover", "target", "--expand=6"]) + .args(["discover", "helper", "--expand=6"]) .output() .unwrap(); assert!( @@ -43,8 +64,12 @@ fn discover_suppresses_offer_for_fully_rendered_target() { ); let stdout = String::from_utf8_lossy(&out.stdout); assert!( - !stdout.contains("> Next: srcwalk show lib.js:1-6"), - "fully-rendered target must not be offered again:\n{stdout}" + !stdout.contains("> Next: srcwalk show lib.rs:1-1"), + "fully-rendered numeric target must not be offered again:\n{stdout}" + ); + assert!( + !stdout.contains("> Next: srcwalk show lib.rs:3-3"), + "fully-rendered numeric target must not be offered again:\n{stdout}" ); assert!( stdout.contains("already shown in full above"), @@ -53,6 +78,38 @@ fn discover_suppresses_offer_for_fully_rendered_target() { let _ = fs::remove_dir_all(&dir); } +#[test] +fn discover_keeps_canonical_offer_for_fully_rendered_target() { + let dir = fixture("canonical"); + // A unique top-level function is canonical. Even when `--expand` renders + // its body inline, the same : stays reusable for context, + // callers, and callees, so the canonical action is kept. + let out = srcwalk() + .current_dir(&dir) + .args(["discover", "target", "--expand=6"]) + .output() + .unwrap(); + assert!( + out.status.success(), + "{}", + String::from_utf8_lossy(&out.stderr) + ); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + stdout.contains("> Next: srcwalk show lib.js:target"), + "fully-rendered canonical target must stay reusable:\n{stdout}" + ); + assert!( + !stdout.contains("> Next: srcwalk show lib.js:1-6"), + "the numeric read must not replace the canonical target:\n{stdout}" + ); + assert!( + !stdout.contains("already shown in full above"), + "a kept canonical target must not claim it was suppressed:\n{stdout}" + ); + let _ = fs::remove_dir_all(&dir); +} + #[test] fn discover_keeps_offer_when_source_not_rendered() { let dir = fixture("kept"); @@ -69,7 +126,7 @@ fn discover_keeps_offer_when_source_not_rendered() { ); let stdout = String::from_utf8_lossy(&out.stdout); assert!( - stdout.contains("> Next: srcwalk show lib.js --section target"), + stdout.contains("> Next: srcwalk show lib.js:target"), "unrendered target should still be offered:\n{stdout}" ); let _ = fs::remove_dir_all(&dir); diff --git a/tests/us065_symbol_section_footer.rs b/tests/us065_symbol_section_footer.rs index 86fe7d5..12566cc 100644 --- a/tests/us065_symbol_section_footer.rs +++ b/tests/us065_symbol_section_footer.rs @@ -1,11 +1,11 @@ //! Symbol-addressed confirmed-structural-target footer + qualified `--section` //! resolution. //! -//! The discover footer points to `srcwalk show --section ` -//! (bare or `Type.method`) instead of a numeric range, so models read the -//! parser-backed symbol body rather than guessing numeric ranges. The -//! `--section` reader resolves `Q.N` receiver/container-qualified symbols -//! matching US-064 discover semantics. +//! The discover footer points to the canonical `srcwalk show :` +//! target (bare or `Type.method`) instead of a numeric range, so models read +//! the parser-backed symbol body rather than guessing numeric ranges. The +//! `--section` reader resolves the same `Q.N` receiver/container-qualified +//! symbols, matching US-064 discover semantics. use std::fs; use std::path::PathBuf; @@ -78,10 +78,7 @@ fn discover_bare_function_emits_symbol_section_command() { ); let out = fx.discover(&["discover", "target", "--scope", "."]); assert!(out.contains("## Confirmed structural targets"), "{out}"); - assert!( - out.contains("> Next: srcwalk show lib.rs --section target"), - "{out}" - ); + assert!(out.contains("> Next: srcwalk show lib.rs:target"), "{out}"); assert!( !out.contains("srcwalk show 'lib.rs:1-3'") && !out.contains("srcwalk show lib.rs:1-3"), "no numeric read command should be emitted:\n{out}" @@ -105,7 +102,7 @@ fn discover_qualified_preserves_qualified_selector() { ); let out = fx.discover(&["discover", "Batch.Set", "--as", "symbol", "--scope", "."]); assert!( - out.contains("> Next: srcwalk show batch.go --section Batch.Set"), + out.contains("> Next: srcwalk show batch.go:Batch.Set"), "{out}" ); } @@ -120,13 +117,16 @@ fn ambiguous_bare_name_falls_back_to_numeric_for_unresolvable_target() { )], ); let out = fx.discover(&["discover", "helper", "--scope", "."]); - // The first `helper` round-trips via the bare symbol; the second same-name - // `helper` cannot round-trip (the bare selector reads the first), so it - // must fall back to a numeric range rather than point at the wrong body. + // With two same-file top-level `helper` definitions, the bare selector is + // ambiguous for BOTH bodies (unique-cardinality check resolves to two + // distinct ranges). No body may be advertised via the canonical symbol + // command (no silent first-match), so every one must fall back to a + // numeric range pointing at its own body. assert!( - out.contains("> Next: srcwalk show lib.rs --section helper"), - "{out}" + !out.contains("> Next: srcwalk show lib.rs:helper"), + "ambiguous same-name bodies must not emit a canonical symbol command:\n{out}" ); + assert!(out.contains("> Next: srcwalk show lib.rs:1-1"), "{out}"); assert!(out.contains("> Next: srcwalk show lib.rs:3-3"), "{out}"); } @@ -135,7 +135,7 @@ fn discover_quotes_space_and_comma_paths_in_symbol_command() { let fx = Fixture::new("quoted", &[("a file.rs", "fn target() -> i32 { 1 }\n")]); let out = fx.discover(&["discover", "target", "--scope", "."]); assert!( - out.contains("> Next: srcwalk show 'a file.rs' --section target"), + out.contains("> Next: srcwalk show 'a file.rs:target'"), "{out}" ); diff --git a/tests/us071_selector_round_trip.rs b/tests/us071_selector_round_trip.rs new file mode 100644 index 0000000..50d7aea --- /dev/null +++ b/tests/us071_selector_round_trip.rs @@ -0,0 +1,468 @@ +//! US-071: the canonical `:` target Discover prints must be +//! copyable, unchanged, into `show`, `context`, `trace callers`, and +//! `trace callees`. +//! +//! Every assertion here consumes the string the binary actually emitted. No +//! hand-authored "ideal" selector is substituted, because the regression being +//! guarded is exactly a printed target that looks canonical but does not +//! resolve (`System.Text.Json.GetTypeInfoInternal` instead of the outline +//! container `JsonSerializerOptions.GetTypeInfoInternal`). +//! +//! The subprocess CWD is always set explicitly, so a target that only resolves +//! because the temp CWD happens to equal `--scope` cannot pass. + +use std::fs; +use std::path::{Path, PathBuf}; +use std::process::Command; + +fn srcwalk() -> Command { + Command::new(env!("CARGO_BIN_EXE_srcwalk")) +} + +struct Fixture { + dir: PathBuf, +} + +impl Fixture { + fn new(name: &str, files: &[(&str, &str)]) -> Self { + let dir = std::env::temp_dir().join(format!( + "us071_round_trip_{name}_{}_{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_or(0, |d| d.as_nanos()) + )); + fs::create_dir_all(&dir).unwrap(); + for (rel, content) in files { + let path = dir.join(rel); + if let Some(parent) = path.parent() { + fs::create_dir_all(parent).unwrap(); + } + fs::write(path, content).unwrap(); + } + Self { dir } + } + + fn run_in(&self, cwd: &Path, args: &[&str]) -> (bool, String) { + let out = srcwalk().current_dir(cwd).args(args).output().unwrap(); + ( + out.status.success(), + String::from_utf8_lossy(&out.stdout).into_owned() + + &String::from_utf8_lossy(&out.stderr), + ) + } + + fn ok_in(&self, cwd: &Path, args: &[&str]) -> String { + let (ok, out) = self.run_in(cwd, args); + assert!(ok, "command {args:?} in {cwd:?} failed:\n{out}"); + out + } + + fn ok(&self, args: &[&str]) -> String { + let dir = self.dir.clone(); + self.ok_in(&dir, args) + } +} + +impl Drop for Fixture { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.dir); + } +} + +/// Split the argument string after `> Next: srcwalk ` into argv with the +/// footer's single-quote shell quoting undone. Fixture names never contain an +/// apostrophe, so plain single-quote toggling is sufficient here. +fn parse_argv(rest: &str) -> Vec { + let mut argv = Vec::new(); + let mut current = String::new(); + let mut quoted = false; + let mut open = false; + for ch in rest.chars() { + match ch { + '\'' => { + quoted = !quoted; + open = true; + } + c if c.is_whitespace() && !quoted => { + if open || !current.is_empty() { + argv.push(std::mem::take(&mut current)); + open = false; + } + } + c => current.push(c), + } + } + if open || !current.is_empty() { + argv.push(current); + } + argv +} + +/// The printed `> Next: srcwalk show ...` command, split into argv. +fn emitted_show_argv(output: &str) -> Vec { + let line = output + .lines() + .find(|l| l.starts_with("> Next: srcwalk show ")) + .unwrap_or_else(|| panic!("no `> Next: srcwalk show` line in:\n{output}")); + let rest = line.trim_start_matches("> Next: srcwalk ").trim(); + let argv = parse_argv(rest); + assert_eq!(argv.first().map(String::as_str), Some("show"), "{output}"); + argv +} + +/// Every `> Next: srcwalk show ...` line, each parsed into `(target, flags)`. +/// Used for multi-symbol output where each per-query section prints its own +/// canonical target. `--section` is never canonical, so its presence fails. +fn emitted_targets_and_flags_all(output: &str) -> Vec<(String, Vec)> { + output + .lines() + .filter(|l| l.starts_with("> Next: srcwalk show ")) + .map(|line| { + let rest = line.trim_start_matches("> Next: srcwalk ").trim(); + let argv = parse_argv(rest); + assert!( + !argv.iter().any(|a| a == "--section"), + "canonical target must be one argument, got {argv:?}:\n{line}" + ); + (argv[1].clone(), argv[2..].to_vec()) + }) + .collect() +} + +/// The single copyable target plus the trailing flags the footer says to keep +/// (`--scope ` when the display is scope-relative). `--section` is not a +/// canonical form, so its presence fails the contract. +fn emitted_target_and_flags(output: &str) -> (String, Vec) { + let argv = emitted_show_argv(output); + assert!( + !argv.iter().any(|a| a == "--section"), + "canonical target must be one argument, got {argv:?}:\n{output}" + ); + (argv[1].clone(), argv[2..].to_vec()) +} + +fn with_flags<'a>(head: &[&'a str], target: &'a str, flags: &'a [String]) -> Vec<&'a str> { + let mut args: Vec<&str> = head.to_vec(); + args.push(target); + args.extend(flags.iter().map(String::as_str)); + args +} + +const OPTIONS_CS: &str = "namespace System.Text.Json\n{\n public sealed class JsonSerializerOptions\n {\n internal JsonTypeInfo? GetTypeInfoInternal(Type type)\n {\n var info = Lookup(type);\n return info;\n }\n\n internal JsonTypeInfo? Lookup(Type type)\n {\n return null;\n }\n }\n}\n"; + +const CALLER_CS: &str = "namespace System.Text.Json\n{\n internal static class Caller\n {\n internal static void Run(JsonSerializerOptions o, Type t)\n {\n o.GetTypeInfoInternal(t);\n }\n }\n}\n"; + +/// C# namespace trap: the display name is `System.Text.Json.GetTypeInfoInternal` +/// but the only selector that resolves is the outline container form +/// `JsonSerializerOptions.GetTypeInfoInternal`. The emitted target is copied +/// unchanged into all four exact-target commands. +#[test] +fn discover_selector_round_trips_unchanged_through_all_four_commands() { + let fx = Fixture::new( + "csharp_namespace", + &[ + ("src/JsonSerializerOptions.cs", OPTIONS_CS), + ("src/Caller.cs", CALLER_CS), + ], + ); + + let discovered = fx.ok(&[ + "discover", + "GetTypeInfoInternal", + "--as", + "symbol", + "--scope", + "src", + ]); + let (target, flags) = emitted_target_and_flags(&discovered); + + // The emitted selector comes from outline primitives, never the display + // qualified name that includes the namespace. This also fails if Discover + // degrades a unique supported target to `path:range`. + assert!( + target.ends_with(":JsonSerializerOptions.GetTypeInfoInternal"), + "emitted target must use the outline container, got `{target}`:\n{discovered}" + ); + assert!( + !target.contains("System.Text.Json."), + "namespace-qualified display name must never be emitted as a target: `{target}`" + ); + assert!( + discovered.contains("trace callers") + && discovered.contains("trace callees") + && discovered.contains("by-name"), + "the reuse note must name all four commands and keep the caller bound:\n{discovered}" + ); + // `src` is under the discover CWD, so the display addresses the file on its + // own and no `--scope` may be appended. + assert!( + flags.is_empty(), + "a CWD-relative display must not repeat --scope, got {flags:?}:\n{discovered}" + ); + + // 1/4 show: the exact body only. + let shown = fx.ok(&with_flags(&["show"], &target, &flags)); + assert!(shown.contains("GetTypeInfoInternal(Type type)"), "{shown}"); + assert!( + !shown.contains("Lookup(Type type)\n"), + "show must read only the requested body:\n{shown}" + ); + + // 2/4 context: same named-file target, no relocation. + let context = fx.ok(&with_flags( + &["context"], + &target, + &["--scope".into(), "src".into()], + )); + assert!( + context.contains("GetTypeInfoInternal"), + "context must root on the same definition:\n{context}" + ); + + // 3/4 trace callers: finds the caller and keeps the by-name caveat. + let callers = fx.ok(&with_flags( + &["trace", "callers"], + &target, + &["--scope".into(), "src".into()], + )); + assert!( + callers.contains("Run"), + "callers must find the call site:\n{callers}" + ); + assert!( + callers.contains("by name") || callers.contains("by-name"), + "callers must keep the by-name evidence bound:\n{callers}" + ); + + // 4/4 trace callees: reads the same body's outgoing calls. + let callees = fx.ok(&with_flags( + &["trace", "callees"], + &target, + &["--scope".into(), "src".into()], + )); + assert!( + callees.contains("Lookup"), + "callees must root on the same body:\n{callees}" + ); +} + +/// The printed command must run verbatim from the CWD of the Discover run, not +/// only from a CWD that happens to equal `--scope`. When `--scope` points +/// outside the CWD the display is scope-relative, so the footer must carry the +/// `--scope` that makes the copied target resolve. +#[test] +fn emitted_command_runs_verbatim_when_scope_is_outside_the_cwd() { + let fx = Fixture::new( + "scope_outside_cwd", + &[ + ("repo/src/JsonSerializerOptions.cs", OPTIONS_CS), + ("repo/src/Caller.cs", CALLER_CS), + ("elsewhere/.keep", ""), + ], + ); + let cwd = fx.dir.join("elsewhere"); + let scope = fx.dir.join("repo").join("src"); + let scope_arg = scope.to_str().unwrap().to_string(); + + let discovered = fx.ok_in( + &cwd, + &[ + "discover", + "GetTypeInfoInternal", + "--as", + "symbol", + "--scope", + &scope_arg, + ], + ); + let (target, flags) = emitted_target_and_flags(&discovered); + assert!( + target.ends_with(":JsonSerializerOptions.GetTypeInfoInternal"), + "expected a canonical target, got `{target}`:\n{discovered}" + ); + // The scope-relative branch must actually be exercised, otherwise this test + // would pass for the same reason a same-CWD test does. + assert!( + flags.iter().any(|f| f == "--scope"), + "a scope-relative display must print the --scope it needs, got {flags:?}:\n{discovered}" + ); + + // Copy the emitted argv byte-for-byte, from the same CWD. + let shown = fx.ok_in(&cwd, &with_flags(&["show"], &target, &flags)); + assert!( + shown.contains("GetTypeInfoInternal(Type type)"), + "printed command must run verbatim from the discover CWD:\n{shown}" + ); + + // The same target plus the same scope reaches the relation commands too. + let callers = fx.ok_in(&cwd, &with_flags(&["trace", "callers"], &target, &flags)); + assert!(callers.contains("Run"), "{callers}"); +} + +/// Same-file overloads make the selector ambiguous, so Discover must fall back +/// to the numeric `:` form. That fallback must also be +/// copyable unchanged into `show`. +#[test] +fn same_file_overload_emits_numeric_fallback_that_still_round_trips() { + let fx = Fixture::new( + "overload", + &[( + "src/Ctx.cs", + "namespace App\n{\n public class Ctx\n {\n public int GetTypeInfo()\n {\n return 1;\n }\n\n public int GetTypeInfo(int id)\n {\n return id;\n }\n }\n}\n", + )], + ); + + let discovered = fx.ok(&[ + "discover", + "GetTypeInfo", + "--as", + "symbol", + "--scope", + "src", + ]); + let (target, flags) = emitted_target_and_flags(&discovered); + + assert!( + !target.contains("Ctx.GetTypeInfo"), + "an ambiguous selector must never be advertised as canonical: `{target}`:\n{discovered}" + ); + assert!( + discovered.contains("numeric fallback"), + "the numeric fallback must be labeled at block level:\n{discovered}" + ); + + let shown = fx.ok(&with_flags(&["show"], &target, &flags)); + assert!(shown.contains("GetTypeInfo"), "{shown}"); + + // The ambiguous symbol form itself must fail explicitly rather than pick + // the first of the two definitions. + let path = target.rsplit_once(':').unwrap().0.to_string(); + let ambiguous = format!("{path}:Ctx.GetTypeInfo"); + let (ok, out) = fx.run_in(&fx.dir, &["show", &ambiguous]); + assert!( + !ok, + "ambiguous selector must not silently first-pick:\n{out}" + ); + assert!(out.contains("definitions"), "{out}"); +} + +/// Nested paths put the OS path separator inside the canonical target, so this +/// also covers the Windows display form (`src/deep/service.rs:Service.run`, +/// normalized from `src\deep\service.rs`): the grammar must split on the +/// separating colon, not on a path character. The target is never hand-written. +#[test] +fn nested_path_target_round_trips_on_the_host_path_separator() { + let fx = Fixture::new( + "nested_path", + &[( + "src/deep/service.rs", + "pub struct Service;\nimpl Service {\n pub fn run(&self) {\n helper();\n }\n}\nfn helper() {}\n", + )], + ); + + let discovered = fx.ok(&[ + "discover", + "Service.run", + "--as", + "symbol", + "--scope", + "src", + ]); + let (target, flags) = emitted_target_and_flags(&discovered); + assert!( + target.ends_with(":Service.run"), + "expected a canonical target, got `{target}`:\n{discovered}" + ); + assert!( + target.contains("deep"), + "target must carry the nested path, got `{target}`:\n{discovered}" + ); + // The file lives under the discover CWD, so the display already addresses + // it: no redundant `--scope` may be printed on any platform. + assert!( + flags.is_empty(), + "a CWD-relative display must not repeat --scope, got {flags:?}:\n{discovered}" + ); + + let shown = fx.ok(&with_flags(&["show"], &target, &flags)); + assert!(shown.contains("pub fn run(&self)"), "{shown}"); + let callees = fx.ok(&with_flags( + &["trace", "callees"], + &target, + &["--scope".into(), "src".into()], + )); + assert!(callees.contains("helper"), "{callees}"); +} + +/// Multi-symbol Discover has one section per term, and each section must emit the +/// same canonical target the single-symbol route would. Every emitted target is +/// copied unchanged through all four exact-target commands. +#[test] +fn multi_symbol_discover_round_trips_each_section_target() { + let fx = Fixture::new( + "multi_symbol", + &[ + ( + "src/alpha.rs", + "pub struct Alpha;\nimpl Alpha {\n pub fn run(&self) {}\n}\n", + ), + ( + "src/beta.rs", + "pub struct Beta;\nimpl Beta {\n pub fn stop(&self) {}\n}\n", + ), + ], + ); + + let discovered = fx.ok(&["discover", "run,stop", "--as", "symbol", "--scope", "src"]); + // Exactly two per-query sections, each with a canonical target. + let emitted = emitted_targets_and_flags_all(&discovered); + assert_eq!( + emitted.len(), + 2, + "expected one target per term:\n{discovered}" + ); + let targets: Vec<&str> = emitted.iter().map(|(t, _)| t.as_str()).collect(); + assert!( + targets.iter().any(|t| t.ends_with(":Alpha.run")), + "expected Alpha.run among {targets:?}:\n{discovered}" + ); + assert!( + targets.iter().any(|t| t.ends_with(":Beta.stop")), + "expected Beta.stop among {targets:?}:\n{discovered}" + ); + // No `--section`, no scope suffix for a CWD-relative display. + for (target, flags) in &emitted { + assert!(flags.is_empty(), "{target} flags {flags:?}:\n{discovered}"); + } + + // Copy each emitted target unchanged through all four commands. + for (target, flags) in &emitted { + let shown = fx.ok(&with_flags(&["show"], target, flags)); + assert!(shown.contains("pub fn"), "show {target}:\n{shown}"); + + let context = fx.ok(&with_flags( + &["context"], + target, + &["--scope".into(), "src".into()], + )); + assert!( + context.contains(target.rsplit(':').next().unwrap()), + "{context}" + ); + + let callers = fx.ok(&with_flags( + &["trace", "callers"], + target, + &["--scope".into(), "src".into()], + )); + assert!(!callers.contains("error"), "callers {target}:\n{callers}"); + + let callees = fx.ok(&with_flags( + &["trace", "callees"], + target, + &["--scope".into(), "src".into()], + )); + assert!(!callees.contains("error"), "callees {target}:\n{callees}"); + } +} diff --git a/tests/windows_paths.rs b/tests/windows_paths.rs index fce2dc8..1c8766b 100644 --- a/tests/windows_paths.rs +++ b/tests/windows_paths.rs @@ -256,3 +256,92 @@ fn windows_suppressed_structural_target_trace_uses_relative_path() { let _ = fs::remove_dir_all(&dir); } + +/// US-071: the canonical `:` target Discover prints must resolve +/// on Windows, where the display path carries a drive letter and the grammar +/// must not split at the drive colon. The target is parsed out of the real +/// output and copied verbatim; nothing is hand-authored. +#[test] +fn windows_discover_canonical_target_runs_verbatim_from_the_discover_cwd() { + let dir = fixture_repo("windows_canonical_target"); + let elsewhere = temp_repo("windows_canonical_cwd"); + let scope = dir.join("src"); + + let output = srcwalk() + .current_dir(&elsewhere) + .args(["discover", "alpha", "--as", "symbol", "--scope"]) + .arg(&scope) + .output() + .unwrap(); + assert!( + output.status.success(), + "discover failed:\n{}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8_lossy(&output.stdout); + let line = stdout + .lines() + .find(|l| l.starts_with("> Next: srcwalk show ")) + .unwrap_or_else(|| panic!("no canonical show target:\n{stdout}")); + let rest = line.trim_start_matches("> Next: srcwalk show ").trim(); + assert!( + !rest.contains("--section"), + "canonical target must be one argument:\n{stdout}" + ); + + // The scope lives outside the CWD, so the footer must carry the --scope + // that makes the copied target resolve. + let (target, scope_arg) = rest + .split_once(" --scope ") + .unwrap_or_else(|| panic!("scope-relative target must print its --scope:\n{stdout}")); + let target = target.trim_matches('\''); + let scope_arg = scope_arg.trim_matches('\''); + assert!(target.ends_with(":alpha"), "{stdout}"); + + let shown = srcwalk() + .current_dir(&elsewhere) + .args(["show", target, "--scope", scope_arg]) + .output() + .unwrap(); + assert!( + shown.status.success(), + "printed command must run verbatim on Windows:\n{}", + String::from_utf8_lossy(&shown.stderr) + ); + let shown = String::from_utf8_lossy(&shown.stdout); + assert!(shown.contains("pub fn alpha()"), "{shown}"); + assert!( + !shown.contains("pub fn beta() {}"), + "must read only the requested body:\n{shown}" + ); + + let _ = fs::remove_dir_all(&dir); + let _ = fs::remove_dir_all(&elsewhere); +} + +/// Both Windows drive forms must split after the file path, never at the drive +/// colon, for the `:` grammar. +#[test] +fn windows_absolute_drive_path_symbol_splits_after_the_file_path() { + let dir = fixture_repo("windows_drive_path_symbol"); + let file = dir.join("src").join("lib.rs"); + let backslash = format!("{}:alpha", file.display()); + let forward = format!("{}:alpha", file.display().to_string().replace('\\', "/")); + + for target in [backslash, forward] { + let out = srcwalk().args(["show", &target]).output().unwrap(); + assert!( + out.status.success(), + "drive path:symbol {target} failed:\n{}", + String::from_utf8_lossy(&out.stderr) + ); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(stdout.contains("pub fn alpha()"), "{target}:\n{stdout}"); + assert!( + !stdout.contains("pub fn beta() {}"), + "{target} must read only the requested body:\n{stdout}" + ); + } + + let _ = fs::remove_dir_all(&dir); +}