caddyfile: Formatter enhancements

2 files changed, 312 insertions, 167 deletions

diff --git a/caddyconfig/caddyfile/formatter.go b/caddyconfig/caddyfile/formatter.go
index 82b8b3c..bf70d11 100644
--- a/caddyconfig/caddyfile/formatter.go
+++ b/caddyconfig/caddyfile/formatter.go
@@ -20,129 +20,194 @@ import (
 	"unicode"
 )
 
-// Format formats a Caddyfile to conventional standards.
-func Format(body []byte) []byte {
-	reader := bytes.NewReader(body)
-	result := new(bytes.Buffer)
+// Format formats the input Caddyfile to a standard, nice-looking
+// appearance. It works by reading each rune of the input and taking
+// control over all the bracing and whitespace that is written; otherwise,
+// words, comments, placeholders, and escaped characters are all treated
+// literally and written as they appear in the input.
+func Format(input []byte) []byte {
+	input = bytes.TrimSpace(input)
+
+	out := new(bytes.Buffer)
+	rdr := bytes.NewReader(input)
 
 	var (
-		commented,
-		quoted,
-		escaped,
-		environ,
-		lineBegin bool
+		last rune // the last character that was written to the result
+
+		space           = true // whether current/previous character was whitespace (beginning of input counts as space)
+		beginningOfLine = true // whether we are at beginning of line
 
-		firstIteration = true
+		openBrace        bool // whether current word/token is or started with open curly brace
+		openBraceWritten bool // if openBrace, whether that brace was written or not
 
-		indentation = 0
+		newLines int // count of newlines consumed
 
-		prev,
-		curr,
-		next rune
+		comment bool // whether we're in a comment
+		quoted  bool // whether we're in a quoted segment
+		escaped bool // whether current char is escaped
 
-		err error
+		nesting int // indentation level
 	)
 
-	insertTabs := func(num int) {
-		for tabs := num; tabs > 0; tabs-- {
-			result.WriteRune('\t')
-		}
+	write := func(ch rune) {
+		out.WriteRune(ch)
+		last = ch
 	}
 
-	for {
-		prev = curr
-		curr = next
-
-		if curr < 0 {
-			break
+	indent := func() {
+		for tabs := nesting; tabs > 0; tabs-- {
+			write('\t')
 		}
+	}
 
-		next, _, err = reader.ReadRune()
+	nextLine := func() {
+		write('\n')
+		beginningOfLine = true
+	}
+
+	for {
+		ch, _, err := rdr.ReadRune()
 		if err != nil {
 			if err == io.EOF {
-				next = -1
+				break
+			}
+			panic(err)
+		}
+
+		if comment {
+			if ch == '\n' {
+				comment = false
 			} else {
-				panic(err)
+				write(ch)
+				continue
+			}
+		}
+
+		if !escaped && ch == '\\' {
+			if space {
+				write(' ')
+				space = false
 			}
+			write(ch)
+			escaped = true
+			continue
 		}
 
-		if firstIteration {
-			firstIteration = false
-			lineBegin = true
+		if escaped {
+			write(ch)
+			escaped = false
 			continue
 		}
 
 		if quoted {
-			if escaped {
-				escaped = false
-			} else {
-				if curr == '\\' {
-					escaped = true
-				}
-				if curr == '"' {
-					quoted = false
-				}
-			}
-			if curr == '\n' {
+			if ch == '"' {
 				quoted = false
 			}
-		} else if commented {
-			if curr == '\n' {
-				commented = false
+			write(ch)
+			continue
+		}
+
+		if space && ch == '"' {
+			quoted = true
+		}
+
+		if unicode.IsSpace(ch) {
+			space = true
+			if ch == '\n' {
+				newLines++
 			}
-		} else {
-			if curr == '"' {
-				quoted = true
+			continue
+		}
+		spacePrior := space
+		space = false
+
+		//////////////////////////////////////////////////////////
+		// I find it helpful to think of the formatting loop in two
+		// main sections; by the time we reach this point, we
+		// know we are in a "regular" part of the file: we know
+		// the character is not a space, not in a literal segment
+		// like a comment or quoted, it's not escaped, etc.
+		//////////////////////////////////////////////////////////
+
+		if ch == '#' {
+			if !spacePrior && !beginningOfLine {
+				write(' ')
 			}
-			if curr == '#' {
-				commented = true
+			comment = true
+		}
+
+		if openBrace && spacePrior && !openBraceWritten {
+			if nesting == 0 && last == '}' {
+				nextLine()
+				nextLine()
 			}
-			if curr == '}' {
-				if environ {
-					environ = false
-				} else if indentation > 0 {
-					indentation--
-				}
+
+			openBrace = false
+			if beginningOfLine {
+				indent()
+			} else {
+				write(' ')
 			}
-			if curr == '{' {
-				if unicode.IsSpace(next) {
-					indentation++
-
-					if !unicode.IsSpace(prev) && !lineBegin {
-						result.WriteRune(' ')
-					}
-				} else {
-					environ = true
-				}
+			write('{')
+			nextLine()
+			newLines = 0
+			nesting++
+		}
+
+		switch {
+		case ch == '{':
+			openBrace = true
+			openBraceWritten = false
+			continue
+
+		case ch == '}' && (spacePrior || !openBrace):
+			if last != '\n' {
+				nextLine()
 			}
-			if lineBegin {
-				if curr == ' ' || curr == '\t' {
-					continue
-				} else {
-					lineBegin = false
-					if curr == '{' && unicode.IsSpace(next) {
-						// If the block is global, i.e., starts with '{'
-						// One less indentation for these blocks.
-						insertTabs(indentation - 1)
-					} else {
-						insertTabs(indentation)
-					}
-				}
-			} else {
-				if prev == '{' &&
-					(curr == ' ' || curr == '\t') &&
-					(next != '\n' && next != '\r') {
-					curr = '\n'
-				}
+			if nesting > 0 {
+				nesting--
 			}
+			indent()
+			write('}')
+			newLines = 0
+			continue
 		}
 
-		if curr == '\n' {
-			lineBegin = true
+		if newLines > 2 {
+			newLines = 2
+		}
+		for i := 0; i < newLines; i++ {
+			nextLine()
+		}
+		newLines = 0
+		if beginningOfLine {
+			indent()
+		}
+		if nesting == 0 && last == '}' {
+			nextLine()
+			nextLine()
 		}
 
-		result.WriteRune(curr)
+		if !beginningOfLine && spacePrior {
+			write(' ')
+		}
+
+		if openBrace && !openBraceWritten {
+			if !beginningOfLine {
+				write(' ')
+			}
+			write('{')
+			openBraceWritten = true
+		}
+		write(ch)
+
+		beginningOfLine = false
 	}
 
-	return result.Bytes()
+	// the Caddyfile does not need any leading or trailing spaces, but...
+	trimmedResult := bytes.TrimSpace(out.Bytes())
+
+	// ...Caddyfiles should, however, end with a newline because
+	// newlines are significant to the syntax of the file
+	return append(trimmedResult, '\n')
 }
diff --git a/caddyconfig/caddyfile/formatter_test.go b/caddyconfig/caddyfile/formatter_test.go
index 2aa6d27..8f2a012 100644
--- a/caddyconfig/caddyfile/formatter_test.go
+++ b/caddyconfig/caddyfile/formatter_test.go
@@ -15,12 +15,28 @@
 package caddyfile
 
 import (
+	"strings"
 	"testing"
 )
 
-func TestFormatBasicIndentation(t *testing.T) {
-	input := []byte(`
-  a
+func TestFormatter(t *testing.T) {
+	for i, tc := range []struct {
+		description string
+		input       string
+		expect      string
+	}{
+		{
+			description: "very simple",
+			input: `abc   def
+	g hi jkl
+mn`,
+			expect: `abc def
+g hi jkl
+mn`,
+		},
+		{
+			description: "basic indentation, line breaks, and nesting",
+			input: `  a
 b
 
 	c {
@@ -30,6 +46,8 @@ b
 e { f
 }
 
+
+
 g {
 h {
 i
@@ -44,22 +62,20 @@ l
 m {
 	n { o
 	}
+	p { q r
+s }
 }
 
-{
-	p
-}
+	{
+{ t
+		u
 
-  { q
-}
+	v
 
-	{
-{ r
-}
+w
 }
-`)
-	expected := []byte(`
-a
+}`,
+			expect: `a
 b
 
 c {
@@ -86,49 +102,58 @@ m {
 	n {
 		o
 	}
+	p {
+		q r
+		s
+	}
 }
 
 {
-	p
-}
+	{
+		t
+		u
 
-{
-	q
-}
+		v
 
-{
-	{
-		r
+		w
 	}
-}
-`)
-	testFormat(t, input, expected)
-}
-
-func TestFormatBasicSpacing(t *testing.T) {
-	input := []byte(`
-a{
+}`,
+		},
+		{
+			description: "block spacing",
+			input: `a{
 	b
 }
 
 c{ d
-}
-`)
-	expected := []byte(`
-a {
+}`,
+			expect: `a {
 	b
 }
 
 c {
 	d
-}
-`)
-	testFormat(t, input, expected)
-}
-
-func TestFormatEnvironmentVariable(t *testing.T) {
-	input := []byte(`
-{$A}
+}`,
+		},
+		{
+			description: "advanced spacing",
+			input: `abc {
+	def
+}ghi{
+	jkl mno
+pqr}`,
+			expect: `abc {
+	def
+}
+
+ghi {
+	jkl mno
+	pqr
+}`,
+		},
+		{
+			description: "env var placeholders",
+			input: `{$A}
 
 b {
 {$C}
@@ -139,9 +164,8 @@ d { {$E}
 
 { {$F}
 }
-`)
-	expected := []byte(`
-{$A}
+`,
+			expect: `{$A}
 
 b {
 	{$C}
@@ -153,49 +177,41 @@ d {
 
 {
 	{$F}
-}
-`)
-	testFormat(t, input, expected)
-}
-
-func TestFormatComments(t *testing.T) {
-	input := []byte(`
-# a "\n"
+}`,
+		},
+		{
+			description: "comments",
+			input: `#a "\n"
 
-# b {
+ #b {
 	c
 }
 
 d {
-e # f
+e#f
 # g
 }
 
 h { # i
-}
-`)
-	expected := []byte(`
-# a "\n"
+}`,
+			expect: `#a "\n"
 
-# b {
+#b {
 c
 }
 
 d {
-	e # f
+	e #f
 	# g
 }
 
 h {
 	# i
-}
-`)
-	testFormat(t, input, expected)
-}
-
-func TestFormatQuotesAndEscapes(t *testing.T) {
-	input := []byte(`
-"a \"b\" #c
+}`,
+		},
+		{
+			description: "quotes and escaping",
+			input: `"a \"b\" "#c
 	d
 
 e {
@@ -204,9 +220,16 @@ e {
 
 g { "h"
 }
-`)
-	expected := []byte(`
-"a \"b\" #c
+
+i {
+	"foo
+bar"
+}
+
+j {
+"\"k\" l m"
+}`,
+			expect: `"a \"b\" " #c
 d
 
 e {
@@ -216,13 +239,70 @@ e {
 g {
 	"h"
 }
-`)
-	testFormat(t, input, expected)
+
+i {
+	"foo
+bar"
 }
 
-func testFormat(t *testing.T, input, expected []byte) {
-	output := Format(input)
-	if string(output) != string(expected) {
-		t.Errorf("Expected:\n%s\ngot:\n%s", string(expected), string(output))
+j {
+	"\"k\" l m"
+}`,
+		},
+		{
+			description: "bad nesting (too many open)",
+			input: `a
+{
+	{
+}`,
+			expect: `a {
+	{
+	}
+`,
+		},
+		{
+			description: "bad nesting (too many close)",
+			input: `a
+{
+	{
+}}}`,
+			expect: `a {
+	{
+	}
+}
+}
+`,
+		},
+		{
+			description: "json",
+			input: `foo
+bar      "{\"key\":34}"
+`,
+			expect: `foo
+bar "{\"key\":34}"`,
+		},
+		{
+			description: "escaping after spaces",
+			input:       `foo \"literal\"`,
+			expect:      `foo \"literal\"`,
+		},
+		{
+			description: "simple placeholders",
+			input:       `foo {bar}`,
+			expect:      `foo {bar}`,
+		},
+	} {
+		// the formatter should output a trailing newline,
+		// even if the tests aren't written to expect that
+		if !strings.HasSuffix(tc.expect, "\n") {
+			tc.expect += "\n"
+		}
+
+		actual := Format([]byte(tc.input))
+
+		if string(actual) != tc.expect {
+			t.Errorf("\n[TEST %d: %s]\n====== EXPECTED ======\n%s\n====== ACTUAL ======\n%s^^^^^^^^^^^^^^^^^^^^^",
+				i, tc.description, string(tc.expect), string(actual))
+		}
 	}
 }