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

common/herrors/file_error.go

@@ -142,7 +142,7 @@ func (e *fileError) Unwrap() error {
 // NewFileError creates a new FileError that wraps err.
 // The value for name should identify the file, the best
 // 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, pos := extractFileTypePos(err)
 	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.
-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, _ := extractFileTypePos(err)
 	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 {
 		panic("err is nil")
 	}
-	if linematcher == nil {
+	f, realFilename, err2 := openFile(pos.Filename, fs)
-		linematcher = SimpleLineMatcher
-	}
-	f, err2 := fs.Open(filename)
 	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()
-	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.

common/herrors/file_error_test.go

@@ -30,7 +30,7 @@ func TestNewFileError(t *testing.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`)
 
 	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},
 	} {
 
-		got := NewFileError("test.txt", test.in)
+		got := NewFileError(test.in, "test.txt")
 
 		errMsg := qt.Commentf("[%d][%T]", i, got)
 

config/configLoader.go

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

docs/content/en/hugo-pipes/postcss.md

@@ -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.
 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:_
 
 use [string]

hugofs/fileinfo.go

@@ -139,6 +139,17 @@ type fileInfoMeta struct {
 	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,
 // 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.

hugolib/config.go

@@ -511,5 +511,5 @@ func (configLoader) loadSiteConfig(cfg config.Provider) (scfg SiteConfig, err er
 }
 
 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)
 }

hugolib/hugo_sites.go

@@ -968,7 +968,7 @@ func (h *HugoSites) errWithFileContext(err error, f source.File) error {
 	}
 	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)
 
 }
 

hugolib/page.go

@@ -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 {
 	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 {

hugolib/shortcode.go

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

langs/i18n/translationProvider.go

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

markup/goldmark/codeblocks/render.go

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

parser/metadecoders/decoder.go

@@ -260,7 +260,7 @@ func (d Decoder) unmarshalORG(data []byte, v any) 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

resources/resource_transformers/js/build.go

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

resources/resource_transformers/postcss/integration_test.go

@@ -140,3 +140,49 @@ func TestTransformPostCSSError(t *testing.T) {
 	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";`))
+
+}

resources/resource_transformers/postcss/postcss.go

@@ -28,6 +28,7 @@ import (
 
 	"github.com/gohugoio/hugo/common/collections"
 	"github.com/gohugoio/hugo/common/hexec"
+	"github.com/gohugoio/hugo/common/text"
 	"github.com/gohugoio/hugo/hugofs"
 
 	"github.com/gohugoio/hugo/common/hugo"
@@ -49,9 +50,10 @@ 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.
 func New(rs *resources.Spec) *Client {
@@ -100,6 +102,12 @@ type Options struct {
 	// so you can have @import anywhere in the file.
 	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
 	Use         string // List of postcss plugins to use
 	Parser      string //  Custom postcss parser
@@ -204,6 +212,7 @@ func (t *postcssTransformation) Transform(ctx *resources.ResourceTransformationC
 	imp := newImportResolver(
 		ctx.From,
 		ctx.InPath,
+		t.options,
 		t.rs.Assets.Fs, t.rs.Logger,
 	)
 
@@ -239,6 +248,7 @@ type fileOffset struct {
 type importResolver struct {
 	r      io.Reader
 	inPath string
+	opts   Options
 
 	contentSeen map[string]bool
 	linemap     map[int]fileOffset
@@ -246,12 +256,13 @@ type importResolver struct {
 	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{
 		r:      r,
 		inPath: inPath,
 		fs:     fs, logger: logger,
 		linemap: make(map[int]fileOffset), contentSeen: make(map[string]bool),
+		opts: opts,
 	}
 }
 
@@ -282,20 +293,31 @@ func (imp *importResolver) importRecursive(
 	i := 0
 	for offset, line := range lines {
 		i++
-		line = strings.TrimSpace(line)
+		lineTrimmed := strings.TrimSpace(line)
+		column := strings.Index(line, lineTrimmed)
+		line = lineTrimmed
 
 		if !imp.shouldImport(line) {
 			trackLine(i, offset, line)
 		} else {
-			i--
 			path := strings.Trim(strings.TrimPrefix(line, importIdentifier), " \"';")
 			filename := filepath.Join(basePath, path)
 			importContent, hash := imp.contentHash(filename)
+
 			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
 				}
+				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] {
 				i++
@@ -399,7 +421,7 @@ func (imp *importResolver) toFileError(output string) error {
 	}
 	defer f.Close()
 
-	ferr := herrors.NewFileError(realFilename, inErr)
+	ferr := herrors.NewFileError(inErr, realFilename)
 	pos := ferr.Position()
 	pos.LineNumber = file.Offset + 1
 	return ferr.UpdatePosition(pos).UpdateContent(f, nil)

resources/resource_transformers/postcss/postcss_test.go

@@ -60,6 +60,7 @@ func TestShouldImport(t *testing.T) {
 		{input: `@import 'navigation.css';`, expect: true},
 		{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 "printstyle.css" print;`, expect: false},
 	} {
 		c.Assert(imp.shouldImport(test.input), qt.Equals, test.expect)
 	}
@@ -88,12 +89,12 @@ A_STYLE2
 @import "b.css";
 LOCAL_STYLE
 @import "c.css";
-@import "e.css";
+@import "e.css";`)
-@import "missing.css";`)
 
 	imp := newImportResolver(
 		mainStyles,
 		"styles.css",
+		Options{},
 		fs, loggers.NewErrorLogger(),
 	)
 
@@ -108,8 +109,7 @@ C_STYLE
 A_STYLE1
 A_STYLE2
 LOCAL_STYLE
-E_STYLE
+E_STYLE`)
-@import "missing.css";`)
 
 	dline := imp.linemap[3]
 	c.Assert(dline, qt.DeepEquals, fileOffset{
@@ -151,6 +151,7 @@ LOCAL_STYLE
 		imp := newImportResolver(
 			strings.NewReader(mainStyles),
 			"styles.css",
+			Options{},
 			fs, logger,
 		)
 

resources/resource_transformers/tocss/dartsass/transform.go

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

tpl/tplimpl/template.go

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

tpl/tplimpl/template_errors.go

@@ -53,7 +53,7 @@ func (t templateInfo) resolveType() templateType {
 
 func (info templateInfo) errWithFileContext(what string, err error) error {
 	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)
 	if err != nil {
 		return err

transform/chain.go

@@ -113,9 +113,9 @@ func (c *Chain) Apply(to io.Writer, from io.Reader) error {
 				filename = tempfile.Name()
 				defer tempfile.Close()
 				_, _ = 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)
 
 		}
 	}