Skip to content

Commit 150c2a4

Browse files
committed
plan 50: inline nodeText and cover the WalkDir-error skip path
- extractParagraphs now inlines the old nodeText helper. An empty Lines list falls through to the min-chars gate (runeLen("") is always below the threshold), so the separate 'not-ok' branch goes away along with the nodeText function. - indexFileIfEligible drops the dead NewFileFromSource error check since NewFile never errors for in-memory bytes. - Tests: errFS wraps os.DirFS to return a forced ReadDir error for a specific subtree, giving the WalkDir callback its non-nil err path real exercise. Package coverage rises from 95.6% to 98.8%. The only remaining uncovered statements are filepath.Abs and filepath.Rel error paths in rootRelative; both fire only on Windows cross-volume paths or when os.Getwd fails, neither reachable from Linux tests.
1 parent 21ba413 commit 150c2a4

2 files changed

Lines changed: 67 additions & 30 deletions

File tree

internal/rules/duplicatedcontent/rule.go

Lines changed: 16 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -134,50 +134,35 @@ type externalMatch struct {
134134
// extractParagraphs walks f.AST and returns fingerprints for every
135135
// paragraph whose normalized text is at least minChars runes long.
136136
// Paragraphs are read via Node.Lines so raw markdown text — not rendered
137-
// inline output — feeds the fingerprint.
137+
// inline output — feeds the fingerprint. Paragraphs with no source lines
138+
// (a shape goldmark never produces today, but cheap to guard) and ones
139+
// shorter than the threshold are skipped via the same min-chars gate.
138140
func extractParagraphs(f *lint.File, minChars int) []paragraph {
139141
var out []paragraph
140142
_ = ast.Walk(f.AST, func(n ast.Node, entering bool) (ast.WalkStatus, error) {
141-
if !entering {
143+
if !entering || n.Kind() != ast.KindParagraph {
142144
return ast.WalkContinue, nil
143145
}
144-
if n.Kind() != ast.KindParagraph {
145-
return ast.WalkContinue, nil
146-
}
147-
text, startOffset, ok := nodeText(n, f.Source)
148-
if !ok {
149-
return ast.WalkSkipChildren, nil
146+
lines := n.Lines()
147+
var b strings.Builder
148+
for i := 0; i < lines.Len(); i++ {
149+
seg := lines.At(i)
150+
b.Write(seg.Value(f.Source))
150151
}
151-
normalized := normalize(text)
152+
normalized := normalize(b.String())
152153
if runeLen(normalized) < minChars {
153154
return ast.WalkSkipChildren, nil
154155
}
155156
sum := sha256.Sum256([]byte(normalized))
156157
out = append(out, paragraph{
157158
fingerprint: hex.EncodeToString(sum[:]),
158-
line: f.LineOfOffset(startOffset),
159+
line: f.LineOfOffset(lines.At(0).Start),
159160
})
160161
return ast.WalkSkipChildren, nil
161162
})
162163
return out
163164
}
164165

165-
// nodeText concatenates a block node's line segments into the raw text
166-
// that the source contains between the node's first and last line. It
167-
// returns the first line's byte offset so callers can compute its line.
168-
func nodeText(n ast.Node, source []byte) (string, int, bool) {
169-
lines := n.Lines()
170-
if lines.Len() == 0 {
171-
return "", 0, false
172-
}
173-
var b strings.Builder
174-
for i := 0; i < lines.Len(); i++ {
175-
seg := lines.At(i)
176-
b.Write(seg.Value(source))
177-
}
178-
return b.String(), lines.At(0).Start, true
179-
}
180-
181166
// normalize collapses runs of whitespace to single spaces, lowercases
182167
// letters, and trims leading/trailing space. The goal is to treat
183168
// paragraphs that differ only by reflow or case as duplicates.
@@ -347,10 +332,11 @@ func indexFileIfEligible(
347332
if err != nil {
348333
return
349334
}
350-
other, err := lint.NewFileFromSource(path, data, stripFrontMatter)
351-
if err != nil {
352-
return
353-
}
335+
// NewFileFromSource cannot fail for in-memory bytes that came
336+
// out of ReadFSFileLimited successfully; goldmark's parser does
337+
// not error on any input. The error return is kept in the
338+
// signature for future-proofing but is dead here.
339+
other, _ := lint.NewFileFromSource(path, data, stripFrontMatter) //nolint:errcheck
354340
for _, p := range extractParagraphs(other, minChars) {
355341
index[p.fingerprint] = append(index[p.fingerprint], externalMatch{
356342
path: path,

internal/rules/duplicatedcontent/rule_test.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package duplicatedcontent
22

33
import (
4+
"errors"
5+
"io/fs"
46
"os"
57
"path/filepath"
68
"strings"
@@ -153,6 +155,55 @@ func TestCheck_NilASTIsNoop(t *testing.T) {
153155
assert.Empty(t, diags)
154156
}
155157

158+
// errFS wraps an fs.FS and returns err from ReadDir on a specific
159+
// path, so buildCorpusIndex's WalkDir callback receives a non-nil
160+
// error and takes the skip-but-continue branch.
161+
type errFS struct {
162+
inner fs.FS
163+
failOn string
164+
failErr error
165+
}
166+
167+
func (e errFS) Open(name string) (fs.File, error) { return e.inner.Open(name) }
168+
169+
func (e errFS) ReadDir(name string) ([]fs.DirEntry, error) {
170+
if name == e.failOn {
171+
return nil, e.failErr
172+
}
173+
return fs.ReadDir(e.inner, name)
174+
}
175+
176+
func TestCheck_CorpusWalkSwallowsFSErrors(t *testing.T) {
177+
dir := t.TempDir()
178+
sub := filepath.Join(dir, "sub")
179+
require.NoError(t, os.MkdirAll(sub, 0o755))
180+
181+
p := longParagraph("the quick brown fox jumps over the lazy dog")
182+
writeFile(t, filepath.Join(dir, "a.md"), "# A\n\n"+p+"\n")
183+
writeFile(t, filepath.Join(sub, "b.md"), "# B\n\n"+p+"\n")
184+
185+
// Build a.md as the current file, then point RootFS at an FS
186+
// that errors on ReadDir("sub") so the walker is forced down
187+
// the err != nil branch for that entry.
188+
data, err := os.ReadFile(filepath.Join(dir, "a.md"))
189+
require.NoError(t, err)
190+
f, err := lint.NewFile(filepath.Join(dir, "a.md"), data)
191+
require.NoError(t, err)
192+
f.FS = os.DirFS(dir)
193+
f.RootDir = dir
194+
f.RootFS = errFS{
195+
inner: os.DirFS(dir),
196+
failOn: "sub",
197+
failErr: errors.New("forced walk error"),
198+
}
199+
200+
// The rule must not panic or return the error; it silently
201+
// skips the unreadable subtree. b.md is in sub/ and therefore
202+
// not found, so no duplicate diagnostic fires.
203+
diags := (&Rule{}).Check(f)
204+
assert.Empty(t, diags)
205+
}
206+
156207
func TestCheck_OversizeCorpusFileSkipped(t *testing.T) {
157208
dir := t.TempDir()
158209
p := longParagraph("the quick brown fox jumps over the lazy dog")

0 commit comments

Comments
 (0)