Improve Tailwind/PostCSS error messages

Fixes #7041
Fixes #7042

6 files changed, 307 insertions, 87 deletions

diff --git a/resources/resource_spec.go b/resources/resource_spec.go
index 66565e4cc..d094998a4 100644
--- a/resources/resource_spec.go
+++ b/resources/resource_spec.go
@@ -22,6 +22,8 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/gohugoio/hugo/common/herrors"
+
 	"github.com/gohugoio/hugo/config"
 
 	"github.com/gohugoio/hugo/hugofs"
@@ -43,6 +45,7 @@ func NewSpec(
 	s *helpers.PathSpec,
 	fileCaches filecache.Caches,
 	logger *loggers.Logger,
+	errorHandler herrors.ErrorSender,
 	outputFormats output.Formats,
 	mimeTypes media.Types) (*Spec, error) {
 
@@ -67,6 +70,7 @@ func NewSpec(
 
 	rs := &Spec{PathSpec: s,
 		Logger:        logger,
+		ErrorSender:   errorHandler,
 		imaging:       imaging,
 		MediaTypes:    mimeTypes,
 		OutputFormats: outputFormats,
@@ -91,7 +95,8 @@ type Spec struct {
 	MediaTypes    media.Types
 	OutputFormats output.Formats
 
-	Logger *loggers.Logger
+	Logger      *loggers.Logger
+	ErrorSender herrors.ErrorSender
 
 	TextTemplates tpl.TemplateParseFinder
 
diff --git a/resources/resource_transformers/htesting/testhelpers.go b/resources/resource_transformers/htesting/testhelpers.go
index 4dfc9855a..752f571f7 100644
--- a/resources/resource_transformers/htesting/testhelpers.go
+++ b/resources/resource_transformers/htesting/testhelpers.go
@@ -51,7 +51,7 @@ func NewTestResourceSpec() (*resources.Spec, error) {
 		return nil, err
 	}
 
-	spec, err := resources.NewSpec(s, filecaches, nil, output.DefaultFormats, media.DefaultTypes)
+	spec, err := resources.NewSpec(s, filecaches, nil, nil, output.DefaultFormats, media.DefaultTypes)
 	return spec, err
 }
 
diff --git a/resources/resource_transformers/postcss/postcss.go b/resources/resource_transformers/postcss/postcss.go
index 5085670c7..15cda898c 100644
--- a/resources/resource_transformers/postcss/postcss.go
+++ b/resources/resource_transformers/postcss/postcss.go
@@ -14,6 +14,7 @@
 package postcss
 
 import (
+	"bytes"
 	"crypto/sha256"
 	"encoding/hex"
 	"io"
@@ -21,13 +22,15 @@ import (
 	"path"
 	"path/filepath"
 	"regexp"
+	"strconv"
 	"strings"
 
-	"github.com/gohugoio/hugo/config"
+	"github.com/gohugoio/hugo/common/loggers"
 
-	"github.com/spf13/afero"
+	"github.com/gohugoio/hugo/config"
 
 	"github.com/gohugoio/hugo/resources/internal"
+	"github.com/spf13/afero"
 	"github.com/spf13/cast"
 
 	"github.com/gohugoio/hugo/hugofs"
@@ -45,6 +48,41 @@ import (
 
 const importIdentifier = "@import"
 
+var cssSyntaxErrorRe = regexp.MustCompile(`> (\d+) \|`)
+
+var shouldImportRe = regexp.MustCompile(`^@import ["'].*["'];?\s*(/\*.*\*/)?$`)
+
+// New creates a new Client with the given specification.
+func New(rs *resources.Spec) *Client {
+	return &Client{rs: rs}
+}
+
+func DecodeOptions(m map[string]interface{}) (opts Options, err error) {
+	if m == nil {
+		return
+	}
+	err = mapstructure.WeakDecode(m, &opts)
+
+	if !opts.NoMap {
+		// There was for a long time a discrepancy between documentation and
+		// implementation for the noMap property, so we need to support both
+		// camel and snake case.
+		opts.NoMap = cast.ToBool(m["no-map"])
+	}
+
+	return
+}
+
+// Client is the client used to do PostCSS transformations.
+type Client struct {
+	rs *resources.Spec
+}
+
+// Process transforms the given Resource with the PostCSS processor.
+func (c *Client) Process(res resources.ResourceTransformer, options Options) (resource.Resource, error) {
+	return res.Transform(&postcssTransformation{rs: c.rs, options: options})
+}
+
 // Some of the options from https://github.com/postcss/postcss-cli
 type Options struct {
 
@@ -68,22 +106,6 @@ type Options struct {
 	Syntax      string // Custom postcss syntax
 }
 
-func DecodeOptions(m map[string]interface{}) (opts Options, err error) {
-	if m == nil {
-		return
-	}
-	err = mapstructure.WeakDecode(m, &opts)
-
-	if !opts.NoMap {
-		// There was for a long time a discrepancy between documentation and
-		// implementation for the noMap property, so we need to support both
-		// camel and snake case.
-		opts.NoMap = cast.ToBool(m["no-map"])
-	}
-
-	return
-}
-
 func (opts Options) toArgs() []string {
 	var args []string
 	if opts.NoMap {
@@ -104,16 +126,6 @@ func (opts Options) toArgs() []string {
 	return args
 }
 
-// Client is the client used to do PostCSS transformations.
-type Client struct {
-	rs *resources.Spec
-}
-
-// New creates a new Client with the given specification.
-func New(rs *resources.Spec) *Client {
-	return &Client{rs: rs}
-}
-
 type postcssTransformation struct {
 	options Options
 	rs      *resources.Spec
@@ -186,8 +198,10 @@ func (t *postcssTransformation) Transform(ctx *resources.ResourceTransformationC
 
 	cmd := exec.Command(binary, cmdArgs...)
 
+	var errBuf bytes.Buffer
+
 	cmd.Stdout = ctx.To
-	cmd.Stderr = os.Stderr
+	cmd.Stderr = io.MultiWriter(os.Stderr, &errBuf)
 	// TODO(bep) somehow generalize this to other external helpers that may need this.
 	env := os.Environ()
 	config.SetEnvVars(&env, "HUGO_ENVIRONMENT", t.rs.Cfg.GetString("environment"))
@@ -199,9 +213,16 @@ func (t *postcssTransformation) Transform(ctx *resources.ResourceTransformationC
 	}
 
 	src := ctx.From
+
+	imp := newImportResolver(
+		ctx.From,
+		ctx.InPath,
+		t.rs.Assets.Fs, t.rs.Logger,
+	)
+
 	if t.options.InlineImports {
 		var err error
-		src, err = t.inlineImports(ctx)
+		src, err = imp.resolve()
 		if err != nil {
 			return err
 		}
@@ -214,69 +235,99 @@ func (t *postcssTransformation) Transform(ctx *resources.ResourceTransformationC
 
 	err = cmd.Run()
 	if err != nil {
-		return err
+		return imp.toFileError(errBuf.String())
 	}
 
 	return nil
 }
 
-func (t *postcssTransformation) inlineImports(ctx *resources.ResourceTransformationCtx) (io.Reader, error) {
+type fileOffset struct {
+	Filename string
+	Offset   int
+}
 
-	const importIdentifier = "@import"
+type importResolver struct {
+	r      io.Reader
+	inPath string
 
-	// Set of content hashes.
-	contentSeen := make(map[string]bool)
+	contentSeen map[string]bool
+	linemap     map[int]fileOffset
+	fs          afero.Fs
+	logger      *loggers.Logger
+}
 
-	content, err := ioutil.ReadAll(ctx.From)
-	if err != nil {
-		return nil, err
+func newImportResolver(r io.Reader, inPath string, 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),
 	}
+}
 
-	contents := string(content)
-
-	newContent, err := t.importRecursive(contentSeen, contents, ctx.InPath)
+func (imp *importResolver) contentHash(filename string) ([]byte, string) {
+	b, err := afero.ReadFile(imp.fs, filename)
 	if err != nil {
-		return nil, err
+		return nil, ""
 	}
-
-	return strings.NewReader(newContent), nil
-
+	h := sha256.New()
+	h.Write(b)
+	return b, hex.EncodeToString(h.Sum(nil))
 }
 
-func (t *postcssTransformation) importRecursive(
-	contentSeen map[string]bool,
+func (imp *importResolver) importRecursive(
+	lineNum int,
 	content string,
-	inPath string) (string, error) {
+	inPath string) (int, string, error) {
 
 	basePath := path.Dir(inPath)
 
 	var replacements []string
 	lines := strings.Split(content, "\n")
 
-	for _, line := range lines {
+	trackLine := func(i, offset int, line string) {
+		// TODO(bep) this is not very efficient.
+		imp.linemap[i+lineNum] = fileOffset{Filename: inPath, Offset: offset}
+	}
+
+	i := 0
+	for offset, line := range lines {
+		i++
 		line = strings.TrimSpace(line)
-		if shouldImport(line) {
+
+		if !imp.shouldImport(line) {
+			trackLine(i, offset, line)
+		} else {
+			i--
 			path := strings.Trim(strings.TrimPrefix(line, importIdentifier), " \"';")
 			filename := filepath.Join(basePath, path)
-			importContent, hash := t.contentHash(filename)
+			importContent, hash := imp.contentHash(filename)
 			if importContent == nil {
-				t.rs.Logger.WARN.Printf("postcss: Failed to resolve CSS @import in %q for path %q", inPath, filename)
+				trackLine(i, offset, "ERROR")
+				imp.logger.WARN.Printf("postcss: Failed to resolve CSS @import in %q for path %q", inPath, filename)
 				continue
 			}
 
-			if contentSeen[hash] {
+			if imp.contentSeen[hash] {
+				i++
 				// Just replace the line with an empty string.
 				replacements = append(replacements, []string{line, ""}...)
+				trackLine(i, offset, "IMPORT")
 				continue
 			}
 
-			contentSeen[hash] = true
+			imp.contentSeen[hash] = true
 
 			// Handle recursive imports.
-			nested, err := t.importRecursive(contentSeen, string(importContent), filepath.ToSlash(filename))
+			l, nested, err := imp.importRecursive(i+lineNum, string(importContent), filepath.ToSlash(filename))
 			if err != nil {
-				return "", err
+				return 0, "", err
 			}
+
+			trackLine(i, offset, line)
+
+			i += l
+
 			importContent = []byte(nested)
 
 			replacements = append(replacements, []string{line, string(importContent)}...)
@@ -288,25 +339,27 @@ func (t *postcssTransformation) importRecursive(
 		content = repl.Replace(content)
 	}
 
-	return content, nil
+	return i, content, nil
 }
 
-func (t *postcssTransformation) contentHash(filename string) ([]byte, string) {
-	b, err := afero.ReadFile(t.rs.Assets.Fs, filename)
+func (imp *importResolver) resolve() (io.Reader, error) {
+	const importIdentifier = "@import"
+
+	content, err := ioutil.ReadAll(imp.r)
 	if err != nil {
-		return nil, ""
+		return nil, err
 	}
-	h := sha256.New()
-	h.Write(b)
-	return b, hex.EncodeToString(h.Sum(nil))
-}
 
-// Process transforms the given Resource with the PostCSS processor.
-func (c *Client) Process(res resources.ResourceTransformer, options Options) (resource.Resource, error) {
-	return res.Transform(&postcssTransformation{rs: c.rs, options: options})
-}
+	contents := string(content)
 
-var shouldImportRe = regexp.MustCompile(`^@import ["'].*["'];?\s*(/\*.*\*/)?$`)
+	_, newContent, err := imp.importRecursive(0, contents, imp.inPath)
+	if err != nil {
+		return nil, err
+	}
+
+	return strings.NewReader(newContent), nil
+
+}
 
 // See https://www.w3schools.com/cssref/pr_import_rule.asp
 // We currently only support simple file imports, no urls, no media queries.
@@ -315,7 +368,7 @@ var shouldImportRe = regexp.MustCompile(`^@import ["'].*["'];?\s*(/\*.*\*/)?$`)
 // This is not:
 //     @import url("navigation.css");
 //     @import "mobstyle.css" screen and (max-width: 768px);
-func shouldImport(s string) bool {
+func (imp *importResolver) shouldImport(s string) bool {
 	if !strings.HasPrefix(s, importIdentifier) {
 		return false
 	}
@@ -325,3 +378,38 @@ func shouldImport(s string) bool {
 
 	return shouldImportRe.MatchString(s)
 }
+
+func (imp *importResolver) toFileError(output string) error {
+	inErr := errors.New(strings.TrimSpace(output))
+
+	match := cssSyntaxErrorRe.FindStringSubmatch(output)
+	if match == nil {
+		return inErr
+	}
+
+	lineNum, err := strconv.Atoi(match[1])
+	if err != nil {
+		return inErr
+	}
+
+	file, ok := imp.linemap[lineNum]
+	if !ok {
+		return inErr
+	}
+
+	fi, err := imp.fs.Stat(file.Filename)
+	if err != nil {
+		return inErr
+	}
+	realFilename := fi.(hugofs.FileMetaInfo).Meta().Filename()
+
+	ferr := herrors.NewFileError("css", -1, file.Offset+1, 1, inErr)
+
+	werr, ok := herrors.WithFileContextForFile(ferr, realFilename, file.Filename, imp.fs, herrors.SimpleLineMatcher)
+
+	if !ok {
+		return ferr
+	}
+
+	return werr
+}
diff --git a/resources/resource_transformers/postcss/postcss_test.go b/resources/resource_transformers/postcss/postcss_test.go
index 02c0ecb55..a49487c97 100644
--- a/resources/resource_transformers/postcss/postcss_test.go
+++ b/resources/resource_transformers/postcss/postcss_test.go
@@ -14,8 +14,17 @@
 package postcss
 
 import (
+	"regexp"
+	"strings"
 	"testing"
 
+	"github.com/gohugoio/hugo/htesting/hqt"
+
+	"github.com/gohugoio/hugo/common/loggers"
+	"github.com/gohugoio/hugo/helpers"
+
+	"github.com/spf13/afero"
+
 	qt "github.com/frankban/quicktest"
 )
 
@@ -40,6 +49,7 @@ func TestDecodeOptions(t *testing.T) {
 
 func TestShouldImport(t *testing.T) {
 	c := qt.New(t)
+	var imp *importResolver
 
 	for _, test := range []struct {
 		input  string
@@ -52,6 +62,106 @@ func TestShouldImport(t *testing.T) {
 		{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},
 	} {
-		c.Assert(shouldImport(test.input), qt.Equals, test.expect)
+		c.Assert(imp.shouldImport(test.input), qt.Equals, test.expect)
+	}
+}
+
+func TestImportResolver(t *testing.T) {
+	c := qt.New(t)
+	fs := afero.NewMemMapFs()
+
+	writeFile := func(name, content string) {
+		c.Assert(afero.WriteFile(fs, name, []byte(content), 0777), qt.IsNil)
+	}
+
+	writeFile("a.css", `@import "b.css";
+@import "c.css";
+A_STYLE1
+A_STYLE2
+`)
+
+	writeFile("b.css", `B_STYLE`)
+	writeFile("c.css", "@import \"d.css\"\nC_STYLE")
+	writeFile("d.css", "@import \"a.css\"\n\nD_STYLE")
+	writeFile("e.css", "E_STYLE")
+
+	mainStyles := strings.NewReader(`@import "a.css";
+@import "b.css";
+LOCAL_STYLE
+@import "c.css";
+@import "e.css";
+@import "missing.css";`)
+
+	imp := newImportResolver(
+		mainStyles,
+		"styles.css",
+		fs, loggers.NewErrorLogger(),
+	)
+
+	r, err := imp.resolve()
+	c.Assert(err, qt.IsNil)
+	rs := helpers.ReaderToString(r)
+	result := regexp.MustCompile(`\n+`).ReplaceAllString(rs, "\n")
+
+	c.Assert(result, hqt.IsSameString, `B_STYLE
+D_STYLE
+C_STYLE
+A_STYLE1
+A_STYLE2
+LOCAL_STYLE
+E_STYLE
+@import "missing.css";`)
+
+	dline := imp.linemap[3]
+	c.Assert(dline, qt.DeepEquals, fileOffset{
+		Offset:   1,
+		Filename: "d.css",
+	})
+
+}
+
+func BenchmarkImportResolver(b *testing.B) {
+	c := qt.New(b)
+	fs := afero.NewMemMapFs()
+
+	writeFile := func(name, content string) {
+		c.Assert(afero.WriteFile(fs, name, []byte(content), 0777), qt.IsNil)
+	}
+
+	writeFile("a.css", `@import "b.css";
+@import "c.css";
+A_STYLE1
+A_STYLE2
+`)
+
+	writeFile("b.css", `B_STYLE`)
+	writeFile("c.css", "@import \"d.css\"\nC_STYLE"+strings.Repeat("\nSTYLE", 12))
+	writeFile("d.css", "@import \"a.css\"\n\nD_STYLE"+strings.Repeat("\nSTYLE", 55))
+	writeFile("e.css", "E_STYLE")
+
+	mainStyles := `@import "a.css";
+@import "b.css";
+LOCAL_STYLE
+@import "c.css";
+@import "e.css";
+@import "missing.css";`
+
+	logger := loggers.NewErrorLogger()
+
+	for i := 0; i < b.N; i++ {
+		b.StopTimer()
+		imp := newImportResolver(
+			strings.NewReader(mainStyles),
+			"styles.css",
+			fs, logger,
+		)
+
+		b.StartTimer()
+
+		_, err := imp.resolve()
+		if err != nil {
+			b.Fatal(err)
+		}
+
 	}
 }
diff --git a/resources/testhelpers_test.go b/resources/testhelpers_test.go
index 5fab0eca0..87652a00f 100644
--- a/resources/testhelpers_test.go
+++ b/resources/testhelpers_test.go
@@ -90,7 +90,7 @@ func newTestResourceSpec(desc specDescriptor) *Spec {
 	filecaches, err := filecache.NewCaches(s)
 	c.Assert(err, qt.IsNil)
 
-	spec, err := NewSpec(s, filecaches, nil, output.DefaultFormats, media.DefaultTypes)
+	spec, err := NewSpec(s, filecaches, nil, nil, output.DefaultFormats, media.DefaultTypes)
 	c.Assert(err, qt.IsNil)
 	return spec
 }
@@ -129,7 +129,7 @@ func newTestResourceOsFs(c *qt.C) (*Spec, string) {
 	filecaches, err := filecache.NewCaches(s)
 	c.Assert(err, qt.IsNil)
 
-	spec, err := NewSpec(s, filecaches, nil, output.DefaultFormats, media.DefaultTypes)
+	spec, err := NewSpec(s, filecaches, nil, nil, output.DefaultFormats, media.DefaultTypes)
 	c.Assert(err, qt.IsNil)
 
 	return spec, workDir
diff --git a/resources/transform.go b/resources/transform.go
index 8d8eb0303..e88307afe 100644
--- a/resources/transform.go
+++ b/resources/transform.go
@@ -28,6 +28,7 @@ import (
 
 	bp "github.com/gohugoio/hugo/bufferpool"
 
+	"github.com/gohugoio/hugo/common/herrors"
 	"github.com/gohugoio/hugo/common/hugio"
 	"github.com/gohugoio/hugo/common/maps"
 	"github.com/gohugoio/hugo/helpers"
@@ -392,16 +393,24 @@ func (r *resourceAdapter) transform(publish, setContent bool) error {
 			}
 		}
 
-		notAvailableErr := func(err error) error {
-			errMsg := err.Error()
-			if tr.Key().Name == "postcss" {
-				// This transformation is not available in this
-				// Most likely because PostCSS is not installed.
-				errMsg += ". Check your PostCSS installation; install with \"npm install postcss-cli\". See https://gohugo.io/hugo-pipes/postcss/"
-			} else if tr.Key().Name == "tocss" {
-				errMsg += ". Check your Hugo installation; you need the extended version to build SCSS/SASS."
+		newErr := func(err error) error {
+
+			msg := fmt.Sprintf("%s: failed to transform %q (%s)", strings.ToUpper(tr.Key().Name), tctx.InPath, tctx.InMediaType.Type())
+
+			if err == herrors.ErrFeatureNotAvailable {
+				var errMsg string
+				if tr.Key().Name == "postcss" {
+					// This transformation is not available in this
+					// Most likely because PostCSS is not installed.
+					errMsg = ". Check your PostCSS installation; install with \"npm install postcss-cli\". See https://gohugo.io/hugo-pipes/postcss/"
+				} else if tr.Key().Name == "tocss" {
+					errMsg = ". Check your Hugo installation; you need the extended version to build SCSS/SASS."
+				}
+
+				return errors.New(msg + errMsg)
 			}
-			return fmt.Errorf("%s: failed to transform %q (%s): %s", strings.ToUpper(tr.Key().Name), tctx.InPath, tctx.InMediaType.Type(), errMsg)
+
+			return errors.Wrap(err, msg)
 
 		}
 
@@ -411,18 +420,22 @@ func (r *resourceAdapter) transform(publish, setContent bool) error {
 			tryFileCache = true
 		} else {
 			err = tr.Transform(tctx)
+			if err != nil && err != herrors.ErrFeatureNotAvailable {
+				return newErr(err)
+			}
+
 			if mayBeCachedOnDisk {
 				tryFileCache = r.spec.BuildConfig.UseResourceCache(err)
 			}
 			if err != nil && !tryFileCache {
-				return notAvailableErr(err)
+				return newErr(err)
 			}
 		}
 
 		if tryFileCache {
 			f := r.target.tryTransformedFileCache(key, updates)
 			if f == nil {
-				return notAvailableErr(errors.Errorf("resource %q not found in file cache", key))
+				return newErr(errors.Errorf("resource %q not found in file cache", key))
 			}
 			transformedContentr = f
 			updates.sourceFs = cache.fileCache.Fs
@@ -525,7 +538,11 @@ func (r *resourceAdapter) initTransform(publish, setContent bool) {
 
 		r.transformationsErr = r.transform(publish, setContent)
 		if r.transformationsErr != nil {
-			r.spec.Logger.ERROR.Printf("Transformation failed: %s", r.transformationsErr)
+			if r.spec.ErrorSender != nil {
+				r.spec.ErrorSender.SendError(r.transformationsErr)
+			} else {
+				r.spec.Logger.ERROR.Printf("Transformation failed: %s", r.transformationsErr)
+			}
 		}
 	})