Linter: Recover panics (grafana#899)

* Linter: Recover panics
Currently, the linter is useful but panics on a few projects. The panics make it unusable on a large set of projects

With this PR, it will fail linting but all projects will still get linted

* Make parallelism <= 0 into an error

pkg/jsonnet/lint.go

@@ -3,6 +3,7 @@ package jsonnet
 import (
 	"bytes"
 	"fmt"
+	"io"
 	"os"
 	"path/filepath"
 	"time"
@@ -22,11 +23,21 @@ type LintOpts struct {
 
 	// Parallelism determines the number of workers that will process files
 	Parallelism int
+
+	Out io.Writer
 }
 
 // Lint takes a list of files and directories, processes them and prints
 // out to stderr if there are linting warnings
 func Lint(fds []string, opts *LintOpts) error {
+	if opts.Parallelism <= 0 {
+		return errors.New("parallelism must be greater than 0")
+	}
+
+	if opts.Out == nil {
+		opts.Out = os.Stdout
+	}
+
 	var paths []string
 	for _, f := range fds {
 		fs, err := FindFiles(f, opts.Excludes)
@@ -37,39 +48,15 @@ func Lint(fds []string, opts *LintOpts) error {
 	}
 
 	type result struct {
-		failed bool
-		output string
+		success bool
+		output  string
 	}
 	fileCh := make(chan string, len(paths))
 	resultCh := make(chan result, len(paths))
 	lintWorker := func(fileCh <-chan string, resultCh chan result) {
 		for file := range fileCh {
-			buf := &bytes.Buffer{}
-			var err error
-			file, err = filepath.Abs(file)
-			if err != nil {
-				fmt.Fprintf(buf, "got an error getting the absolute path for %s: %v\n\n", file, err)
-				resultCh <- result{failed: true, output: buf.String()}
-				continue
-			}
-
-			log.Debug().Str("file", file).Msg("linting file")
-			startTime := time.Now()
-
-			vm := MakeVM(Opts{})
-			jpaths, _, _, err := jpath.Resolve(file, true)
-			if err != nil {
-				fmt.Fprintf(buf, "got an error getting JPATH for %s: %v\n\n", file, err)
-				resultCh <- result{failed: true, output: buf.String()}
-				continue
-			}
-
-			vm.Importer(NewExtendedImporter(jpaths))
-
-			content, _ := os.ReadFile(file)
-			failed := linter.LintSnippet(vm, buf, []linter.Snippet{{FileName: file, Code: string(content)}})
-			resultCh <- result{failed: failed, output: buf.String()}
-			log.Debug().Str("file", file).Dur("duration_ms", time.Since(startTime)).Msg("linted file")
+			buf, success := lintWithRecover(file)
+			resultCh <- result{success: success, output: buf.String()}
 		}
 	}
 
@@ -85,9 +72,9 @@ func Lint(fds []string, opts *LintOpts) error {
 	lintingFailed := false
 	for i := 0; i < len(paths); i++ {
 		result := <-resultCh
-		lintingFailed = lintingFailed || result.failed
+		lintingFailed = lintingFailed || !result.success
 		if result.output != "" {
-			fmt.Print(result.output)
+			fmt.Fprint(opts.Out, result.output)
 		}
 	}
 
@@ -96,3 +83,37 @@ func Lint(fds []string, opts *LintOpts) error {
 	}
 	return nil
 }
+
+func lintWithRecover(file string) (buf bytes.Buffer, success bool) {
+	file, err := filepath.Abs(file)
+	if err != nil {
+		fmt.Fprintf(&buf, "got an error getting the absolute path for %s: %v\n\n", file, err)
+		return
+	}
+
+	log.Debug().Str("file", file).Msg("linting file")
+	startTime := time.Now()
+	defer func() {
+		if err := recover(); err != nil {
+			fmt.Fprintf(&buf, "caught a panic while linting %s: %v\n\n", file, err)
+		}
+		log.Debug().Str("file", file).Dur("duration_ms", time.Since(startTime)).Msg("linted file")
+	}()
+
+	content, err := os.ReadFile(file)
+	if err != nil {
+		fmt.Fprintf(&buf, "got an error reading file %s: %v\n\n", file, err)
+		return
+	}
+
+	vm := MakeVM(Opts{})
+	jpaths, _, _, err := jpath.Resolve(file, true)
+	if err != nil {
+		fmt.Fprintf(&buf, "got an error getting jpath for %s: %v\n\n", file, err)
+		return
+	}
+
+	vm.Importer(NewExtendedImporter(jpaths))
+	failed := linter.LintSnippet(vm, &buf, []linter.Snippet{{FileName: file, Code: string(content)}})
+	return buf, !failed
+}

pkg/jsonnet/lint_test.go

@@ -0,0 +1,29 @@
+package jsonnet
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestLint(t *testing.T) {
+	t.Run("no error", func(t *testing.T) {
+		opts := &LintOpts{Parallelism: 4}
+		err := Lint([]string{"testdata/importTree"}, opts)
+		assert.NoError(t, err)
+	})
+
+	t.Run("error", func(t *testing.T) {
+		buf := &bytes.Buffer{}
+		opts := &LintOpts{Out: buf, Parallelism: 4}
+		err := Lint([]string{"testdata/lintingError"}, opts)
+		assert.EqualError(t, err, "Linting has failed for at least one file")
+		assert.Equal(t, absPath(t, "testdata/lintingError/main.jsonnet")+`:1:7-22 Unused variable: unused
+
+local unused = 'test';
+
+
+`, buf.String())
+	})
+}

pkg/jsonnet/testdata/lintingError/jsonnetfile.json

@@ -0,0 +1 @@
+{}

pkg/jsonnet/testdata/lintingError/main.jsonnet

@@ -0,0 +1,2 @@
+local unused = 'test';
+{}