chore: render CLI errors as TOON

cliv2/cmd/cliv2/behavior/output.go

@@ -0,0 +1,152 @@
+package behavior
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io"
+	"regexp"
+	"strings"
+
+	"github.com/snyk/error-catalog-golang-public/cli"
+	"github.com/snyk/error-catalog-golang-public/errorcodes"
+	"github.com/snyk/error-catalog-golang-public/snyk_errors"
+	"github.com/snyk/go-application-framework/pkg/configuration"
+	"github.com/snyk/go-application-framework/pkg/local_workflows/output_workflow"
+)
+
+var toonNumericString = regexp.MustCompile(`^-?\d+(?:\.\d+)?(?:[eE][+-]?\d+)?$`)
+
+type StructuredError struct {
+	Ok       bool   `json:"ok"`
+	ErrorMsg string `json:"error"`
+	Path     string `json:"path"`
+}
+
+type OutputFormat string
+
+const (
+	outputFormatTOON  OutputFormat = output_workflow.OUTPUT_CONFIG_KEY_TOON
+	outputFormatJSON  OutputFormat = output_workflow.OUTPUT_CONFIG_KEY_JSON
+	outputFormatSARIF OutputFormat = output_workflow.OUTPUT_CONFIG_KEY_SARIF
+	outputFormatHTML  OutputFormat = output_workflow.OUTPUT_CONFIG_KEY_HTML
+)
+
+var outputFormats = []OutputFormat{
+	outputFormatTOON,
+	outputFormatSARIF,
+	outputFormatJSON,
+	outputFormatHTML,
+}
+
+// A format's alternate keys (e.g. sarif accepts json as a fallback for code
+// test/secrets test, so the two are interchangeable there) are treated as one
+// selection rather than two, so that doesn't get flagged as a conflict.
+func ValidateOutputFormatSelection(command string, config configuration.Configuration) error {
+	selected := []string{command}
+	aliasedAway := map[string]bool{}
+	for _, format := range outputFormats {
+		key := string(format)
+		if aliasedAway[key] || !config.GetBool(key) {
+			continue
+		}
+
+		selected = append(selected, key)
+		if len(selected) > 2 {
+			detail := "The following option combination is not currently supported: " + strings.Join(selected, " + ")
+			return cli.NewInvalidFlagOptionError(detail)
+		}
+
+		for _, alt := range config.GetAlternativeKeys(key) {
+			aliasedAway[alt] = true
+		}
+	}
+
+	return nil
+}
+
+func SelectErrorOutputWriter(config configuration.Configuration, stdout, stderr io.Writer) io.Writer {
+	if output_workflow.DefaultOutputIsStructured(config) {
+		return stderr
+	}
+
+	return stdout
+}
+
+func StructuredErrorOutputFormat(config configuration.Configuration) (OutputFormat, bool) {
+	for _, format := range []OutputFormat{outputFormatTOON, outputFormatJSON} {
+		if config.GetBool(string(format)) {
+			return format, true
+		}
+	}
+
+	return "", false
+}
+
+func IsDataRenderingError(err error) bool {
+	var catalogError snyk_errors.Error
+	return errors.As(err, &catalogError) && catalogError.ErrorCode == errorcodes.CLI.DataRenderingError
+}
+
+func RenderStructuredError(format OutputFormat, err StructuredError) ([]byte, error) {
+	switch format {
+	case outputFormatTOON:
+		return renderTOONError(err), nil
+	case outputFormatJSON:
+		return json.MarshalIndent(err, "", "  ")
+	case outputFormatSARIF, outputFormatHTML:
+	}
+
+	return nil, fmt.Errorf("unsupported structured error output format: %s", format)
+}
+
+// ok is omitted here: this renderer only ever runs from the error path, where
+// it is always false, so it carries no information for a TOON consumer.
+func renderTOONError(err StructuredError) []byte {
+	var document strings.Builder
+	document.WriteString("error: ")
+	document.WriteString(formatTOONString(err.ErrorMsg))
+	document.WriteString("\npath: ")
+	document.WriteString(formatTOONString(err.Path))
+	return []byte(document.String())
+}
+
+func formatTOONString(value string) string {
+	needsQuotes := value == "" ||
+		strings.TrimSpace(value) != value ||
+		value == "true" || value == "false" || value == "null" ||
+		toonNumericString.MatchString(value) ||
+		strings.ContainsAny(value, ":\\\"[]{}") ||
+		strings.IndexFunc(value, func(character rune) bool { return character < 0x20 }) >= 0 ||
+		strings.HasPrefix(value, "-") ||
+		strings.HasPrefix(value, "#")
+	if !needsQuotes {
+		return value
+	}
+
+	var escaped strings.Builder
+	escaped.Grow(len(value) + 2)
+	escaped.WriteByte('"')
+	for _, character := range value {
+		switch character {
+		case '\\':
+			escaped.WriteString(`\\`)
+		case '"':
+			escaped.WriteString(`\"`)
+		case '\n':
+			escaped.WriteString(`\n`)
+		case '\r':
+			escaped.WriteString(`\r`)
+		case '\t':
+			escaped.WriteString(`\t`)
+		default:
+			if character < 0x20 {
+				escaped.WriteString(fmt.Sprintf(`\u%04x`, character))
+			} else {
+				escaped.WriteRune(character)
+			}
+		}
+	}
+	escaped.WriteByte('"')
+	return escaped.String()
+}

