From a16e0053c0250010de56083c4c2290e9d01232f5 Mon Sep 17 00:00:00 2001 From: Yonghwan SO Date: Sat, 1 Oct 2022 22:11:38 +0900 Subject: [PATCH 1/2] added more test cases to check #157 and side effects of #164 --- if_test.go | 15 +++++++++++++++ math_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/if_test.go b/if_test.go index e788146..ecd8515 100644 --- a/if_test.go +++ b/if_test.go @@ -314,3 +314,18 @@ func Test_Render_If_Variable_Not_Set_But_Or_Condition_Right_Node_Is_True(t *test r.NoError(err) r.Equal("hi", s) } + +func Test_Condition_UnsetIsNil(t *testing.T) { + r := require.New(t) + ctx := NewContext() + + input := `<%= paths == nil %>` + s, err := Render(input, ctx) + r.NoError(err) + r.Equal("true", s) + + input = `<%= nil == paths %>` + s, err = Render(input, ctx) + r.NoError(err) + r.Equal("true", s) +} diff --git a/math_test.go b/math_test.go index 19eacd3..1649cfb 100644 --- a/math_test.go +++ b/math_test.go @@ -93,6 +93,47 @@ func Test_Render_String_Math(t *testing.T) { } } +func Test_Render_Operator_UndefinedVar(t *testing.T) { + tests := []struct { + operator string + result interface{} + errorExpected bool + }{ + {"+", "", true}, + {"-", "", true}, + {"/", "", true}, + {"*", "", true}, + {">", "", true}, + {">=", "", true}, + {"<=", "", true}, + {"<", "", true}, + {"==", "false", false}, + {"!=", "true", false}, + } + for _, tc := range tests { + t.Run(tc.operator, func(t *testing.T) { + r := require.New(t) + input := fmt.Sprintf("<%%= undefined %s 3 %%>", tc.operator) + s, err := Render(input, NewContext()) + if tc.errorExpected { + r.Error(err, "undefined %s 3 --> '%v'", tc.operator, tc.result) + } else { + r.NoError(err, "undefined %s 3 --> '%v'", tc.operator, tc.result) + } + r.Equal(tc.result, s, "undefined %s 3", tc.operator) + + input = fmt.Sprintf("<%%= 3 %s unknown %%>", tc.operator) + s, err = Render(input, NewContext()) + if tc.errorExpected { + r.Error(err, "3 %s undefined --> '%v'", tc.operator, tc.result) + } else { + r.NoError(err, "3 %s undefined --> '%v'", tc.operator, tc.result) + } + r.Equal(tc.result, s, "undefined %s 3", tc.operator) + }) + } +} + func Test_Render_String_Concat_Multiple(t *testing.T) { r := require.New(t) From 4684281dd889b036047ea44cca392289299cdfd5 Mon Sep 17 00:00:00 2001 From: Yonghwan SO Date: Sat, 1 Oct 2022 22:30:28 +0900 Subject: [PATCH 2/2] fixed #164's bug by focusing on the operator instead of the value --- compiler.go | 51 +++++++++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/compiler.go b/compiler.go index 7841ddc..f103c77 100644 --- a/compiler.go +++ b/compiler.go @@ -451,30 +451,29 @@ func (c *compiler) evalIdentifier(node *ast.Identifier) (interface{}, error) { func (c *compiler) evalInfixExpression(node *ast.InfixExpression) (interface{}, error) { lres, err := c.evalExpression(node.Left) - if err != nil { - if _, ok := err.(*ErrUnknownIdentifier); ok { - lres = false - } else { - return nil, err - } - } - if node.Operator == "&&" { - if !c.isTruthy(lres) { - return false, nil - } + if err != nil && node.Operator != "==" && node.Operator != "!=" { + return nil, err + } // nil lres is acceptable only for '==' and '!=' + + switch { // fast return + case node.Operator == "&&" && !c.isTruthy(lres): + return false, nil + case node.Operator == "||" && c.isTruthy(lres): + return true, nil } rres, err := c.evalExpression(node.Right) - if err != nil { - if _, ok := err.(*ErrUnknownIdentifier); !ok { - return nil, err - } else { - rres = false - } - } + if err != nil && node.Operator != "==" && node.Operator != "!=" { + return nil, err + } // nil rres is acceptable only for '==' and '!=' + switch node.Operator { case "&&", "||": - return c.boolsOperator(lres, rres, node.Operator) + return c.isTruthy(rres), nil + } // fast return or this. '&&' and '||' end here + + if nil == lres || nil == rres { + return c.simpleOperator(lres, rres, node.Operator) } switch t := lres.(type) { @@ -493,10 +492,7 @@ func (c *compiler) evalInfixExpression(node *ast.InfixExpression) (interface{}, return c.floatsOperator(t, r, node.Operator) } case bool: - return c.boolsOperator(lres, rres, node.Operator) - case nil: - return nil, nil } return nil, fmt.Errorf("unable to operate (%s) on %T and %T ", node.Operator, lres, rres) } @@ -605,6 +601,17 @@ func (c *compiler) stringsOperator(l string, r interface{}, op string) (interfac return nil, fmt.Errorf("unknown operator for string %s", op) } +func (c *compiler) simpleOperator(l interface{}, r interface{}, op string) (interface{}, error) { + switch op { + case "!=": + return l != r, nil + case "==": + return l == r, nil + default: + return nil, fmt.Errorf("unknown operator '%s' on '%T' and '%T' ", op, l, r) + } +} + func (c *compiler) evalCallExpression(node *ast.CallExpression) (interface{}, error) { var rv reflect.Value if node.Callee != nil {