refactor: improve void element parsing perf, fixes #886 and #857 (#887)

Author: a-h

generator/test-html/template.templ

@@ -10,5 +10,5 @@ templ render(p person) {
 	<hr noshade?={ true }/>
 	<hr optionA optionB?={ true } optionC="other" optionD?={ false }/>
 	<hr noshade/>
-	<input name="test">Text</input>
+	<input name="test"/>Text
 }

generator/test-html/template_templ.go

@@ -0,0 +1,34 @@
+package parser
+
+import (
+	_ "embed"
+	"strings"
+	"testing"
+)
+
+//go:embed benchmarktestdata/benchmark.txt
+var benchmarkTemplate string
+
+func BenchmarkParse(b *testing.B) {
+	b.ReportAllocs()
+	for i := 0; i < b.N; i++ {
+		if _, err := ParseString(benchmarkTemplate); err != nil {
+			b.Fatal(err)
+		}
+	}
+}
+
+func BenchmarkFormat(b *testing.B) {
+	b.ReportAllocs()
+	sb := new(strings.Builder)
+	for i := 0; i < b.N; i++ {
+		tf, err := ParseString(benchmarkTemplate)
+		if err != nil {
+			b.Fatal(err)
+		}
+		if err = tf.Write(sb); err != nil {
+			b.Fatal(err)
+		}
+		sb.Reset()
+	}
+}

parser/v2/benchmarks_test.go

@@ -0,0 +1,18 @@
+package benchmarktestdata
+
+templ Benchmark() {
+				<div>Hello</div>
+				<br>
+				<br>
+				<br>
+				</br>
+				<div>Lorem ipsum dolor sit amet, consectetur adipiscing elit.</div>
+				<br>
+				<br>
+				<ul>
+				<li>Item 1</li>
+				<li>Item 2</li>
+				<li>Item 3</li>
+				</ul>
+}
+

parser/v2/benchmarktestdata/benchmark.txt

@@ -2,7 +2,6 @@ package parser
 
 import (
 	"errors"
-	"fmt"
 )
 
 type diagnoser func(Node) ([]Diagnostic, error)
