Skip to content

Commit aa9ba84

Browse files
authored
fix(tools): grep stops where a file's line breaks stop (#1160)
* fix(tools): grep stops where a file's line breaks stop scanGrepFile treated EOF after a trailing line break - and an empty file - as the START of another line, and handed the matcher an empty line one number past the real content. Every pattern that matches an empty string then reported a match on a line no reader sees: a.txt holds "x\n"; grep pattern "^$" -> a.txt:2: (grep -c '' GNU-style reports no such line) same file, pattern ".*", count mode -> "2 matches found" (one real line, one phantom) an empty file, pattern "^$" -> a.txt:1: (the file has no lines at all) read_file and the file tracker's countLines already state the rule the scan violated: "the trailing newline does not open a line", and an empty file has no lines. The scan now stops there too, so anchored and empty-matching patterns can only match the lines the content carries. Real matches, numbering, count-per-hit accounting for non-empty patterns, file mode, glob and symlink handling are unchanged. * test(tools): make the grep line-number parity test catch the phantom The parity test matched only "needle", a pattern that cannot match the empty line the old EOF branch invented, so its absence-of-line-4 assertion passed on the code being fixed. The pattern now also matches an empty line, and the expected output is asserted exactly: on the pre-fix scanner the same run reported grep = "a.txt:2: needle two\na.txt:4: ", want exactly the line read_file numbers 2 so the phantom is visible right beside the real line, and the test is about the numbering claim it states. Also gofmt-aligns the case table that made the CI formatting check fail.
1 parent a97db88 commit aa9ba84

2 files changed

Lines changed: 89 additions & 7 deletions

File tree

‎internal/tools/grep.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -387,24 +387,24 @@ func scanGrepFile(ctx context.Context, resolvedRoot string, absolutePaths bool,
387387

388388
reader := bufio.NewReader(handle)
389389
lineNumber := 1
390-
sawLine := false
391-
lastEnded := false
392390
for {
393391
if err := ctx.Err(); err != nil {
394392
return err
395393
}
396394
raw, ended, err := readRawLine(reader)
395+
// At EOF the scan stops where the line breaks stopped. A trailing
396+
// line break is the boundary that CLOSED the line it ends — the rule
397+
// the file tracker's countLines states for everything a reader
398+
// reports, and an empty file has no lines at all. Emitting at EOF
399+
// anyway handed every pattern that matches an empty string (anchors
400+
// among them: ^$, \b, x*) a phantom match on a line that does not
401+
// exist, numbered one past the real content.
397402
if err == io.EOF {
398-
if !sawLine || lastEnded {
399-
return emitGrepLine(matcher, fileLabel, lineNumber, nil, emit)
400-
}
401403
return nil
402404
}
403405
if err != nil {
404406
return nil
405407
}
406-
sawLine = true
407-
lastEnded = ended
408408
line := trimTrailingCarriageReturns(trimLineBreak(raw, ended))
409409
if err := emitGrepLine(matcher, fileLabel, lineNumber, line, emit); err != nil {
410410
return err
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package tools
2+
3+
import (
4+
"context"
5+
"path/filepath"
6+
"strings"
7+
"testing"
8+
)
9+
10+
// A LINE BREAK ENDS THE LINE IT TRAILS; IT DOES NOT OPEN THE ONE AFTER IT.
11+
//
12+
// The scan used to hand the matcher an empty line past the file's real content
13+
// at EOF, so every pattern that matches an empty string — the anchored ones ^$
14+
// and \b among them — reported a match on a line no reader sees, numbered one
15+
// past the real content, and count mode charged that phantom to its total.
16+
// read_file and the file tracker's countLines both report "x\n" as one line
17+
// and an empty file as none; the grep scan now stops with them.
18+
func TestGrepStopsAtTheLineBreaksTheFileHas(t *testing.T) {
19+
for name, testCase := range map[string]struct {
20+
content string
21+
pattern string
22+
want string
23+
}{
24+
"a trailing newline closes the line": {"x\n", `^$`, "content:No matches found."},
25+
"an unterminated last line stands alone": {"x", `^$`, "content:No matches found."},
26+
"an empty file has no lines": {"", `^$`, "content:No matches found."},
27+
"the middle empty line is the only empty one": {"x\n\nz\n", `^$`, "content:a.txt:2: "},
28+
"dot star does not run past the break": {"x\n", ".*", "content:a.txt:1: x"},
29+
"count stops at the trailing break": {"x\n", `^$`, "count:0 matches found"},
30+
"count charges only real lines": {"x\n\nz\n", `^$`, "count:1 matches found"},
31+
"count takes the trailing empty lines": {"z\n\n\n", `^$`, "count:2 matches found"},
32+
"an empty file counts nothing": {"", `^$`, "count:0 matches found"},
33+
} {
34+
t.Run(name, func(t *testing.T) {
35+
root := t.TempDir()
36+
writeTestFile(t, filepath.Join(root, "a.txt"), testCase.content)
37+
38+
mode, want, found := strings.Cut(testCase.want, ":")
39+
if !found {
40+
t.Fatalf("SETUP INVALID: %s carries no mode", testCase.want)
41+
}
42+
43+
result := NewScopedGrepTool(root, nil).Run(context.Background(), map[string]any{
44+
"pattern": testCase.pattern,
45+
"output_mode": mode,
46+
})
47+
if result.Status != StatusOK {
48+
t.Fatalf("grep status = %s: %s", result.Status, result.Output)
49+
}
50+
if result.Output != want {
51+
t.Fatalf("grep = %q, want %q", result.Output, want)
52+
}
53+
})
54+
}
55+
}
56+
57+
// THE LINE NUMBERS THE SCAN REPORTS ARE THE LINE NUMBERS read_file USES, and
58+
// neither of them runs past the breaks the content carries. The pattern also
59+
// matches an empty line, so the line the scan used to invent at EOF would
60+
// appear right beside the real one: numbering a real line wrong in the same
61+
// file would fail the read side first, and the phantom would fail as a match
62+
// the content does not carry.
63+
func TestGrepLineNumbersAgreeWithReadFile(t *testing.T) {
64+
root := t.TempDir()
65+
writeTestFile(t, filepath.Join(root, "a.txt"), "one\nneedle two\nthree\n")
66+
67+
read := NewScopedReadFileTool(root, nil).Run(context.Background(), map[string]any{
68+
"path": "a.txt",
69+
})
70+
if read.Status != StatusOK || !strings.Contains(read.Output, "2→needle two") {
71+
t.Fatalf("SETUP INVALID: read_file does not number the needle line 2: %s", read.Output)
72+
}
73+
result := NewScopedGrepTool(root, nil).Run(context.Background(), map[string]any{
74+
"pattern": "needle|^$",
75+
})
76+
if result.Status != StatusOK {
77+
t.Fatalf("grep status = %s: %s", result.Status, result.Output)
78+
}
79+
if result.Output != "a.txt:2: needle two" {
80+
t.Fatalf("grep = %q, want exactly the line read_file numbers 2; an extra empty match is a line the content does not have, so the scan opened one past the trailing break", result.Output)
81+
}
82+
}

0 commit comments

Comments
 (0)