From 733d1c5bd2cf03d75316db3c3a52f56188287ee5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 24 Jun 2026 02:20:33 +0000 Subject: [PATCH 1/4] audit(2026-06-24): flag test-debt and rename dedup tax Appends the 2026-06-24 audit entry (range 1599c9f..09f22d3) to the architecture audit log. Four tax items were found: missing unit tests for helpers in locate.go, lsp/rename.go, and export/export.go, plus duplicated normalizedLabel/refDefBracketBytes between lsp and rename packages. Each item is tracked in a new plan file. Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_017DMVDTLs2U9kcVMEyLNZgK --- PLAN.md | 4 + docs/development/architecture-audit.md | 54 ++++++++++++- ...2606240211_arch-fix-locate-helper-tests.md | 68 ++++++++++++++++ ...240212_arch-fix-lsp-rename-helper-tests.md | 74 +++++++++++++++++ ...2606240213_arch-fix-export-helper-tests.md | 81 +++++++++++++++++++ plan/2606240214_arch-fix-rename-dedup.md | 66 +++++++++++++++ 6 files changed, 346 insertions(+), 1 deletion(-) create mode 100644 plan/2606240211_arch-fix-locate-helper-tests.md create mode 100644 plan/2606240212_arch-fix-lsp-rename-helper-tests.md create mode 100644 plan/2606240213_arch-fix-export-helper-tests.md create mode 100644 plan/2606240214_arch-fix-rename-dedup.md diff --git a/PLAN.md b/PLAN.md index 4cd4175e4..974d21f98 100644 --- a/PLAN.md +++ b/PLAN.md @@ -224,4 +224,8 @@ footer: | | 2606211910 | βœ… | | [arch-fix: add trivial-accessor exemption comments in workspace.go](plan/2606211910_arch-fix-workspace-exemptions.md) | | 2606231013 | βœ… | sonnet | [Add dedicated unit tests for inline_scan.go helpers](plan/2606231013_arch-fix-inline-scan-helper-tests.md) | | 2606231014 | βœ… | sonnet | [Add dedicated unit tests for samefileanchor helper functions](plan/2606231014_arch-fix-samefileanchor-helper-tests.md) | +| 2606240211 | πŸ”² | sonnet | [Add dedicated unit tests for locate.go helpers](plan/2606240211_arch-fix-locate-helper-tests.md) | +| 2606240212 | πŸ”² | sonnet | [Add dedicated unit tests for lsp/rename.go helpers](plan/2606240212_arch-fix-lsp-rename-helper-tests.md) | +| 2606240213 | πŸ”² | sonnet | [Add dedicated unit tests for export.go helpers and two small rename helpers](plan/2606240213_arch-fix-export-helper-tests.md) | +| 2606240214 | πŸ”² | sonnet | [Remove duplicated helpers between lsp/rename.go and rename/rename.go](plan/2606240214_arch-fix-rename-dedup.md) | diff --git a/docs/development/architecture-audit.md b/docs/development/architecture-audit.md index f34056350..43621793f 100644 --- a/docs/development/architecture-audit.md +++ b/docs/development/architecture-audit.md @@ -6,7 +6,7 @@ summary: >- solid-architecture skill (audit mode) appends here; blockers are also filed as plans. -audit-from: 1599c9f17336b36d4d06d10677b2510bfe33665b +audit-from: 09f22d3a59ea9cc07911df1db2d462da7d5fccb1 --- # Architecture audit log @@ -220,3 +220,55 @@ layering impact. [2606231013]: ../../plan/2606231013_arch-fix-inline-scan-helper-tests.md [2606231014]: ../../plan/2606231014_arch-fix-samefileanchor-helper-tests.md + +## Audit 2026-06-24 (range: 1599c9f..09f22d3) + +Perf series (struct-alignment, Sprintfβ†’strconv, +`[]byte` FindSubmatch, Builder). Plans 2606231013 +and 2606231014 closed. Benchmark docs and security +SARIF retired. No TypeScript changes. 273 Go +sources outside fixtures. + +No blockers. No rule-to-rule imports. No DIP +violations. No file crossed 1 000 lines. + +### tax (2026-06-24) + +- `internal/index/locate.go` β€” 12 unexported + helpers lack dedicated unit tests. Tests doc + Β§"every function by name" β€” + [plan/2606240211][2606240211]. + +- `internal/lsp/rename.go` β€” 15 unexported + helpers lack dedicated unit tests. Tests doc + Β§"every function by name" β€” + [plan/2606240212][2606240212]. + +- `internal/export/export.go` β€” 11 unexported + helpers lack dedicated unit tests. Tests doc + Β§"every function by name" β€” + [plan/2606240213][2606240213]. + +- `internal/lsp/rename.go` and + `internal/rename/rename.go` β€” `normalizedLabel` + and `refDefBracketBytes` are duplicated. Both + have identical bodies. Hub Β§"Anti-patterns" β€” + [plan/2606240214][2606240214]. + +- `internal/rules/concisenessscoring/rule.go` + and `internal/rename/rename.go` β€” + `countClassifierTokens` and + `contentBlockLines` lack dedicated unit tests. + Batched into [plan/2606240213][2606240213]. + +### nice-to-have (2026-06-24) + +- `internal/index/locate.go` β€” + `isGlobPattern` is a trivial one-liner with no + branch. Add "// no test by design" so the audit + can distinguish it from forgotten test debt. + +[2606240211]: ../../plan/2606240211_arch-fix-locate-helper-tests.md +[2606240212]: ../../plan/2606240212_arch-fix-lsp-rename-helper-tests.md +[2606240213]: ../../plan/2606240213_arch-fix-export-helper-tests.md +[2606240214]: ../../plan/2606240214_arch-fix-rename-dedup.md diff --git a/plan/2606240211_arch-fix-locate-helper-tests.md b/plan/2606240211_arch-fix-locate-helper-tests.md new file mode 100644 index 000000000..fa93baa64 --- /dev/null +++ b/plan/2606240211_arch-fix-locate-helper-tests.md @@ -0,0 +1,68 @@ +--- +id: 2606240211 +title: Add dedicated unit tests for locate.go helpers +status: "πŸ”²" +model: sonnet +summary: >- + internal/index/locate.go has 12 unexported + helpers without dedicated unit tests. Add a + named test for each so the audit policy is + satisfied. +--- +# Add dedicated unit tests for locate.go helpers + +## Goal + +Add a named unit test for each of the 12 unexported +helpers in `internal/index/locate.go`. The 2026-06-24 +audit requires it. + +## Background + +Go arch doc Β§"Tests" requires every production function +to have a dedicated test by name. The 2026-06-24 audit +(range: 1599c9f..09f22d3) flagged this file. + +Functions without a dedicated test as of 09f22d3: + +- `headingInfo` β€” extract text, level, anchor from + a heading node +- `locateInAST` β€” walk AST at a line/col offset +- `linkContainsOffset` β€” byte offset inside a link +- `linkCloseOffset` β€” find closing bracket offset +- `scanForByte` β€” scan source for a target byte +- `linkToLocate` β€” project `*ast.Link` to result +- `piToLocate` β€” project PI node to result +- `listItemValue` β€” parse `- key: value` line +- `headingOnLine` β€” find heading at a given line +- `frontMatterListItem` β€” parse front-matter row +- `frontMatterParentKey` β€” walk up to parent key +- `offsetAt` β€” convert (line, col) to byte offset + +`isGlobPattern` is a trivial one-liner with no branch. +Add a "// no test by design" exemption comment. + +## Tasks + +1. For each of the 12 functions above, add at least + one `TestFunctionName` in + `internal/index/locate_test.go`. Drive the helper + directly, not through `Locate`. +2. Add a `// no test by design` comment on + `isGlobPattern` in `internal/index/locate.go`. +3. `go test ./internal/index/...` passes. +4. `go vet ./internal/index/...` passes. + +## Acceptance Criteria + +- [ ] `locate_test.go` contains `TestheadingInfo`, + `TestlocateInAST`, `TestlinkContainsOffset`, + `TestlinkCloseOffset`, `TestscanForByte`, + `TestlinkToLocate`, `TestpiToLocate`, + `TestlistItemValue`, `TestheadingOnLine`, + `TestfrontMatterListItem`, + `TestfrontMatterParentKey`, `TestoffsetAt`. +- [ ] `isGlobPattern` carries a "// no test by + design" comment. +- [ ] `go test ./internal/index/...` is green. +- [ ] `mdsmith check .` is green. diff --git a/plan/2606240212_arch-fix-lsp-rename-helper-tests.md b/plan/2606240212_arch-fix-lsp-rename-helper-tests.md new file mode 100644 index 000000000..a8b019bcf --- /dev/null +++ b/plan/2606240212_arch-fix-lsp-rename-helper-tests.md @@ -0,0 +1,74 @@ +--- +id: 2606240212 +title: Add dedicated unit tests for lsp/rename.go helpers +status: "πŸ”²" +model: sonnet +summary: >- + internal/lsp/rename.go has 15 unexported + helpers without dedicated unit tests. Add a + named test for each so the audit policy is + satisfied. +--- +# Add dedicated unit tests for lsp/rename.go helpers + +## Goal + +Add a named unit test for each of the 15 unexported +helpers in `internal/lsp/rename.go`. The 2026-06-24 +audit requires it. + +## Background + +Go arch doc Β§"Tests" requires every production function +to have a dedicated test by name. The 2026-06-24 audit +(range: 1599c9f..09f22d3) flagged this file. + +`atxHeadingTextByteRange` already has a test. The +three trivial pass-through methods on the workspace +adapter carry exemption comments. The 15 helpers +below need dedicated tests: + +- `isValidRefDefLine` +- `headingPrepareRange` +- `atxHeadingTextStart` +- `trimTrailingHashRun` +- `skipLeadingSpaces` +- `trimRightSpace` +- `trimmedRange` +- `refDefPrepareRange` +- `refDefBracketBytes` +- `refUsePrepareRange` +- `refUseLabelBytes` (one partial test exists; + add broader coverage) +- `matchLeadingPair` +- `matchTrailingPair` +- `normalizedLabel` +- `bracketPairs` + +## Tasks + +1. For each function above, add at least one + `TestFunctionName` in + `internal/lsp/rename_test.go`. Drive the helper + directly with a byte-slice input. +2. `go test ./internal/lsp/...` passes. +3. `go vet ./internal/lsp/...` passes. + +## Acceptance Criteria + +- [ ] `rename_test.go` contains + `TestisValidRefDefLine`, + `TestheadingPrepareRange`, + `TestatxHeadingTextStart`, + `TesttrimTrailingHashRun`, + `TestskipLeadingSpaces`, + `TesttrimRightSpace`, `TesttrimmedRange`, + `TestrefDefPrepareRange`, + `TestrefDefBracketBytes`, + `TestrefUsePrepareRange`, + `TestrefUseLabelBytes`, + `TestmatchLeadingPair`, + `TestmatchTrailingPair`, + `TestnormalizedLabel`, `TestbracketPairs`. +- [ ] `go test ./internal/lsp/...` is green. +- [ ] `mdsmith check .` is green. diff --git a/plan/2606240213_arch-fix-export-helper-tests.md b/plan/2606240213_arch-fix-export-helper-tests.md new file mode 100644 index 000000000..e8158a7b1 --- /dev/null +++ b/plan/2606240213_arch-fix-export-helper-tests.md @@ -0,0 +1,81 @@ +--- +id: 2606240213 +title: >- + Add dedicated unit tests for export.go helpers + and two small rename helpers +status: "πŸ”²" +model: sonnet +summary: >- + internal/export/export.go has 11 unexported + helpers without dedicated tests. Two small + helpers from concisenessscoring and rename are + batched here. Flagged by the 2026-06-24 audit. +--- +# Add dedicated unit tests for export helpers + +## Goal + +Add named unit tests for 11 unexported helpers in +`internal/export/export.go`. Two small helpers from +other files are batched here as well. + +## Background + +Go arch doc Β§"Tests" requires every production function +to have a dedicated test by name. The 2026-06-24 audit +(range: 1599c9f..09f22d3) flagged all three files. + +### internal/export/export.go + +Functions without a dedicated test: + +- `selectDirectives` β€” pick rules with directives +- `allDirectiveNames` β€” list built-in directive names +- `regenerate` β€” rebuild generated sections +- `hydrate` β€” copy diagnostics to original file +- `checkStaleness` β€” flag stale generated bodies +- `inGeneratedRange` β€” check if line is generated +- `stripDirectives` β€” remove directive PI markers +- `piLineRange` β€” compute PI block line range +- `overlapsAny` β€” check if `[from,to]` hits line set +- `emitLines` β€” emit non-stripped lines to buffer +- `normalizeBlankLines` β€” collapse blank-line runs + +### internal/rules/concisenessscoring/rule.go + +- `countClassifierTokens` β€” count tokens without + allocating a regex match per call + +### internal/rename/rename.go + +- `contentBlockLines` β€” lines inside code blocks + +## Tasks + +1. Add a `TestFunctionName` in + `internal/export/export_test.go` for each of the + 11 export helpers above. +2. Add `TestcountClassifierTokens` in + `internal/rules/concisenessscoring/rule_test.go`. +3. Add `TestcontentBlockLines` in + `internal/rename/rename_test.go`. +4. `go test ./internal/export/...` passes. +5. `go test ./internal/rules/concisenessscoring/...` + passes. +6. `go test ./internal/rename/...` passes. + +## Acceptance Criteria + +- [ ] `export_test.go` contains + `TestselectDirectives`, `TestallDirectiveNames`, + `Testregenerate`, `Testhydrate`, + `TestcheckStaleness`, `TestinGeneratedRange`, + `TeststripDirectives`, `TestpiLineRange`, + `TestoverlapsAny`, `TestemitLines`, + `TestnormalizeBlankLines`. +- [ ] `rule_test.go` contains + `TestcountClassifierTokens`. +- [ ] `rename_test.go` contains + `TestcontentBlockLines`. +- [ ] All three `go test` commands are green. +- [ ] `mdsmith check .` is green. diff --git a/plan/2606240214_arch-fix-rename-dedup.md b/plan/2606240214_arch-fix-rename-dedup.md new file mode 100644 index 000000000..08873b3a9 --- /dev/null +++ b/plan/2606240214_arch-fix-rename-dedup.md @@ -0,0 +1,66 @@ +--- +id: 2606240214 +title: >- + Remove duplicated helpers between lsp/rename.go + and rename/rename.go +status: "πŸ”²" +model: sonnet +summary: >- + normalizedLabel and refDefBracketBytes are + duplicated with identical bodies in two packages. + Export them from rename and remove the lsp copies. + Flagged by the 2026-06-24 audit. +--- +# Remove duplicated rename helpers + +## Goal + +Export `normalizedLabel` and `refDefBracketBytes` from +`internal/rename` and delete the copies in +`internal/lsp/rename.go`. + +## Background + +The 2026-06-24 audit (range: 1599c9f..09f22d3) found +that two private helpers are duplicated with identical +bodies: + +- `normalizedLabel(b []byte) string` β€” wraps + `util.ToLinkReference(b)` +- `refDefBracketBytes(row []byte) []int` β€” parses + bracket bounds on a `[label]:` line + +`internal/lsp` already imports `internal/rename`. +Hub Β§"Anti-patterns we have actually hit" flags this +kind of silent copy as maintenance friction. +If either copy is updated, the other diverges without +a compile error. + +## Tasks + +1. In `internal/rename/rename.go`, rename + `normalizedLabel` β†’ `NormalizedLabel` and + `refDefBracketBytes` β†’ `RefDefBracketBytes`. +2. Update all callers in `internal/rename/` to use + the exported names. +3. Add or update tests in + `internal/rename/rename_test.go` for both. +4. In `internal/lsp/rename.go`, delete the private + copies of both functions. +5. Replace every call site in `lsp/rename.go` with + `rename.NormalizedLabel` and + `rename.RefDefBracketBytes`. +6. `go build ./...` passes. +7. `go test ./internal/lsp/... ./internal/rename/...` + passes. + +## Acceptance Criteria + +- [ ] `internal/lsp/rename.go` has no private + `normalizedLabel` or `refDefBracketBytes`. +- [ ] `internal/rename/rename.go` exports + `NormalizedLabel` and `RefDefBracketBytes`. +- [ ] `internal/rename/rename_test.go` has dedicated + tests for both helpers. +- [ ] `go test ./...` is green. +- [ ] `mdsmith check .` is green. From 4cebe15a5c7bbdd16767512689d686d7329401e2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 24 Jun 2026 02:28:26 +0000 Subject: [PATCH 2/4] test(index): add dedicated tests for 12 locate.go helpers Closes plan/2606240211. Adds TestHeadingInfo, TestLocateInAST, TestLinkContainsOffset, TestLinkCloseOffset, TestScanForByte, TestLinkToLocate, TestPiToLocate, TestListItemValue, TestHeadingOnLine, TestFrontMatterListItem, TestFrontMatterParentKey, and TestOffsetAt to internal/index/locate_test.go. Each helper is exercised directly with a byte-slice or AST input, not through Locator.Locate. isGlobPattern gains a "// no test by design" comment. Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_017DMVDTLs2U9kcVMEyLNZgK --- PLAN.md | 2 +- internal/index/locate.go | 1 + internal/index/locate_test.go | 283 ++++++++++++++++++ ...2606240211_arch-fix-locate-helper-tests.md | 35 ++- 4 files changed, 305 insertions(+), 16 deletions(-) diff --git a/PLAN.md b/PLAN.md index 974d21f98..7f3e2bc20 100644 --- a/PLAN.md +++ b/PLAN.md @@ -224,7 +224,7 @@ footer: | | 2606211910 | βœ… | | [arch-fix: add trivial-accessor exemption comments in workspace.go](plan/2606211910_arch-fix-workspace-exemptions.md) | | 2606231013 | βœ… | sonnet | [Add dedicated unit tests for inline_scan.go helpers](plan/2606231013_arch-fix-inline-scan-helper-tests.md) | | 2606231014 | βœ… | sonnet | [Add dedicated unit tests for samefileanchor helper functions](plan/2606231014_arch-fix-samefileanchor-helper-tests.md) | -| 2606240211 | πŸ”² | sonnet | [Add dedicated unit tests for locate.go helpers](plan/2606240211_arch-fix-locate-helper-tests.md) | +| 2606240211 | βœ… | sonnet | [Add dedicated unit tests for locate.go helpers](plan/2606240211_arch-fix-locate-helper-tests.md) | | 2606240212 | πŸ”² | sonnet | [Add dedicated unit tests for lsp/rename.go helpers](plan/2606240212_arch-fix-lsp-rename-helper-tests.md) | | 2606240213 | πŸ”² | sonnet | [Add dedicated unit tests for export.go helpers and two small rename helpers](plan/2606240213_arch-fix-export-helper-tests.md) | | 2606240214 | πŸ”² | sonnet | [Remove duplicated helpers between lsp/rename.go and rename/rename.go](plan/2606240214_arch-fix-rename-dedup.md) | diff --git a/internal/index/locate.go b/internal/index/locate.go index 4b62fe54e..4a0864ffd 100644 --- a/internal/index/locate.go +++ b/internal/index/locate.go @@ -487,6 +487,7 @@ var piArgRE = regexp.MustCompile(`^\s*([A-Za-z_][A-Za-z0-9_-]*)\s*:\s*(.*?)\s*$` var piListItemRE = regexp.MustCompile(`^\s*-\s+(.*?)\s*$`) // isGlobPattern reports whether p contains doublestar glob metacharacters. +// no test by design: trivial one-liner with no branch. func isGlobPattern(p string) bool { return strings.ContainsAny(p, "*?[{") } // headingOnLine returns the heading whose first source line equals diff --git a/internal/index/locate_test.go b/internal/index/locate_test.go index 71355bcdb..0b358eb83 100644 --- a/internal/index/locate_test.go +++ b/internal/index/locate_test.go @@ -1,10 +1,18 @@ package index import ( + "bytes" "strings" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/jeduden/mdsmith/internal/lint" + "github.com/jeduden/mdsmith/internal/piparser" + goldast "github.com/jeduden/mdsmith/pkg/goldmark/ast" + "github.com/jeduden/mdsmith/pkg/goldmark/parser" + "github.com/jeduden/mdsmith/pkg/goldmark/text" ) func TestLocateHeading(t *testing.T) { @@ -260,3 +268,278 @@ func TestEnclosingListKey_FindsParentKey(t *testing.T) { got := enclosingListKey(lines, 4) assert.Equal(t, "inputs", got) } + +// --- Dedicated tests for unexported helpers in locate.go --- + +// parseDoc parses body-only markdown (no front matter) and returns root + source. +func parseDoc(src string) (goldast.Node, []byte) { + b := []byte(src) + root := lint.NewParser().Parse(text.NewReader(b), parser.WithContext(parser.NewContext())) + return root, b +} + +// firstLink returns the first *ast.Link in root, or nil. +func firstLink(root goldast.Node) *goldast.Link { + var found *goldast.Link + _ = goldast.Walk(root, func(n goldast.Node, entering bool) (goldast.WalkStatus, error) { + if !entering { + return goldast.WalkContinue, nil + } + if l, ok := n.(*goldast.Link); ok { + found = l + return goldast.WalkStop, nil + } + return goldast.WalkContinue, nil + }) + return found +} + +// firstPI returns the first *piparser.ProcessingInstruction in root, or nil. +func firstPI(root goldast.Node) *piparser.ProcessingInstruction { + var found *piparser.ProcessingInstruction + _ = goldast.Walk(root, func(n goldast.Node, entering bool) (goldast.WalkStatus, error) { + if !entering { + return goldast.WalkContinue, nil + } + if pi, ok := n.(*piparser.ProcessingInstruction); ok { + found = pi + return goldast.WalkStop, nil + } + return goldast.WalkContinue, nil + }) + return found +} + +// nthHeading returns the n-th (1-based) heading in root, or nil. +func nthHeading(root goldast.Node, n int) *goldast.Heading { + count := 0 + var found *goldast.Heading + _ = goldast.Walk(root, func(node goldast.Node, entering bool) (goldast.WalkStatus, error) { + if !entering { + return goldast.WalkContinue, nil + } + h, ok := node.(*goldast.Heading) + if !ok { + return goldast.WalkContinue, nil + } + count++ + if count == n { + found = h + return goldast.WalkStop, nil + } + return goldast.WalkContinue, nil + }) + return found +} + +func TestHeadingInfo(t *testing.T) { + t.Parallel() + src := "# Alpha\n\n# Alpha\n\n## Beta\n" + root, b := parseDoc(src) + + h1 := nthHeading(root, 1) + h2 := nthHeading(root, 2) + h3 := nthHeading(root, 3) + require.NotNil(t, h1) + require.NotNil(t, h2) + require.NotNil(t, h3) + + anchor, level, name := headingInfo(h1, b, root) + assert.Equal(t, "alpha", anchor) + assert.Equal(t, 1, level) + assert.Equal(t, "Alpha", name) + + // Duplicate slug disambiguated with suffix. + anchor2, _, _ := headingInfo(h2, b, root) + assert.Equal(t, "alpha-1", anchor2) + + anchor3, level3, name3 := headingInfo(h3, b, root) + assert.Equal(t, "beta", anchor3) + assert.Equal(t, 2, level3) + assert.Equal(t, "Beta", name3) +} + +func TestLocateInAST(t *testing.T) { + t.Parallel() + src := "# T\n\n[link](./a.md)\n" + root, b := parseDoc(src) + lines := bytes.Split(b, []byte("\n")) + + // Line 3, col 3 is inside the link text "link". + res, ok := locateInAST("doc.md", root, b, lines, 3, 3) + assert.True(t, ok) + assert.Equal(t, TokenFileLink, res.Tag) + assert.Equal(t, "a.md", res.TargetFile) + + // Line 1, col 1 is on the heading β€” no link or PI. + _, ok = locateInAST("doc.md", root, b, lines, 1, 1) + assert.False(t, ok) +} + +func TestLinkContainsOffset(t *testing.T) { + t.Parallel() + // "# T\n\n" = 5 bytes; "[text](./a.md)" starts at offset 5. + src := "# T\n\n[text](./a.md)\n" + root, b := parseDoc(src) + l := firstLink(root) + require.NotNil(t, l) + + // Offset 7 is 'e' in "text" β€” inside the link. + assert.True(t, linkContainsOffset(b, l, 7)) + // Offset 0 is '#' β€” before the link. + assert.False(t, linkContainsOffset(b, l, 0)) +} + +func TestLinkCloseOffset(t *testing.T) { + t.Parallel() + // "[text](dest)\n": '[' 0, text 1-4, ']' 5, '(' 6, dest 7-10, ')' 11, '\n' 12. + // With nil link (inline path): after=5, source[5]=']' so i advances to 6, + // source[6]='(' triggers depth scan, ')' found at offset 11. + src := []byte("[text](dest)\n") + assert.Equal(t, 11, linkCloseOffset(src, nil, 5)) + + // Newline before the closing ')' β†’ -1. + srcBroken := []byte("[text](dest\nmore\n") + assert.Equal(t, -1, linkCloseOffset(srcBroken, nil, 5)) +} + +func TestScanForByte(t *testing.T) { + t.Parallel() + src := []byte("ab]cd") + assert.Equal(t, 2, scanForByte(src, 0, ']')) + // Start past the target. + assert.Equal(t, -1, scanForByte(src, 3, ']')) + // Newline stops the scan before the target. + src2 := []byte("ab\n]") + assert.Equal(t, -1, scanForByte(src2, 0, ']')) + // Target not present at all. + assert.Equal(t, -1, scanForByte(src, 0, 'z')) +} + +func TestLinkToLocate(t *testing.T) { + t.Parallel() + + // Inline file link. + root, b := parseDoc("# T\n\n[text](./a.md)\n") + l := firstLink(root) + require.NotNil(t, l) + res := linkToLocate("doc.md", l, b) + assert.Equal(t, TokenFileLink, res.Tag) + assert.Equal(t, "a.md", res.TargetFile) + + // Reference-use link. + root2, b2 := parseDoc("# T\n\n[text][label]\n\n[label]: https://x.com\n") + l2 := firstLink(root2) + require.NotNil(t, l2) + res2 := linkToLocate("doc.md", l2, b2) + assert.Equal(t, TokenRefUse, res2.Tag) + assert.Equal(t, "label", res2.Label) + + // Anchor-only link. + root3, b3 := parseDoc("# T\n\n[here](#sec)\n") + l3 := firstLink(root3) + require.NotNil(t, l3) + res3 := linkToLocate("doc.md", l3, b3) + assert.Equal(t, TokenAnchorLink, res3.Tag) + assert.Equal(t, "sec", res3.TargetAnchor) +} + +func TestPiToLocate(t *testing.T) { + t.Parallel() + src := "# T\n\n\n\n" + root, b := parseDoc(src) + pi := firstPI(root) + require.NotNil(t, pi) + + lines := bytes.Split(b, []byte("\n")) + // Line 4 is `file: "x.md"`. + res := piToLocate(pi, b, lines, 4, 8) + assert.Equal(t, TokenDirectiveArg, res.Tag) + assert.Equal(t, "include", res.DirectiveName) + assert.Equal(t, "file", res.DirectiveArg) + assert.Equal(t, "x.md", res.DirectiveValue) + assert.Equal(t, "x.md", res.DirectiveTargetFile) +} + +func TestListItemValue(t *testing.T) { + t.Parallel() + v, ok := listItemValue(" - foo") + assert.True(t, ok) + assert.Equal(t, "foo", v) + + v, ok = listItemValue(` - "bar"`) + assert.True(t, ok) + assert.Equal(t, "bar", v) + + _, ok = listItemValue("not a list item") + assert.False(t, ok) + + _, ok = listItemValue("key: value") + assert.False(t, ok) +} + +func TestHeadingOnLine(t *testing.T) { + t.Parallel() + src := "# Top\n\nSome text\n\n## Sub\n" + root, b := parseDoc(src) + + h := headingOnLine(root, b, 1) + require.NotNil(t, h) + assert.Equal(t, 1, h.Level) + + h = headingOnLine(root, b, 3) + assert.Nil(t, h) + + h = headingOnLine(root, b, 5) + require.NotNil(t, h) + assert.Equal(t, 2, h.Level) +} + +func TestFrontMatterListItem(t *testing.T) { + t.Parallel() + v, ok := frontMatterListItem("- foo") + assert.True(t, ok) + assert.Equal(t, "foo", v) + + v, ok = frontMatterListItem(" - bar") + assert.True(t, ok) + assert.Equal(t, "bar", v) + + v, ok = frontMatterListItem("-") + assert.True(t, ok) + assert.Equal(t, "", v) + + _, ok = frontMatterListItem("key: val") + assert.False(t, ok) +} + +func TestFrontMatterParentKey(t *testing.T) { + t.Parallel() + lines := [][]byte{ + []byte("kinds:"), + []byte(" - guide"), + []byte(" - reference"), + } + assert.Equal(t, "kinds", frontMatterParentKey(lines, 2)) + assert.Equal(t, "kinds", frontMatterParentKey(lines, 1)) + assert.Equal(t, "", frontMatterParentKey(lines, 0)) +} + +func TestOffsetAt(t *testing.T) { + t.Parallel() + lines := [][]byte{ + []byte("abc"), + []byte("de"), + []byte("f"), + } + // (1,1) β†’ 0 + assert.Equal(t, 0, offsetAt(lines, 1, 1)) + // (1,2) β†’ 1 + assert.Equal(t, 1, offsetAt(lines, 1, 2)) + // (2,1) β†’ len("abc")+newline = 4 + assert.Equal(t, 4, offsetAt(lines, 2, 1)) + // (2,2) β†’ 5 + assert.Equal(t, 5, offsetAt(lines, 2, 2)) + // Clamp: line < 1 β†’ treated as line 1 + assert.Equal(t, 0, offsetAt(lines, 0, 1)) +} diff --git a/plan/2606240211_arch-fix-locate-helper-tests.md b/plan/2606240211_arch-fix-locate-helper-tests.md index fa93baa64..bfb3dbe3d 100644 --- a/plan/2606240211_arch-fix-locate-helper-tests.md +++ b/plan/2606240211_arch-fix-locate-helper-tests.md @@ -1,7 +1,7 @@ --- id: 2606240211 title: Add dedicated unit tests for locate.go helpers -status: "πŸ”²" +status: "βœ…" model: sonnet summary: >- internal/index/locate.go has 12 unexported @@ -42,27 +42,32 @@ Functions without a dedicated test as of 09f22d3: `isGlobPattern` is a trivial one-liner with no branch. Add a "// no test by design" exemption comment. +Note: Go vet requires test function names to start with +an uppercase letter after `Test`. The names below follow +the established codebase pattern (capitalize the first +letter of the helper name). + ## Tasks -1. For each of the 12 functions above, add at least +1. [x] For each of the 12 functions above, add at least one `TestFunctionName` in `internal/index/locate_test.go`. Drive the helper directly, not through `Locate`. -2. Add a `// no test by design` comment on +2. [x] Add a `// no test by design` comment on `isGlobPattern` in `internal/index/locate.go`. -3. `go test ./internal/index/...` passes. -4. `go vet ./internal/index/...` passes. +3. [x] `go test ./internal/index/...` passes. +4. [x] `go vet ./internal/index/...` passes. ## Acceptance Criteria -- [ ] `locate_test.go` contains `TestheadingInfo`, - `TestlocateInAST`, `TestlinkContainsOffset`, - `TestlinkCloseOffset`, `TestscanForByte`, - `TestlinkToLocate`, `TestpiToLocate`, - `TestlistItemValue`, `TestheadingOnLine`, - `TestfrontMatterListItem`, - `TestfrontMatterParentKey`, `TestoffsetAt`. -- [ ] `isGlobPattern` carries a "// no test by +- [x] `locate_test.go` contains `TestHeadingInfo`, + `TestLocateInAST`, `TestLinkContainsOffset`, + `TestLinkCloseOffset`, `TestScanForByte`, + `TestLinkToLocate`, `TestPiToLocate`, + `TestListItemValue`, `TestHeadingOnLine`, + `TestFrontMatterListItem`, + `TestFrontMatterParentKey`, `TestOffsetAt`. +- [x] `isGlobPattern` carries a "// no test by design" comment. -- [ ] `go test ./internal/index/...` is green. -- [ ] `mdsmith check .` is green. +- [x] `go test ./internal/index/...` is green. +- [x] `mdsmith check .` is green. From 3eae15677ff76a6dacbaddf167b2f516c73f823f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 24 Jun 2026 02:36:25 +0000 Subject: [PATCH 3/4] test(index): fix review-round-1 gaps in locate helper tests - TestOffsetAt: add col-clamping case (col > line length), exercising the branch at locate.go:639-641 that was previously uncovered - TestPiToLocate_GlobInputSuppressed: new test verifying that a glob pattern in a build directive inputs list does NOT populate DirectiveTargetFile (the !isGlobPattern guard was untested) - TestLinkCloseOffset: add shortcut-reference and full-reference cases using real parsed *ast.Link nodes so the l.Reference != nil branches are exercised directly, not just the nil/inline path Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_017DMVDTLs2U9kcVMEyLNZgK --- internal/index/locate_test.go | 66 +++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/internal/index/locate_test.go b/internal/index/locate_test.go index 0b358eb83..c3c0fc79c 100644 --- a/internal/index/locate_test.go +++ b/internal/index/locate_test.go @@ -392,15 +392,55 @@ func TestLinkContainsOffset(t *testing.T) { func TestLinkCloseOffset(t *testing.T) { t.Parallel() + // Inline link via nil (exercises the l==nil path, identical to the + // real production path where l!=nil, l.Reference==nil). // "[text](dest)\n": '[' 0, text 1-4, ']' 5, '(' 6, dest 7-10, ')' 11, '\n' 12. - // With nil link (inline path): after=5, source[5]=']' so i advances to 6, - // source[6]='(' triggers depth scan, ')' found at offset 11. src := []byte("[text](dest)\n") assert.Equal(t, 11, linkCloseOffset(src, nil, 5)) // Newline before the closing ')' β†’ -1. srcBroken := []byte("[text](dest\nmore\n") assert.Equal(t, -1, linkCloseOffset(srcBroken, nil, 5)) + + // Shortcut reference [label]: close must land on the single ']'. + root, b := parseDoc("# T\n\n[label]\n\n[label]: https://x.com\n") + shortcut := firstLink(root) + require.NotNil(t, shortcut) + var shortcutAfter int + _ = goldast.Walk(shortcut, func(n goldast.Node, entering bool) (goldast.WalkStatus, error) { + if !entering { + return goldast.WalkContinue, nil + } + if tx, ok := n.(*goldast.Text); ok { + shortcutAfter = tx.Segment.Stop + return goldast.WalkStop, nil + } + return goldast.WalkContinue, nil + }) + off := linkCloseOffset(b, shortcut, shortcutAfter) + assert.True(t, off >= 0, "shortcut ref close offset must be β‰₯ 0") + assert.Equal(t, byte(']'), b[off], "shortcut ref must close at ']'") + + // Full reference [text][label]: close must land on the ']' of the label part. + root2, b2 := parseDoc("# T\n\n[text][label]\n\n[label]: https://x.com\n") + full := firstLink(root2) + require.NotNil(t, full) + var fullAfter int + _ = goldast.Walk(full, func(n goldast.Node, entering bool) (goldast.WalkStatus, error) { + if !entering { + return goldast.WalkContinue, nil + } + if tx, ok := n.(*goldast.Text); ok { + fullAfter = tx.Segment.Stop + return goldast.WalkStop, nil + } + return goldast.WalkContinue, nil + }) + off2 := linkCloseOffset(b2, full, fullAfter) + assert.True(t, off2 >= 0, "full ref close offset must be β‰₯ 0") + assert.Equal(t, byte(']'), b2[off2], "full ref must close at ']' of label part") + // The close must be past the text-closing ']' (i.e., farther into the source). + assert.Greater(t, off2, fullAfter, "full ref close must be past the text bracket") } func TestScanForByte(t *testing.T) { @@ -461,6 +501,25 @@ func TestPiToLocate(t *testing.T) { assert.Equal(t, "x.md", res.DirectiveTargetFile) } +func TestPiToLocate_GlobInputSuppressed(t *testing.T) { + t.Parallel() + // A inputs list item that is a glob pattern must NOT populate + // DirectiveTargetFile β€” go-to-definition must not fire on a pattern. + src := "# T\n\n\n\n" + root, b := parseDoc(src) + pi := firstPI(root) + require.NotNil(t, pi) + + lines := bytes.Split(b, []byte("\n")) + // Line 6 (1-based) is ` - "*.md"` β€” a glob pattern. + res := piToLocate(pi, b, lines, 6, 5) + assert.Equal(t, TokenDirectiveArg, res.Tag) + assert.Equal(t, "build", res.DirectiveName) + assert.Equal(t, "inputs", res.DirectiveArg) + assert.Equal(t, "*.md", res.DirectiveValue) + assert.Equal(t, "", res.DirectiveTargetFile, "glob pattern must not populate DirectiveTargetFile") +} + func TestListItemValue(t *testing.T) { t.Parallel() v, ok := listItemValue(" - foo") @@ -542,4 +601,7 @@ func TestOffsetAt(t *testing.T) { assert.Equal(t, 5, offsetAt(lines, 2, 2)) // Clamp: line < 1 β†’ treated as line 1 assert.Equal(t, 0, offsetAt(lines, 0, 1)) + // Clamp: col past end of line β†’ clamped to line length. + // Line 1 "abc" has length 3; col 99 β†’ offset = 0 + 3 = 3. + assert.Equal(t, 3, offsetAt(lines, 1, 99)) } From 457ab7310118a1f135fba1c96a056b1a4aa8de48 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 24 Jun 2026 02:44:28 +0000 Subject: [PATCH 4/4] test(index): fix review-round-2 gaps in locate helper tests - Use require.True (not assert.True) before b[off] indexing in TestLinkCloseOffset: assert continues on failure, causing a panic on the immediately following b[-1] index if the offset is -1. - Add third duplicate heading to TestHeadingInfo to exercise the disambiguation counter beyond c==1 (verifies "alpha-2" suffix). - Add bare-dash case to TestListItemValue to document that " -" does not match piListItemRE (contrast with frontMatterListItem). - Remove section-separator comment (CLAUDE.md: no "what" comments). Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_017DMVDTLs2U9kcVMEyLNZgK --- internal/index/locate_test.go | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/internal/index/locate_test.go b/internal/index/locate_test.go index c3c0fc79c..b49094774 100644 --- a/internal/index/locate_test.go +++ b/internal/index/locate_test.go @@ -269,8 +269,6 @@ func TestEnclosingListKey_FindsParentKey(t *testing.T) { assert.Equal(t, "inputs", got) } -// --- Dedicated tests for unexported helpers in locate.go --- - // parseDoc parses body-only markdown (no front matter) and returns root + source. func parseDoc(src string) (goldast.Node, []byte) { b := []byte(src) @@ -334,29 +332,34 @@ func nthHeading(root goldast.Node, n int) *goldast.Heading { func TestHeadingInfo(t *testing.T) { t.Parallel() - src := "# Alpha\n\n# Alpha\n\n## Beta\n" + src := "# Alpha\n\n# Alpha\n\n# Alpha\n\n## Beta\n" root, b := parseDoc(src) h1 := nthHeading(root, 1) h2 := nthHeading(root, 2) h3 := nthHeading(root, 3) + h4 := nthHeading(root, 4) require.NotNil(t, h1) require.NotNil(t, h2) require.NotNil(t, h3) + require.NotNil(t, h4) anchor, level, name := headingInfo(h1, b, root) assert.Equal(t, "alpha", anchor) assert.Equal(t, 1, level) assert.Equal(t, "Alpha", name) - // Duplicate slug disambiguated with suffix. + // Second and third occurrences get sequential suffixes. anchor2, _, _ := headingInfo(h2, b, root) assert.Equal(t, "alpha-1", anchor2) - anchor3, level3, name3 := headingInfo(h3, b, root) - assert.Equal(t, "beta", anchor3) - assert.Equal(t, 2, level3) - assert.Equal(t, "Beta", name3) + anchor3, _, _ := headingInfo(h3, b, root) + assert.Equal(t, "alpha-2", anchor3) + + anchor4, level4, name4 := headingInfo(h4, b, root) + assert.Equal(t, "beta", anchor4) + assert.Equal(t, 2, level4) + assert.Equal(t, "Beta", name4) } func TestLocateInAST(t *testing.T) { @@ -418,7 +421,7 @@ func TestLinkCloseOffset(t *testing.T) { return goldast.WalkContinue, nil }) off := linkCloseOffset(b, shortcut, shortcutAfter) - assert.True(t, off >= 0, "shortcut ref close offset must be β‰₯ 0") + require.True(t, off >= 0, "shortcut ref close offset must be β‰₯ 0") assert.Equal(t, byte(']'), b[off], "shortcut ref must close at ']'") // Full reference [text][label]: close must land on the ']' of the label part. @@ -437,7 +440,7 @@ func TestLinkCloseOffset(t *testing.T) { return goldast.WalkContinue, nil }) off2 := linkCloseOffset(b2, full, fullAfter) - assert.True(t, off2 >= 0, "full ref close offset must be β‰₯ 0") + require.True(t, off2 >= 0, "full ref close offset must be β‰₯ 0") assert.Equal(t, byte(']'), b2[off2], "full ref must close at ']' of label part") // The close must be past the text-closing ']' (i.e., farther into the source). assert.Greater(t, off2, fullAfter, "full ref close must be past the text bracket") @@ -535,6 +538,12 @@ func TestListItemValue(t *testing.T) { _, ok = listItemValue("key: value") assert.False(t, ok) + + // Bare dash without a value does not match (piListItemRE requires + // whitespace after the dash); contrast with frontMatterListItem which + // accepts bare "-". + _, ok = listItemValue(" -") + assert.False(t, ok) } func TestHeadingOnLine(t *testing.T) {