@@ -35,7 +34,6 @@ func walkNodes(t []Node, f func(Node) bool) {
 
 var diagnosers = []diagnoser{
 	useOfLegacyCallSyntaxDiagnoser,
-	voidElementWithChildrenDiagnoser,
 }
 
 func Diagnose(t TemplateFile) ([]Diagnostic, error) {
@@ -64,20 +62,3 @@ func useOfLegacyCallSyntaxDiagnoser(n Node) ([]Diagnostic, error) {
 	}
 	return nil, nil
 }
-
-func voidElementWithChildrenDiagnoser(n Node) (d []Diagnostic, err error) {
-	e, ok := n.(Element)
-	if !ok {
-		return
-	}
-	if !e.IsVoidElement() {
-		return
-	}
-	if len(e.Children) == 0 {
-		return
-	}
-	return []Diagnostic{{
-		Message: fmt.Sprintf("void element <%s> should not have child content", e.Name),
-		Range:   e.NameRange,
-	}}, nil
-}

parser/v2/diagnostics.go

@@ -134,21 +134,6 @@ templ template () {
 }`,
 			want: nil,
 		},
-		{
-			name: "voidElementWithChildrenDiagnoser: with diagnostics",
-			template: `
-package main
-
-templ template () {
-	<div>
-	  <input>Child content</input>
-	</div>
-}`,
-			want: []Diagnostic{{
-				Message: "void element <input> should not have child content",
-				Range:   Range{Position{46, 5, 4}, Position{51, 5, 9}},
-			}},
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

parser/v2/diagnostics_test.go

@@ -375,6 +375,34 @@ var (
 	})
 )
 
+// Void element closer.
+var voidElementCloser voidElementCloserParser
+
+type voidElementCloserParser struct{}
+
+var voidElementCloseTags = []string{"</area>", "</base>", "</br>", "</col>", "</command>", "</embed>", "</hr>", "</img>", "</input>", "</keygen>", "</link>", "</meta>", "</param>", "</source>", "</track>", "</wbr>"}
+
+func (voidElementCloserParser) Parse(pi *parse.Input) (n Node, ok bool, err error) {
+	var ve string
+	for _, ve = range voidElementCloseTags {
+		s, canPeekLen := pi.Peek(len(ve))
+		if !canPeekLen {
+			continue
+		}
+		if !strings.EqualFold(s, ve) {
+			continue
+		}
+		// Found a match.
+		ok = true
+		break
+	}
+	if !ok {
+		return nil, false, nil
+	}
+	pi.Take(len(ve))
+	return nil, true, nil
+}
+
 // Element.
 var element elementParser
 
@@ -396,28 +424,19 @@ func (elementParser) Parse(pi *parse.Input) (n Node, ok bool, err error) {
 
 	// Once we've got an open tag, the rest must be present.
 	l := pi.Position().Line
-	endOfOpenTag := pi.Index()
 
 	// If the element is self-closing, even if it's not really a void element (br, hr etc.), we can return early.
-	if ot.Void {
+	if ot.Void || r.IsVoidElement() {
 		// Escape early, no need to try to parse children for self-closing elements.
 		return addTrailingSpaceAndValidate(start, r, pi)
 	}
 
-	// Void elements _might_ have children, even though it's invalid.
-	// We want to allow this to be parsed.
+	// Parse children.
 	closer := StripType(parse.All(parse.String("</"), parse.String(ot.Name), parse.Rune('>')))
 	tnp := newTemplateNodeParser(closer, fmt.Sprintf("<%s>: close tag", ot.Name))
 	nodes, _, err := tnp.Parse(pi)
 	if err != nil {
 		notFoundErr, isNotFoundError := err.(UntilNotFoundError)
-		if r.IsVoidElement() && isNotFoundError {
-			// Void elements shouldn't have children, or a close tag, so this is expected.
-			// When the template is reformatted, we won't reach here, because <hr> will be converted to <hr/>.
-			// Return the element as we have it.
-			pi.Seek(endOfOpenTag)
-			return addTrailingSpaceAndValidate(start, r, pi)
-		}
 		if isNotFoundError {
 			err = notFoundErr.ParseError
 		}
@@ -443,6 +462,10 @@ func (elementParser) Parse(pi *parse.Input) (n Node, ok bool, err error) {
 }
 
 func addTrailingSpaceAndValidate(start parse.Position, e Element, pi *parse.Input) (n Node, ok bool, err error) {
+	// Elide any void close tags.
+	if _, _, err = voidElementCloser.Parse(pi); err != nil {
+		return e, false, err
+	}
 	// Add trailing space.
 	ws, _, err := parse.Whitespace.Parse(pi)
 	if err != nil {

parser/v2/elementparser.go

@@ -550,6 +550,20 @@ if test {
 	}
 }
 
+func TestVoidElementCloserParser(t *testing.T) {
+	t.Run("all void elements are parsed", func(t *testing.T) {
+		for _, input := range voidElementCloseTags {
+			_, ok, err := voidElementCloser.Parse(parse.NewInput(input))
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if !ok {
+				t.Fatalf("failed to parse %q", input)
+			}
+		}
+	})
+}
+
 func TestElementParser(t *testing.T) {
 	tests := []struct {
 		name     string
@@ -662,15 +676,9 @@ func TestElementParser(t *testing.T) {
 					From: Position{Index: 1, Line: 0, Col: 1},
 					To:   Position{Index: 6, Line: 0, Col: 6},
 				},
-				Children: []Node{
-					Text{
-						Value: "Text",
-						Range: Range{
-							From: Position{Index: 7, Line: 0, Col: 7},
-							To:   Position{Index: 11, Line: 0, Col: 11},
-						},
-					},
-				},
+				// <input> is a void element, so text is not a child of the input.
+				// </input> is ignored.
+				Children: nil,
 			},
 		},
 		{
@@ -736,23 +744,17 @@ func TestElementParser(t *testing.T) {
 			},
 		},
 		{
-			name:  "element: void nesting others is OK (br/hr)",
+			name:  "element: void nesting is ignored",
 			input: `<br><hr></br>`,
 			expected: Element{
 				Name: "br",
 				NameRange: Range{
 					From: Position{Index: 1, Line: 0, Col: 1},
 					To:   Position{Index: 3, Line: 0, Col: 3},
 				},
-				Children: []Node{
-					Element{
-						Name: "hr",
-						NameRange: Range{
-							From: Position{Index: 5, Line: 0, Col: 5},
-							To:   Position{Index: 7, Line: 0, Col: 7},
-						},
-					},
-				},
+				// <br> is a void element, so <hr> is not a child of the <br>.
+				// </br> is ignored.
+				Children: nil,
 			},
 		},
 		{

parser/v2/elementparser_test.go

@@ -20,6 +20,7 @@ templ input(value, validation string) {
 	<img></img>
 	<input>
 	<input></input>
+	<input>Text
 	<input>Text</input>
 	<keygen>
 	<keygen></keygen>
@@ -59,7 +60,8 @@ templ input(value, validation string) {
 	<img/>
 	<input/>
 	<input/>
-	<input>Text</input>
+	<input/>Text
+	<input/>Text
 	<keygen/>
 	<keygen/>
 	<link/>

parser/v2/formattestdata/void_elements_are_converted_to_self_closing_elements.txt

@@ -61,6 +61,10 @@ type templateNodeParser[TUntil any] struct {
 
 var rawElements = parse.Any(styleElement, scriptElement)
 
+var templateNodeSkipParsers = []parse.Parser[Node]{
+	voidElementCloser, // </br>, </img> etc. - should be ignored.
+}
+
 var templateNodeParsers = []parse.Parser[Node]{
 	docType,                // <!DOCTYPE html>
 	htmlComment,            // <!--
@@ -80,6 +84,7 @@ var templateNodeParsers = []parse.Parser[Node]{
 }
 
 func (p templateNodeParser[T]) Parse(pi *parse.Input) (op Nodes, ok bool, err error) {
+outer:
 	for {
 		// Check if we've reached the end.
 		if p.until != nil {
@@ -94,6 +99,17 @@ func (p templateNodeParser[T]) Parse(pi *parse.Input) (op Nodes, ok bool, err er
 			}
 		}
 
+		// Skip any nodes that we don't care about.
+		for _, p := range templateNodeSkipParsers {
+			_, matched, err := p.Parse(pi)
+			if err != nil {
+				return Nodes{}, false, err
+			}
+			if matched {
+				continue outer
+			}
+		}
+
 		// Attempt to parse a node.
 		// Loop through the parsers and try to parse a node.
 		var matched bool

parser/v2/templateparser.go

@@ -806,6 +806,44 @@ func TestTemplateParser(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "template: void element closers are ignored",
+			input: `templ Name() {
+	<br></br><br>
+}`,
+			expected: HTMLTemplate{
+				Range: Range{
+					From: Position{Index: 0, Line: 0, Col: 0},
+					To:   Position{Index: 31, Line: 2, Col: 1},
+				},
+				Expression: Expression{
+					Value: "Name()",
+					Range: Range{
+						From: Position{Index: 6, Line: 0, Col: 6},
+						To:   Position{Index: 12, Line: 0, Col: 12},
+					},
+				},
+				Children: []Node{
+					Whitespace{Value: "\t"},
+					Element{
+						Name: "br",
+						NameRange: Range{
+							From: Position{Index: 17, Line: 1, Col: 2},
+							To:   Position{Index: 19, Line: 1, Col: 4},
+						},
+						TrailingSpace: SpaceNone,
+					},
+					Element{
+						Name: "br",
+						NameRange: Range{
+							From: Position{Index: 26, Line: 1, Col: 11},
+							To:   Position{Index: 28, Line: 1, Col: 13},
+						},
+						TrailingSpace: SpaceVertical,
+					},
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		tt := tt

parser/v2/templateparser_test.go

@@ -25,6 +25,11 @@ var textParser = parse.Func(func(pi *parse.Input) (n Node, ok bool, err error) {
 	}
 	t.Range = NewRange(from, pi.Position())
 
+	// Elide any void element closing tags.
+	if _, _, err = voidElementCloser.Parse(pi); err != nil {
+		return
+	}
+
 	// Parse trailing whitespace.
 	ws, _, err := parse.Whitespace.Parse(pi)
 	if err != nil {