From 8e2dd5079c27af50a3288502b3a82f683e3737fe Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Sat, 11 Apr 2026 09:17:55 +1000 Subject: [PATCH] caddyfile: Improve import/global options UX for imports before global options (#7642) * caddyfile: improve import/global options UX Keep standalone global-options braces stable in fmt when they follow import lines. Also improve validate output for imports before the global options block with a clearer error message. Add focused formatter and parser regression coverage * caddyfile: satisfy staticcheck in formatter --- caddyconfig/caddyfile/formatter.go | 40 ++++++++++++++++++++++++- caddyconfig/caddyfile/formatter_test.go | 15 ++++++++++ caddyconfig/caddyfile/parse.go | 17 +++++++++++ caddyconfig/caddyfile/parse_test.go | 30 +++++++++++++++++++ 4 files changed, 101 insertions(+), 1 deletion(-) diff --git a/caddyconfig/caddyfile/formatter.go b/caddyconfig/caddyfile/formatter.go index dfd316b16..14315a3f1 100644 --- a/caddyconfig/caddyfile/formatter.go +++ b/caddyconfig/caddyfile/formatter.go @@ -63,8 +63,33 @@ func Format(input []byte) []byte { heredocClosingMarker []rune nesting int // indentation level + + currentToken strings.Builder + currentLineFirstToken string + previousLineWasTopLevelImport bool + openBraceOwnLine bool ) + finishToken := func() { + if currentToken.Len() == 0 { + return + } + if currentLineFirstToken == "" { + currentLineFirstToken = currentToken.String() + } + currentToken.Reset() + } + + finishLine := func() { + finishToken() + if currentLineFirstToken != "" { + previousLineWasTopLevelImport = nesting == 0 && currentLineFirstToken == "import" + } else if !openBrace || !openBraceOwnLine || openBraceWritten { + previousLineWasTopLevelImport = false + } + currentLineFirstToken = "" + } + write := func(ch rune) { out.WriteRune(ch) last = ch @@ -220,9 +245,11 @@ func Format(input []byte) []byte { } if unicode.IsSpace(ch) { + finishToken() space = true heredocEscaped = false if ch == '\n' { + finishLine() newLines++ } continue @@ -249,13 +276,19 @@ func Format(input []byte) []byte { } openBrace = false - if beginningOfLine { + if openBraceOwnLine && previousLineWasTopLevelImport { + if last != '\n' { + nextLine() + } + indent() + } else if beginningOfLine { indent() } else if !openBraceSpace || !unicode.IsSpace(last) { write(' ') } write('{') openBraceWritten = true + openBraceOwnLine = false nextLine() newLines = 0 // prevent infinite nesting from ridiculous inputs (issue #4169) @@ -266,8 +299,10 @@ func Format(input []byte) []byte { switch { case ch == '{': + finishToken() openBrace = true openBraceSpace = spacePrior && !beginningOfLine + openBraceOwnLine = newLines > 0 if openBraceSpace && newLines == 0 { write(' ') } @@ -275,11 +310,13 @@ func Format(input []byte) []byte { if quotes == "`" { write('{') openBraceWritten = true + openBraceOwnLine = false continue } continue case ch == '}' && (spacePrior || !openBrace): + finishToken() if quotes == "`" { write('}') continue @@ -324,6 +361,7 @@ func Format(input []byte) []byte { space = true } + currentToken.WriteRune(ch) write(ch) beginningOfLine = false diff --git a/caddyconfig/caddyfile/formatter_test.go b/caddyconfig/caddyfile/formatter_test.go index 6ab293615..3d586f813 100644 --- a/caddyconfig/caddyfile/formatter_test.go +++ b/caddyconfig/caddyfile/formatter_test.go @@ -475,6 +475,21 @@ Hope this helps.` + "`" + ` }`, expect: "https://localhost:8953 {\n\trespond `Here are some random numbers:\n\n{{randNumeric 16}}\n\nHope this helps.`\n}", }, + { + description: "imports before global options block keep standalone brace", + input: `import ./conf.d/matcher_my_subnet.caddy +import ./conf.d/matcher_not_my_subnet.caddy +{ + order crowdsec first + order appsec after crowdsec +}`, + expect: `import ./conf.d/matcher_my_subnet.caddy +import ./conf.d/matcher_not_my_subnet.caddy +{ + order crowdsec first + order appsec after crowdsec +}`, + }, } { // the formatter should output a trailing newline, // even if the tests aren't written to expect that diff --git a/caddyconfig/caddyfile/parse.go b/caddyconfig/caddyfile/parse.go index e9f27dfbf..6a4db5bbb 100644 --- a/caddyconfig/caddyfile/parse.go +++ b/caddyconfig/caddyfile/parse.go @@ -682,11 +682,28 @@ func (p *parser) directive() error { // a opening curly brace. It does NOT advance the token. func (p *parser) openCurlyBrace() error { if p.Val() != "{" { + if p.valLooksLikeGlobalOptionsAfterImportedSnippets() { + return p.Err("global options block must appear before import directives; move the global options block to the top of the Caddyfile") + } return p.SyntaxErr("{") } return nil } +func (p *parser) valLooksLikeGlobalOptionsAfterImportedSnippets() bool { + if p.Val() != "import" || len(p.block.Keys) == 0 { + return false + } + + for _, key := range p.block.Keys { + if !strings.HasPrefix(key.Text, "(") || !strings.HasSuffix(key.Text, ")") { + return false + } + } + + return true +} + // closeCurlyBrace expects the current token to be // a closing curly brace. This acts like an assertion // because it returns an error if the token is not diff --git a/caddyconfig/caddyfile/parse_test.go b/caddyconfig/caddyfile/parse_test.go index bf149e635..516b7dfd9 100644 --- a/caddyconfig/caddyfile/parse_test.go +++ b/caddyconfig/caddyfile/parse_test.go @@ -930,6 +930,36 @@ func TestAcceptSiteImportWithBraces(t *testing.T) { } } +func TestGlobalOptionsAfterImportedSnippetsGivesHelpfulError(t *testing.T) { + tempDir := t.TempDir() + importFile1 := filepath.Join(tempDir, "matcher_snippet_1.caddy") + importFile2 := filepath.Join(tempDir, "matcher_snippet_2.caddy") + + err := os.WriteFile(importFile1, []byte(`(matcher1)`), 0o644) + if err != nil { + t.Fatalf("writing first import file: %v", err) + } + + err = os.WriteFile(importFile2, []byte(`(matcher2)`), 0o644) + if err != nil { + t.Fatalf("writing second import file: %v", err) + } + + _, err = Parse("Testfile", []byte(`import `+importFile1+` +import `+importFile2+` +{ + debug +}`)) + if err == nil { + t.Fatal("Expected an error, but got nil") + } + + expected := "global options block must appear before import directives; move the global options block to the top of the Caddyfile" + if !strings.HasPrefix(err.Error(), expected) { + t.Errorf("Expected error to start with '%s' but got '%v'", expected, err) + } +} + func testParser(input string) parser { return parser{Dispenser: NewTestDispenser(input)} }