Skip to content

Commit 7ebbc9d

Browse files
committed
fix: lint issues and mixed quote escaping in fuzzy edit matching
Fix 5 lint issues from CI: - unused whitespaceRE variable - strings.Index → strings.Cut - classic for loops → integer range (Go 1.22+) - len(s) == 0 → s == "" (gocritic) Fix mixed escaping bug: when a file has both escaped and unescaped quotes (e.g. "${DIR}" with plain quotes and echo \"hello\" with escaped ones), the global strings.ReplaceAll approach fails. Add a regex fallback (Case 3) that matches each " as optionally preceded by a backslash.
1 parent 9405568 commit 7ebbc9d

2 files changed

Lines changed: 56 additions & 17 deletions

File tree

pkg/tools/builtin/filesystem.go

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -530,11 +530,11 @@ func FindAndReplace(content, oldText, newText string) (string, bool) {
530530

531531
for _, replacer := range replacers {
532532
for _, candidate := range replacer(content, oldText) {
533-
idx := strings.Index(content, candidate)
534-
if idx == -1 {
533+
before, after, found := strings.Cut(content, candidate)
534+
if !found {
535535
continue
536536
}
537-
return content[:idx] + newText + content[idx+len(candidate):], true
537+
return before + newText + after, true
538538
}
539539
}
540540

@@ -558,9 +558,9 @@ func lineTrimmedCandidates(content, find string) []string {
558558
}
559559

560560
var candidates []string
561-
for i := 0; i <= len(contentLines)-len(searchLines); i++ {
561+
for i := range len(contentLines) - len(searchLines) + 1 {
562562
match := true
563-
for j := 0; j < len(searchLines); j++ {
563+
for j := range len(searchLines) {
564564
if strings.TrimSpace(contentLines[i+j]) != strings.TrimSpace(searchLines[j]) {
565565
match = false
566566
break
@@ -586,7 +586,7 @@ func indentFlexibleCandidates(content, find string) []string {
586586
normalizedFind := stripCommonIndent(searchLines)
587587

588588
var candidates []string
589-
for i := 0; i <= len(contentLines)-len(searchLines); i++ {
589+
for i := range len(contentLines) - len(searchLines) + 1 {
590590
block := contentLines[i : i+len(searchLines)]
591591
if stripCommonIndent(block) == normalizedFind {
592592
candidates = append(candidates, joinOriginalLines(contentLines, i, len(searchLines)))
@@ -601,7 +601,7 @@ func stripCommonIndent(lines []string) string {
601601
minIndent := -1
602602
for _, line := range lines {
603603
trimmed := strings.TrimLeft(line, " \t")
604-
if len(trimmed) == 0 {
604+
if trimmed == "" {
605605
continue
606606
}
607607
indent := len(line) - len(trimmed)
@@ -614,7 +614,7 @@ func stripCommonIndent(lines []string) string {
614614
}
615615
out := make([]string, len(lines))
616616
for i, line := range lines {
617-
if len(strings.TrimSpace(line)) == 0 {
617+
if strings.TrimSpace(line) == "" {
618618
out[i] = line
619619
} else if minIndent < len(line) {
620620
out[i] = line[minIndent:]
@@ -645,35 +645,58 @@ func lineContinuationCandidates(content, find string) []string {
645645
return flexibleWhitespaceMatch(content, find, `(?:\s*\\\s*\n\s*|\s+)`)
646646
}
647647

648-
// escapeNormalizedCandidates handles the case where the LLM dropped
649-
// backslash escapes (e.g., sent "hello" when the file has \"hello\").
650-
// It tries two approaches:
651-
// 1. Unescape \" → " in the search text and look for it directly.
652-
// 2. Escape " → \" in the search text and look for it directly.
648+
// escapeNormalizedCandidates handles the case where the LLM dropped or
649+
// added backslash escapes on double quotes. It handles three scenarios:
650+
// 1. All quotes need escaping (" → \")
651+
// 2. All escaped quotes need unescaping (\" → ")
652+
// 3. Mixed: some quotes are escaped in the file and some aren't —
653+
// builds a regex where each " can optionally be preceded by \.
653654
func escapeNormalizedCandidates(content, find string) []string {
655+
if !strings.Contains(find, `"`) && !strings.Contains(find, `\"`) {
656+
return nil
657+
}
658+
654659
var candidates []string
655660

656-
// LLM sent unescaped quotes, file has escaped quotes.
661+
// Case 1: LLM sent unescaped quotes, file has escaped quotes.
657662
if strings.Contains(find, `"`) && strings.Contains(content, `\"`) {
658663
escaped := strings.ReplaceAll(find, `"`, `\"`)
659664
if strings.Contains(content, escaped) {
660665
candidates = append(candidates, escaped)
661666
}
662667
}
663668

664-
// LLM sent escaped quotes, file has unescaped quotes.
669+
// Case 2: LLM sent escaped quotes, file has unescaped quotes.
665670
if strings.Contains(find, `\"`) {
666671
unescaped := strings.ReplaceAll(find, `\"`, `"`)
667672
if strings.Contains(content, unescaped) {
668673
candidates = append(candidates, unescaped)
669674
}
670675
}
671676

677+
// Case 3: mixed escaping — file has both " and \".
678+
// Build a regex where each " matches an optional preceding \.
679+
if len(candidates) == 0 &&
680+
strings.Contains(content, `"`) && strings.Contains(content, `\"`) {
681+
normalized := strings.ReplaceAll(find, `\"`, `"`)
682+
parts := strings.Split(normalized, `"`)
683+
if len(parts) > 1 {
684+
escaped := make([]string, len(parts))
685+
for i, p := range parts {
686+
escaped[i] = regexp.QuoteMeta(p)
687+
}
688+
pattern := strings.Join(escaped, `\\?"`)
689+
if re, err := regexp.Compile(pattern); err == nil {
690+
if m := re.FindString(content); m != "" {
691+
candidates = append(candidates, m)
692+
}
693+
}
694+
}
695+
}
696+
672697
return candidates
673698
}
674699

675-
var whitespaceRE = regexp.MustCompile(`\s+`)
676-
677700
// whitespaceCollapsedCandidates matches when the only difference is
678701
// whitespace quantity. It splits find into non-whitespace tokens and
679702
// builds a regex that allows flexible whitespace between them.

pkg/tools/builtin/filesystem_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,22 @@ func TestFindAndReplace(t *testing.T) {
553553
wantOK: true,
554554
want: "REPLACED",
555555
},
556+
{
557+
name: "mixed escaped and unescaped quotes",
558+
content: `if [ ! -x "${DIR}/bin" ]; then echo \"install failed\"; exit 1; fi`,
559+
oldText: `if [ ! -x "${DIR}/bin" ]; then echo "install failed"; exit 1; fi`,
560+
newText: "REPLACED",
561+
wantOK: true,
562+
want: "REPLACED",
563+
},
564+
{
565+
name: "mixed escaping in Dockerfile brew block",
566+
content: `if [ ! -x "${BREW_DIR}/bin/brew" ]; then echo \"brew install failed\"; exit 1; fi`,
567+
oldText: `if [ ! -x "${BREW_DIR}/bin/brew" ]; then echo "brew install failed"; exit 1; fi`,
568+
newText: "REPLACED",
569+
wantOK: true,
570+
want: "REPLACED",
571+
},
556572
}
557573

558574
for _, tc := range tests {

0 commit comments

Comments
 (0)