From 956e4dda7cda49dccf1fbfe6ef19a31363b36313 Mon Sep 17 00:00:00 2001
From: Scott Leggett <scott@sl.id.au>
Date: Mon, 1 Nov 2021 23:25:28 +0800
Subject: [PATCH] feat: improve error diagnosis of lint issues

---
 internal/lagoonyml/lint.go | 71 ++++++++++++++++++++++++--------------
 1 file changed, 46 insertions(+), 25 deletions(-)

diff --git a/internal/lagoonyml/lint.go b/internal/lagoonyml/lint.go
index fde2604..c29201b 100644
--- a/internal/lagoonyml/lint.go
+++ b/internal/lagoonyml/lint.go
@@ -1,6 +1,8 @@
 package lagoonyml
 
 import (
+	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 
@@ -10,19 +12,34 @@ import (
 // Linter validates the given Lagoon struct.
 type Linter func(*Lagoon) error
 
-// LintFile takes a file path, reads it, and applies `.lagoon.yml` lint policy to
-// it. Lint returns an error of type ErrLint if it finds problems with the
-// file, a regular error if something else went wrong, and nil if the
-// `.lagoon.yml` is valid.
-func LintFile(path string, linters ...Linter) error {
+func lint(rawYAML []byte, linters []Linter) error {
 	var l Lagoon
-	rawYAML, err := os.ReadFile(path)
-	if err != nil {
-		return fmt.Errorf("couldn't read %v: %v", path, err)
-	}
-	err = yaml.Unmarshal(rawYAML, &l)
-	if err != nil {
-		return fmt.Errorf("couldn't unmarshal %v: %v", path, err)
+	yamlErr := yaml.Unmarshal(rawYAML, &l)
+	if yamlErr != nil {
+		// try to extract more detail from the Unmarshal error
+		rawJSON, err := yaml.YAMLToJSON(rawYAML)
+		if err != nil {
+			// can't even convert this so return the original error
+			return yamlErr
+		}
+		// json.Unmarshal returns richer errors than yaml.Unmarshal, which helps to
+		// diagnose exactly what went wrong
+		err = json.Unmarshal(rawJSON, &l)
+		var jTypeErr *json.UnmarshalTypeError
+		if errors.As(err, &jTypeErr) {
+			// this rawJSON slice will be partial JSON, but JSONToYAML ignores junk
+			// at the end of the snippet.
+			badYAML, err := yaml.JSONToYAML(rawJSON[jTypeErr.Offset:])
+			if err != nil {
+				// can't convert this snippet, so return the original error
+				return yamlErr
+			}
+			return fmt.Errorf(
+				"couldn't unmarshal YAML: %v.\nThere appears to be invalid YAML in the `%s` field:\n\n%s",
+				yamlErr, jTypeErr.Field, badYAML)
+		}
+		// this isn't a json.UnmarshalTypeError, so just return the original error
+		return yamlErr
 	}
 	for _, linter := range linters {
 		if err := linter(&l); err != nil {
@@ -34,21 +51,25 @@ func LintFile(path string, linters ...Linter) error {
 	return nil
 }
 
-// LintYAML takes a byte slice containing raw YAML and applies `.lagoon.yml` lint policy to
-// it. Lint returns an error of type ErrLint if it finds problems with the
-// file, a regular error if something else went wrong, and nil if the
+// LintFile takes a file path, reads it, and applies `.lagoon.yml` lint policy
+// to it. LintFile returns an error of type ErrLint if it finds problems with
+// the file, a regular error if something else went wrong, and nil if the
 // `.lagoon.yml` is valid.
-func LintYAML(rawYAML []byte, linters ...Linter) error {
-	var l Lagoon
-	if err := yaml.Unmarshal(rawYAML, &l); err != nil {
-		return fmt.Errorf("couldn't unmarshal YAML: %v", err)
+func LintFile(path string, linters ...Linter) error {
+	rawYAML, err := os.ReadFile(path)
+	if err != nil {
+		return fmt.Errorf("couldn't read %v: %v", path, err)
 	}
-	for _, linter := range linters {
-		if err := linter(&l); err != nil {
-			return &ErrLint{
-				Detail: err.Error(),
-			}
-		}
+	if err = lint(rawYAML, linters); err != nil {
+		return fmt.Errorf("couldn't validate: %v: %v", path, err)
 	}
 	return nil
 }
+
+// LintYAML takes a byte slice containing raw YAML and applies `.lagoon.yml`
+// lint policy to it. LintYAML returns an error of type ErrLint if it finds
+// problems with the YAML, a regular error if something else went wrong, and
+// nil if the YAML is valid.
+func LintYAML(rawYAML []byte, linters ...Linter) error {
+	return lint(rawYAML, linters)
+}