Skip to content

Commit eb12af4

Browse files
committed
fix: add { to glob-char guards and use single-pass template replacer
Three correctness fixes found during round-3 code review: 1. validatePathEntry (allowGlob=false) was missing { from the rejected glob metacharacter set. The comment already documented { as forbidden for outputs; the code now enforces it consistently with *, ?, and [. 2. isGlobPattern in locate.go used "*?[" — missing { for brace expansion ({a,b}) which doublestar supports. Misclassifying a brace-glob input as a literal path caused LSP go-to-definition to navigate to a nonexistent file. Now matches hasGlobMeta in linkgraph/directives.go. 3. generateBody used two sequential strings.ReplaceAll calls. If an output path contained {alt}, the first pass would introduce {alt} into the body and the second pass would expand it again. Replaced with strings.NewReplacer (single-pass, immune to this). https://claude.ai/code/session_0166NMrRbK1uDGk847FENqZ9
1 parent 0a514c1 commit eb12af4

4 files changed

Lines changed: 15 additions & 5 deletions

File tree

internal/index/locate.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,7 @@ var piArgRE = regexp.MustCompile(`^\s*([A-Za-z_][A-Za-z0-9_-]*)\s*:\s*(.*?)\s*$`
485485
var piListItemRE = regexp.MustCompile(`^\s*-\s+(.*?)\s*$`)
486486

487487
// isGlobPattern reports whether p contains doublestar glob metacharacters.
488-
func isGlobPattern(p string) bool { return strings.ContainsAny(p, "*?[") }
488+
func isGlobPattern(p string) bool { return strings.ContainsAny(p, "*?[{") }
489489

490490
// headingOnLine returns the heading whose first source line equals
491491
// line, or nil.

internal/index/locate_coverage_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,3 +270,14 @@ func TestLocateBuildDirectiveInputsLiteral(t *testing.T) {
270270
assert.Equal(t, "inputs", res.DirectiveArg)
271271
assert.Equal(t, "src.svg", res.DirectiveTargetFile)
272272
}
273+
274+
// TestLocateBuildDirectiveInputsBraceGlob: brace-expansion inputs must not set DirectiveTargetFile.
275+
func TestLocateBuildDirectiveInputsBraceGlob(t *testing.T) {
276+
t.Parallel()
277+
src := "# T\n\n<?build\nrecipe: render\noutputs:\n - \"out.png\"\ninputs:\n - \"{a,b}.md\"\n?>\n<?/build?>\n"
278+
res := Locator{Path: "a.md"}.Locate([]byte(src), 8, 5)
279+
assert.Equal(t, TokenDirectiveArg, res.Tag)
280+
assert.Equal(t, "build", res.DirectiveName)
281+
assert.Equal(t, "inputs", res.DirectiveArg)
282+
assert.Empty(t, res.DirectiveTargetFile, "brace-expansion input must not set DirectiveTargetFile")
283+
}

internal/rules/build/rule.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -296,8 +296,7 @@ func (r *Rule) generateBody(
296296
rendered := make([]string, 0, len(outputs))
297297
for _, output := range outputs {
298298
alt := fmt.Sprintf("%s output: %s", recipeName, output)
299-
body := strings.ReplaceAll(tmpl, "{output}", output)
300-
body = strings.ReplaceAll(body, "{alt}", alt)
299+
body := strings.NewReplacer("{output}", output, "{alt}", alt).Replace(tmpl)
301300
rendered = append(rendered, body)
302301
}
303302
body := strings.Join(rendered, "\n")
@@ -370,7 +369,7 @@ func validatePathEntry(p string, allowGlob bool) string {
370369
if strings.HasPrefix(p, "/") || strings.HasPrefix(p, "~") {
371370
return "must be a relative path"
372371
}
373-
if !allowGlob && strings.ContainsAny(p, "*?[") {
372+
if !allowGlob && strings.ContainsAny(p, "*?[{") {
374373
return "must not contain glob characters"
375374
}
376375
cleaned := path.Clean(p)

internal/rules/build/rule_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,7 @@ func TestValidatePathEntry_UnderMdsmithDir(t *testing.T) {
753753
}
754754

755755
func TestValidatePathEntry_OutputsRejectGlobChars(t *testing.T) {
756-
for _, p := range []string{"out*.png", "out?.png", "out[1].png"} {
756+
for _, p := range []string{"out*.png", "out?.png", "out[1].png", "out{a,b}.png"} {
757757
assert.NotEmpty(t, validatePathEntry(p, false), "path %q should be rejected for outputs", p)
758758
}
759759
}

0 commit comments

Comments
 (0)