compiler: support recursive extends

This commit adds support for 'extends' declarations in files that have been extended. Co-authored-by: Marco Gazerro <[email protected]>
open2b · Sep 9, 2021 · 4ad04d5 · 4ad04d5
1 parent c49f9e8
commit 4ad04d5
Show file tree

Hide file tree

Showing 7 changed files with 138 additions and 17 deletions.
diff --git a/internal/compiler/checker.go b/internal/compiler/checker.go
@@ -76,25 +76,34 @@ func typecheck(tree *ast.Tree, importer native.Importer, opts checkerOptions) (m
 
 	// If tree extends another template file, transform it swapping the files
 	// and adding a dummy 'import' declaration that imports the extending file.
-	var extending bool
-	if extends, ok := getExtends(tree.Nodes); ok {
+	// This is done recursively for every file that extends another file, so:
+	//
+	//    A --extends--> B --extends--> C
+	//
+	// becomes:
+	//
+	//    C --imports--> B --imports--> A
+	//
+	for {
+		extends, ok := getExtends(tree.Nodes)
+		if !ok {
+			break
+		}
 		dummyImport := ast.NewImport(nil, ast.NewIdentifier(nil, "."), tree.Path, nil)
 		dummyImport.Tree = ast.NewTree(tree.Path, tree.Nodes, tree.Format)
-		compilation.extendingTrees[dummyImport.Tree] = true
+		compilation.extendingTrees[dummyImport.Tree.Path] = true
+		compilation.extendedTrees[extends.Tree.Path] = true
 		tree.Nodes = append([]ast.Node{dummyImport}, extends.Tree.Nodes...)
 		tree.Path = extends.Tree.Path
 		tc.path = extends.Tree.Path
-		extending = true
 	}
 
 	// Type check a template file or a script.
 	var err error
-	tc.inExtendedFile = extending // if tree was "extending", after the swap it becomes "extended"
 	tree.Nodes, err = tc.checkNodesInNewScopeError(tree, tree.Nodes)
 	if err != nil {
 		return nil, err
 	}
-	tc.inExtendedFile = false
 	mainPkgInfo := &packageInfo{}
 	mainPkgInfo.IndirectVars = tc.compilation.indirectVars
 	mainPkgInfo.TypeInfos = tc.compilation.typeInfos
@@ -180,10 +189,6 @@ type typechecker struct {
 	// of its fields have been declared.
 	structDeclPkg map[reflect.Type]string
 
-	// inExtendedFile reports whether the type checker is type checking an
-	// extended file.
-	inExtendedFile bool
-
 	// withinUsingAffectedStmt reports whether the type checker is currently
 	// checking the affected statement of a 'using' statement.
 	withinUsingAffectedStmt bool

diff --git a/internal/compiler/checker_expressions.go b/internal/compiler/checker_expressions.go
@@ -2307,7 +2307,7 @@ func (tc *typechecker) checkDefault(expr *ast.Default, show bool) typeInfoPair {
 		}
 
 	case *ast.Call:
-		if !tc.inExtendedFile {
+		if !tc.compilation.extendedTrees[tc.path] {
 			panic(tc.errorf(n, "use of default with call in non-extended file"))
 		}
 		ident := n.Func.(*ast.Identifier)

diff --git a/internal/compiler/checker_statements.go b/internal/compiler/checker_statements.go
@@ -1030,7 +1030,7 @@ func (tc *typechecker) checkImport(impor *ast.Import) error {
 	}
 
 	// Check the package and retrieve the package infos.
-	err := checkPackage(tc.compilation, impor.Tree.Nodes[0].(*ast.Package), impor.Tree.Path, tc.importer, tc.opts, tc.compilation.extendingTrees[impor.Tree])
+	err := checkPackage(tc.compilation, impor.Tree.Nodes[0].(*ast.Package), impor.Tree.Path, tc.importer, tc.opts, tc.compilation.extendingTrees[impor.Tree.Path])
 	if err != nil {
 		return err
 	}

diff --git a/internal/compiler/compilation.go b/internal/compiler/compilation.go
@@ -66,10 +66,17 @@ type compilation struct {
 	// globalScope is the global scope.
 	globalScope map[string]scopeName
 
-	// extendingTrees reports if a tree was extending another file.
+	// extendingTrees reports if a tree with a certain path is extending
+	// another file.
 	// This information must be kept here because it becomes lost after
 	// transforming the tree in case of extends.
-	extendingTrees map[*ast.Tree]bool
+	extendingTrees map[string]bool
+
+	// extendedTrees reports if a tree with a certain path has been extended by
+	// another file.
+	// This information must be kept here because it becomes lost after
+	// transforming the tree in case of extends.
+	extendedTrees map[string]bool
 }
 
 type renderIR struct {
@@ -88,7 +95,8 @@ func newCompilation(globalScope map[string]scopeName) *compilation {
 		renderImportMacro: map[*ast.Tree]renderIR{},
 		currentIteaIndex:  -1,
 		globalScope:       globalScope,
-		extendingTrees:    map[*ast.Tree]bool{},
+		extendingTrees:    map[string]bool{},
+		extendedTrees:     map[string]bool{},
 	}
 }
 

diff --git a/internal/compiler/compiler.go b/internal/compiler/compiler.go
@@ -390,6 +390,10 @@ func getExtends(nodes []ast.Node) (*ast.Extends, bool) {
 	for _, node := range nodes {
 		switch n := node.(type) {
 		case *ast.Comment, *ast.Text:
+		case *ast.Import:
+			// This is the import declaration added when transforming the tree
+			// in case of extends. It must be skipped because it is added
+			// before the 'extends' declaration.
 		case *ast.Extends:
 			return n, true
 		case *ast.Statements:

diff --git a/internal/compiler/parser_template.go b/internal/compiler/parser_template.go
@@ -46,6 +46,7 @@ func ParseTemplate(fsys fs.FS, name string, noParseShow, dollarIdentifier bool)
 		fsys:             fsys,
 		trees:            map[string]parsedTree{},
 		paths:            []string{},
+		canExtend:        true,
 		noParseShow:      noParseShow,
 		dollarIdentifier: dollarIdentifier,
 	}
@@ -68,6 +69,7 @@ type templateExpansion struct {
 	fsys             fs.FS
 	trees            map[string]parsedTree
 	paths            []string
+	canExtend        bool
 	noParseShow      bool
 	dollarIdentifier bool
 }
@@ -214,8 +216,8 @@ func (pp *templateExpansion) expand(nodes []ast.Node) error {
 		case *ast.Extends:
 			// extends "path"
 
-			if len(pp.paths) > 1 {
-				return syntaxError(n.Pos(), "extended, imported and rendered files can not have extends")
+			if !pp.canExtend {
+				return syntaxError(n.Pos(), "imported and rendered files can not have extends")
 			}
 			var err error
 			n.Tree, err = pp.parseNodeFile(n)
@@ -244,6 +246,7 @@ func (pp *templateExpansion) expand(nodes []ast.Node) error {
 
 			// Try to import the path as a template file
 			var err error
+			pp.canExtend = false
 			n.Tree, err = pp.parseNodeFile(n)
 			if err != nil && !errors.Is(err, os.ErrNotExist) {
 				if e, ok := err.(*CycleError); ok {
@@ -274,6 +277,7 @@ func (pp *templateExpansion) expand(nodes []ast.Node) error {
 			}
 
 			var err error
+			pp.canExtend = false
 			r.Tree, err = pp.parseNodeFile(r)
 			if err != nil && (!special || !errors.Is(err, os.ErrNotExist)) {
 				parent := pp.paths[len(pp.paths)-1]

diff --git a/test/misc/templates_test.go b/test/misc/templates_test.go
@@ -3731,6 +3731,106 @@ var templateMultiFileCases = map[string]struct {
 		},
 		expectedBuildErr: "undefined: T",
 	},
+
+	"Multiple extends - simple case": {
+		sources: fstest.Files{
+			"index.html":     `{% extends "extended1.html" %}`,
+			"extended1.html": `{% extends "extended2.html" %}`,
+			"extended2.html": `extends 2`,
+		},
+		expectedOut: "extends 2",
+	},
+
+	"Multiple extends - redeclaration error": {
+		sources: fstest.Files{
+			"index.html":     `{% extends "extended1.html" %}`,
+			"extended1.html": `{% extends "extended2.html" %}`,
+			"extended2.html": `{% extends "extended3.html" %}`,
+			"extended3.html": `{% extends "extended4.html" %}{% var V4 = 4 %}`,
+			"extended4.html": `{% extends "extended5.html" %}{% var V4 = 4 %}`,
+			"extended5.html": `{{ V4 }}`,
+		},
+		expectedBuildErr: "extended4.html:1:38: V4 redeclared in this block\n\textended4.html:<nil>: previous declaration during import . \"extended3.html\"",
+	},
+
+	"Multiple extends - many extended files": {
+		sources: fstest.Files{
+			"index.html":     `{% extends "extended1.html" %}{% var S0 = "0" %}`,
+			"extended1.html": `{% extends "extended2.html" %}{% var S1 = S0 + "1" %}`,
+			"extended2.html": `{% extends "extended3.html" %}{% var S2 = S1 + "2" %}`,
+			"extended3.html": `{% extends "extended4.html" %}{% var S3 = S2 + "3" %}`,
+			"extended4.html": `{% extends "extended5.html" %}{% var S4 = S3 + "4" %}`,
+			"extended5.html": `{{ S4 }}`,
+		},
+		expectedOut: "01234",
+	},
+
+	"Multiple extends - error when referring to undefined name": {
+		sources: fstest.Files{
+			"index.html":     `{% extends "extended1.html" %}`,
+			"extended1.html": `{% extends "extended2.html" %}`,
+			"extended2.html": `{% extends "extended3.html" %}`,
+			"extended3.html": `{% extends "extended4.html" %}{% var V3 = 3 %}`,
+			"extended4.html": `{% extends "extended5.html" %}{% var V4 = 4 %}`,
+			"extended5.html": `{{ V3 }}{{ V4 }}`,
+		},
+		expectedBuildErr: "extended5.html:1:4: undefined: V3",
+	},
+
+	"Multiple extends - with imports": {
+		sources: fstest.Files{
+			"index.html":     `{% extends "extended1.html" %}{% import "imported.html" %}{% var S0 = "0" + I %}`,
+			"extended1.html": `{% extends "extended2.html" %}{% var S1 = S0 + "1" %}`,
+			"extended2.html": `{% extends "extended3.html" %}{% import "imported.html" %}{% var S2 = S1 + "2" + I %}`,
+			"extended3.html": `{% extends "extended4.html" %}{% var S3 = S2 + "3" %}`,
+			"extended4.html": `{% extends "extended5.html" %}{% var S4 = S3 + "4" %}`,
+			"extended5.html": `{% import "imported.html" %}{{ S4 }}{{ I }}`,
+			"imported.html":  `{% var I = "imported" %}`,
+		},
+		expectedOut: "0imported12imported34imported",
+	},
+
+	"Multiple extends - using 'default' in extended file that extends another file": {
+		sources: fstest.Files{
+			"index.html":     `{% extends "extended1.html" %}`,
+			"extended1.html": `{% extends "extended2.html" %}{% macro M %}{{ Undef() default "hello" }}{% end macro %}`,
+			"extended2.html": `M: {{ M() }}`,
+		},
+		expectedOut: "M: hello",
+	},
+
+	"Import a file with an extends statement": {
+		sources: fstest.Files{
+			"index.html":    `{% import "imported.html" %}`,
+			"imported.html": `{% extends "extended.html" %}`,
+			"extended.html": ``,
+		},
+		expectedBuildErr: "imported and rendered files can not have extends",
+	},
+
+	"Render a file with an extends statement": {
+		sources: fstest.Files{
+			"index.html":    `{{ render "partial.html" }}`,
+			"partial.html":  `{% extends "extended.html" %}`,
+			"extended.html": ``,
+		},
+		expectedBuildErr: "imported and rendered files can not have extends",
+	},
+
+	"Cyclic extends is not allowed": {
+		sources: fstest.Files{
+			"index.html": `
+				{% extends "extended1.html" %}
+			`,
+			"extended1.html": `
+				{% extends "extended2.html" %}
+			`,
+			"extended2.html": `
+				{% extends "extended1.html" %}
+			`,
+		},
+		expectedBuildErr: "file index.html\n\textends extended1.html\n\textends extended2.html\n\textends extended1.html: cycle not allowed",
+	},
 }
 
 var structWithUnexportedFields = &struct {