eval: tolerate syntax errors

CHANGELOG_PENDING.md

@@ -9,4 +9,6 @@
 
 ### Bug Fixes
 
+- Environment declarations are now returned even in the face of syntax errors.
+
 ### Breaking changes

ast/expr.go

@@ -892,7 +892,7 @@ func parseFromBase64(node *syntax.ObjectNode, name *StringExpr, args Expr) (Expr
 func parseSecret(node *syntax.ObjectNode, name *StringExpr, value Expr) (Expr, syntax.Diagnostics) {
 	if arg, ok := value.(*ObjectExpr); ok && len(arg.Entries) == 1 {
 		kvp := arg.Entries[0]
-		if kvp.Key.Value == "ciphertext" {
+		if kvp.Key.GetValue() == "ciphertext" {
 			if str, ok := kvp.Value.(*StringExpr); ok {
 				return CiphertextSyntax(node, name, arg, str), nil
 			}

eval/eval.go

@@ -70,10 +70,6 @@ func LoadYAMLBytes(filename string, source []byte) (*ast.EnvironmentDecl, syntax
 
 	t, tdiags := ast.ParseEnvironment(source, syn)
 	diags.Extend(tdiags...)
-	if tdiags.HasErrors() {
-		return nil, diags, nil
-	}
-
 	return t, diags, nil
 }
 
@@ -391,11 +387,14 @@ func declare[Expr exprNode](e *evalContext, path string, x Expr, base *value) *e
 	case *ast.ObjectExpr:
 		properties := make(map[string]*expr, len(x.Entries))
 		for _, entry := range x.Entries {
-			k := entry.Key.Value
-			if _, ok := properties[k]; ok {
-				e.errorf(entry.Key, "duplicate key %q", k)
-			} else {
-				properties[k] = declare(e, util.JoinKey(path, k), entry.Value, base.property(entry.Key, k))
+			// Possible during parse errors. We elide properties with nil keys.
+			if entry.Key != nil {
+				k := entry.Key.Value
+				if _, ok := properties[k]; ok {
+					e.errorf(entry.Key, "duplicate key %q", k)
+				} else {
+					properties[k] = declare(e, util.JoinKey(path, k), entry.Value, base.property(entry.Key, k))
+				}
 			}
 		}
 		repr := &objectExpr{node: x, properties: properties}

eval/eval_test.go

@@ -252,6 +252,10 @@ type testEnvironments struct {
 }
 
 func (e *testEnvironments) LoadEnvironment(ctx context.Context, name string) ([]byte, Decrypter, error) {
+	if e.root == "" {
+		return nil, nil, os.ErrNotExist
+	}
+
 	bytes, err := os.ReadFile(filepath.Join(e.root, name+".yaml"))
 	if err != nil {
 		return nil, nil, err
@@ -600,3 +604,37 @@ func BenchmarkEvalEnvLoad(b *testing.B) {
 func BenchmarkEvalAll(b *testing.B) {
 	benchmarkEval(b, 10*time.Millisecond, 10*time.Millisecond)
 }
+
+// TestSyntaxErrorCheck provides additional insurance that we are able to parse and analyze environments that contain
+// syntax errors. It is important that we are able to provide as much information about an environment as we can so
+// that tools that depend on an environment's typed AST or implied schema needs this fix can operate properly, even on
+// environment definitions that contain syntax errors. Most notably we need this for autocomplete, but other
+// type/schema-based features also benefit (signature help, go-to-def, et cetera).
+//
+// This is also covered by tests under TestEval, but it can be easy to miss the magnitude of changes to those tests.
+func TestSyntaxErrorCheck(t *testing.T) {
+	environmentName := "syntax-error"
+	envBytes := []byte(`values:
+  foo:
+    bar: ${foo.}
+`)
+
+	env, loadDiags, err := LoadYAMLBytes(environmentName, envBytes)
+	require.NoError(t, err)
+	sortEnvironmentDiagnostics(loadDiags)
+
+	execContext, err := esc.NewExecContext(map[string]esc.Value{
+		"pulumi": esc.NewValue(map[string]esc.Value{
+			"user": esc.NewValue(map[string]esc.Value{
+				"id": esc.NewValue("USER_123"),
+			}),
+		}),
+	})
+	assert.NoError(t, err)
+
+	check, checkDiags := CheckEnvironment(context.Background(), environmentName, env, rot128{}, testProviders{},
+		&testEnvironments{}, execContext, false)
+
+	assert.True(t, checkDiags.HasErrors())
+	assert.NotNil(t, check)
+}

eval/expr.go

@@ -300,7 +300,9 @@ func (x *expr) export(environment string) esc.Expr {
 	case *objectExpr:
 		ex.KeyRanges = make(map[string]esc.Range, len(repr.node.Entries))
 		for _, kvp := range repr.node.Entries {
-			ex.KeyRanges[kvp.Key.Value] = convertRange(kvp.Key.Syntax().Syntax().Range(), environment)
+			if kvp.Key != nil {
+				ex.KeyRanges[kvp.Key.Value] = convertRange(kvp.Key.Syntax().Syntax().Range(), environment)
+			}
 		}
 
 		ex.Object = make(map[string]esc.Expr, len(repr.properties))