cliv2/cmd/cliv2/behavior/output_test.go

@@ -0,0 +1,228 @@
+package behavior
+
+import (
+	"bytes"
+	"io"
+	"os"
+	"testing"
+
+	"github.com/snyk/error-catalog-golang-public/snyk_errors"
+	"github.com/snyk/go-application-framework/pkg/configuration"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestValidateOutputFormatSelection(t *testing.T) {
+	formats := []OutputFormat{
+		outputFormatTOON,
+		outputFormatSARIF,
+		outputFormatJSON,
+		outputFormatHTML,
+	}
+
+	for _, format := range formats {
+		t.Run("allows "+string(format)+" by itself", func(t *testing.T) {
+			config := outputConfig(format)
+
+			assert.NoError(t, ValidateOutputFormatSelection("test", config))
+		})
+	}
+
+	for index, first := range formats {
+		for _, second := range formats[index+1:] {
+			t.Run("rejects "+string(first)+" with "+string(second), func(t *testing.T) {
+				config := outputConfig(first, second)
+
+				err := ValidateOutputFormatSelection("test", config)
+				require.Error(t, err)
+				var catalogError snyk_errors.Error
+				require.ErrorAs(t, err, &catalogError)
+				assert.Equal(t, "SNYK-CLI-0004", catalogError.ErrorCode)
+				assert.Equal(
+					t,
+					"The following option combination is not currently supported: test + "+string(first)+" + "+string(second),
+					catalogError.Detail,
+				)
+			})
+		}
+	}
+}
+
+func TestValidateOutputFormatSelection_ConfigFileConflict(t *testing.T) {
+	// A CLI-1828 regression case: nothing on the command line changed the flags,
+	// but the resolved config still carries both formats (e.g. from a config
+	// file or env var), so the conflict must still be caught.
+	config := configuration.NewWithOpts()
+	config.Set(string(outputFormatTOON), true)
+	config.Set(string(outputFormatJSON), true)
+
+	err := ValidateOutputFormatSelection("test", config)
+
+	require.Error(t, err)
+	var catalogError snyk_errors.Error
+	require.ErrorAs(t, err, &catalogError)
+	assert.Equal(t, "SNYK-CLI-0004", catalogError.ErrorCode)
+}
+
+func TestValidateOutputFormatSelection_RejectsConfigConflict(t *testing.T) {
+	// Exact repro from the CLI-1828 review: no flag was ever Changed, but the
+	// config file alone enables two conflicting formats.
+	t.Chdir(t.TempDir())
+	require.NoError(t, os.WriteFile(
+		"output-conflict.json",
+		[]byte(`{"toon":true,"json":true}`),
+		0o600,
+	))
+	config := configuration.NewWithOpts(configuration.WithFiles("output-conflict"))
+	require.True(t, config.GetBool(string(outputFormatTOON)))
+	require.True(t, config.GetBool(string(outputFormatJSON)))
+
+	err := ValidateOutputFormatSelection("test", config)
+
+	require.Error(t, err, "toon and json from config must conflict")
+}
+
+func TestValidateOutputFormatSelection_AllowsAliasedFormats(t *testing.T) {
+	// code test/secrets test register sarif and json as interchangeable via
+	// AddAlternativeKeys; that pair must not be flagged as a conflict.
+	config := configuration.NewWithOpts()
+	config.AddAlternativeKeys(string(outputFormatSARIF), []string{string(outputFormatJSON)})
+	config.Set(string(outputFormatJSON), true)
+
+	assert.NoError(t, ValidateOutputFormatSelection("code test", config))
+}
+
+func outputConfig(selected ...OutputFormat) configuration.Configuration {
+	config := configuration.NewWithOpts()
+	for _, format := range selected {
+		config.Set(string(format), true)
+	}
+	return config
+}
+
+func TestStructuredErrorOutputFormat(t *testing.T) {
+	for _, testCase := range []struct {
+		format   OutputFormat
+		selected bool
+	}{
+		{format: outputFormatJSON, selected: true},
+		{format: outputFormatTOON, selected: true},
+		{format: outputFormatSARIF, selected: false},
+		{format: outputFormatHTML, selected: false},
+	} {
+		t.Run(string(testCase.format), func(t *testing.T) {
+			config := configuration.NewWithOpts()
+			config.Set(string(testCase.format), true)
+
+			actual, selected := StructuredErrorOutputFormat(config)
+
+			assert.Equal(t, testCase.selected, selected)
+			if selected {
+				assert.Equal(t, testCase.format, actual)
+			}
+		})
+	}
+}
+
+func TestSelectErrorOutputWriter(t *testing.T) {
+	for _, testCase := range []struct {
+		name       string
+		selected   []OutputFormat
+		wantStderr bool
+	}{
+		{name: "no format selected (legacy human output)", selected: nil, wantStderr: false},
+		{name: "json alone", selected: []OutputFormat{outputFormatJSON}, wantStderr: true},
+		{name: "toon alone", selected: []OutputFormat{outputFormatTOON}, wantStderr: true},
+		{name: "sarif alone", selected: []OutputFormat{outputFormatSARIF}, wantStderr: true},
+		{name: "html alone", selected: []OutputFormat{outputFormatHTML}, wantStderr: true},
+		{name: "sarif and json", selected: []OutputFormat{outputFormatSARIF, outputFormatJSON}, wantStderr: true},
+		{name: "sarif and html", selected: []OutputFormat{outputFormatSARIF, outputFormatHTML}, wantStderr: true},
+	} {
+		t.Run(testCase.name, func(t *testing.T) {
+			config := configuration.NewWithOpts()
+			for _, format := range testCase.selected {
+				config.Set(string(format), true)
+			}
+			stdout := &bytes.Buffer{}
+			stderr := &bytes.Buffer{}
+
+			writer := SelectErrorOutputWriter(config, stdout, stderr)
+
+			if testCase.wantStderr {
+				assert.Same(t, io.Writer(stderr), writer)
+			} else {
+				assert.Same(t, io.Writer(stdout), writer)
+			}
+		})
+	}
+}
+
+func TestRenderStructuredError_matchesApprovedToonFixture(t *testing.T) {
+	expected, err := os.ReadFile("testdata/error.toon")
+	require.NoError(t, err)
+	expected = bytes.TrimSuffix(expected, []byte("\n"))
+
+	actual, err := RenderStructuredError(outputFormatTOON, StructuredError{
+		Ok:       false,
+		ErrorMsg: "No supported files found",
+		Path:     "/workspace",
+	})
+
+	require.NoError(t, err)
+	assert.Equal(t, string(expected), string(actual))
+}
+
+func TestRenderStructuredError_quotesToonStrings(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    StructuredError
+		expected string
+	}{
+		{
+			name: "TOON syntax and whitespace",
+			input: StructuredError{
+				Ok:       false,
+				ErrorMsg: "scan: failed\nretry",
+				Path:     " /workspace ",
+			},
+			expected: "error: \"scan: failed\\nretry\"\npath: \" /workspace \"",
+		},
+		{
+			name: "control characters",
+			input: StructuredError{
+				Ok:       false,
+				ErrorMsg: "invalid\x01detail",
+			},
+			expected: "error: \"invalid\\u0001detail\"\npath: \"\"",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			actual, err := RenderStructuredError(outputFormatTOON, test.input)
+
+			require.NoError(t, err)
+			assert.Equal(t, test.expected, string(actual))
+		})
+	}
+}
+
+func TestRenderStructuredError_keepsSafeToonStringsUnquoted(t *testing.T) {
+	actual, err := RenderStructuredError(outputFormatTOON, StructuredError{
+		Ok:       false,
+		ErrorMsg: "scan failed, retry",
+		Path:     `\\server\share`,
+	})
+
+	require.NoError(t, err)
+	assert.Equal(t, "error: scan failed, retry\npath: \"\\\\\\\\server\\\\share\"", string(actual))
+
+	actual, err = RenderStructuredError(outputFormatTOON, StructuredError{
+		Ok:       false,
+		ErrorMsg: "true",
+		Path:     "#comment",
+	})
+
+	require.NoError(t, err)
+	assert.Equal(t, "error: \"true\"\npath: \"#comment\"", string(actual))
+}

cliv2/cmd/cliv2/behavior/testdata/error.toon

@@ -0,0 +1,2 @@
+error: No supported files found
+path: /workspace