address PR comments and minor improvements

Author: diegommm

checker/nature/nature.go

@@ -190,23 +190,36 @@ func ArrayFromType(c *Cache, t reflect.Type) Nature {
 func (n *Nature) SetCache(c *Cache) {
 	n.cache = c
 	if n.Kind == reflect.Struct {
-		n.structData.cache = c
 		if c.structs == nil {
+			n.structData.setCache(c)
 			c.structs = map[reflect.Type]Nature{
 				n.Type: *n,
 			}
 		} else if nt, ok := c.structs[n.Type]; ok {
 			// invalidate local, use shared from cache
 			n.Optional.structData = nt.Optional.structData
 		} else {
+			n.structData.setCache(c)
 			c.structs[n.Type] = *n
 		}
 	}
-	if n.Optional != nil {
-		if s, ok := c.methods[n.Type]; ok {
-			// invalidate local if set, use shared from cache
+	hasMethodset := n.Optional != nil && n.Optional.methodset != nil
+	if c.methods != nil || hasMethodset {
+		if c.methods == nil {
+			// Cache is new and the type already gathered some methods
+			n.Optional.methodset.setCache(c)
+			c.methods = map[reflect.Type]*methodset{
+				n.Type: n.Optional.methodset,
+			}
+		} else if s, ok := c.methods[n.Type]; ok {
+			if n.Optional == nil {
+				n.Optional = new(Optional)
+			}
+			// Cache is not new. Invalidate local if set
 			n.Optional.methodset = s
-		} else if n.Optional.methodset != nil {
+		} else if hasMethodset {
+			// Cache miss and the type already gathered some methods
+			n.Optional.methodset.setCache(c)
 			c.methods[n.Type] = n.Optional.methodset
 		}
 	}
@@ -228,7 +241,7 @@ func (n *Nature) String() string {
 }
 
 func (n *Nature) Deref() Nature {
-	t, _, changed := derefTypeKind(n.Type, n.Kind)
+	t, _, changed := deref.TypeKind(n.Type, n.Kind)
 	if !changed {
 		return *n
 	}
@@ -294,7 +307,7 @@ func (n *Nature) NumMethods() int {
 
 func (n *Nature) MethodByName(name string) (Nature, bool) {
 	if s := n.getMethodset(); s != nil {
-		if m, ok := s.method(name); ok {
+		if m := s.method(name); m != nil {
 			return m.nature, true
 		}
 	}
@@ -378,7 +391,7 @@ func (n *Nature) FieldByName(name string) (Nature, bool) {
 	} else {
 		sd = n.cache.getStruct(n.Type).structData
 	}
-	if sf, ok := sd.structField(nil, name); ok {
+	if sf := sd.structField(nil, name); sf != nil {
 		return sf.Nature, true
 	}
 	return Nature{}, false
@@ -419,7 +432,7 @@ func (n *Nature) getSlow(name string) (Nature, bool) {
 		return nt, true
 	}
 	if n.Kind == reflect.Struct {
-		if sf, ok := n.structField(nil, name); ok {
+		if sf := n.structField(nil, name); sf != nil {
 			return sf.Nature, true
 		}
 	}
@@ -430,7 +443,7 @@ func (n *Nature) FieldIndex(name string) ([]int, bool) {
 	if n.Kind != reflect.Struct {
 		return nil, false
 	}
-	if sf, ok := n.structField(nil, name); ok {
+	if sf := n.structField(nil, name); sf != nil {
 		return sf.Index, true
 	}
 	return nil, false

checker/nature/utils.go

@@ -2,16 +2,9 @@ package nature
 
 import (
 	"reflect"
-)
 
-func derefTypeKind(t reflect.Type, k reflect.Kind) (_ reflect.Type, _ reflect.Kind, changed bool) {
-	for k == reflect.Pointer {
-		changed = true
-		t = t.Elem()
-		k = t.Kind()
-	}
-	return t, k, changed
-}
+	"github.com/expr-lang/expr/internal/deref"
+)
 
 func fieldName(fieldName string, tag reflect.StructTag) (string, bool) {
 	switch taggedName := tag.Get("expr"); taggedName {
@@ -27,7 +20,7 @@ func fieldName(fieldName string, tag reflect.StructTag) (string, bool) {
 type structData struct {
 	cache                     *Cache
 	rType                     reflect.Type
-	fields                    map[string]structField
+	fields                    map[string]*structField
 	numField, ownIdx, anonIdx int
 
 	curParent, curChild *structData
@@ -39,22 +32,29 @@ type structField struct {
 	Index []int
 }
 
+func (s *structData) setCache(c *Cache) {
+	s.cache = c
+	for _, sf := range s.fields {
+		sf.SetCache(c)
+	}
+}
+
 func (s *structData) finished() bool {
 	return s.ownIdx >= s.numField && // no own fields left to visit
 		s.anonIdx >= s.numField && // no embedded fields to visit
 		s.curChild == nil // no child in process of visiting
 }
 
-func (s *structData) structField(parentEmbed *structData, name string) (structField, bool) {
+func (s *structData) structField(parentEmbed *structData, name string) *structField {
 	if s.fields == nil {
 		if s.numField > 0 {
-			s.fields = make(map[string]structField, s.numField)
+			s.fields = make(map[string]*structField, s.numField)
 		}
-	} else if f, ok := s.fields[name]; ok {
-		return f, true
+	} else if f := s.fields[name]; f != nil {
+		return f
 	}
 	if s.finished() {
-		return structField{}, false
+		return nil
 	}
 
 	// Lookup own fields first.
@@ -73,7 +73,7 @@ func (s *structData) structField(parentEmbed *structData, name string) (structFi
 			continue
 		}
 		nt := s.cache.FromType(field.Type)
-		sf := structField{
+		sf := &structField{
 			Nature: nt,
 			Index:  field.Index,
 		}
@@ -82,14 +82,14 @@ func (s *structData) structField(parentEmbed *structData, name string) (structFi
 			parentEmbed.trySet(fName, sf)
 		}
 		if fName == name {
-			return sf, true
+			return sf
 		}
 	}
 
 	if s.curChild != nil {
-		sf, ok := s.findInEmbedded(parentEmbed, s.curChild, s.curChildIndex, name)
-		if ok {
-			return sf, true
+		sf := s.findInEmbedded(parentEmbed, s.curChild, s.curChildIndex, name)
+		if sf != nil {
+			return sf
 		}
 	}
 
@@ -101,26 +101,26 @@ func (s *structData) structField(parentEmbed *structData, name string) (structFi
 		if !field.Anonymous {
 			continue
 		}
-		t, k, _ := derefTypeKind(field.Type, field.Type.Kind())
+		t, k, _ := deref.TypeKind(field.Type, field.Type.Kind())
 		if k != reflect.Struct {
 			continue
 		}
 
 		childEmbed := s.cache.getStruct(t).structData
-		sf, ok := s.findInEmbedded(parentEmbed, childEmbed, field.Index, name)
-		if ok {
-			return sf, true
+		sf := s.findInEmbedded(parentEmbed, childEmbed, field.Index, name)
+		if sf != nil {
+			return sf
 		}
 	}
 
-	return structField{}, false
+	return nil
 }
 
 func (s *structData) findInEmbedded(
 	parentEmbed, childEmbed *structData,
 	childIndex []int,
 	name string,
-) (structField, bool) {
+) *structField {
 	// Set current parent/child data. This allows trySet to handle child fields
 	// and add them to our struct and to the parent as well if needed
 	s.curParent = parentEmbed
@@ -145,29 +145,29 @@ func (s *structData) findInEmbedded(
 	}
 
 	// Recheck if we have what we needed from the above sync
-	if sf, ok := s.fields[name]; ok {
-		return sf, true
+	if sf := s.fields[name]; sf != nil {
+		return sf
 	}
 
 	// Try finding in the child again in case it hasn't finished
 	if !childEmbed.finished() {
-		if _, ok := childEmbed.structField(s, name); ok {
-			return s.fields[name], true
+		if childEmbed.structField(s, name) != nil {
+			return s.fields[name]
 		}
 	}
 
-	return structField{}, false
+	return nil
 }
 
-func (s *structData) trySet(name string, sf structField) {
+func (s *structData) trySet(name string, sf *structField) {
 	if _, ok := s.fields[name]; ok {
 		return
 	}
-	sf.Index = append(s.curChildIndex, sf.Index...)
-	s.fields[name] = structField{
+	sf = &structField{
 		Nature: sf.Nature,
-		Index:  sf.Index,
+		Index:  append(s.curChildIndex, sf.Index...),
 	}
+	s.fields[name] = sf
 	if s.curParent != nil {
 		s.curParent.trySet(name, sf)
 	}
@@ -178,7 +178,7 @@ func StructFields(c *Cache, t reflect.Type) map[string]Nature {
 	if t == nil {
 		return table
 	}
-	t, k, _ := derefTypeKind(t, t.Kind())
+	t, k, _ := deref.TypeKind(t, t.Kind())
 	if k == reflect.Struct {
 		// lookup for a field with an empty name, which will cause to never find a
 		// match, meaning everything will have been cached.
@@ -195,7 +195,7 @@ type methodset struct {
 	cache          *Cache
 	rType          reflect.Type
 	kind           reflect.Kind
-	methods        map[string]method
+	methods        map[string]*method
 	numMethod, idx int
 }
 
@@ -204,11 +204,18 @@ type method struct {
 	nature Nature
 }
 
-func (s *methodset) method(name string) (method, bool) {
+func (s *methodset) setCache(c *Cache) {
+	s.cache = c
+	for _, m := range s.methods {
+		m.nature.SetCache(c)
+	}
+}
+
+func (s *methodset) method(name string) *method {
 	if s.methods == nil {
-		s.methods = make(map[string]method, s.numMethod)
-	} else if m, ok := s.methods[name]; ok {
-		return m, true
+		s.methods = make(map[string]*method, s.numMethod)
+	} else if m := s.methods[name]; m != nil {
+		return m
 	}
 	for ; s.idx < s.numMethod; s.idx++ {
 		rm := s.rType.Method(s.idx)
@@ -227,14 +234,14 @@ func (s *methodset) method(name string) (method, bool) {
 			// different indexes for different types which implement
 			// the same interface.
 		}
-		m := method{
+		m := &method{
 			Method: rm,
 			nature: nt,
 		}
 		s.methods[rm.Name] = m
 		if rm.Name == name {
-			return m, true
+			return m
 		}
 	}
-	return method{}, false
+	return nil
 }

conf/config.go

@@ -63,7 +63,7 @@ func New(env any) *Config {
 
 func (c *Config) WithEnv(env any) {
 	c.EnvObject = env
-	c.Env = Env(&c.NtCache, env)
+	c.Env = EnvWithCache(&c.NtCache, env)
 	c.Strict = c.Env.Strict
 }
 

conf/env.go

@@ -9,7 +9,14 @@ import (
 	"github.com/expr-lang/expr/types"
 )
 
-func Env(c *Cache, env any) Nature {
+// Env returns the Nature of the given environment.
+//
+// Deprecated: use EnvWithCache instead.
+func Env(env any) Nature {
+	return EnvWithCache(new(Cache), env)
+}
+
+func EnvWithCache(c *Cache, env any) Nature {
 	if env == nil {
 		n := c.NatureOf(map[string]any{})
 		n.Strict = true

docgen/docgen.go

@@ -85,7 +85,7 @@ func CreateDoc(i any) *Context {
 		PkgPath:   deref.Type(reflect.TypeOf(i)).PkgPath(),
 	}
 
-	env := conf.Env(new(nature.Cache), i)
+	env := conf.EnvWithCache(new(nature.Cache), i)
 	for name, t := range env.All() {
 		if _, ok := c.Variables[Identifier(name)]; ok {
 			continue

internal/deref/deref.go

@@ -45,3 +45,12 @@ func Value(v reflect.Value) reflect.Value {
 	}
 	return v
 }
+
+func TypeKind(t reflect.Type, k reflect.Kind) (_ reflect.Type, _ reflect.Kind, changed bool) {
+	for k == reflect.Pointer {
+		changed = true
+		t = t.Elem()
+		k = t.Kind()
+	}
+	return t, k, changed
+}