diff --git a/PLAN.md b/PLAN.md index 4cd4175e4..7f3e2bc20 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/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..b49094774 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,349 @@ func TestEnclosingListKey_FindsParentKey(t *testing.T) { got := enclosingListKey(lines, 4) assert.Equal(t, "inputs", got) } + +// 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# 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) + + // Second and third occurrences get sequential suffixes. + anchor2, _, _ := headingInfo(h2, b, root) + assert.Equal(t, "alpha-1", anchor2) + + 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) { + 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() + // 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. + 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) + 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. + 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) + 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") +} + +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 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") + 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) + + // 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) { + 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)) + // 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)) +} 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..bfb3dbe3d --- /dev/null +++ b/plan/2606240211_arch-fix-locate-helper-tests.md @@ -0,0 +1,73 @@ +--- +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. + +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. [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. [x] Add a `// no test by design` comment on + `isGlobPattern` in `internal/index/locate.go`. +3. [x] `go test ./internal/index/...` passes. +4. [x] `go vet ./internal/index/...` passes. + +## Acceptance Criteria + +- [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. +- [x] `go test ./internal/index/...` is green. +- [x] `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.