Fix race in the checker around reference types (#1224)

Author: TristonianJones

checker/checker_test.go

@@ -2355,15 +2355,14 @@ func TestCheck(t *testing.T) {
 			}
 
 			reg, err := types.NewRegistry(&proto2pb.TestAllTypes{}, &proto3pb.TestAllTypes{})
+			if err != nil {
+				t.Fatalf("types.NewRegistry() failed: %v", err)
+			}
 			if tc.env.optionalSyntax {
-				err = reg.RegisterType(types.OptionalType)
-				if err != nil {
+				if err := reg.RegisterType(types.OptionalType); err != nil {
 					t.Fatalf("reg.RegisterType(optional_type) failed: %v", err)
 				}
 			}
-			if err != nil {
-				t.Fatalf("types.NewRegistry() failed: %v", err)
-			}
 			cont, err := containers.NewContainer(containers.Name(tc.container))
 			if err != nil {
 				t.Fatalf("containers.NewContainer() failed: %v", err)
@@ -2453,6 +2452,11 @@ func BenchmarkCheck(b *testing.B) {
 			if err != nil {
 				b.Fatalf("types.NewRegistry() failed: %v", err)
 			}
+			if tc.env.optionalSyntax {
+				if err := reg.RegisterType(types.OptionalType); err != nil {
+					b.Fatalf("reg.RegisterType(optional_type) failed: %v", err)
+				}
+			}
 			cont, err := containers.NewContainer(containers.Name(tc.container))
 			if err != nil {
 				b.Fatalf("containers.NewContainer() failed: %v", err)

checker/env.go

@@ -137,29 +137,20 @@ func (e *Env) LookupIdent(name string) *decls.VariableDecl {
 			return ident
 		}
 
-		// Next try to import the name as a reference to a message type. If found,
-		// the declaration is added to the outest (global) scope of the
-		// environment, so next time we can access it faster.
+		// Next try to import the name as a reference to a message type.
 		if t, found := e.provider.FindStructType(candidate); found {
-			decl := decls.NewVariable(candidate, t)
-			e.declarations.AddIdent(decl)
-			return decl
+			return decls.NewVariable(candidate, t)
 		}
-
 		if i, found := e.provider.FindIdent(candidate); found {
 			if t, ok := i.(*types.Type); ok {
-				decl := decls.NewVariable(candidate, types.NewTypeTypeWithParam(t))
-				e.declarations.AddIdent(decl)
-				return decl
+				return decls.NewVariable(candidate, types.NewTypeTypeWithParam(t))
 			}
 		}
 
 		// Next try to import this as an enum value by splitting the name in a type prefix and
 		// the enum inside.
 		if enumValue := e.provider.EnumValue(candidate); enumValue.Type() != types.ErrType {
-			decl := decls.NewConstant(candidate, types.IntType, enumValue)
-			e.declarations.AddIdent(decl)
-			return decl
+			return decls.NewConstant(candidate, types.IntType, enumValue)
 		}
 	}
 	return nil

ext/native_test.go

@@ -1009,6 +1009,51 @@ func TestNativeTypesVersion(t *testing.T) {
 	}
 }
 
+type Custom struct {
+	Name string `cel:"name"`
+}
+
+func TestTypeResolutionRace(t *testing.T) {
+	customType := reflect.TypeFor[*Custom]()
+	env, err := cel.NewEnv(
+		cel.Container("ext"),
+		NativeTypes(
+			ParseStructTag("cel"),
+			customType,
+		),
+	)
+	if err != nil {
+		t.Fatal("NewEnv:", err)
+	}
+
+	tests := []struct {
+		name string
+		expr string
+	}{
+		{name: "custom1", expr: `Custom{ name: "name1" }`},
+		{name: "custom2", expr: `Custom{ name: "name2" }`},
+		{name: "custom3", expr: `Custom{ name: "name3" }`},
+		{name: "custom4", expr: `Custom{ name: "name4" }`},
+		{name: "custom5", expr: `Custom{ name: "name5" }`},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			ast, iss := env.Compile(test.expr)
+			if err := iss.Err(); err != nil {
+				t.Fatal("Compile:", err)
+			}
+			prg, err := env.Program(ast)
+			if err != nil {
+				t.Fatalf("env.Program() failed: %s", err)
+			}
+			prg.Eval(cel.NoVars())
+		})
+	}
+}
+
 // testEnv initializes the test environment common to all tests.
 func testNativeEnv(t *testing.T, opts ...any) *cel.Env {
 	t.Helper()