From 9d863764db6e178f25b6dc89b6957ee42c9e5c4d Mon Sep 17 00:00:00 2001 From: 0xLeif Date: Thu, 11 Jun 2026 09:12:47 -0600 Subject: [PATCH] fix: address hands-on testing findings in new, scaffold, and check --fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - new/scaffold: fall back to the project's single non-test source file (e.g. a fresh cargo crate with only src/lib.rs) when no directory or file matches the module name, so the README quickstart flow produces a spec with real files: and pre-populated exports instead of an empty files: [] that immediately fails validation; new prints a warning with guidance when nothing can be detected - check --fix: bypass the hash cache's unchanged-skip (like --strict and --force already did) — a spec that failed check --strict was still recorded in the cache, so a follow-up check --fix printed "All specs unchanged", fixed nothing, and exited 0 - check --fix: promote bare API-kind headings under ## Public API (### Functions, ### Methods, ### Types, …) to ### Exported and skip symbols already documented in any Public API table, instead of appending a duplicate export table for the same symbols - check: print the partial "N/M exports documented" summary as ⚠ — it is counted as a warning, so the summary's warning count now matches the printed ⚠ lines Updated the affected specs (6) and added 6 integration tests plus unit tests for find_single_source_fallback and get_all_api_table_symbols. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 4 + specs/cmd_check/cmd_check.spec.md | 8 +- specs/cmd_new/cmd_new.spec.md | 7 +- specs/cmd_scaffold/cmd_scaffold.spec.md | 5 +- specs/commands/commands.spec.md | 3 +- specs/generator/generator.spec.md | 4 +- specs/parser/parser.spec.md | 4 +- src/commands/check.rs | 52 +++- src/commands/mod.rs | 5 +- src/commands/new.rs | 20 ++ src/commands/scaffold.rs | 10 +- src/generator.rs | 89 +++++++ src/parser.rs | 71 ++++++ tests/integration.rs | 300 ++++++++++++++++++++++++ 14 files changed, 561 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4830fc5..4c71518 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **`specsync new`/`scaffold` auto-detect the source in single-source-file projects** — when no directory or file matches the module name but the project has exactly one non-test source file (the README quickstart's fresh cargo crate with only `src/lib.rs`), that file is used, so `new greeter` produces a spec with real `files:` and pre-populated exports instead of an empty `files: []` that immediately fails validation. When nothing can be detected, `new` prints a warning explaining that the `files:` list must be filled in instead of silently writing a spec that fails `check`. +- **`check --fix` is never a silent no-op** — `--fix` now bypasses the hash cache's unchanged-skip (like `--strict` and `--force` already did). Previously a spec that failed `check --strict` was still recorded in the cache, so a follow-up `check --fix` printed "All specs unchanged", fixed nothing, and exited 0. +- **`check --fix` no longer appends a duplicate export table for symbols a human already documented** — bare API-kind headings under `## Public API` (`### Functions`, `### Methods`, `### Types`, …) are promoted to `### Exported ` during `--fix` so the existing rows become the recognized export table, and `--fix` skips any symbol that already appears in *any* table within the Public API section. +- **Warning count matches the warnings shown** — the partial "N/M exports documented" summary line is counted as a warning, so it now prints with ⚠ instead of a green ✓ (previously the summary could say "2 warning(s)" while only one ⚠ line was visible). - **Fresh `specsync init` now creates the v4 layout** — init writes `.specsync/config.toml`, a `4.0.0` version stamp, `.specsync/.gitignore`, and the `lifecycle/`/`changes/`/`archive/` directories (what `specsync migrate` produces) instead of a legacy root-level `specsync.json`, so a brand-new project no longer sees the "Legacy 3.x layout detected" migration nag on its first `check`. - **`init-registry` respects the v4 layout** — the registry is written to `.specsync/registry.toml` in v4 projects instead of recreating a root-level `specsync-registry.toml` (which re-triggered the legacy nag after migration). Un-migrated 3.x projects keep the legacy path. `load_registry`/`register_module` now resolve the same location via the new `registry::local_registry_path`. - **Draft specs no longer pass validation silently** — when a draft skips section/export checks (by design), `check` now prints explicit "Section validation skipped (status: draft)" / "Export validation skipped (status: draft)" notices instead of misleading "✓ All required sections present" lines, plus a summary hint: "N draft spec(s) skipped section and export validation — set `status: active` to enable full checks". diff --git a/specs/cmd_check/cmd_check.spec.md b/specs/cmd_check/cmd_check.spec.md index 0d3a10a..b1fcaa1 100644 --- a/specs/cmd_check/cmd_check.spec.md +++ b/specs/cmd_check/cmd_check.spec.md @@ -1,6 +1,6 @@ --- module: cmd_check -version: 3 +version: 4 status: stable files: - src/commands/check.rs @@ -36,8 +36,9 @@ Implements the `specsync check` command — the primary validation entry point. ## Invariants 1. When `--fix` is passed, auto-fix runs in two phases: (a) add undocumented exports to spec markdown tables with generated review prompts — type exports are routed to the "… Types" table and functions/values to the "… Functions"/"… Methods" table (falling back to the last export subsection), with rows padded to the target table's column count, (b) AI-regenerate specs whose requirements have drifted -2. Near-miss header correction (e.g., "Exported Functions" → "### Exported Functions") runs as part of auto-fix -3. Hash cache is consulted before validation unless `--force` is set — unchanged specs are skipped +2. Near-miss header correction runs as part of auto-fix — Levenshtein-close typos are renamed to canonical export headers, and bare API-kind headings under `## Public API` (e.g. `### Functions`, `### Methods`, `### Types`) are promoted to `### Exported ` so hand-written tables become the export table instead of being duplicated +3. Hash cache is consulted before validation unless `--force`, `--strict`, `--fix`, or a spec filter is set — an explicit `--fix` is never silently skipped because a previous failing/warning run recorded the hashes +3a. `--fix` never adds a symbol that already appears in any table within `## Public API` (including informational subsections) 4. After auto-fix, validation is re-run to verify fixes resolved the issues 5. JSON output mode collects all errors/warnings into a structured object instead of printing inline 6. `--create-issues` groups errors by spec path and creates one GitHub issue per affected spec @@ -100,6 +101,7 @@ Implements the `specsync check` command — the primary validation entry point. | Date | Change | |------|--------| +| 2026-06-11 | v4: `--fix` bypasses the hash cache (no more silent no-op after a cached warning run); bare API-kind headings are promoted to export headers and symbols already documented in any Public API table are not re-added; partial export-coverage summary prints as ⚠ so the warning count matches printed warnings | | 2026-06-11 | v3: `--fix` routes exports to the matching table by kind; unmatched spec filters exit 1 without contradictory output | | 2026-06-07 | Document generated review prompts for `--fix` export rows | | 2026-04-09 | Initial spec | diff --git a/specs/cmd_new/cmd_new.spec.md b/specs/cmd_new/cmd_new.spec.md index a7889e4..a7bab86 100644 --- a/specs/cmd_new/cmd_new.spec.md +++ b/specs/cmd_new/cmd_new.spec.md @@ -1,6 +1,6 @@ --- module: cmd_new -version: 2 +version: 3 status: stable files: - src/commands/new.rs @@ -28,7 +28,7 @@ Implements the `specsync new` command. Quick-creates a minimal spec with auto-de ## Invariants -1. Auto-detects source files by scanning source dirs for module name matches +1. Auto-detects source files by scanning source dirs for module name matches; when nothing matches and the project has exactly one non-test source file (e.g. only `src/lib.rs`), that file is used as the module's source 2. Extracts exports to pre-populate Public API tables 3. `--full` generates companion files (tasks.md, context.md, requirements.md, testing.md) via `generator::generate_companion_files_for_spec()`; design.md is included only when `companions.design` is enabled in config 4. Includes custom `chrono_lite_today()` for dates without chrono dependency @@ -53,7 +53,7 @@ Implements the `specsync new` command. Quick-creates a minimal spec with auto-de | Condition | Behavior | |-----------|----------| | Spec already exists | Exits 1 | -| No source files found | Creates spec with empty `files:` | +| No source files found | Creates spec with empty `files:` and prints a ⚠ explaining that the `files:` list must be filled in before `check` passes | | Dir creation fails | Exits 1 | ## Dependencies @@ -76,6 +76,7 @@ Implements the `specsync new` command. Quick-creates a minimal spec with auto-de | Date | Change | |------|--------| +| 2026-06-11 | Fall back to the project's single source file when no name match exists (README quickstart flow); warn instead of silently writing an empty `files:` list | | 2026-06-07 | Replace unfinished-marker generated rows with review prompts | | 2026-04-09 | Initial spec | | 2026-04-13 | Document testing.md and conditional design.md in companion generation | diff --git a/specs/cmd_scaffold/cmd_scaffold.spec.md b/specs/cmd_scaffold/cmd_scaffold.spec.md index d177290..59474ff 100644 --- a/specs/cmd_scaffold/cmd_scaffold.spec.md +++ b/specs/cmd_scaffold/cmd_scaffold.spec.md @@ -1,6 +1,6 @@ --- module: cmd_scaffold -version: 2 +version: 3 status: stable files: - src/commands/scaffold.rs @@ -30,7 +30,7 @@ Implements `specsync add-spec` and `specsync scaffold` commands. Creates new spe ## Invariants -1. Both scan source dirs for module name matches +1. Both scan source dirs for module name matches; `cmd_scaffold` falls back to the project's single non-test source file (e.g. only `src/lib.rs`) when no name match exists 2. `cmd_scaffold` supports custom templates and auto-appends to registry 3. Neither overwrites existing specs 4. Companion files (tasks.md, context.md, requirements.md, testing.md) are always generated with guided starter content; design.md is generated only when `companions.design` is enabled in config @@ -72,6 +72,7 @@ Implements `specsync add-spec` and `specsync scaffold` commands. Creates new spe | Date | Change | |------|--------| +| 2026-06-11 | `cmd_scaffold` falls back to the project's single source file when no module name match exists | | 2026-06-07 | Document guided starter content in generated companions | | 2026-04-09 | Initial spec | | 2026-04-13 | Document companions.design flag for conditional design.md generation | diff --git a/specs/commands/commands.spec.md b/specs/commands/commands.spec.md index 03d16f4..52a4c8a 100644 --- a/specs/commands/commands.spec.md +++ b/specs/commands/commands.spec.md @@ -1,6 +1,6 @@ --- module: commands -version: 2 +version: 3 status: stable files: - src/commands/mod.rs @@ -143,6 +143,7 @@ Shared command infrastructure used by all CLI subcommands. Provides config loadi | Date | Change | |------|--------| +| 2026-06-11 | v3: Partial export-coverage summary ("N/M exports documented") prints as ⚠ — it is counted as a warning, so the summary's warning count now matches the printed ⚠ lines | | 2026-06-11 | v2: Draft specs report skipped section/export validation explicitly; failing frontmatter renders a negated label | | 2026-04-09 | Initial spec | | 2026-04-11 | Add lifecycle submodule and filter_by_status function | diff --git a/specs/generator/generator.spec.md b/specs/generator/generator.spec.md index 00adfe6..cfc1f06 100644 --- a/specs/generator/generator.spec.md +++ b/specs/generator/generator.spec.md @@ -1,6 +1,6 @@ --- module: generator -version: 3 +version: 4 status: stable files: - src/generator.rs @@ -28,6 +28,7 @@ Scaffolds spec files and companion files (tasks.md, context.md, requirements.md, | `generate_specs_for_unspecced_modules_paths` | `root, report, config, provider` | `GenerationOutcome` | Generate specs for all unspecced modules without per-file progress output (JSON/MCP callers), returning the generation outcome | | `generate_companion_files_for_spec` | `spec_dir, module_name, design_enabled` | `()` | Generate companion files (tasks.md, context.md, requirements.md, testing.md, and design.md if enabled) alongside a spec | | `find_files_for_module` | `root, module_name, config` | `Vec` | Find source files for a module by checking config definitions, subdirectories, then flat files | +| `find_single_source_fallback` | `root, config` | `Option` | Root-relative path of the project's only non-test source file (e.g. `src/lib.rs`), or `None` when there are zero or multiple candidates — fallback for `new`/`scaffold` when no name match exists | | `generate_spec` | `module_name, source_files, root, specs_dir` | `String` | Generate a spec from a template (custom or language-aware default) | | `generate_spec_from_custom_template` | `template_dir, module_name, source_files, root` | `String` | Generate a spec using files from a custom template directory | | `generate_companion_files_from_template` | `spec_dir, module_name, template_dir, design_enabled` | `()` | Generate companion files from a custom template directory with fallback to defaults; creates design.md only when `design_enabled` is true | @@ -118,3 +119,4 @@ Scaffolds spec files and companion files (tasks.md, context.md, requirements.md, | 2026-04-13 | Fix generate_companion_files_from_template signature to include design_enabled; update scenario for conditional design.md | | 2026-06-07 | Replace unfinished-marker built-in template content with guided starter content | | 2026-06-11 | Return `GenerationOutcome` (count, paths, AI errors) from both generation entry points so AI failures surface with a non-zero exit | +| 2026-06-11 | Add `find_single_source_fallback` so `new`/`scaffold` auto-detect the source in single-source-file projects (e.g. a fresh cargo crate with only `src/lib.rs`) | diff --git a/specs/parser/parser.spec.md b/specs/parser/parser.spec.md index a8f503b..49b7f7e 100644 --- a/specs/parser/parser.spec.md +++ b/specs/parser/parser.spec.md @@ -1,6 +1,6 @@ --- module: parser -version: 1 +version: 2 status: stable files: - src/parser.rs @@ -37,6 +37,7 @@ Parses spec markdown files — extracts YAML frontmatter into structured data, e | `find_section_offset` | `body: &str, section: &str` | `Option` | Returns byte offset of the `## Section` heading line, using anchored regex with trailing-whitespace tolerance | | `body_has_section` | `body: &str, section: &str` | `bool` | Returns true if the spec body contains an exact `## Section` heading (delegates to `find_section_offset`) | | `get_near_miss_sections` | `body: &str, required_sections: &[String]` | `Vec<(String, String)>` | For each missing required section, returns `(canonical_name, found_heading)` pairs where a `## Heading` exists within Levenshtein distance ≤ 2 — used to detect typos and suggest `--fix` | +| `get_all_api_table_symbols` | `body: &str` | `Vec` | Extract the first backtick-quoted symbol from every table row in `## Public API`, including informational subsections that `get_spec_symbols` skips — used by `check --fix` to avoid appending duplicate rows | ## Invariants @@ -101,3 +102,4 @@ Parses spec markdown files — extracts YAML frontmatter into structured data, e | Date | Change | |------|--------| | 2026-03-25 | Initial spec | +| 2026-06-11 | Add `get_all_api_table_symbols` so `check --fix` treats symbols documented under any Public API table (e.g. a bare `### Functions` heading) as already documented | diff --git a/src/commands/check.rs b/src/commands/check.rs index 1441117..3da97bd 100644 --- a/src/commands/check.rs +++ b/src/commands/check.rs @@ -127,6 +127,13 @@ pub fn cmd_check( let (specs_to_validate, change_classifications) = if force || strict || !spec_filters.is_empty() { (spec_files.clone(), Vec::new()) + } else if fix { + // --fix bypasses the unchanged-skip: an explicit fix request must + // never be a silent no-op because a previous (failing or warning) run + // recorded the hashes. Classifications are still computed so + // requirements-drift regeneration keeps working. + let classifications = hash_cache::classify_all_changes(root, &spec_files, &cache); + (spec_files.clone(), classifications) } else { let classifications = hash_cache::classify_all_changes(root, &spec_files, &cache); let changed: Vec = classifications @@ -410,7 +417,9 @@ pub fn cmd_check( let coverage = compute_coverage(root, &spec_files, &config); // Update hash cache after validation (only when no errors). - // Specs with warnings are still cached — --strict forces re-validation separately. + // Specs with warnings are still cached, which is why --fix, --strict, and + // --force all bypass the unchanged-skip above — an explicit fix or strict + // run must never trust hashes recorded by a run that had findings. if total_errors == 0 { hash_cache::update_cache(root, &specs_to_validate, &mut cache); let _ = cache.save(root); @@ -697,6 +706,32 @@ fn fix_near_miss_headers(content: &mut String) -> bool { let new = format!("### {canonical}"); new_section = new_section.replacen(&old, &new, 1); modified = true; + continue; + } + + // Bare API-kind headings ("### Functions", "### Methods", "### Types", …) + // describe export tables but fail is_export_header, so their rows are + // informational-only and --fix would append a duplicate export table + // for the same symbols. Promote them to "### Exported ". + let bare_kinds: &[&str] = &[ + "functions", + "methods", + "types", + "classes", + "constants", + "components", + "hooks", + "interfaces", + "enums", + "structs", + "traits", + "protocols", + ]; + if bare_kinds.contains(&lower.trim()) { + let old = format!("### {header_text}"); + let new = format!("### Exported {}", header_text.trim()); + new_section = new_section.replacen(&old, &new, 1); + modified = true; } } @@ -735,7 +770,7 @@ fn auto_fix_specs( dry_run: bool, ) -> usize { use crate::exports::get_exported_symbols_full; - use crate::parser::{get_spec_symbols, parse_frontmatter}; + use crate::parser::{get_all_api_table_symbols, get_spec_symbols, parse_frontmatter}; let mut fixed_count = 0; let sub_re = regex::Regex::new(r"(?m)^### ").unwrap(); @@ -795,14 +830,17 @@ fn auto_fix_specs( let mut seen = std::collections::HashSet::new(); all_exports.retain(|s| seen.insert(s.clone())); - // Find which exports are already documented - let spec_symbols = get_spec_symbols(&parsed.body); - let spec_set: std::collections::HashSet<&str> = - spec_symbols.iter().map(|s| s.as_str()).collect(); + // Find which exports are already documented — in recognized export + // tables AND in any other table within ## Public API. A symbol that a + // human already documented under an informational heading must not be + // appended a second time. + let mut documented: std::collections::HashSet = + get_spec_symbols(&parsed.body).into_iter().collect(); + documented.extend(get_all_api_table_symbols(&parsed.body)); let undocumented: Vec<&str> = all_exports .iter() - .filter(|s| !spec_set.contains(s.as_str())) + .filter(|s| !documented.contains(s.as_str())) .map(|s| s.as_str()) .collect(); diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 08a01c1..0a806a2 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -374,7 +374,10 @@ pub fn run_validation( "⊘".yellow() ); } else if let Some(line) = api_line { - println!(" {} {line}", "✓".green()); + // The partial-coverage summary is recorded (and counted) as a + // warning — print it as one so the summary's warning count matches + // the number of ⚠ lines shown. + println!(" {} {line}", "⚠".yellow()); } else if let Some(ref summary) = result.export_summary { println!(" {} {summary}", "✓".green()); } diff --git a/src/commands/new.rs b/src/commands/new.rs index 1338d63..db5c182 100644 --- a/src/commands/new.rs +++ b/src/commands/new.rs @@ -30,6 +30,18 @@ pub fn cmd_new(root: &Path, module_name: &str, full: bool) { // Auto-detect source files for this module let source_files = detect_module_sources(root, module_name, &config); + if source_files.is_empty() { + eprintln!( + "{} No source files matched module '{module_name}' — the spec is created with an empty `files:` list.", + "⚠".yellow() + ); + eprintln!( + " Add the module's source path(s) to the `files:` list in the spec frontmatter," + ); + eprintln!( + " or define the module in your config — `specsync check` fails on empty `files:`." + ); + } let files_yaml = if source_files.is_empty() { "files: []\n".to_string() } else { @@ -179,6 +191,14 @@ fn detect_module_sources( } } + // Fallback: a single-source-file project (e.g. only src/lib.rs) has exactly + // one possible source — use it even though the name doesn't match. + if files.is_empty() + && let Some(single) = generator::find_single_source_fallback(root, config) + { + files.push(single); + } + files.sort(); files } diff --git a/src/commands/scaffold.rs b/src/commands/scaffold.rs index db2d1ac..17600ab 100644 --- a/src/commands/scaffold.rs +++ b/src/commands/scaffold.rs @@ -218,8 +218,14 @@ pub fn cmd_scaffold( process::exit(1); } - // Auto-detect source files matching the module name - let module_files = generator::find_files_for_module(root, module_name, &config); + // Auto-detect source files matching the module name; for single-source-file + // projects (e.g. only src/lib.rs) fall back to that file. + let mut module_files = generator::find_files_for_module(root, module_name, &config); + if module_files.is_empty() + && let Some(single) = generator::find_single_source_fallback(root, &config) + { + module_files.push(single); + } // Generate spec content let spec_content = if let Some(ref tpl_dir) = template { diff --git a/src/generator.rs b/src/generator.rs index 0859f38..e1dd7ca 100644 --- a/src/generator.rs +++ b/src/generator.rs @@ -601,6 +601,42 @@ pub fn find_files_for_module( module_files } +/// Fallback source detection for single-source-file projects. +/// +/// When a module name matches no source directory or file (e.g. `greeter` in a +/// cargo project whose only source is `src/lib.rs`), but the project contains +/// exactly one non-test source file across all configured source directories, +/// that file is unambiguously the module's source. Returns the root-relative +/// path, or `None` when there are zero or multiple candidates. +pub fn find_single_source_fallback(root: &Path, config: &SpecSyncConfig) -> Option { + let mut found: Option = None; + for src_dir in &config.source_dirs { + let base = root.join(src_dir); + if !base.is_dir() { + continue; + } + for entry in WalkDir::new(&base).into_iter().filter_map(|e| e.ok()) { + let path = entry.path(); + if !path.is_file() + || !has_extension(path, &config.source_extensions) + || is_test_file(path) + { + continue; + } + let rel = path + .strip_prefix(root) + .unwrap_or(path) + .to_string_lossy() + .replace('\\', "/"); + if found.is_some() { + return None; // More than one source file — ambiguous, no fallback + } + found = Some(rel); + } + } + found +} + /// Generate a spec from a template, using language-aware defaults. pub fn generate_spec( module_name: &str, @@ -1494,6 +1530,59 @@ mod tests { assert!(files.is_empty()); } + // ── find_single_source_fallback ──────────────────────────────── + + #[test] + fn single_source_fallback_returns_only_file() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + let src_dir = root.join("src"); + fs::create_dir_all(&src_dir).unwrap(); + fs::write(src_dir.join("lib.rs"), "pub fn greet() {}").unwrap(); + + let config = SpecSyncConfig::default(); + let file = find_single_source_fallback(root, &config); + assert_eq!(file.as_deref(), Some("src/lib.rs")); + } + + #[test] + fn single_source_fallback_ambiguous_returns_none() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + let src_dir = root.join("src"); + fs::create_dir_all(&src_dir).unwrap(); + fs::write(src_dir.join("lib.rs"), "pub fn greet() {}").unwrap(); + fs::write(src_dir.join("util.rs"), "pub fn helper() {}").unwrap(); + + let config = SpecSyncConfig::default(); + assert_eq!(find_single_source_fallback(root, &config), None); + } + + #[test] + fn single_source_fallback_ignores_test_files() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + let src_dir = root.join("src"); + fs::create_dir_all(&src_dir).unwrap(); + fs::write(src_dir.join("lib.ts"), "export function greet() {}").unwrap(); + fs::write(src_dir.join("lib.test.ts"), "test('greet', () => {})").unwrap(); + + let mut config = SpecSyncConfig::default(); + config.source_extensions = vec!["ts".to_string()]; + let file = find_single_source_fallback(root, &config); + assert_eq!(file.as_deref(), Some("src/lib.ts")); + } + + #[test] + fn single_source_fallback_empty_project_returns_none() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + fs::create_dir_all(root.join("src")).unwrap(); + + let config = SpecSyncConfig::default(); + assert_eq!(find_single_source_fallback(root, &config), None); + } + #[test] fn find_files_user_defined_module() { let tmp = TempDir::new().unwrap(); diff --git a/src/parser.rs b/src/parser.rs index 07e5eee..ef9b03c 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -261,6 +261,42 @@ pub fn get_spec_symbols(body: &str) -> Vec { symbols } +/// Extract the first backtick-quoted symbol from EVERY table row in the +/// Public API section, including informational subsections that +/// `get_spec_symbols` skips (e.g. "### API Endpoints", "### Functions"). +/// `check --fix` uses this to avoid appending a duplicate row for a symbol +/// that a human already documented under a non-export heading. +pub fn get_all_api_table_symbols(body: &str) -> Vec { + let mut symbols = Vec::new(); + + let api_start = match find_section_offset(body, "Public API") { + Some(pos) => pos, + None => return symbols, + }; + let after_header = match body[api_start..].find('\n') { + Some(pos) => api_start + pos + 1, + None => return symbols, + }; + let rest = &body[after_header..]; + let heading_re = Regex::new(r"(?m)^## [^#]").unwrap(); + let api_section = match heading_re.find(rest) { + Some(m) => &rest[..m.start()], + None => rest, + }; + + for line in api_section.lines() { + if let Some(caps) = TABLE_ROW_RE.captures(line) + && let Some(sym) = caps.get(1) + { + symbols.push(sym.as_str().to_string()); + } + } + + let mut seen = HashSet::new(); + symbols.retain(|s| seen.insert(s.clone())); + symbols +} + /// Check which required sections are missing from the spec body. pub fn get_missing_sections(body: &str, required_sections: &[String]) -> Vec { let mut missing = Vec::new(); @@ -585,6 +621,41 @@ Something assert_eq!(symbols, vec!["authenticate", "AuthConfig"]); } + #[test] + fn test_get_all_api_table_symbols_includes_informational_tables() { + let body = r#"# Auth + +## Public API + +### Functions + +| Function | Description | +|----------|-------------| +| `greet` | Says hello | + +### API Endpoints + +| Endpoint | Description | +|----------|-------------| +| `login` | Login route | + +## Error Cases + +| Condition | Behavior | +|-----------|----------| +| `OutsideApiSection` | Not collected | +"#; + let symbols = get_all_api_table_symbols(body); + // Tables under ANY ### heading within ## Public API count, but tables + // in other ## sections do not. + assert_eq!(symbols, vec!["greet", "login"]); + } + + #[test] + fn test_get_all_api_table_symbols_no_api_section() { + assert!(get_all_api_table_symbols("# Title\n\n## Purpose\n\nText.\n").is_empty()); + } + #[test] fn test_parse_frontmatter_implements_list() { let content = "---\nmodule: auth\nversion: 1\nstatus: active\nfiles:\n - src/auth.ts\nimplements:\n - 42\n - 57\ntracks:\n - 10\n---\n\n# Auth\n"; diff --git a/tests/integration.rs b/tests/integration.rs index 2bcd88d..759d59d 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -5285,3 +5285,303 @@ fn nonexistent_root_errors_with_nonzero_exit() { .code(2) .stderr(predicate::str::contains("does not exist")); } + +// ─── Hands-on findings round 2 ─────────────────────────────────────────── + +/// `specsync new` in a single-source-file project (e.g. a fresh cargo crate +/// with only src/lib.rs) must auto-detect that file even though its name +/// doesn't match the module — the README quickstart promises exactly this. +#[test] +fn new_auto_detects_single_source_file() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src")).unwrap(); + fs::write( + root.join("src/lib.rs"), + "/// Says hello.\npub fn greet(name: &str) -> String { format!(\"Hello, {name}!\") }\n", + ) + .unwrap(); + + specsync() + .args(["new", "greeter", "--root", root.to_str().unwrap()]) + .assert() + .success() + .stdout(predicate::str::contains("Auto-detected 1 source file(s)")); + + let spec = fs::read_to_string(root.join("specs/greeter/greeter.spec.md")).unwrap(); + assert!( + spec.contains("- src/lib.rs"), + "expected src/lib.rs in frontmatter files, got:\n{spec}" + ); + assert!( + spec.contains("`greet`"), + "expected pre-populated `greet` export, got:\n{spec}" + ); + assert!( + !spec.contains("files: []"), + "files must not be empty, got:\n{spec}" + ); + + // The quickstart flow: the generated spec must pass `specsync check` + specsync() + .args(["check", "--root", root.to_str().unwrap()]) + .assert() + .success(); +} + +/// When nothing can be auto-detected (multiple source files, none matching), +/// `new` must say so instead of silently writing a spec that fails validation. +#[test] +fn new_warns_when_no_source_files_match() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src")).unwrap(); + fs::write(root.join("src/alpha.rs"), "pub fn a() {}").unwrap(); + fs::write(root.join("src/beta.rs"), "pub fn b() {}").unwrap(); + + specsync() + .args(["new", "gamma", "--root", root.to_str().unwrap()]) + .assert() + .success() + .stderr(predicate::str::contains("No source files matched")); +} + +/// `scaffold` gets the same single-source-file fallback as `new`. +#[test] +fn scaffold_auto_detects_single_source_file() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src")).unwrap(); + fs::write(root.join("src/lib.rs"), "pub fn greet() {}").unwrap(); + + specsync() + .args(["scaffold", "greeter", "--root", root.to_str().unwrap()]) + .assert() + .success(); + + let spec = fs::read_to_string(root.join("specs/greeter/greeter.spec.md")).unwrap(); + assert!( + spec.contains("src/lib.rs"), + "expected src/lib.rs in scaffolded spec, got:\n{spec}" + ); +} + +/// The original repro: a spec with a warning gets recorded in the hash cache, +/// then `check --fix` (without --force) reports "All specs unchanged" and +/// fixes nothing. An explicit --fix must never be a silent no-op. +#[test] +fn fix_runs_even_when_cache_marks_specs_unchanged() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src/utils")).unwrap(); + fs::write( + root.join("src/utils/helpers.ts"), + "export function documented() {}\nexport function undocumented() {}\n", + ) + .unwrap(); + + fs::create_dir_all(root.join("specs/utils")).unwrap(); + let spec = valid_spec("utils", &["src/utils/helpers.ts"]).replace( + "| Function | Parameters | Returns | Description |\n|----------|-----------|---------|-------------|", + "| Function | Parameters | Returns | Description |\n|----------|-----------|---------|-------------|\n| `documented` | — | void | Documented helper |", + ); + fs::write(root.join("specs/utils/utils.spec.md"), spec).unwrap(); + + // Failing strict run — this records the spec (and its sources) in the + // hash cache even though it produced a warning + specsync() + .args(["check", "--strict", "--root", root.to_str().unwrap()]) + .assert() + .failure(); + + // Sanity: without --fix/--force/--strict the cache now skips the spec + let assert = specsync() + .args(["check", "--root", root.to_str().unwrap()]) + .assert() + .success(); + let stdout = String::from_utf8_lossy(&assert.get_output().stdout).to_string(); + assert!( + stdout.contains("All specs unchanged"), + "expected the cache to skip the unchanged spec, got:\n{stdout}" + ); + + // --fix without --force must actually fix, not report "All specs unchanged" + let assert = specsync() + .args(["check", "--fix", "--root", root.to_str().unwrap()]) + .assert() + .success(); + let stdout = String::from_utf8_lossy(&assert.get_output().stdout).to_string(); + assert!( + !stdout.contains("All specs unchanged"), + "--fix must bypass the hash cache, got:\n{stdout}" + ); + + let updated = fs::read_to_string(root.join("specs/utils/utils.spec.md")).unwrap(); + assert!( + updated.contains("`undocumented`"), + "--fix must add the undocumented export, got:\n{updated}" + ); + + // And all exports are documented on a forced re-check + let assert = specsync() + .args(["check", "--force", "--root", root.to_str().unwrap()]) + .assert() + .success(); + let stdout = String::from_utf8_lossy(&assert.get_output().stdout).to_string(); + assert!( + stdout.contains("2/2 exports documented"), + "expected full export coverage after --fix, got:\n{stdout}" + ); +} + +/// A symbol documented in a hand-written table under a bare heading like +/// "### Functions" (not on the export-header allowlist) must not be added a +/// second time by --fix. The bare heading is promoted to "### Exported +/// Functions" so the existing rows become the export table. +#[test] +fn fix_does_not_duplicate_symbols_documented_under_bare_headings() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src/greeter")).unwrap(); + fs::write(root.join("src/greeter/lib.rs"), "pub fn greet() {}\n").unwrap(); + + fs::create_dir_all(root.join("specs/greeter")).unwrap(); + let spec = r#"--- +module: greeter +version: 1 +status: active +files: + - src/greeter/lib.rs +db_tables: [] +depends_on: [] +--- + +# Greeter + +## Purpose + +Greets people. + +## Public API + +### Functions + +| Function | Description | +|----------|-------------| +| `greet` | Says hello | + +## Invariants + +1. Always valid. + +## Behavioral Examples + +### Scenario: Basic + +- **Given** precondition +- **When** action +- **Then** result + +## Error Cases + +| Condition | Behavior | +|-----------|----------| + +## Dependencies + +None + +## Change Log + +| Date | Author | Change | +|------|--------|--------| +"#; + fs::write(root.join("specs/greeter/greeter.spec.md"), spec).unwrap(); + + specsync() + .args(["check", "--fix", "--root", root.to_str().unwrap()]) + .assert() + .success(); + + let updated = fs::read_to_string(root.join("specs/greeter/greeter.spec.md")).unwrap(); + assert_eq!( + updated.matches("`greet`").count(), + 1, + "greet must not be duplicated, got:\n{updated}" + ); + assert!( + updated.contains("### Exported Functions"), + "bare heading must be promoted to an export header, got:\n{updated}" + ); + + // The promoted table makes the export count as documented on re-check + let assert = specsync() + .args(["check", "--force", "--root", root.to_str().unwrap()]) + .assert() + .success(); + let stdout = String::from_utf8_lossy(&assert.get_output().stdout).to_string(); + assert!( + stdout.contains("1/1 exports documented"), + "expected greet to count as documented after promotion, got:\n{stdout}" + ); +} + +/// The summary's warning count must match the number of ⚠ lines printed — +/// the partial "N/M exports documented" line is counted as a warning, so it +/// must render as one (previously it rendered as a green ✓). +#[test] +fn warning_count_matches_printed_warning_lines() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + + write_config(root, "specs", &["src"]); + fs::create_dir_all(root.join("src/utils")).unwrap(); + fs::write( + root.join("src/utils/helpers.ts"), + "export function documented() {}\nexport function undocumented() {}\n", + ) + .unwrap(); + fs::create_dir_all(root.join("specs/utils")).unwrap(); + let spec = valid_spec("utils", &["src/utils/helpers.ts"]).replace( + "| Function | Parameters | Returns | Description |\n|----------|-----------|---------|-------------|", + "| Function | Parameters | Returns | Description |\n|----------|-----------|---------|-------------|\n| `documented` | — | void | Documented helper |", + ); + fs::write(root.join("specs/utils/utils.spec.md"), spec).unwrap(); + + let assert = specsync() + .args(["check", "--root", root.to_str().unwrap()]) + .assert() + .success(); + let stdout = String::from_utf8_lossy(&assert.get_output().stdout).to_string(); + + // The partial export-coverage summary must render as a ⚠ line + assert!( + stdout.contains("⚠ 1/2 exports documented"), + "partial export coverage must print as a warning, got:\n{stdout}" + ); + + // Extract N from the "… N warning(s) …" summary and compare with the + // number of ⚠ lines actually printed + let counted: usize = stdout + .lines() + .find_map(|line| { + let idx = line.find(" warning(s)")?; + line[..idx].rsplit(' ').next()?.parse().ok() + }) + .expect("summary line with warning count"); + assert_eq!( + stdout.matches('⚠').count(), + counted, + "warning count must match printed ⚠ lines, got:\n{stdout}" + ); +}