diff --git a/cmd/goal/clerk.go b/cmd/goal/clerk.go index e467064ee1..dce74a81d4 100644 --- a/cmd/goal/clerk.go +++ b/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) diff --git a/cmd/goal/multisig.go b/cmd/goal/multisig.go index e4526f44f8..5db3d09af9 100644 --- a/cmd/goal/multisig.go +++ b/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 == "" { diff --git a/cmd/pingpong/runCmd.go b/cmd/pingpong/runCmd.go index ac95b16e07..1ebf4fc95c 100644 --- a/cmd/pingpong/runCmd.go +++ b/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 diff --git a/daemon/algod/api/server/v2/handlers.go b/daemon/algod/api/server/v2/handlers.go index e314de31a7..f1b342268f 100644 --- a/daemon/algod/api/server/v2/handlers.go +++ b/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) diff --git a/daemon/algod/api/server/v2/test/handlers_test.go b/daemon/algod/api/server/v2/test/handlers_test.go index 5584224941..dedd902c94 100644 --- a/daemon/algod/api/server/v2/test/handlers_test.go +++ b/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 diff --git a/data/transactions/logic/assembler.go b/data/transactions/logic/assembler.go index 3ed539feab..9e8da08d1a 100644 --- a/data/transactions/logic/assembler.go +++ b/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 diff --git a/data/transactions/logic/assembler_test.go b/data/transactions/logic/assembler_test.go index 24a3afaf50..cc5cd97a44 100644 --- a/data/transactions/logic/assembler_test.go +++ b/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) +} diff --git a/test/scripts/e2e_subs/e2e-app-simple.sh b/test/scripts/e2e_subs/e2e-app-simple.sh index e1f1458ce4..660487621d 100755 --- a/test/scripts/e2e_subs/e2e-app-simple.sh +++ 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