From 4dae52af680e6ff2c8cdeb4ce1f219330b27001c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Mon, 25 Mar 2019 18:18:34 +0100 Subject: [PATCH] Avoid nilpointer on no File on Page Fixes #5781 --- codegen/methods.go | 19 ++++ commands/convert.go | 2 +- deps/deps.go | 5 ++ hugolib/hugo_sites.go | 2 +- hugolib/page.go | 8 +- hugolib/page__meta.go | 8 +- hugolib/page__new.go | 4 + hugolib/page__paths.go | 2 +- hugolib/page_test.go | 8 ++ hugolib/site.go | 4 +- magefile.go | 1 + .../page_generate/generate_page_wrappers.go | 66 ++++++++++++++ resources/page/pages_sort.go | 6 +- resources/page/zero_file.autogen.go | 88 +++++++++++++++++++ source/fileInfo.go | 6 ++ 15 files changed, 213 insertions(+), 16 deletions(-) create mode 100644 resources/page/zero_file.autogen.go diff --git a/codegen/methods.go b/codegen/methods.go index 007384f9b..ed8dba923 100644 --- a/codegen/methods.go +++ b/codegen/methods.go @@ -288,6 +288,12 @@ func (m Method) Declaration(receiver string) string { return fmt.Sprintf("func (%s %s) %s%s %s", receiverShort(receiver), receiver, m.Name, m.inStr(), m.outStr()) } +// DeclarationNamed creates a method declaration (without any body) for the given receiver +// with named return values. +func (m Method) DeclarationNamed(receiver string) string { + return fmt.Sprintf("func (%s %s) %s%s %s", receiverShort(receiver), receiver, m.Name, m.inStr(), m.outStrNamed()) +} + // Delegate creates a delegate call string. func (m Method) Delegate(receiver, delegate string) string { ret := "" @@ -336,6 +342,19 @@ func (m Method) outStr() string { return "(" + strings.Join(m.Out, ", ") + ")" } +func (m Method) outStrNamed() string { + if len(m.Out) == 0 { + return "" + } + + outs := make([]string, len(m.Out)) + for i := 0; i < len(outs); i++ { + outs[i] = fmt.Sprintf("o%d %s", i, m.Out[i]) + } + + return "(" + strings.Join(outs, ", ") + ")" +} + // Methods represents a list of methods for one or more interfaces. // The order matches the defined order in their source file(s). type Methods []Method diff --git a/commands/convert.go b/commands/convert.go index e7ba572bc..d0a46a641 100644 --- a/commands/convert.go +++ b/commands/convert.go @@ -143,7 +143,7 @@ func (cc *convertCmd) convertAndSavePage(p page.Page, site *hugolib.Site, target } } - if p.File() == nil { + if p.File().IsZero() { // No content file. return nil } diff --git a/deps/deps.go b/deps/deps.go index 47159d017..fa62fe5ae 100644 --- a/deps/deps.go +++ b/deps/deps.go @@ -34,6 +34,9 @@ type Deps struct { // Used to log errors that may repeat itself many times. DistinctErrorLog *helpers.DistinctLogger + // Used to log warnings that may repeat itself many times. + DistinctWarningLog *helpers.DistinctLogger + // The templates to use. This will usually implement the full tpl.TemplateHandler. Tmpl tpl.TemplateFinder `json:"-"` @@ -233,11 +236,13 @@ func New(cfg DepsCfg) (*Deps, error) { } distinctErrorLogger := helpers.NewDistinctLogger(logger.ERROR) + distinctWarnLogger := helpers.NewDistinctLogger(logger.WARN) d := &Deps{ Fs: fs, Log: logger, DistinctErrorLog: distinctErrorLogger, + DistinctWarningLog: distinctWarnLogger, templateProvider: cfg.TemplateProvider, translationProvider: cfg.TranslationProvider, WithTemplate: cfg.WithTemplate, diff --git a/hugolib/hugo_sites.go b/hugolib/hugo_sites.go index 0c79f4bdb..9fe2a5bdb 100644 --- a/hugolib/hugo_sites.go +++ b/hugolib/hugo_sites.go @@ -577,7 +577,7 @@ func (cfg *BuildCfg) shouldRender(p *pageState) bool { return true } - if cfg.whatChanged != nil && p.File() != nil { + if cfg.whatChanged != nil && !p.File().IsZero() { return cfg.whatChanged.files[p.File().Filename()] } diff --git a/hugolib/page.go b/hugolib/page.go index 576342cfa..99b771eee 100644 --- a/hugolib/page.go +++ b/hugolib/page.go @@ -236,7 +236,7 @@ func (p *pageState) TranslationKey() string { p.translationKeyInit.Do(func() { if p.m.translationKey != "" { p.translationKey = p.Kind() + "/" + p.m.translationKey - } else if p.IsPage() && p.File() != nil { + } else if p.IsPage() && !p.File().IsZero() { p.translationKey = path.Join(p.Kind(), filepath.ToSlash(p.File().Dir()), p.File().TranslationBaseName()) } else if p.IsNode() { p.translationKey = path.Join(p.Kind(), p.SectionsPath()) @@ -462,7 +462,7 @@ func (p *pageState) Render(layout ...string) template.HTML { func (p *pageState) wrapError(err error) error { var filename string - if p.File() != nil { + if !p.File().IsZero() { filename = p.File().Filename() } @@ -665,7 +665,7 @@ func (p *pageState) parseError(err error, input []byte, offset int) error { } func (p *pageState) pathOrTitle() string { - if p.File() != nil { + if !p.File().IsZero() { return p.File().Filename() } @@ -763,7 +763,7 @@ func (p *pageState) sortParentSections() { // For pages that do not (sections witout content page etc.), it returns the // virtual path, consistent with where you would add a source file. func (p *pageState) sourceRef() string { - if p.File() != nil { + if !p.File().IsZero() { sourcePath := p.File().Path() if sourcePath != "" { return "/" + filepath.ToSlash(sourcePath) diff --git a/hugolib/page__meta.go b/hugolib/page__meta.go index 8532f5016..2c6b0a85d 100644 --- a/hugolib/page__meta.go +++ b/hugolib/page__meta.go @@ -222,7 +222,7 @@ func (p *pageMeta) Params() map[string]interface{} { } func (p *pageMeta) Path() string { - if p.File() != nil { + if !p.File().IsZero() { return p.File().Path() } return p.SectionsPath() @@ -256,7 +256,7 @@ func (p *pageMeta) Section() string { return p.sections[0] } - if p.File() != nil { + if !p.File().IsZero() { return p.File().Section() } @@ -536,8 +536,8 @@ func (pm *pageMeta) setMetadata(p *pageState, frontmatter map[string]interface{} func (p *pageMeta) applyDefaultValues() error { if p.markup == "" { - if p.File() != nil { - // Fall back to {file extension + if !p.File().IsZero() { + // Fall back to file extension p.markup = helpers.GuessType(p.File().Ext()) } if p.markup == "" { diff --git a/hugolib/page__new.go b/hugolib/page__new.go index 0f419b5da..0ac14a5e8 100644 --- a/hugolib/page__new.go +++ b/hugolib/page__new.go @@ -95,6 +95,10 @@ func newPageBase(metaProvider *pageMeta) (*pageState, error) { } func newPageFromMeta(metaProvider *pageMeta) (*pageState, error) { + if metaProvider.f == nil { + metaProvider.f = page.NewZeroFile(metaProvider.s.DistinctWarningLog) + } + ps, err := newPageBase(metaProvider) if err != nil { return nil, err diff --git a/hugolib/page__paths.go b/hugolib/page__paths.go index 0a5dad5ef..8bc7a7535 100644 --- a/hugolib/page__paths.go +++ b/hugolib/page__paths.go @@ -99,7 +99,7 @@ func createTargetPathDescriptor(s *Site, p page.Page, pm *pageMeta) (page.Target d := s.Deps - if p.File() != nil { + if !p.File().IsZero() { dir = p.File().Dir() baseName = p.File().TranslationBaseName() } diff --git a/hugolib/page_test.go b/hugolib/page_test.go index 5e9ac696c..1e362e0dc 100644 --- a/hugolib/page_test.go +++ b/hugolib/page_test.go @@ -18,6 +18,8 @@ import ( "html/template" "os" + "github.com/gohugoio/hugo/common/loggers" + "path/filepath" "strings" "testing" @@ -1166,6 +1168,12 @@ Content:{{ .Content }} } +// https://github.com/gohugoio/hugo/issues/5781 +func TestPageWithZeroFile(t *testing.T) { + newTestSitesBuilder(t).WithLogger(loggers.NewWarningLogger()).WithSimpleConfigFile(). + WithTemplatesAdded("index.html", "{{ .File.Filename }}{{ with .File }}{{ .Dir }}{{ end }}").Build(BuildCfg{}) +} + func TestShouldBuild(t *testing.T) { t.Parallel() var past = time.Date(2009, 11, 17, 20, 34, 58, 651387237, time.UTC) diff --git a/hugolib/site.go b/hugolib/site.go index 145ae2d49..1bf2d639c 100644 --- a/hugolib/site.go +++ b/hugolib/site.go @@ -789,11 +789,11 @@ func (s *siteRefLinker) refLink(ref string, source interface{}, relative bool, o if refURL.Fragment != "" { _ = target link = link + "#" + refURL.Fragment - if pctx, ok := target.(pageContext); ok && target.File() != nil && !pctx.getRenderingConfig().PlainIDAnchors { + if pctx, ok := target.(pageContext); ok && !target.File().IsZero() && !pctx.getRenderingConfig().PlainIDAnchors { if refURL.Path != "" { link = link + ":" + target.File().UniqueID() } - } else if pctx, ok := p.(pageContext); ok && p.File() != nil && !pctx.getRenderingConfig().PlainIDAnchors { + } else if pctx, ok := p.(pageContext); ok && !p.File().IsZero() && !pctx.getRenderingConfig().PlainIDAnchors { link = link + ":" + p.File().UniqueID() } diff --git a/magefile.go b/magefile.go index 04f0499a2..3b74a7e94 100644 --- a/magefile.go +++ b/magefile.go @@ -89,6 +89,7 @@ func Generate() error { // TODO(bep) check: stat ./resources/page/*autogen*: no such file or directory "./resources/page/page_marshaljson.autogen.go", "./resources/page/page_wrappers.autogen.go", + "./resources/page/zero_file.autogen.go", } for _, pattern := range goFmtPatterns { diff --git a/resources/page/page_generate/generate_page_wrappers.go b/resources/page/page_generate/generate_page_wrappers.go index 54e1f272e..739d7a00e 100644 --- a/resources/page/page_generate/generate_page_wrappers.go +++ b/resources/page/page_generate/generate_page_wrappers.go @@ -63,6 +63,10 @@ func Generate(c *codegen.Inspector) error { return errors.Wrap(err, "failed to generate deprecate wrappers") } + if err := generateFileIsZeroWrappers(c); err != nil { + return errors.Wrap(err, "failed to generate file wrappers") + } + return nil } @@ -199,6 +203,68 @@ type pageDeprecated struct { return nil } +func generateFileIsZeroWrappers(c *codegen.Inspector) error { + filename := filepath.Join(c.ProjectRootDir, packageDir, "zero_file.autogen.go") + f, err := os.Create(filename) + if err != nil { + return err + } + defer f.Close() + + // Generate warnings for zero file access + + warning := func(name string, tp reflect.Type) string { + msg := fmt.Sprintf(".File.%s on zero object. Wrap it in if or with: {{ with .File }}{{ .%s }}{{ end }}", name, name) + + return fmt.Sprintf("z.log.Println(%q)", msg) + } + + var buff bytes.Buffer + + methods := c.MethodsFromTypes([]reflect.Type{reflect.TypeOf((*source.File)(nil)).Elem()}, nil) + + for _, m := range methods { + if m.Name == "IsZero" { + continue + } + fmt.Fprint(&buff, m.DeclarationNamed("zeroFile")) + fmt.Fprintln(&buff, " {") + fmt.Fprintf(&buff, "\t%s\n", warning(m.Name, m.Owner)) + if len(m.Out) > 0 { + fmt.Fprintln(&buff, "\treturn") + } + fmt.Fprintln(&buff, "}") + + } + + pkgImports := append(methods.Imports(), "github.com/gohugoio/hugo/helpers", "github.com/gohugoio/hugo/source") + + fmt.Fprintf(f, `%s + +package page + +%s + +// ZeroFile represents a zero value of source.File with warnings if invoked. +type zeroFile struct { + log *helpers.DistinctLogger +} + +func NewZeroFile(log *helpers.DistinctLogger) source.File { + return zeroFile{log: log} +} + +func (zeroFile) IsZero() bool { + return true +} + +%s + +`, header, importsString(pkgImports), buff.String()) + + return nil +} + func importsString(imps []string) string { if len(imps) == 0 { return "" diff --git a/resources/page/pages_sort.go b/resources/page/pages_sort.go index eb3a28247..7b2a34a6a 100644 --- a/resources/page/pages_sort.go +++ b/resources/page/pages_sort.go @@ -51,8 +51,8 @@ var DefaultPageSort = func(p1, p2 Page) bool { if p1.Weight() == p2.Weight() { if p1.Date().Unix() == p2.Date().Unix() { if p1.LinkTitle() == p2.LinkTitle() { - if p1.File() == nil || p2.File() == nil { - return p1.File() == nil + if p1.File().IsZero() || p2.File().IsZero() { + return p1.File().IsZero() } return p1.File().Filename() < p2.File().Filename() } @@ -77,7 +77,7 @@ var languagePageSort = func(p1, p2 Page) bool { if p1.Language().Weight == p2.Language().Weight { if p1.Date().Unix() == p2.Date().Unix() { if p1.LinkTitle() == p2.LinkTitle() { - if p1.File() != nil && p2.File() != nil { + if !p1.File().IsZero() && !p2.File().IsZero() { return p1.File().Filename() < p2.File().Filename() } } diff --git a/resources/page/zero_file.autogen.go b/resources/page/zero_file.autogen.go new file mode 100644 index 000000000..eec1dd66d --- /dev/null +++ b/resources/page/zero_file.autogen.go @@ -0,0 +1,88 @@ +// Copyright 2019 The Hugo Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// This file is autogenerated. + +package page + +import ( + "github.com/gohugoio/hugo/helpers" + "github.com/gohugoio/hugo/source" + "os" +) + +// ZeroFile represents a zero value of source.File with warnings if invoked. +type zeroFile struct { + log *helpers.DistinctLogger +} + +func NewZeroFile(log *helpers.DistinctLogger) source.File { + return zeroFile{log: log} +} + +func (zeroFile) IsZero() bool { + return true +} + +func (z zeroFile) Path() (o0 string) { + z.log.Println(".File.Path on zero object. Wrap it in if or with: {{ with .File }}{{ .Path }}{{ end }}") + return +} +func (z zeroFile) Section() (o0 string) { + z.log.Println(".File.Section on zero object. Wrap it in if or with: {{ with .File }}{{ .Section }}{{ end }}") + return +} +func (z zeroFile) Lang() (o0 string) { + z.log.Println(".File.Lang on zero object. Wrap it in if or with: {{ with .File }}{{ .Lang }}{{ end }}") + return +} +func (z zeroFile) Filename() (o0 string) { + z.log.Println(".File.Filename on zero object. Wrap it in if or with: {{ with .File }}{{ .Filename }}{{ end }}") + return +} +func (z zeroFile) Dir() (o0 string) { + z.log.Println(".File.Dir on zero object. Wrap it in if or with: {{ with .File }}{{ .Dir }}{{ end }}") + return +} +func (z zeroFile) Extension() (o0 string) { + z.log.Println(".File.Extension on zero object. Wrap it in if or with: {{ with .File }}{{ .Extension }}{{ end }}") + return +} +func (z zeroFile) Ext() (o0 string) { + z.log.Println(".File.Ext on zero object. Wrap it in if or with: {{ with .File }}{{ .Ext }}{{ end }}") + return +} +func (z zeroFile) LogicalName() (o0 string) { + z.log.Println(".File.LogicalName on zero object. Wrap it in if or with: {{ with .File }}{{ .LogicalName }}{{ end }}") + return +} +func (z zeroFile) BaseFileName() (o0 string) { + z.log.Println(".File.BaseFileName on zero object. Wrap it in if or with: {{ with .File }}{{ .BaseFileName }}{{ end }}") + return +} +func (z zeroFile) TranslationBaseName() (o0 string) { + z.log.Println(".File.TranslationBaseName on zero object. Wrap it in if or with: {{ with .File }}{{ .TranslationBaseName }}{{ end }}") + return +} +func (z zeroFile) ContentBaseName() (o0 string) { + z.log.Println(".File.ContentBaseName on zero object. Wrap it in if or with: {{ with .File }}{{ .ContentBaseName }}{{ end }}") + return +} +func (z zeroFile) UniqueID() (o0 string) { + z.log.Println(".File.UniqueID on zero object. Wrap it in if or with: {{ with .File }}{{ .UniqueID }}{{ end }}") + return +} +func (z zeroFile) FileInfo() (o0 os.FileInfo) { + z.log.Println(".File.FileInfo on zero object. Wrap it in if or with: {{ with .File }}{{ .FileInfo }}{{ end }}") + return +} diff --git a/source/fileInfo.go b/source/fileInfo.go index 3f262fb5e..072b55b6c 100644 --- a/source/fileInfo.go +++ b/source/fileInfo.go @@ -53,6 +53,8 @@ type fileOverlap interface { // Lang is the language code for this page. It will be the // same as the site's language code. Lang() string + + IsZero() bool } type FileWithoutOverlap interface { @@ -187,6 +189,10 @@ func (fi *FileInfo) Open() (hugio.ReadSeekCloser, error) { return f, err } +func (fi *FileInfo) IsZero() bool { + return fi == nil +} + // We create a lot of these FileInfo objects, but there are parts of it used only // in some cases that is slightly expensive to construct. func (fi *FileInfo) init() {