Enhancement: goal Error UX Improvement

cmd/goal/clerk.go

@@ -962,7 +962,7 @@ func assembleFileImpl(fname string, printWarnings bool) *logic.OpStream {
 	}
 	ops, err := logic.AssembleString(string(text))
 	if err != nil {
-		ops.ReportProblems(fname, os.Stderr)
+		ops.ReportMultipleErrors(fname, os.Stderr)
 		reportErrorf("%s: %s", fname, err)
 	}
 	_, params := getProto(protoVersion)

cmd/goal/multisig.go

@@ -163,7 +163,7 @@ var signProgramCmd = &cobra.Command{
 			}
 			ops, err := logic.AssembleString(string(text))
 			if err != nil {
-				ops.ReportProblems(programSource, os.Stderr)
+				ops.ReportMultipleErrors(programSource, os.Stderr)
 				reportErrorf("%s: %s", programSource, err)
 			}
 			if outname == "" {

cmd/pingpong/runCmd.go

@@ -316,7 +316,7 @@ var runCmd = &cobra.Command{
 			}
 			ops, err := logic.AssembleString(programStr)
 			if err != nil {
-				ops.ReportProblems(teal, os.Stderr)
+				ops.ReportMultipleErrors(teal, os.Stderr)
 				reportErrorf("Internal error, cannot assemble %v \n", programStr)
 			}
 			cfg.Program = ops.Program

daemon/algod/api/server/v2/handlers.go

@@ -1528,7 +1528,7 @@ func (v2 *Handlers) TealCompile(ctx echo.Context, params model.TealCompileParams
 	ops, err := logic.AssembleString(source)
 	if err != nil {
 		sb := strings.Builder{}
-		ops.ReportProblems("", &sb)
+		ops.ReportMultipleErrors("", &sb)
 		return badRequest(ctx, err, sb.String(), v2.Log)
 	}
 	pd := logic.HashProgram(ops.Program)

daemon/algod/api/server/v2/test/handlers_test.go

@@ -815,7 +815,7 @@ func TestTealCompile(t *testing.T) {
 	t.Parallel()
 
 	params := model.TealCompileParams{}
-	tealCompileTest(t, nil, 200, true, params, nil) // nil program should work
+	tealCompileTest(t, nil, 400, true, params, nil) // nil program should NOT work
 
 	goodProgram := fmt.Sprintf(`#pragma version %d
 int 1

data/transactions/logic/assembler.go

@@ -1622,7 +1622,7 @@ func isFullSpec(spec OpSpec) bool {
 }
 
 // mergeProtos allows us to support typetracking of pseudo-ops which are given an improper number of immediates
-//by creating a new proto that is a combination of all the pseudo-op's possibilities
+// by creating a new proto that is a combination of all the pseudo-op's possibilities
 func mergeProtos(specs map[int]OpSpec) (Proto, uint64, bool) {
 	var args StackTypes
 	var returns StackTypes
@@ -1857,10 +1857,13 @@ func splitTokens(tokens []string) (current, rest []string) {
 
 // assemble reads text from an input and accumulates the program
 func (ops *OpStream) assemble(text string) error {
-	fin := strings.NewReader(text)
 	if ops.Version > LogicVersion && ops.Version != assemblerNoVersion {
 		return ops.errorf("Can not assemble version %d", ops.Version)
 	}
+	if strings.TrimSpace(text) == "" {
+		return ops.errorf("Cannot assemble empty program text")
+	}
+	fin := strings.NewReader(text)
 	scanner := bufio.NewScanner(fin)
 	for scanner.Scan() {
 		ops.sourceLine++
@@ -2411,8 +2414,19 @@ func (ops *OpStream) warnf(format string, a ...interface{}) error {
 	return ops.warn(fmt.Errorf(format, a...))
 }
 
-// ReportProblems issues accumulated warnings and outputs errors to an io.Writer.
-func (ops *OpStream) ReportProblems(fname string, writer io.Writer) {
+// ReportMultipleErrors issues accumulated warnings and outputs errors to an io.Writer.
+// In the case of exactly 1 error and no warnings, a slightly different format is provided
+// to handle the cases when the original error is or isn't reported elsewhere.
+// In the case of > 10 errors, only the first 10 errors will be reported.
+func (ops *OpStream) ReportMultipleErrors(fname string, writer io.Writer) {
+	if len(ops.Errors) == 1 && len(ops.Warnings) == 0 {
+		prefix := ""
+		if fname != "" {
+			prefix = fmt.Sprintf("%s: ", fname)
+		}
+		fmt.Fprintf(writer, "%s1 error: %s\n", prefix, ops.Errors[0])
+		return
+	}
 	for i, e := range ops.Errors {
 		if i > 9 {
 			break

data/transactions/logic/assembler_test.go

@@ -19,6 +19,7 @@ package logic
 import (
 	"bytes"
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"regexp"
 	"strings"
@@ -1457,7 +1458,7 @@ done:`
 	require.Equal(t, expectedProgBytes, ops.Program)
 }
 
-func TestMultipleErrors(t *testing.T) {
+func TestSeveralErrors(t *testing.T) {
 	partitiontest.PartitionTest(t)
 	t.Parallel()
 
@@ -3053,3 +3054,108 @@ func TestAssemblePushConsts(t *testing.T) {
 	source = `pushbytess "x" "y"; +`
 	testProg(t, source, AssemblerMaxVersion, Expect{1, "+ arg 1 wanted type uint64 got []byte"})
 }
+
+func TestAssembleEmpty(t *testing.T) {
+	partitiontest.PartitionTest(t)
+	t.Parallel()
+
+	emptyExpect := Expect{0, "Cannot assemble empty program text"}
+	emptyPrograms := []string{
+		"",
+		"     ",
+		"   \n\t\t\t\n\n    ",
+		"   \n \t   \t \t  \n   \n    \n\n",
+	}
+
+	nonEmpty := "   \n \t   \t \t  int 1   \n   \n \t \t   \n\n"
+
+	for version := uint64(1); version <= AssemblerMaxVersion; version++ {
+		for _, prog := range emptyPrograms {
+			testProg(t, prog, version, emptyExpect)
+		}
+		testProg(t, nonEmpty, version)
+	}
+}
+
+func TestReportMultipleErrors(t *testing.T) {
+	partitiontest.PartitionTest(t)
+	t.Parallel()
+
+	assertWithMsg := func(t *testing.T, expectedOutput string, b bytes.Buffer) {
+		if b.String() != expectedOutput {
+			t.Errorf("Unexpected output: got %q, want %q", b.String(), expectedOutput)
+		}
+	}
+
+	ops := &OpStream{
+		Errors: []lineError{
+			{Line: 1, Err: errors.New("error 1")},
+			{Err: errors.New("error 2")},
+			{Line: 3, Err: errors.New("error 3")},
+		},
+		Warnings: []error{
+			errors.New("warning 1"),
+			errors.New("warning 2"),
+		},
+	}
+
+	// Test the case where fname is not empty
+	var b bytes.Buffer
+	ops.ReportMultipleErrors("test.txt", &b)
+	expected := `test.txt: 1: error 1
+test.txt: 0: error 2
+test.txt: 3: error 3
+test.txt: warning 1
+test.txt: warning 2
+`
+	assertWithMsg(t, expected, b)
+
+	// Test the case where fname is empty
+	b.Reset()
+	ops.ReportMultipleErrors("", &b)
+	expected = `1: error 1
+0: error 2
+3: error 3
+warning 1
+warning 2
+`
+	assertWithMsg(t, expected, b)
+
+	// no errors or warnings at all
+	ops = &OpStream{}
+	b.Reset()
+	ops.ReportMultipleErrors("blah blah", &b)
+	expected = ""
+	assertWithMsg(t, expected, b)
+
+	// more than 10 errors:
+	file := "great-file.go"
+	les := []lineError{}
+	expectedStrs := []string{}
+	for i := 1; i <= 11; i++ {
+		errS := fmt.Errorf("error %d", i)
+		les = append(les, lineError{i, errS})
+		if i <= 10 {
+			expectedStrs = append(expectedStrs, fmt.Sprintf("%s: %d: %s", file, i, errS))
+		}
+	}
+	expected = strings.Join(expectedStrs, "\n") + "\n"
+	ops = &OpStream{Errors: les}
+	b.Reset()
+	ops.ReportMultipleErrors(file, &b)
+	assertWithMsg(t, expected, b)
+
+	// exactly 1 error + filename
+	ops = &OpStream{Errors: []lineError{{42, errors.New("super annoying error")}}}
+	b.Reset()
+	ops.ReportMultipleErrors("galaxy.py", &b)
+	expected = "galaxy.py: 1 error: 42: super annoying error\n"
+	assertWithMsg(t, expected, b)
+
+	// exactly 1 error w/o filename
+	ops = &OpStream{Errors: []lineError{{42, errors.New("super annoying error")}}}
+	b.Reset()
+	ops.ReportMultipleErrors("", &b)
+	expected = "1 error: 42: super annoying error\n"
+	assertWithMsg(t, expected, b)
+}

test/scripts/e2e_subs/e2e-app-simple.sh

@@ -122,3 +122,27 @@ ${gcmd} app optin --app-id $APPID --from $ACCOUNT
 
 # Succeed in clearing state for the app
 ${gcmd} app clear --app-id $APPID --from $ACCOUNT
+
+
+# Empty program:
+printf '     ' > "${TEMPDIR}/empty_clear.teal"
+
+# Fail to compile an empty program
+RES=$(${gcmd} clerk compile "${TEMPDIR}/empty_clear.teal" 2>&1 | tr -d '\n' || true)
+EXPERROR='Cannot assemble empty program text'
+if [[ $RES != *"${EXPERROR}"* ]]; then
+    echo RES="$RES"
+    echo EXPERROR="$EXPERROR"
+    date '+clerk-compile-test FAIL wrong error for compiling empty program %Y%m%d_%H%M%S'
+    false
+fi
+
+# Fail to create an app because the clear program is empty
+RES=$(${gcmd} app create --creator "${ACCOUNT}" --approval-prog "${TEMPDIR}/simple.teal" --clear-prog "${TEMPDIR}/empty_clear.teal" --global-byteslices 0 --global-ints 0 --local-byteslices 0 --local-ints 0 2>&1 | tr -d '\n' || true)
+EXPERROR='Cannot assemble empty program text'
+if [[ $RES != *"${EXPERROR}"* ]]; then
+    echo RES="$RES"
+    echo EXPERROR="$EXPERROR"
+    date '+app-create-test FAIL wrong error for creating app with empty clear program %Y%m%d_%H%M%S'
+    false
+fi