From 49c82b7fa7a5f5f0d73e0d844cdcfbe23f0bed50 Mon Sep 17 00:00:00 2001 From: Tommy Graves Date: Tue, 10 Sep 2024 09:33:02 -0400 Subject: [PATCH] Adds support for new user message format for runs and linting (#106) Co-authored-by: Doug Mayer --- internal/api/client.go | 30 ++++- internal/api/config.go | 44 +++++-- internal/cli/service.go | 64 ++++----- internal/cli/service_test.go | 161 +++++++++++++++++++++-- internal/messages/messages.go | 45 +++++++ internal/messages/messages_suite_test.go | 13 ++ internal/messages/messages_test.go | 34 +++++ 7 files changed, 330 insertions(+), 61 deletions(-) create mode 100644 internal/messages/messages.go create mode 100644 internal/messages/messages_suite_test.go create mode 100644 internal/messages/messages_test.go diff --git a/internal/api/client.go b/internal/api/client.go index 5e23e84..621d04b 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -7,10 +7,12 @@ import ( "io" "net/http" "net/url" + "strings" "github.com/rwx-research/mint-cli/cmd/mint/config" "github.com/rwx-research/mint-cli/internal/accesstoken" "github.com/rwx-research/mint-cli/internal/errors" + "github.com/rwx-research/mint-cli/internal/messages" ) // Client is an API Client for Mint @@ -367,15 +369,39 @@ func (c Client) GetLeafVersions() (*LeafVersionsResult, error) { return &respBody, nil } +type ErrorMessage struct { + Message string `json:"message"` + StackTrace []messages.StackEntry `json:"stack_trace,omitempty"` + Frame string `json:"frame"` + Advice string `json:"advice"` +} + // extractErrorMessage is a small helper function for parsing an API error message func extractErrorMessage(reader io.Reader) string { errorStruct := struct { - Error string + Error string `json:"error,omitempty"` + ErrorMessages []ErrorMessage `json:"error_messages,omitempty"` }{} if err := json.NewDecoder(reader).Decode(&errorStruct); err != nil { return "" } - return errorStruct.Error + if len(errorStruct.ErrorMessages) > 0 { + var message strings.Builder + for _, errorMessage := range errorStruct.ErrorMessages { + message.WriteString("\n\n") + message.WriteString(messages.FormatUserMessage(errorMessage.Message, errorMessage.Frame, errorMessage.StackTrace, errorMessage.Advice)) + } + + return message.String() + } + + // Fallback to Error field + if errorStruct.Error != "" { + return errorStruct.Error + } + + // Fallback to an empty string + return "" } diff --git a/internal/api/config.go b/internal/api/config.go index d56f159..6386d8f 100644 --- a/internal/api/config.go +++ b/internal/api/config.go @@ -7,6 +7,7 @@ import ( "github.com/rwx-research/mint-cli/internal/accesstoken" "github.com/rwx-research/mint-cli/internal/errors" + "github.com/rwx-research/mint-cli/internal/messages" ) type Config struct { @@ -70,26 +71,45 @@ func (c LintConfig) Validate() error { } type LintProblem struct { - Severity string `json:"severity"` - Message string `json:"message"` - FileName string `json:"file_name"` - Line NullInt `json:"line"` - Column NullInt `json:"column"` - Advice string `json:"advice"` + Severity string `json:"severity"` + Message string `json:"message"` + FileName string `json:"file_name"` + Line NullInt `json:"line"` + Column NullInt `json:"column"` + Advice string `json:"advice"` + StackTrace []messages.StackEntry `json:"stack_trace,omitempty"` + Frame string `json:"frame"` } func (lf LintProblem) FileLocation() string { - if len(lf.FileName) > 0 { + fileName := lf.FileName + line := lf.Line + column := lf.Column + + if (len(lf.StackTrace) > 0) { + lastStackEntry := lf.StackTrace[len(lf.StackTrace) - 1] + fileName = lastStackEntry.FileName + line = NullInt{ + Value: lastStackEntry.Line, + IsNull: false, + } + column = NullInt{ + Value: lastStackEntry.Column, + IsNull: false, + } + } + + if len(fileName) > 0 { var buf bytes.Buffer w := io.Writer(&buf) - fmt.Fprint(w, lf.FileName) + fmt.Fprint(w, fileName) - if !lf.Line.IsNull { - fmt.Fprintf(w, ":%d", lf.Line.Value) + if !line.IsNull { + fmt.Fprintf(w, ":%d", line.Value) } - if !lf.Column.IsNull { - fmt.Fprintf(w, ":%d", lf.Column.Value) + if !column.IsNull { + fmt.Fprintf(w, ":%d", column.Value) } return buf.String() diff --git a/internal/cli/service.go b/internal/cli/service.go index bef273d..f6b1670 100644 --- a/internal/cli/service.go +++ b/internal/cli/service.go @@ -1,7 +1,6 @@ package cli import ( - "cmp" "encoding/json" "fmt" "io" @@ -17,6 +16,7 @@ import ( "github.com/rwx-research/mint-cli/internal/api" "github.com/rwx-research/mint-cli/internal/dotenv" "github.com/rwx-research/mint-cli/internal/errors" + "github.com/rwx-research/mint-cli/internal/messages" "github.com/briandowns/spinner" "golang.org/x/crypto/ssh" @@ -158,7 +158,7 @@ func (s Service) InitiateRun(cfg InitiateRunConfig) (*api.InitiateRunResult, err UseCache: !cfg.NoCache, }) if err != nil { - return nil, errors.Wrap(err, "unable to initiate run") + return nil, errors.Wrap(err, "Failed to initiate run") } return runResult, nil @@ -258,9 +258,9 @@ func (s Service) Lint(cfg LintConfig) (*api.LintResult, error) { switch cfg.OutputFormat { case LintOutputOneLine: - err = outputLintOneLine(cfg.Output, sortLintProblems(lintResult.Problems)) + err = outputLintOneLine(cfg.Output, lintResult.Problems) case LintOutputMultiLine: - err = outputLintMultiLine(cfg.Output, sortLintProblems(lintResult.Problems), len(targetPaths)) + err = outputLintMultiLine(cfg.Output, lintResult.Problems, len(targetPaths)) } if err != nil { return nil, errors.Wrap(err, "unable to output lint results") @@ -270,32 +270,40 @@ func (s Service) Lint(cfg LintConfig) (*api.LintResult, error) { } func outputLintMultiLine(w io.Writer, problems []api.LintProblem, fileCount int) error { - for i, lf := range problems { - if i > 0 { + for _, lf := range problems { + fmt.Fprintln(w) + + if len(lf.StackTrace) > 0 { + fmt.Fprint(w, "[", lf.Severity, "] ") + fmt.Fprintln(w, messages.FormatUserMessage(lf.Message, lf.Frame, lf.StackTrace, lf.Advice)) + } else { + if fileLoc := lf.FileLocation(); len(fileLoc) > 0 { + fmt.Fprint(w, fileLoc, " ") + } + fmt.Fprint(w, "[", lf.Severity, "]") fmt.Fprintln(w) - } - if fileLoc := lf.FileLocation(); len(fileLoc) > 0 { - fmt.Fprint(w, fileLoc, " ") - } - fmt.Fprint(w, "[", lf.Severity, "]") - fmt.Fprintln(w) + fmt.Fprint(w, lf.Message) - fmt.Fprint(w, lf.Message) + if len(lf.Advice) > 0 { + fmt.Fprint(w, "\n", lf.Advice) + } - if len(lf.Advice) > 0 { - fmt.Fprint(w, "\n", lf.Advice) + fmt.Fprintln(w) } - - fmt.Fprintln(w) } - pluralized := "problems" + pluralizedProblems := "problems" if len(problems) == 1 { - pluralized = "problem" + pluralizedProblems = "problem" + } + + pluralizedFiles := "files" + if fileCount == 1 { + pluralizedFiles = "file" } - fmt.Fprintf(w, "\nChecked %d files and found %d %s.\n", fileCount, len(problems), pluralized) + fmt.Fprintf(w, "\nChecked %d %s and found %d %s.\n", fileCount, pluralizedFiles, len(problems), pluralizedProblems) return nil } @@ -319,22 +327,6 @@ func outputLintOneLine(w io.Writer, lintedFiles []api.LintProblem) error { return nil } -func sortLintProblems(problems []api.LintProblem) []api.LintProblem { - sortedProblems := append([]api.LintProblem(nil), problems...) - slices.SortFunc(sortedProblems, func(a, b api.LintProblem) int { - if n := cmp.Compare(a.FileName, b.FileName); n != 0 { - return n - } - - if n := cmp.Compare(a.Line.Value, b.Line.Value); n != 0 { - return n - } - - return cmp.Compare(a.Column.Value, b.Column.Value) - }) - return sortedProblems -} - // InitiateRun will connect to the Cloud API and start a new run in Mint. func (s Service) Login(cfg LoginConfig) error { err := cfg.Validate() diff --git a/internal/cli/service_test.go b/internal/cli/service_test.go index 69aad0b..8a88a4a 100644 --- a/internal/cli/service_test.go +++ b/internal/cli/service_test.go @@ -19,6 +19,7 @@ import ( "github.com/rwx-research/mint-cli/internal/errors" "github.com/rwx-research/mint-cli/internal/fs" "github.com/rwx-research/mint-cli/internal/memoryfs" + "github.com/rwx-research/mint-cli/internal/messages" "github.com/rwx-research/mint-cli/internal/mocks" "golang.org/x/crypto/ssh" @@ -1452,13 +1453,13 @@ AAAEC6442PQKevgYgeT0SIu9zwlnEMl6MF59ZgM+i0ByMv4eLJPqG3xnZcEQmktHj/GY2i lintConfig.OutputFormat = cli.LintOutputOneLine }) - It("orders and lists only files", func() { + It("lists only files", func() { _, err := service.Lint(lintConfig) Expect(err).NotTo(HaveOccurred()) - Expect(stdout.String()).To(Equal(`warning mint1.yml - message 4 -warning mint1.yml:2:6 - message 3 -error mint1.yml:11:22 - message 1 message 1a + Expect(stdout.String()).To(Equal(`error mint1.yml:11:22 - message 1 message 1a error mint1.yml:15:4 - message 2 message 2a +warning mint1.yml:2:6 - message 3 +warning mint1.yml - message 4 `)) }) }) @@ -1468,26 +1469,164 @@ error mint1.yml:15:4 - message 2 message 2a lintConfig.OutputFormat = cli.LintOutputMultiLine }) - It("orders and lists only files", func() { + It("lists all the data from the problem", func() { _, err := service.Lint(lintConfig) Expect(err).NotTo(HaveOccurred()) - Expect(stdout.String()).To(Equal(`mint1.yml [warning] -message 4 + Expect(stdout.String()).To(Equal(` +mint1.yml:11:22 [error] +message 1 +message 1a +advice 1 +advice 1a + +mint1.yml:15:4 [error] +message 2 +message 2a mint1.yml:2:6 [warning] message 3 advice 3 advice 3a -mint1.yml:11:22 [error] -message 1 +mint1.yml [warning] +message 4 + +Checked 2 files and found 4 problems. +`)) + }) + }) + + Context("using none output", func() { + BeforeEach(func() { + lintConfig.OutputFormat = cli.LintOutputNone + }) + + It("doesn't output", func() { + _, err := service.Lint(lintConfig) + Expect(err).NotTo(HaveOccurred()) + Expect(stdout.String()).To(Equal("")) + }) + }) + }) + + Context("with multiple errors including stack traces", func() { + BeforeEach(func() { + Expect(memfs.WriteFileString("mint1.yml", "mint1 contents")).NotTo(HaveOccurred()) + Expect(memfs.WriteFileString(".mint/base.yml", ".mint/base.yml contents")).NotTo(HaveOccurred()) + Expect(memfs.WriteFileString(".mint/base.json", ".mint/base.json contents")).NotTo(HaveOccurred()) + + lintConfig.MintFilePaths = []string{"mint1.yml", ".mint/base.yml"} + + mockAPI.MockLint = func(cfg api.LintConfig) (*api.LintResult, error) { + Expect(cfg.TaskDefinitions).To(HaveLen(2)) + return &api.LintResult{ + Problems: []api.LintProblem{ + { + Severity: "error", + Message: "message 1\nmessage 1a", + StackTrace: []messages.StackEntry{ + { + FileName: "mint1.yml", + Line: 11, + Column: 22, + }, + }, + Frame: " 4 | run: echo hi\n> 5 | bad: true\n | ^\n 6 | env:\n 7 | A:", + Advice: "advice 1\nadvice 1a", + }, + { + Severity: "error", + Message: "message 2\nmessage 2a", + StackTrace: []messages.StackEntry{ + { + FileName: "mint1.yml", + Line: 22, + Column: 11, + Name: "*alias", + }, + { + FileName: "mint1.yml", + Line: 5, + Column: 22, + }, + }, + }, + { + Severity: "warning", + Message: "message 3", + StackTrace: []messages.StackEntry{ + { + FileName: "mint1.yml", + Line: 2, + Column: 6, + }, + }, + Advice: "advice 3\nadvice 3a", + }, + { + Severity: "warning", + Message: "message 4", + StackTrace: []messages.StackEntry{ + { + FileName: "mint1.yml", + Line: 7, + Column: 9, + }, + }, + }, + }, + }, nil + } + }) + + Context("using oneline output", func() { + BeforeEach(func() { + lintConfig.OutputFormat = cli.LintOutputOneLine + }) + + It("lists only files", func() { + _, err := service.Lint(lintConfig) + Expect(err).NotTo(HaveOccurred()) + Expect(stdout.String()).To(Equal(`error mint1.yml:11:22 - message 1 message 1a +error mint1.yml:5:22 - message 2 message 2a +warning mint1.yml:2:6 - message 3 +warning mint1.yml:7:9 - message 4 +`)) + }) + }) + + Context("using multiline output", func() { + BeforeEach(func() { + lintConfig.OutputFormat = cli.LintOutputMultiLine + }) + + It("lists all the data from the problem", func() { + _, err := service.Lint(lintConfig) + Expect(err).NotTo(HaveOccurred()) + Expect(stdout.String()).To(Equal(` +[error] message 1 message 1a + 4 | run: echo hi +> 5 | bad: true + | ^ + 6 | env: + 7 | A: + at mint1.yml:11:22 advice 1 advice 1a -mint1.yml:15:4 [error] -message 2 +[error] message 2 message 2a + at mint1.yml:5:22 + at *alias (mint1.yml:22:11) + +[warning] message 3 + at mint1.yml:2:6 +advice 3 +advice 3a + +[warning] message 4 + at mint1.yml:7:9 Checked 2 files and found 4 problems. `)) diff --git a/internal/messages/messages.go b/internal/messages/messages.go new file mode 100644 index 0000000..362ec1b --- /dev/null +++ b/internal/messages/messages.go @@ -0,0 +1,45 @@ +package messages + +import ( + "fmt" + "strings" +) + +type StackEntry struct { + FileName string `json:"file_name"` + Line int `json:"line"` + Column int `json:"column"` + Name string `json:"name"` +} + +func FormatUserMessage(message string, frame string, stackTrace []StackEntry, advice string) string { + var builder strings.Builder + + if (message != "") { + builder.WriteString(message) + } + + if (frame != "") { + builder.WriteString("\n") + builder.WriteString(frame) + } + + if (len(stackTrace) > 0) { + for i := len(stackTrace) - 1; i >= 0; i-- { + stackEntry := stackTrace[i] + builder.WriteString("\n") + if (stackEntry.Name != "") { + builder.WriteString(fmt.Sprintf(" at %s (%s:%d:%d)", stackEntry.Name, stackEntry.FileName, stackEntry.Line, stackEntry.Column)) + } else { + builder.WriteString(fmt.Sprintf(" at %s:%d:%d", stackEntry.FileName, stackEntry.Line, stackEntry.Column)) + } + } + } + + if (advice != "") { + builder.WriteString("\n") + builder.WriteString(advice) + } + + return builder.String() +} diff --git a/internal/messages/messages_suite_test.go b/internal/messages/messages_suite_test.go new file mode 100644 index 0000000..2f84bd8 --- /dev/null +++ b/internal/messages/messages_suite_test.go @@ -0,0 +1,13 @@ +package messages_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestCli(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Messages Suite") +} diff --git a/internal/messages/messages_test.go b/internal/messages/messages_test.go new file mode 100644 index 0000000..40d90d7 --- /dev/null +++ b/internal/messages/messages_test.go @@ -0,0 +1,34 @@ +package messages_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + messages "github.com/rwx-research/mint-cli/internal/messages" +) + +var _ = Describe("FormatUserMessage", func() { + It("builds a string based on the available data", func() { + Expect(messages.FormatUserMessage("message", "", []messages.StackEntry{}, "")).To(Equal("message")) + Expect(messages.FormatUserMessage("message", "frame", []messages.StackEntry{}, "")).To(Equal("message\nframe")) + Expect(messages.FormatUserMessage("message", "frame", []messages.StackEntry{}, "advice")).To(Equal("message\nframe\nadvice")) + + stackTrace := []messages.StackEntry{ + { + FileName: "mint1.yml", + Line: 22, + Column: 11, + Name: "*alias", + }, + { + FileName: "mint1.yml", + Line: 5, + Column: 22, + }, + } + Expect(messages.FormatUserMessage("message", "frame", stackTrace, "advice")).To(Equal(`message +frame + at mint1.yml:5:22 + at *alias (mint1.yml:22:11) +advice`)) + }) +})