postcss: Fix import error handling

Note that we will now fail if `inlineImports` is enabled and we cannot resolve an import.

You can work around this by either:

* Use url imports or imports with media queries.
* Set `skipInlineImportsNotFound=true` in the options

Also get the argument order in the different NewFileError* funcs in line.

Fixes #9895
This commit is contained in:
Bjørn Erik Pedersen 2022-05-14 15:51:04 +02:00
parent c2fa0a3320
commit 4b189d8fd9
20 changed files with 159 additions and 43 deletions

View file

@ -142,7 +142,7 @@ func (e *fileError) Unwrap() error {
// NewFileError creates a new FileError that wraps err. // NewFileError creates a new FileError that wraps err.
// The value for name should identify the file, the best // The value for name should identify the file, the best
// being the full filename to the file on disk. // being the full filename to the file on disk.
func NewFileError(name string, err error) FileError { func NewFileError(err error, name string) FileError {
// Filetype is used to determine the Chroma lexer to use. // Filetype is used to determine the Chroma lexer to use.
fileType, pos := extractFileTypePos(err) fileType, pos := extractFileTypePos(err)
pos.Filename = name pos.Filename = name
@ -155,7 +155,7 @@ func NewFileError(name string, err error) FileError {
} }
// NewFileErrorFromPos will use the filename and line number from pos to create a new FileError, wrapping err. // NewFileErrorFromPos will use the filename and line number from pos to create a new FileError, wrapping err.
func NewFileErrorFromPos(pos text.Position, err error) FileError { func NewFileErrorFromPos(err error, pos text.Position) FileError {
// Filetype is used to determine the Chroma lexer to use. // Filetype is used to determine the Chroma lexer to use.
fileType, _ := extractFileTypePos(err) fileType, _ := extractFileTypePos(err)
if fileType == "" { if fileType == "" {
@ -165,20 +165,52 @@ func NewFileErrorFromPos(pos text.Position, err error) FileError {
} }
// NewFileErrorFromFile is a convenience method to create a new FileError from a file. func NewFileErrorFromFileInPos(err error, pos text.Position, fs afero.Fs, linematcher LineMatcherFn) FileError {
func NewFileErrorFromFile(err error, filename, realFilename string, fs afero.Fs, linematcher LineMatcherFn) FileError {
if err == nil { if err == nil {
panic("err is nil") panic("err is nil")
} }
if linematcher == nil { f, realFilename, err2 := openFile(pos.Filename, fs)
linematcher = SimpleLineMatcher
}
f, err2 := fs.Open(filename)
if err2 != nil { if err2 != nil {
return NewFileError(realFilename, err) return NewFileErrorFromPos(err, pos)
}
pos.Filename = realFilename
defer f.Close()
return NewFileErrorFromPos(err, pos).UpdateContent(f, linematcher)
}
// NewFileErrorFromFile is a convenience method to create a new FileError from a file.
func NewFileErrorFromFile(err error, filename string, fs afero.Fs, linematcher LineMatcherFn) FileError {
if err == nil {
panic("err is nil")
}
f, realFilename, err2 := openFile(filename, fs)
if err2 != nil {
return NewFileError(err, realFilename)
} }
defer f.Close() defer f.Close()
return NewFileError(realFilename, err).UpdateContent(f, linematcher) return NewFileError(err, realFilename).UpdateContent(f, linematcher)
}
func openFile(filename string, fs afero.Fs) (afero.File, string, error) {
realFilename := filename
// We want the most specific filename possible in the error message.
fi, err2 := fs.Stat(filename)
if err2 == nil {
if s, ok := fi.(interface {
Filename() string
}); ok {
realFilename = s.Filename()
}
}
f, err2 := fs.Open(filename)
if err2 != nil {
return nil, realFilename, err2
}
return f, realFilename, nil
} }
// Cause returns the underlying error or itself if it does not implement Unwrap. // Cause returns the underlying error or itself if it does not implement Unwrap.

View file

@ -30,7 +30,7 @@ func TestNewFileError(t *testing.T) {
c := qt.New(t) c := qt.New(t)
fe := NewFileError("foo.html", errors.New("bar")) fe := NewFileError(errors.New("bar"), "foo.html")
c.Assert(fe.Error(), qt.Equals, `"foo.html:1:1": bar`) c.Assert(fe.Error(), qt.Equals, `"foo.html:1:1": bar`)
lines := "" lines := ""
@ -70,7 +70,7 @@ func TestNewFileErrorExtractFromMessage(t *testing.T) {
{errors.New(`execute of template failed: template: index.html:2:5: executing "index.html" at <partial "foo.html" .>: error calling partial: "/layouts/partials/foo.html:3:6": execute of template failed: template: partials/foo.html:3:6: executing "partials/foo.html" at <.ThisDoesNotExist>: can't evaluate field ThisDoesNotExist in type *hugolib.pageStat`), 0, 2, 5}, {errors.New(`execute of template failed: template: index.html:2:5: executing "index.html" at <partial "foo.html" .>: error calling partial: "/layouts/partials/foo.html:3:6": execute of template failed: template: partials/foo.html:3:6: executing "partials/foo.html" at <.ThisDoesNotExist>: can't evaluate field ThisDoesNotExist in type *hugolib.pageStat`), 0, 2, 5},
} { } {
got := NewFileError("test.txt", test.in) got := NewFileError(test.in, "test.txt")
errMsg := qt.Commentf("[%d][%T]", i, got) errMsg := qt.Commentf("[%d][%T]", i, got)

View file

@ -59,7 +59,7 @@ func FromConfigString(config, configType string) (Provider, error) {
func FromFile(fs afero.Fs, filename string) (Provider, error) { func FromFile(fs afero.Fs, filename string) (Provider, error) {
m, err := loadConfigFromFile(fs, filename) m, err := loadConfigFromFile(fs, filename)
if err != nil { if err != nil {
return nil, herrors.NewFileErrorFromFile(err, filename, filename, fs, nil) return nil, herrors.NewFileErrorFromFile(err, filename, fs, nil)
} }
return NewFrom(m), nil return NewFrom(m), nil
} }

View file

@ -44,6 +44,10 @@ URL imports (e.g. `@import url('https://fonts.googleapis.com/css?family=Open+San
Note that this import routine does not care about the CSS spec, so you can have @import anywhere in the file. Note that this import routine does not care about the CSS spec, so you can have @import anywhere in the file.
Hugo will look for imports relative to the module mount and will respect theme overrides. Hugo will look for imports relative to the module mount and will respect theme overrides.
skipInlineImportsNotFound [bool] {{< new-in "0.99.0" >}}
Before Hugo 0.99.0 when `inlineImports` was enabled and we failed to resolve an import, we logged it as a warning. We now fail the build. If you have regular CSS imports in your CSS that you want to preserve, you can either use imports with URL or media queries (Hugo does not try to resolve those) or set `skipInlineImportsNotFound` to true.
_If no configuration file is used:_ _If no configuration file is used:_
use [string] use [string]

View file

@ -139,6 +139,17 @@ type fileInfoMeta struct {
m *FileMeta m *FileMeta
} }
type filenameProvider interface {
Filename() string
}
var _ filenameProvider = (*fileInfoMeta)(nil)
// Filename returns the full filename.
func (fi *fileInfoMeta) Filename() string {
return fi.m.Filename
}
// Name returns the file's name. Note that we follow symlinks, // Name returns the file's name. Note that we follow symlinks,
// if supported by the file system, and the Name given here will be the // if supported by the file system, and the Name given here will be the
// name of the symlink, which is what Hugo needs in all situations. // name of the symlink, which is what Hugo needs in all situations.

View file

@ -511,5 +511,5 @@ func (configLoader) loadSiteConfig(cfg config.Provider) (scfg SiteConfig, err er
} }
func (l configLoader) wrapFileError(err error, filename string) error { func (l configLoader) wrapFileError(err error, filename string) error {
return herrors.NewFileErrorFromFile(err, filename, filename, l.Fs, nil) return herrors.NewFileErrorFromFile(err, filename, l.Fs, nil)
} }

View file

@ -968,7 +968,7 @@ func (h *HugoSites) errWithFileContext(err error, f source.File) error {
} }
realFilename := fim.Meta().Filename realFilename := fim.Meta().Filename
return herrors.NewFileErrorFromFile(err, realFilename, realFilename, h.SourceSpec.Fs.Source, nil) return herrors.NewFileErrorFromFile(err, realFilename, h.SourceSpec.Fs.Source, nil)
} }

View file

@ -588,7 +588,7 @@ func (p *pageState) wrapError(err error) error {
} }
} }
return herrors.NewFileErrorFromFile(err, filename, filename, p.s.SourceSpec.Fs.Source, herrors.NopLineMatcher) return herrors.NewFileErrorFromFile(err, filename, p.s.SourceSpec.Fs.Source, herrors.NopLineMatcher)
} }
@ -788,7 +788,7 @@ func (p *pageState) outputFormat() (f output.Format) {
func (p *pageState) parseError(err error, input []byte, offset int) error { func (p *pageState) parseError(err error, input []byte, offset int) error {
pos := p.posFromInput(input, offset) pos := p.posFromInput(input, offset)
return herrors.NewFileError(p.File().Filename(), err).UpdatePosition(pos) return herrors.NewFileError(err, p.File().Filename()).UpdatePosition(pos)
} }
func (p *pageState) pathOrTitle() string { func (p *pageState) pathOrTitle() string {

View file

@ -298,7 +298,7 @@ func renderShortcode(
var err error var err error
tmpl, err = s.TextTmpl().Parse(templName, templStr) tmpl, err = s.TextTmpl().Parse(templName, templStr)
if err != nil { if err != nil {
fe := herrors.NewFileError(p.File().Filename(), err) fe := herrors.NewFileError(err, p.File().Filename())
pos := fe.Position() pos := fe.Position()
pos.LineNumber += p.posOffset(sc.pos).LineNumber pos.LineNumber += p.posOffset(sc.pos).LineNumber
fe = fe.UpdatePosition(pos) fe = fe.UpdatePosition(pos)
@ -391,7 +391,7 @@ func renderShortcode(
result, err := renderShortcodeWithPage(s.Tmpl(), tmpl, data) result, err := renderShortcodeWithPage(s.Tmpl(), tmpl, data)
if err != nil && sc.isInline { if err != nil && sc.isInline {
fe := herrors.NewFileError(p.File().Filename(), err) fe := herrors.NewFileError(err, p.File().Filename())
pos := fe.Position() pos := fe.Position()
pos.LineNumber += p.posOffset(sc.pos).LineNumber pos.LineNumber += p.posOffset(sc.pos).LineNumber
fe = fe.UpdatePosition(pos) fe = fe.UpdatePosition(pos)

View file

@ -138,6 +138,6 @@ func errWithFileContext(inerr error, r source.File) error {
} }
defer f.Close() defer f.Close()
return herrors.NewFileError(realFilename, inerr).UpdateContent(f, nil) return herrors.NewFileError(inerr, realFilename).UpdateContent(f, nil)
} }

View file

@ -130,7 +130,7 @@ func (r *htmlRenderer) renderCodeBlock(w util.BufWriter, src []byte, node ast.No
ctx.AddIdentity(cr) ctx.AddIdentity(cr)
if err != nil { if err != nil {
return ast.WalkContinue, herrors.NewFileErrorFromPos(cbctx.createPos(), err) return ast.WalkContinue, herrors.NewFileErrorFromPos(err, cbctx.createPos())
} }
return ast.WalkContinue, nil return ast.WalkContinue, nil

View file

@ -260,7 +260,7 @@ func (d Decoder) unmarshalORG(data []byte, v any) error {
} }
func toFileError(f Format, data []byte, err error) error { func toFileError(f Format, data []byte, err error) error {
return herrors.NewFileError(fmt.Sprintf("_stream.%s", f), err).UpdateContent(bytes.NewReader(data), nil) return herrors.NewFileError(err, fmt.Sprintf("_stream.%s", f)).UpdateContent(bytes.NewReader(data), nil)
} }
// stringifyMapKeys recurses into in and changes all instances of // stringifyMapKeys recurses into in and changes all instances of

View file

@ -165,7 +165,7 @@ func (t *buildTransformation) Transform(ctx *resources.ResourceTransformationCtx
if err == nil { if err == nil {
fe := herrors. fe := herrors.
NewFileError(path, errors.New(errorMessage)). NewFileError(errors.New(errorMessage), path).
UpdatePosition(text.Position{Offset: -1, LineNumber: loc.Line, ColumnNumber: loc.Column}). UpdatePosition(text.Position{Offset: -1, LineNumber: loc.Line, ColumnNumber: loc.Column}).
UpdateContent(f, nil) UpdateContent(f, nil)

View file

@ -48,7 +48,7 @@ class-in-b {
@tailwind base; @tailwind base;
@tailwind components; @tailwind components;
@tailwind utilities; @tailwind utilities;
@import "components/all.css"; @import "components/all.css";
h1 { h1 {
@apply text-2xl font-bold; @apply text-2xl font-bold;
} }
@ -140,3 +140,49 @@ func TestTransformPostCSSError(t *testing.T) {
c.Assert(err.Error(), qt.Contains, "a.css:4:2") c.Assert(err.Error(), qt.Contains, "a.css:4:2")
} }
// #9895
func TestTransformPostCSSImportError(t *testing.T) {
if !htesting.IsCI() {
t.Skip("Skip long running test when running locally")
}
c := qt.New(t)
s, err := hugolib.NewIntegrationTestBuilder(
hugolib.IntegrationTestConfig{
T: c,
NeedsOsFS: true,
NeedsNpmInstall: true,
LogLevel: jww.LevelInfo,
TxtarString: strings.ReplaceAll(postCSSIntegrationTestFiles, `@import "components/all.css";`, `@import "components/doesnotexist.css";`),
}).BuildE()
s.AssertIsFileError(err)
c.Assert(err.Error(), qt.Contains, "styles.css:4:3")
c.Assert(err.Error(), qt.Contains, filepath.FromSlash(`failed to resolve CSS @import "css/components/doesnotexist.css"`))
}
func TestTransformPostCSSImporSkipInlineImportsNotFound(t *testing.T) {
if !htesting.IsCI() {
t.Skip("Skip long running test when running locally")
}
c := qt.New(t)
files := strings.ReplaceAll(postCSSIntegrationTestFiles, `@import "components/all.css";`, `@import "components/doesnotexist.css";`)
files = strings.ReplaceAll(files, `{{ $options := dict "inlineImports" true }}`, `{{ $options := dict "inlineImports" true "skipInlineImportsNotFound" true }}`)
s := hugolib.NewIntegrationTestBuilder(
hugolib.IntegrationTestConfig{
T: c,
NeedsOsFS: true,
NeedsNpmInstall: true,
LogLevel: jww.LevelInfo,
TxtarString: files,
}).Build()
s.AssertFileContent("public/css/styles.css", filepath.FromSlash(`@import "components/doesnotexist.css";`))
}

View file

@ -28,6 +28,7 @@ import (
"github.com/gohugoio/hugo/common/collections" "github.com/gohugoio/hugo/common/collections"
"github.com/gohugoio/hugo/common/hexec" "github.com/gohugoio/hugo/common/hexec"
"github.com/gohugoio/hugo/common/text"
"github.com/gohugoio/hugo/hugofs" "github.com/gohugoio/hugo/hugofs"
"github.com/gohugoio/hugo/common/hugo" "github.com/gohugoio/hugo/common/hugo"
@ -49,9 +50,10 @@ import (
const importIdentifier = "@import" const importIdentifier = "@import"
var cssSyntaxErrorRe = regexp.MustCompile(`> (\d+) \|`) var (
cssSyntaxErrorRe = regexp.MustCompile(`> (\d+) \|`)
var shouldImportRe = regexp.MustCompile(`^@import ["'].*["'];?\s*(/\*.*\*/)?$`) shouldImportRe = regexp.MustCompile(`^@import ["'].*["'];?\s*(/\*.*\*/)?$`)
)
// New creates a new Client with the given specification. // New creates a new Client with the given specification.
func New(rs *resources.Spec) *Client { func New(rs *resources.Spec) *Client {
@ -100,6 +102,12 @@ type Options struct {
// so you can have @import anywhere in the file. // so you can have @import anywhere in the file.
InlineImports bool InlineImports bool
// When InlineImports is enabled, we fail the build if an import cannot be resolved.
// You can enable this to allow the build to continue and leave the import statement in place.
// Note that the inline importer does not process url location or imports with media queries,
// so those will be left as-is even without enabling this option.
SkipInlineImportsNotFound bool
// Options for when not using a config file // Options for when not using a config file
Use string // List of postcss plugins to use Use string // List of postcss plugins to use
Parser string // Custom postcss parser Parser string // Custom postcss parser
@ -204,6 +212,7 @@ func (t *postcssTransformation) Transform(ctx *resources.ResourceTransformationC
imp := newImportResolver( imp := newImportResolver(
ctx.From, ctx.From,
ctx.InPath, ctx.InPath,
t.options,
t.rs.Assets.Fs, t.rs.Logger, t.rs.Assets.Fs, t.rs.Logger,
) )
@ -239,6 +248,7 @@ type fileOffset struct {
type importResolver struct { type importResolver struct {
r io.Reader r io.Reader
inPath string inPath string
opts Options
contentSeen map[string]bool contentSeen map[string]bool
linemap map[int]fileOffset linemap map[int]fileOffset
@ -246,12 +256,13 @@ type importResolver struct {
logger loggers.Logger logger loggers.Logger
} }
func newImportResolver(r io.Reader, inPath string, fs afero.Fs, logger loggers.Logger) *importResolver { func newImportResolver(r io.Reader, inPath string, opts Options, fs afero.Fs, logger loggers.Logger) *importResolver {
return &importResolver{ return &importResolver{
r: r, r: r,
inPath: inPath, inPath: inPath,
fs: fs, logger: logger, fs: fs, logger: logger,
linemap: make(map[int]fileOffset), contentSeen: make(map[string]bool), linemap: make(map[int]fileOffset), contentSeen: make(map[string]bool),
opts: opts,
} }
} }
@ -282,20 +293,31 @@ func (imp *importResolver) importRecursive(
i := 0 i := 0
for offset, line := range lines { for offset, line := range lines {
i++ i++
line = strings.TrimSpace(line) lineTrimmed := strings.TrimSpace(line)
column := strings.Index(line, lineTrimmed)
line = lineTrimmed
if !imp.shouldImport(line) { if !imp.shouldImport(line) {
trackLine(i, offset, line) trackLine(i, offset, line)
} else { } else {
i--
path := strings.Trim(strings.TrimPrefix(line, importIdentifier), " \"';") path := strings.Trim(strings.TrimPrefix(line, importIdentifier), " \"';")
filename := filepath.Join(basePath, path) filename := filepath.Join(basePath, path)
importContent, hash := imp.contentHash(filename) importContent, hash := imp.contentHash(filename)
if importContent == nil { if importContent == nil {
trackLine(i, offset, "ERROR") if imp.opts.SkipInlineImportsNotFound {
imp.logger.Warnf("postcss: Failed to resolve CSS @import in %q for path %q", inPath, filename) trackLine(i, offset, line)
continue continue
} }
pos := text.Position{
Filename: inPath,
LineNumber: offset + 1,
ColumnNumber: column + 1,
}
return 0, "", herrors.NewFileErrorFromFileInPos(fmt.Errorf("failed to resolve CSS @import %q", filename), pos, imp.fs, nil)
}
i--
if imp.contentSeen[hash] { if imp.contentSeen[hash] {
i++ i++
@ -399,7 +421,7 @@ func (imp *importResolver) toFileError(output string) error {
} }
defer f.Close() defer f.Close()
ferr := herrors.NewFileError(realFilename, inErr) ferr := herrors.NewFileError(inErr, realFilename)
pos := ferr.Position() pos := ferr.Position()
pos.LineNumber = file.Offset + 1 pos.LineNumber = file.Offset + 1
return ferr.UpdatePosition(pos).UpdateContent(f, nil) return ferr.UpdatePosition(pos).UpdateContent(f, nil)

View file

@ -60,6 +60,7 @@ func TestShouldImport(t *testing.T) {
{input: `@import 'navigation.css';`, expect: true}, {input: `@import 'navigation.css';`, expect: true},
{input: `@import url("navigation.css");`, expect: false}, {input: `@import url("navigation.css");`, expect: false},
{input: `@import url('https://fonts.googleapis.com/css?family=Open+Sans:400,400i,800,800i&display=swap');`, expect: false}, {input: `@import url('https://fonts.googleapis.com/css?family=Open+Sans:400,400i,800,800i&display=swap');`, expect: false},
{input: `@import "printstyle.css" print;`, expect: false},
} { } {
c.Assert(imp.shouldImport(test.input), qt.Equals, test.expect) c.Assert(imp.shouldImport(test.input), qt.Equals, test.expect)
} }
@ -88,12 +89,12 @@ A_STYLE2
@import "b.css"; @import "b.css";
LOCAL_STYLE LOCAL_STYLE
@import "c.css"; @import "c.css";
@import "e.css"; @import "e.css";`)
@import "missing.css";`)
imp := newImportResolver( imp := newImportResolver(
mainStyles, mainStyles,
"styles.css", "styles.css",
Options{},
fs, loggers.NewErrorLogger(), fs, loggers.NewErrorLogger(),
) )
@ -108,8 +109,7 @@ C_STYLE
A_STYLE1 A_STYLE1
A_STYLE2 A_STYLE2
LOCAL_STYLE LOCAL_STYLE
E_STYLE E_STYLE`)
@import "missing.css";`)
dline := imp.linemap[3] dline := imp.linemap[3]
c.Assert(dline, qt.DeepEquals, fileOffset{ c.Assert(dline, qt.DeepEquals, fileOffset{
@ -151,6 +151,7 @@ LOCAL_STYLE
imp := newImportResolver( imp := newImportResolver(
strings.NewReader(mainStyles), strings.NewReader(mainStyles),
"styles.css", "styles.css",
Options{},
fs, logger, fs, logger,
) )

View file

@ -125,7 +125,7 @@ func (t *transform) Transform(ctx *resources.ResourceTransformationCtx) error {
return -1 return -1
} }
return herrors.NewFileErrorFromFile(sassErr, filename, filename, hugofs.Os, offsetMatcher) return herrors.NewFileErrorFromFile(sassErr, filename, hugofs.Os, offsetMatcher)
} }
return err return err

View file

@ -554,7 +554,7 @@ func (t *templateHandler) addFileContext(templ tpl.Template, inerr error) error
} }
defer f.Close() defer f.Close()
fe := herrors.NewFileError(info.realFilename, inErr) fe := herrors.NewFileError(inErr, info.realFilename)
fe.UpdateContent(f, lineMatcher) fe.UpdateContent(f, lineMatcher)
if !fe.ErrorContext().Position.IsValid() { if !fe.ErrorContext().Position.IsValid() {

View file

@ -53,7 +53,7 @@ func (t templateInfo) resolveType() templateType {
func (info templateInfo) errWithFileContext(what string, err error) error { func (info templateInfo) errWithFileContext(what string, err error) error {
err = fmt.Errorf(what+": %w", err) err = fmt.Errorf(what+": %w", err)
fe := herrors.NewFileError(info.realFilename, err) fe := herrors.NewFileError(err, info.realFilename)
f, err := info.fs.Open(info.filename) f, err := info.fs.Open(info.filename)
if err != nil { if err != nil {
return err return err

View file

@ -113,9 +113,9 @@ func (c *Chain) Apply(to io.Writer, from io.Reader) error {
filename = tempfile.Name() filename = tempfile.Name()
defer tempfile.Close() defer tempfile.Close()
_, _ = io.Copy(tempfile, fb.from) _, _ = io.Copy(tempfile, fb.from)
return herrors.NewFileErrorFromFile(err, filename, filename, hugofs.Os, nil) return herrors.NewFileErrorFromFile(err, filename, hugofs.Os, nil)
} }
return herrors.NewFileError(filename, err).UpdateContent(fb.from, nil) return herrors.NewFileError(err, filename).UpdateContent(fb.from, nil)
} }
} }