Skip to content

Commit 8841c48

Browse files
antonmedvantonmedv
authored
Merge branch 'master' into vm-and-runtime-improvements
2 parents 7ea86c9 + 3d0aec6 commit 8841c48

File tree

5 files changed

+218
-11
lines changed

5 files changed

+218
-11
lines changed

‎checker/checker_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -760,8 +760,8 @@ func TestCheck_TaggedFieldName(t *testing.T) {
760760

761761
config := conf.CreateNew()
762762
expr.Env(struct {
763-
x struct {
764-
y bool `expr:"bar"`
763+
X struct {
764+
Y bool `expr:"bar"`
765765
} `expr:"foo"`
766766
}{})(config)
767767
expr.AsBool()(config)

‎checker/nature/nature.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -404,8 +404,15 @@ func (n *Nature) getSlow(c *Cache, name string) (Nature, bool) {
404404
if nt, ok := n.MethodByName(c, name); ok {
405405
return nt, true
406406
}
407-
if n.Kind == reflect.Struct {
408-
if sf := n.structField(c, nil, name); sf != nil {
407+
t, k, changed := deref.TypeKind(n.Type, n.Kind)
408+
if k == reflect.Struct {
409+
var sd *structData
410+
if changed {
411+
sd = c.getStruct(t).structData
412+
} else {
413+
sd = n.structData
414+
}
415+
if sf := sd.structField(c, nil, name); sf != nil {
409416
return sf.Nature, true
410417
}
411418
}

‎checker/nature/utils.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,13 @@ func (s *structData) structField(c *Cache, parentEmbed *structData, name string)
5252
// Lookup own fields first.
5353
for ; s.ownIdx < s.numField; s.ownIdx++ {
5454
field := s.rType.Field(s.ownIdx)
55-
// BUG: we should skip if !field.IsExported() here
56-
5755
if field.Anonymous && s.anonIdx < 0 {
5856
// start iterating anon fields on the first instead of zero
5957
s.anonIdx = s.ownIdx
6058
}
59+
if !field.IsExported() {
60+
continue
61+
}
6162
fName, ok := fieldName(field.Name, field.Tag)
6263
if !ok || fName == "" {
6364
// name can still be empty for a type created at runtime with

‎test/issues/840/issue_test.go‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,23 +8,28 @@ import (
88
)
99

1010
func TestEnvFieldMethods(t *testing.T) {
11-
program, err := expr.Compile(`Func(0)`, expr.Env(&Env{}))
11+
program, err := expr.Compile(`Func() + Int`, expr.Env(&Env{}))
1212
require.NoError(t, err)
1313

1414
env := &Env{}
1515
env.Func = func() int {
16-
return 42
16+
return 40
17+
}
18+
env.EmbeddedEnv = &EmbeddedEnv{
19+
Int: 2,
1720
}
1821

19-
out, err := expr.Run(program, Env{})
22+
out, err := expr.Run(program, env)
2023
require.NoError(t, err)
2124

2225
require.Equal(t, 42, out)
2326
}
2427

2528
type Env struct {
26-
EmbeddedEnv
29+
*EmbeddedEnv
2730
Func func() int
2831
}
2932

30-
type EmbeddedEnv struct{}
33+
type EmbeddedEnv struct {
34+
Int int
35+
}

‎test/issues/844/issue_test.go‎

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
package main
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
"github.com/expr-lang/expr"
8+
"github.com/expr-lang/expr/checker"
9+
"github.com/expr-lang/expr/conf"
10+
"github.com/expr-lang/expr/internal/testify/require"
11+
"github.com/expr-lang/expr/parser"
12+
)
13+
14+
func TestIssue844(t *testing.T) {
15+
testCases := []struct {
16+
name string
17+
env any
18+
expression string
19+
shouldFail bool
20+
}{
21+
{
22+
name: "exported env, exported field",
23+
env: ExportedEnv{},
24+
expression: `ExportedEmbedded`,
25+
shouldFail: false,
26+
},
27+
{
28+
name: "exported env, unexported field",
29+
env: ExportedEnv{},
30+
expression: `unexportedEmbedded`,
31+
shouldFail: true,
32+
},
33+
{
34+
name: "exported env, exported field inherited from exported field",
35+
env: ExportedEnv{},
36+
expression: `Str`,
37+
shouldFail: false,
38+
},
39+
{
40+
name: "exported env, unexported field inherited from exported field",
41+
env: ExportedEnv{},
42+
expression: `str`,
43+
shouldFail: true,
44+
},
45+
{
46+
name: "exported env, exported field inherited from exported field",
47+
env: ExportedEnv{},
48+
expression: `Integer`,
49+
shouldFail: false,
50+
},
51+
{
52+
name: "exported env, unexported field inherited from exported field",
53+
env: ExportedEnv{},
54+
expression: `integer`,
55+
shouldFail: true,
56+
},
57+
{
58+
name: "exported env, exported field directly accessed from exported field",
59+
env: ExportedEnv{},
60+
expression: `ExportedEmbedded.Str`,
61+
shouldFail: false,
62+
},
63+
{
64+
name: "exported env, unexported field directly accessed from exported field",
65+
env: ExportedEnv{},
66+
expression: `ExportedEmbedded.str`,
67+
shouldFail: true,
68+
},
69+
{
70+
name: "exported env, exported field directly accessed from exported field",
71+
env: ExportedEnv{},
72+
expression: `unexportedEmbedded.Integer`,
73+
shouldFail: true,
74+
},
75+
{
76+
name: "exported env, unexported field directly accessed from exported field",
77+
env: ExportedEnv{},
78+
expression: `unexportedEmbedded.integer`,
79+
shouldFail: true,
80+
},
81+
{
82+
name: "unexported env, exported field",
83+
env: unexportedEnv{},
84+
expression: `ExportedEmbedded`,
85+
shouldFail: false,
86+
},
87+
{
88+
name: "unexported env, unexported field",
89+
env: unexportedEnv{},
90+
expression: `unexportedEmbedded`,
91+
shouldFail: true,
92+
},
93+
{
94+
name: "unexported env, exported field inherited from exported field",
95+
env: unexportedEnv{},
96+
expression: `Str`,
97+
shouldFail: false,
98+
},
99+
{
100+
name: "unexported env, unexported field inherited from exported field",
101+
env: unexportedEnv{},
102+
expression: `str`,
103+
shouldFail: true,
104+
},
105+
{
106+
name: "unexported env, exported field inherited from exported field",
107+
env: unexportedEnv{},
108+
expression: `Integer`,
109+
shouldFail: false,
110+
},
111+
{
112+
name: "unexported env, unexported field inherited from exported field",
113+
env: unexportedEnv{},
114+
expression: `integer`,
115+
shouldFail: true,
116+
},
117+
{
118+
name: "unexported env, exported field directly accessed from exported field",
119+
env: unexportedEnv{},
120+
expression: `ExportedEmbedded.Str`,
121+
shouldFail: false,
122+
},
123+
{
124+
name: "unexported env, unexported field directly accessed from exported field",
125+
env: unexportedEnv{},
126+
expression: `ExportedEmbedded.str`,
127+
shouldFail: true,
128+
},
129+
{
130+
name: "unexported env, exported field directly accessed from exported field",
131+
env: unexportedEnv{},
132+
expression: `unexportedEmbedded.Integer`,
133+
shouldFail: true,
134+
},
135+
{
136+
name: "unexported env, unexported field directly accessed from exported field",
137+
env: unexportedEnv{},
138+
expression: `unexportedEmbedded.integer`,
139+
shouldFail: true,
140+
},
141+
}
142+
143+
for _, tc := range testCases {
144+
t.Run(tc.name, func(t *testing.T) {
145+
config := conf.New(tc.env)
146+
tree, err := parser.ParseWithConfig(tc.expression, config)
147+
require.NoError(t, err)
148+
149+
_, err = new(checker.Checker).PatchAndCheck(tree, config)
150+
if tc.shouldFail {
151+
require.Error(t, err)
152+
errStr := err.Error()
153+
if !strings.Contains(errStr, "unknown name") &&
154+
!strings.Contains(errStr, " has no field ") {
155+
t.Fatalf("expected a different error, got: %v", err)
156+
}
157+
} else {
158+
require.NoError(t, err)
159+
}
160+
161+
// We add this because the issue was actually not catching something
162+
// that sometimes failed with the error:
163+
// reflect.Value.Interface: cannot return value obtained from unexported field or method
164+
// This way, we test that everything we allow passing is also
165+
// allowed later
166+
_, err = expr.Eval(tc.expression, tc.env)
167+
if tc.shouldFail {
168+
require.Error(t, err)
169+
} else {
170+
require.NoError(t, err)
171+
}
172+
})
173+
}
174+
}
175+
176+
type ExportedEnv struct {
177+
ExportedEmbedded
178+
unexportedEmbedded
179+
}
180+
181+
type unexportedEnv struct {
182+
ExportedEmbedded
183+
unexportedEmbedded
184+
}
185+
186+
type ExportedEmbedded struct {
187+
Str string
188+
str string
189+
}
190+
191+
type unexportedEmbedded struct {
192+
Integer int
193+
integer int
194+
}

0 commit comments

Comments
 (0)