pkg/eval: Annotate partial compilation errors.

This has both false positives (errors are not actually partial being marked as
partial) and false negatives (the other way around) for now and could use more
work, but it's at least good enough for the editor to not show any compilation
error during the process of typing out "if $true { echo $pid } else { }".

0.21.0-release-notes.md

@@ -23,6 +23,12 @@
 -   Temporary assignments now work correctly on map and list elements
     ([#1515](https://b.elv.sh/1515)).
 
+-   The terminal line editor is now more aggressive in suppressing compilation
+    errors caused by the code not being complete.
+
+    For example, during the process of typing out `echo $pid`, the editor no
+    longer complains that `$p` is undefined when the user has typed `echo $p`.
+
 # Deprecations
 
 -   The implicit cd feature is now deprecated. Use `cd` or location mode

pkg/edit/highlight/highlighter_test.go

@@ -92,16 +92,16 @@ func TestHighlighter_AutofixesAndCheckErrors(t *testing.T) {
 
 	tt.Test(t, tt.Fn(hl.Get).Named("hl.Get"),
 		// Check error is highlighted and returned
-		Args("ls $a").Rets(
+		Args("ls $a ").Rets(
 			ui.MarkLines(
-				"ls $a", styles,
-				"vv ??"),
+				"ls $a ", styles,
+				"vv ?? "),
 			matchTexts("1:4")),
 		// Multiple check errors
-		Args("ls $a $b").Rets(
+		Args("ls $a $b ").Rets(
 			ui.MarkLines(
-				"ls $a $b", styles,
-				"vv ?? ??"),
+				"ls $a $b ", styles,
+				"vv ?? ?? "),
 			matchTexts("1:4", "1:7")),
 		// Check errors at the end are ignored
 		Args("set _").Rets(any, noTips),
@@ -110,9 +110,8 @@ func TestHighlighter_AutofixesAndCheckErrors(t *testing.T) {
 		Args("nop $mod1:").Rets(
 			ui.MarkLines(
 				"nop $mod1:", styles,
-				"vvv ??????"),
+				"vvv $$$$$$"),
 			matchTexts(
-				"1:5",               // error
 				"autofix: use mod1", // autofix
 			)),
 	)

pkg/edit/highlight_test.go

@@ -27,10 +27,10 @@ func TestHighlighter(t *testing.T) {
 	)
 
 	// Check errors
-	feedInput(f.TTYCtrl, "x")
+	feedInput(f.TTYCtrl, "x ")
 	f.TestTTY(t,
-		"~> put $truex", Styles,
-		"   vvv ??????", term.DotHere, "\n",
+		"~> put $truex ", Styles,
+		"   vvv ?????? ", term.DotHere, "\n",
 		"compilation error: [interactive]:1:5-10: variable $truex not found",
 	)
 }
@@ -42,8 +42,7 @@ func TestHighlighter_Autofix(t *testing.T) {
 	feedInput(f.TTYCtrl, "put $mod1:")
 	f.TestTTY(t,
 		"~> put $mod1:", Styles,
-		"   vvv ??????", term.DotHere, "\n",
-		"compilation error: [interactive]:1:5-10: variable $mod1: not found\n",
+		"   vvv $$$$$$", term.DotHere, "\n",
 		"Ctrl-A autofix: use mod1 Tab Enter autofix first", Styles,
 		"++++++                   +++ +++++",
 	)

pkg/eval/builtin_special.go

@@ -114,13 +114,14 @@ func compileTmp(cp *compiler, fn *parse.Form) effectOp {
 
 func compileWith(cp *compiler, fn *parse.Form) effectOp {
 	if len(fn.Args) < 2 {
-		cp.errorpf(fn, "with requires at least two arguments")
+		cp.errorpfPartial(fn, "with requires at least two arguments")
 		return nopOp{}
 	}
 	lastArg := fn.Args[len(fn.Args)-1]
 	bodyNode, ok := cmpd.Lambda(lastArg)
 	if !ok {
-		cp.errorpf(lastArg, "last argument must be a lambda")
+		// It's possible that we just haven't seen the last argument yet.
+		cp.errorpfPartial(diag.PointRanging(fn.To), "last argument must be a lambda")
 		return nopOp{}
 	}
 
@@ -194,7 +195,7 @@ func (op *withOp) exec(fm *Frame) (opExc Exception) {
 func compileLHSRHS(cp *compiler, args []*parse.Compound, end int, lf lvalueFlag) (lvaluesGroup, valuesOp) {
 	lhs, rhs := compileLHSOptionalRHS(cp, args, end, lf)
 	if rhs == nil {
-		cp.errorpf(diag.PointRanging(end), "need = and right-hand-side")
+		cp.errorpfPartial(diag.PointRanging(end), "need = and right-hand-side")
 	}
 	return lhs, rhs
 }
@@ -240,7 +241,7 @@ func compileDel(cp *compiler, fn *parse.Form) effectOp {
 		var f effectOp
 		ref := resolveVarRef(cp, qname, nil)
 		if ref == nil {
-			cp.errorpf(cn, "no variable $%s", head.Value)
+			cp.errorpfPartial(cn, "no variable $%s", head.Value)
 			continue
 		}
 		if len(indices) == 0 {
@@ -810,7 +811,7 @@ func compileTry(cp *compiler, fn *parse.Form) effectOp {
 	if elseNode != nil && catchNode == nil {
 		cp.errorpf(fn, "try with an else block requires a catch block")
 	} else if catchNode == nil && finallyNode == nil {
-		cp.errorpf(fn, "try must be followed by a catch block or a finally block")
+		cp.errorpfPartial(fn, "try must be followed by a catch block or a finally block")
 	}
 
 	var catchVar lvalue
@@ -904,11 +905,11 @@ func compilePragma(cp *compiler, fn *parse.Form) effectOp {
 		case "external":
 			cp.currentPragma().unknownCommandIsExternal = true
 		default:
-			cp.errorpf(valueNode,
+			cp.errorpfPartial(valueNode,
 				"invalid value for unknown-command: %s", parse.Quote(value))
 		}
 	default:
-		cp.errorpf(fn.Args[0], "unknown pragma %s", parse.Quote(name))
+		cp.errorpfPartial(fn.Args[0], "unknown pragma %s", parse.Quote(name))
 	}
 	return nopOp{}
 }

pkg/eval/builtin_special_test.elvts

@@ -376,7 +376,7 @@ Compilation error: with requires at least two arguments
 ~> var x
    with x = val foobar
 Compilation error: last argument must be a lambda
-  [tty]:2:14-19: with x = val foobar
+  [tty]:2:20: with x = val foobar
 
 ## compound expressions ##
 ~> with a'x' = foo { }

pkg/eval/compile_effect.go

@@ -197,7 +197,7 @@ func (cp *compiler) formBody(n *parse.Form) formBody {
 			if cp.currentPragma().unknownCommandIsExternal || fsutil.DontSearch(head) {
 				headOp = literalValues(n.Head, NewExternalCmd(head))
 			} else {
-				cp.errorpf(n.Head, "unknown command disallowed by current pragma")
+				cp.errorpfPartial(n.Head, "unknown command disallowed by current pragma")
 			}
 		}
 	} else {

pkg/eval/compile_lvalue.go

@@ -63,7 +63,7 @@ func (cp *compiler) compileIndexingLValue(n *parse.Indexing, f lvalueFlag) lvalu
 	varUse := n.Head.Value
 	sigil, qname := SplitSigil(varUse)
 	if qname == "" {
-		cp.errorpf(n, "variable name must not be empty")
+		cp.errorpfPartial(n, "variable name must not be empty")
 		return dummyLValuesGroup
 	}
 
@@ -78,7 +78,7 @@ func (cp *compiler) compileIndexingLValue(n *parse.Indexing, f lvalueFlag) lvalu
 	if ref == nil {
 		if f&newLValue == 0 {
 			cp.autofixUnresolvedVar(qname)
-			cp.errorpf(n, "cannot find variable $%s", parse.Quote(qname))
+			cp.errorpfPartial(n, "cannot find variable $%s", parse.Quote(qname))
 			return dummyLValuesGroup
 		}
 		if len(n.Indices) > 0 {

pkg/eval/compile_value.go

@@ -263,7 +263,11 @@ func (cp *compiler) primaryOp(n *parse.Primary) valuesOp {
 		ref := resolveVarRef(cp, qname, n)
 		if ref == nil {
 			cp.autofixUnresolvedVar(qname)
-			cp.errorpf(n, "variable $%s not found", parse.Quote(qname))
+			// The variable name might be a prefix of a valid variable name.
+			// Ideally, we'd want to match the variable name to all possible
+			// names to check if that's actually the case, but it's a bit
+			// expensive and let's call this good enough for now.
+			cp.errorpfPartial(n, "variable $%s not found", parse.Quote(qname))
 		}
 		return &variableOp{n.Range(), sigil != "", qname, ref}
 	case parse.Wildcard:
@@ -390,7 +394,7 @@ func (cp *compiler) lambda(n *parse.Primary) valuesOp {
 				cp.errorpf(arg, "argument name must be unqualified")
 			}
 			if name == "" {
-				cp.errorpf(arg, "argument name must not be empty")
+				cp.errorpfPartial(arg, "argument name must not be empty")
 			}
 			if sigil == "@" {
 				if restArg != -1 {
@@ -400,7 +404,7 @@ func (cp *compiler) lambda(n *parse.Primary) valuesOp {
 			}
 			if name != "_" {
 				if seenName[name] {
-					cp.errorpf(arg, "duplicate argument name '%s'", name)
+					cp.errorpfPartial(arg, "duplicate argument name '%s'", name)
 				} else {
 					seenName[name] = true
 				}
@@ -418,11 +422,11 @@ func (cp *compiler) lambda(n *parse.Primary) valuesOp {
 				cp.errorpf(opt.Key, "option name must be unqualified")
 			}
 			if name == "" {
-				cp.errorpf(opt.Key, "option name must not be empty")
+				cp.errorpfPartial(opt.Key, "option name must not be empty")
 			}
 			optNames[i] = name
 			if opt.Value == nil {
-				cp.errorpf(opt.Key, "option must have default value")
+				cp.errorpfPartial(opt.Key, "option must have default value")
 			} else {
 				optDefaultOps[i] = cp.compoundOp(opt.Value)
 			}

pkg/eval/compiler.go

@@ -84,13 +84,24 @@ type CompilationErrorTag struct{}
 
 func (CompilationErrorTag) ErrorTag() string { return "compilation error" }
 
+// Reports a compilation error.
 func (cp *compiler) errorpf(r diag.Ranger, format string, args ...any) {
+	cp.errorpfInner(r, fmt.Sprintf(format, args...), false)
+}
+
+// Reports a compilation error, and mark it as partial iff the end of r happens
+// to coincide with the end of the source code.
+func (cp *compiler) errorpfPartial(r diag.Ranger, format string, args ...any) {
+	cp.errorpfInner(r, fmt.Sprintf(format, args...), r.Range().To == len(cp.src.Code))
+}
+
+func (cp *compiler) errorpfInner(r diag.Ranger, msg string, partial bool) {
 	cp.errors = append(cp.errors, &CompilationError{
-		Message: fmt.Sprintf(format, args...),
+		Message: msg,
 		Context: *diag.NewContext(cp.src.Name, cp.src.Code, r),
 		// TODO: This criteria is too strict and only captures a small subset of
 		// partial compilation errors.
-		Partial: r.Range().From == len(cp.src.Code),
+		Partial: partial,
 	})
 }
 

pkg/eval/compiler_test.go

@@ -2,8 +2,10 @@ package eval_test
 
 import (
 	"testing"
+	"unicode/utf8"
 
 	"github.com/google/go-cmp/cmp"
+	"src.elv.sh/pkg/diag"
 	. "src.elv.sh/pkg/eval"
 	"src.elv.sh/pkg/parse"
 )
@@ -64,3 +66,38 @@ func TestAutofix(t *testing.T) {
 		})
 	}
 }
+
+// TODO: Turn this into a fuzz test.
+func TestPartialCompilationError(t *testing.T) {
+	for _, code := range transcriptCodes {
+		testPartialError(t, code, func(src parse.Source) []*CompilationError {
+			_, _, err := NewEvaler().Check(src, nil)
+			return UnpackCompilationErrors(err)
+		})
+	}
+}
+
+// TODO: Deduplicate this against a similar helper in pkg/parse/fuzz_test.go
+func testPartialError[T diag.ErrorTag](t *testing.T, full string, fn func(src parse.Source) []*diag.Error[T]) {
+	if !utf8.ValidString(full) {
+		t.Skip("not valid UTF-8")
+	}
+	errs := fn(parse.Source{Name: "fuzz.elv", Code: full})
+	if len(errs) > 0 {
+		t.Skip("code itself has error")
+	}
+	// If code has no error, then every prefix of it (as long as it's valid
+	// UTF-8) should have either no errors or only partial errors.
+	for i := range full {
+		if i == 0 {
+			continue
+		}
+		prefix := full[:i]
+		errs := fn(parse.Source{Name: "fuzz.elv", Code: prefix})
+		for _, err := range errs {
+			if !err.Partial {
+				t.Errorf("\n%s\n===========\nnon-partial error: %v\nfull code:\n===========\n%s\n", prefix, err, full)
+			}
+		}
+	}
+}

pkg/eval/evaltest/test_transcript.go

@@ -148,6 +148,11 @@ func TestTranscriptsInFS(t *testing.T, fsys fs.FS, setupPairs ...any) {
 	if err != nil {
 		t.Fatalf("parse transcript sessions: %v", err)
 	}
+	TestTranscriptNodes(t, nodes, setupPairs...)
+}
+
+// TestTranscriptsInFS runs parsed Elvish transcript nodes as tests.
+func TestTranscriptNodes(t *testing.T, nodes []*transcript.Node, setupPairs ...any) {
 	var run *runCfg
 	if runEnv := os.Getenv("ELVISH_TRANSCRIPT_RUN"); runEnv != "" {
 		filename, lineNo, ok := parseFileNameAndLineNo(runEnv)

pkg/eval/node_utils.go

@@ -11,7 +11,11 @@ import (
 func stringLiteralOrError(cp *compiler, n *parse.Compound, what string) string {
 	s, err := cmpd.StringLiteralOrError(n, what)
 	if err != nil {
-		cp.errorpf(n, "%v", err)
+		if len(n.Indexings) == 0 {
+			cp.errorpfPartial(n, "%v", err)
+		} else {
+			cp.errorpf(n, "%v", err)
+		}
 	}
 	return s
 }
@@ -34,12 +38,19 @@ func (ag *argsGetter) errorpf(r diag.Ranger, format string, args ...any) {
 	}
 }
 
+func (ag *argsGetter) errorpfPartial(r diag.Ranger, format string, args ...any) {
+	if ag.ok {
+		ag.cp.errorpfPartial(r, format, args...)
+		ag.ok = false
+	}
+}
+
 func (ag *argsGetter) get(i int, what string) *argAsserter {
 	if ag.n < i+1 {
 		ag.n = i + 1
 	}
 	if i >= len(ag.fn.Args) {
-		ag.errorpf(diag.PointRanging(ag.fn.To), "need %s", what)
+		ag.errorpfPartial(diag.PointRanging(ag.fn.To), "need %s", what)
 		return &argAsserter{ag, what, nil}
 	}
 	return &argAsserter{ag, what, ag.fn.Args[i]}
@@ -64,7 +75,15 @@ func (ag *argsGetter) optionalKeywordBody(i int, kw string) *parse.Primary {
 
 func (ag *argsGetter) finish() bool {
 	if ag.n < len(ag.fn.Args) {
-		ag.errorpf(
+		// In general, the "superfluous" argument may actually be incomplete
+		// optional keywords (like "el" in an "if" form), hence this calls
+		// errorpfPartial instead of errorpf.
+		//
+		// TODO: Make this more accurate. We should have all the information we
+		// need to say that whether this is partial or not, but in order to
+		// retain that information we'll probably need a more declarative API
+		// for parsing special forms.
+		ag.errorpfPartial(
 			diag.Ranging{From: ag.fn.Args[ag.n].Range().From, To: ag.fn.To},
 			"superfluous arguments")
 	}
@@ -99,8 +118,15 @@ func (aa *argAsserter) lambda() *parse.Primary {
 	}
 	lambda, ok := cmpd.Lambda(aa.node)
 	if !ok {
-		aa.ag.errorpf(aa.node,
-			"%s must be lambda, found %s", aa.what, cmpd.Shape(aa.node))
+		if p, ok := cmpd.Primary(aa.node); ok && p.Type == parse.Braced {
+			// If we have seen just a "{", the parser will parse a braced
+			// expression, but this could as well be the start of a lambda.
+			aa.ag.errorpfPartial(aa.node,
+				"%s must be lambda, found %s", aa.what, cmpd.Shape(aa.node))
+		} else {
+			aa.ag.errorpf(aa.node,
+				"%s must be lambda, found %s", aa.what, cmpd.Shape(aa.node))
+		}
 		return nil
 	}
 	return lambda