Skip to content

Commit ed2c65a

Browse files
committed
fix(noreferencestyle): keep nil return on zero footnote matches
make([]footnoteOccurrence, 0, len(matches)) is non-nil even when len(matches) == 0 (Go's make does not special-case a zero-capacity slice back to nil), so the earlier pre-sizing commit silently broke the "return nil, not []T{}" convention this same PR applies to headingincrement and tableformat for the common zero-match case. Return nil directly before the pre-sized make when there are no matches. Caught by code review round 2. Also corrects the extractHeadings doc/test comments in requiredstructure: checkSingleFileSchemaFromData (single-source) and checkComposedSources's per-source loop (N>=2 extends sources) are mutually exclusive within one Check, so the walk count saved is N, not "N+" as the round-1 correction still implied. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BTLNzEVChZtqSc7m7ph5Ah
1 parent 11fe5e1 commit ed2c65a

4 files changed

Lines changed: 41 additions & 10 deletions

File tree

internal/rules/noreferencestyle/alloc_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,3 +95,25 @@ func TestScanFootnoteDefinitions_PresizedAllocs(t *testing.T) {
9595
assert.LessOrEqualf(t, allocs, 43.0,
9696
"scanFootnoteDefinitions allocs regressed: got %v, want <= 43", allocs)
9797
}
98+
99+
// TestScanFootnoteReferences_NoMatches_ReturnsNil and
100+
// TestScanFootnoteDefinitions_NoMatches_ReturnsNil pin
101+
// docs/development/high-performance-go.md's "return nil, not []T{}"
102+
// convention for the zero-match case: make([]footnoteOccurrence, 0,
103+
// len(matches)) with len(matches) == 0 still returns a non-nil empty
104+
// slice in Go, so the pre-sizing fix must special-case the no-match
105+
// return to stay nil, matching the convention this same PR applies to
106+
// headingincrement and tableformat.
107+
func TestScanFootnoteReferences_NoMatches_ReturnsNil(t *testing.T) {
108+
f, err := lint.NewFile("clean.md", []byte("No footnotes here.\n"))
109+
require.NoError(t, err)
110+
out := scanFootnoteReferences(f, map[int]struct{}{}, nil)
111+
assert.Nil(t, out, "scanFootnoteReferences must return nil when there are no matches")
112+
}
113+
114+
func TestScanFootnoteDefinitions_NoMatches_ReturnsNil(t *testing.T) {
115+
f, err := lint.NewFile("clean.md", []byte("No footnotes here.\n"))
116+
require.NoError(t, err)
117+
out := scanFootnoteDefinitions(f, map[int]struct{}{})
118+
assert.Nil(t, out, "scanFootnoteDefinitions must return nil when there are no matches")
119+
}

internal/rules/noreferencestyle/rule.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,9 @@ func scanFootnoteReferences(
373373
) []footnoteOccurrence {
374374
source := f.Source
375375
matches := footnoteRefRE.FindAllSubmatchIndex(source, -1)
376+
if len(matches) == 0 {
377+
return nil
378+
}
376379
out := make([]footnoteOccurrence, 0, len(matches))
377380
for _, m := range matches {
378381
start := m[0]
@@ -403,6 +406,9 @@ func scanFootnoteDefinitions(
403406
) []footnoteOccurrence {
404407
source := f.Source
405408
matches := footnoteDefRE.FindAllSubmatchIndex(source, -1)
409+
if len(matches) == 0 {
410+
return nil
411+
}
406412
out := make([]footnoteOccurrence, 0, len(matches))
407413
for _, m := range matches {
408414
start := m[0]

internal/rules/requiredstructure/rule.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1874,12 +1874,15 @@ type docHeading struct {
18741874
}
18751875

18761876
// extractHeadings walks the AST and collects all headings. Memoized
1877-
// per File via lint.File.MemoFile: on the document File, this is
1878-
// called from checkSingleFileSchemaFromData, the per-source loop in
1879-
// checkComposedSources (via bodySyncDiagnostics), and fixBodySyncIn,
1880-
// so a document validated against a composed schema with N extends
1881-
// sources would otherwise re-walk the same AST N+ times per Check.
1882-
// parseSchemaWithRootFS also calls this, but on a throwaway
1877+
// per File via lint.File.MemoFile: within one Check, the document
1878+
// File sees this called once, from checkSingleFileSchemaFromData,
1879+
// when the rule has a single schema source, or once per source from
1880+
// checkComposedSources's bodySyncDiagnostics loop when the rule
1881+
// composes N>=2 extends sources — Check's dispatch on len(sources)
1882+
// makes the two paths mutually exclusive, so a document composing N
1883+
// extends sources would otherwise re-walk the same AST N times per
1884+
// Check. fixBodySyncIn (the Fix path, single-source only) calls this
1885+
// too. parseSchemaWithRootFS also calls this, but on a throwaway
18831886
// schema-content File built fresh per call, so the memo there is a
18841887
// no-op rather than a shared cache.
18851888
func extractHeadings(f *lint.File) []docHeading {

internal/rules/requiredstructure/rule_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2004,10 +2004,10 @@ func TestHeadingText_WithLink(t *testing.T) {
20042004
}
20052005

20062006
// TestExtractHeadings_Memoized pins that repeated calls on the same
2007-
// File share the same backing slice: a composed schema with N extends
2008-
// sources calls extractHeadings once per source (bodySyncDiagnostics)
2009-
// plus once from checkSingleFileSchemaFromData/fixBodySyncIn, so an
2010-
// unmemoized walk re-scans the same AST N+ times per Check.
2007+
// File share the same backing slice: a schema composing N>=2 extends
2008+
// sources calls extractHeadings once per source from
2009+
// checkComposedSources's bodySyncDiagnostics loop, so an unmemoized
2010+
// walk would re-scan the same AST N times within one Check.
20112011
func TestExtractHeadings_Memoized(t *testing.T) {
20122012
f := newTestFile(t, "doc.md", "# H1\n\n## H2\n\n### H3\n")
20132013
h1 := extractHeadings(f)

0 commit comments

Comments
 (0)