diff --git a/compiler/parser.go b/compiler/parser.go index f1c359d11..6d374c424 100644 --- a/compiler/parser.go +++ b/compiler/parser.go @@ -72,11 +72,31 @@ func syntaxError(pos *ast.Position, format string, a ...interface{}) *SyntaxErro return &SyntaxError{"", *pos, fmt.Sprintf(format, a...)} } -// cycleError implements an error indicating the presence of a cycle. -type cycleError string +// CycleError implements an error indicating the presence of a cycle. +type CycleError struct { + path string + pos ast.Position + msg string +} + +func (e *CycleError) Error() string { + return e.msg +} -func (e cycleError) Error() string { - return string(e) +// Path returns the path of the first referenced file in the cycle. +func (e *CycleError) Path() string { + return e.path +} + +// Position returns the position of the extends, import or show statement +// referring the second file in the cycle. +func (e *CycleError) Position() ast.Position { + return e.pos +} + +// Message returns the message of cycle error, without position and path. +func (e *CycleError) Message() string { + return e.msg } // containsOnlySpaces reports whether b contains only white space characters diff --git a/compiler/parser_cycle_test.go b/compiler/parser_cycle_test.go index 2abcb7a79..7de45c250 100644 --- a/compiler/parser_cycle_test.go +++ b/compiler/parser_cycle_test.go @@ -14,49 +14,179 @@ import ( "github.com/google/go-cmp/cmp" ) -var cyclicProgram = mapStringLoader{ - "main": "package main\nimport _ \"cycle/foo\"\nfunc main() {}", - "cycle/foo": "package foo\nimport _ \"cycle/foo2\"", - "cycle/foo2": "package foo2\nimport _ \"cycle/foo3\"", - "cycle/foo3": "package foo3\nimport _ \"cycle/foo\"", -} +var cycleProgramTests = []struct { + name string + program mapStringLoader + path string + pos ast.Position + msg string +}{ -const cyclicProgramErrorMessage = `package main + { + name: "Package import cycle", + program: mapStringLoader{ + "main": "package main\nimport _ \"cycle/foo\"\nfunc main() {}", + "cycle/foo": "package foo\nimport _ \"cycle/foo2\"", + "cycle/foo2": "package foo2\nimport _ \"cycle/foo3\"", + "cycle/foo3": "package foo3\nimport _ \"cycle/foo\"", + }, + path: "cycle/foo", + pos: ast.Position{Line: 2, Column: 8, Start: 20, End: 32}, + msg: `package main imports cycle/foo imports cycle/foo2 imports cycle/foo3 - imports cycle/foo: import cycle not allowed` + imports cycle/foo: import cycle not allowed`, + }, -func TestCyclicProgram(t *testing.T) { - _, err := ParseProgram(cyclicProgram) - if err == nil { - t.Fatal("expecting cycle error, got no error") - } - if diff := cmp.Diff(cyclicProgramErrorMessage, err.Error()); diff != "" { - t.Fatalf("unexpected error message (-want, +got):\n%s", diff) - } + { + name: "Package that imports itself", + program: mapStringLoader{ + "main": "package main\nimport _ \"cycle/foo\"\nfunc main() {}", + "cycle/foo": "package foo\nimport _ \"cycle/foo\"", + }, + path: "cycle/foo", + pos: ast.Position{Line: 2, Column: 8, Start: 19, End: 31}, + msg: `package main + imports cycle/foo + imports cycle/foo: import cycle not allowed`, + }, } -var cyclicTemplate = mapStringReader{ - "/index.html": `{% extends "/layout.html" %}`, - "/layout.html": `{% import "/macros/macro.html" %}`, - "/partials/partial.html": `{% import "/macros/macro.html" %}`, - "/macros/macro.html": `{% macro A() %}\n\t{% show "/partials/partial.html" %}\n{% end %}`, +func TestCyclicPrograms(t *testing.T) { + for _, test := range cycleProgramTests { + t.Run(test.name, func(t *testing.T) { + _, err := ParseProgram(test.program) + if err == nil { + t.Fatal("expecting cycle error, got no error") + } + e, ok := err.(*CycleError) + if !ok { + t.Fatalf("expecting *CycleError value, got %T value", err) + } + if diff := cmp.Diff(test.msg, e.Error()); diff != "" { + t.Fatalf("unexpected error message (-want, +got):\n%s", diff) + } + if e.path != test.path { + t.Fatalf(`expecting path %q, got %q`, test.path, e.path) + } + if e.pos != test.pos { + t.Fatalf(`expecting %#v, got %#v`, test.pos, e.pos) + } + }) + } } -const cyclicTemplateErrorMessage = `file /index.html - extends /layout.html - imports /macros/macro.html - shows /partials/partial.html - imports /macros/macro.html: cycle not allowed` +var cycleTemplateTests = []struct { + name string + template mapStringReader + path string + pos ast.Position + msg string +}{ -func TestCyclicTemplate(t *testing.T) { - _, err := ParseTemplate("index.html", cyclicTemplate, ast.LanguageHTML, false, nil) - if err == nil { - t.Fatal("expecting cycle error, got no error") - } - if diff := cmp.Diff(cyclicTemplateErrorMessage, err.Error()); diff != "" { - t.Fatalf("unexpected error message (-want, +got):\n%s", diff) + { + name: "Template cycle", + template: mapStringReader{ + "/index.html": `{% extends "/layout.html" %}`, + "/layout.html": `{% import "/macros/macro.html" %}`, + "/partials/partial.html": `{% import "/macros/macro.html" %}`, + "/macros/macro.html": `{% macro A() %}\n\t{% show "/partials/partial.html" %}\n{% end %}`, + }, + path: "/macros/macro.html", + pos: ast.Position{Line: 1, Column: 23, Start: 22, End: 50}, + msg: `file /index.html + extends /layout.html + imports /macros/macro.html + shows /partials/partial.html + imports /macros/macro.html: cycle not allowed`, + }, + + { + name: "Template cycle on index file", + template: mapStringReader{ + "/index.html": `{% extends "/layout.html" %}`, + "/layout.html": `{% import "/macros/macro.html" %}`, + "/macros/macro.html": `{% macro A() %}\n\t{% show "/index.html" %}\n{% end %}`, + }, + path: "/index.html", + pos: ast.Position{Line: 1, Column: 4, Start: 3, End: 24}, + msg: `file /index.html + extends /layout.html + imports /macros/macro.html + shows /index.html: cycle not allowed`, + }, + + { + name: "Template cycle on last showd file", + template: mapStringReader{ + "/index.html": `{% extends "/layout.html" %}`, + "/layout.html": `{% import "/macros/macro.html" %}`, + "/macros/macro.html": `{% macro A() %}\n\t{% show "/macros/macro.html" %}\n{% end %}`, + }, + path: "/macros/macro.html", + pos: ast.Position{Line: 1, Column: 23, Start: 22, End: 46}, + msg: `file /index.html + extends /layout.html + imports /macros/macro.html + shows /macros/macro.html: cycle not allowed`, + }, + + { + name: "Template file that extends itself", + template: mapStringReader{ + "/index.html": `{% extends "/index.html" %}`, + }, + path: "/index.html", + pos: ast.Position{Line: 1, Column: 4, Start: 3, End: 23}, + msg: `file /index.html + extends /index.html: cycle not allowed`, + }, + + { + name: "Template file that imports itself", + template: mapStringReader{ + "/index.html": `{% import "/index.html" %}`, + }, + path: "/index.html", + pos: ast.Position{Line: 1, Column: 11, Start: 10, End: 22}, + msg: `file /index.html + imports /index.html: cycle not allowed`, + }, + + { + name: "Template file that shows itself", + template: mapStringReader{ + "/index.html": `{% show "/index.html" %}`, + }, + path: "/index.html", + pos: ast.Position{Line: 1, Column: 4, Start: 3, End: 20}, + msg: `file /index.html + shows /index.html: cycle not allowed`, + }, +} + +func TestCyclicTemplates(t *testing.T) { + for _, test := range cycleTemplateTests { + t.Run(test.name, func(t *testing.T) { + _, err := ParseTemplate("index.html", test.template, ast.LanguageHTML, false, nil) + if err == nil { + t.Fatal("expecting cycle error, got no error") + } + e, ok := err.(*CycleError) + if !ok { + t.Fatalf("expecting *CycleError value, got %T value", err) + } + if diff := cmp.Diff(test.msg, e.Error()); diff != "" { + t.Fatalf("unexpected error message (-want, +got):\n%s", diff) + } + if e.path != test.path { + t.Fatalf(`expecting path %q, got %q`, test.path, e.path) + } + if e.pos != test.pos { + t.Fatalf(`expecting %#v, got %#v`, test.pos, e.pos) + } + }) } } diff --git a/compiler/parser_program.go b/compiler/parser_program.go index abd3324cd..7bfef4239 100644 --- a/compiler/parser_program.go +++ b/compiler/parser_program.go @@ -81,15 +81,19 @@ func ParseProgram(packages PackageLoader) (*ast.Tree, error) { for i, p := range imports { if p.Path == imp.Path { // There is a cycle. - err := "package " + err := &CycleError{ + path: p.Path, + pos: *(imp.Pos()), + } + err.msg = "package " for i, imp = range imports { if i > 0 { - err += "\n\timports " + err.msg += "\n\timports " } - err += imp.Path + err.msg += imp.Path } - err += "\n\timports " + p.Path + ": import cycle not allowed" - return nil, cycleError(err) + err.msg += "\n\timports " + p.Path + ": import cycle not allowed" + return nil, err } } imp.Tree = tree diff --git a/compiler/parser_template.go b/compiler/parser_template.go index 3382682d8..13dc8c948 100644 --- a/compiler/parser_template.go +++ b/compiler/parser_template.go @@ -54,8 +54,8 @@ func ParseTemplate(path string, reader FileReader, lang ast.Language, relaxedBoo if err != nil { if err2, ok := err.(*SyntaxError); ok && err2.path == "" { err2.path = path - } else if err2, ok := err.(cycleError); ok { - err = cycleError("file " + path + string(err2) + ": cycle not allowed") + } else if e, ok := err.(*CycleError); ok { + e.msg = "file " + path + e.msg + ": cycle not allowed" } else if os.IsNotExist(err) { err = ErrNotExist } @@ -94,7 +94,7 @@ func (pp *templateExpansion) parseFile(name string, lang ast.Language) (*ast.Tre // Check if there is a cycle. for _, p := range pp.paths { if p == name { - return nil, cycleError("") + return nil, &CycleError{path: p} } } @@ -121,13 +121,13 @@ func (pp *templateExpansion) parseFile(name string, lang ast.Language) (*ast.Tre // Expand the nodes. pp.paths = append(pp.paths, name) err = pp.expand(tree.Nodes) + pp.paths = pp.paths[:len(pp.paths)-1] if err != nil { if e, ok := err.(*SyntaxError); ok && e.path == "" { e.path = name } return nil, err } - pp.paths = pp.paths[:len(pp.paths)-1] // Add the tree to the cache. pp.trees.Add(name, lang, tree) @@ -228,8 +228,11 @@ func (pp *templateExpansion) expand(nodes []ast.Node) error { err = fmt.Errorf("invalid path %q at %s", n.Path, n.Pos()) } else if os.IsNotExist(err) { err = syntaxError(n.Pos(), "extends path %q does not exist", absPath) - } else if err2, ok := err.(cycleError); ok { - err = cycleError("\n\textends " + absPath + string(err2)) + } else if e, ok := err.(*CycleError); ok { + e.msg = "\n\textends " + absPath + e.msg + if e.path == pp.paths[len(pp.paths)-1] { + e.pos = *(n.Pos()) + } } return err } @@ -264,8 +267,11 @@ func (pp *templateExpansion) expand(nodes []ast.Node) error { err = fmt.Errorf("invalid path %q at %s", n.Path, n.Pos()) } else if os.IsNotExist(err) { err = syntaxError(n.Pos(), "import path %q does not exist", absPath) - } else if err2, ok := err.(cycleError); ok { - err = cycleError("\n\timports " + absPath + string(err2)) + } else if e, ok := err.(*CycleError); ok { + e.msg = "\n\timports " + absPath + e.msg + if e.path == pp.paths[len(pp.paths)-1] { + e.pos = *(n.Pos()) + } } return err } @@ -283,8 +289,11 @@ func (pp *templateExpansion) expand(nodes []ast.Node) error { err = fmt.Errorf("invalid path %q at %s", n.Path, n.Pos()) } else if os.IsNotExist(err) { err = syntaxError(n.Pos(), "shown path %q does not exist", absPath) - } else if err2, ok := err.(cycleError); ok { - err = cycleError("\n\tshows " + absPath + string(err2)) + } else if e, ok := err.(*CycleError); ok { + e.msg = "\n\tshows " + absPath + e.msg + if e.path == pp.paths[len(pp.paths)-1] { + e.pos = *(n.Pos()) + } } return err }