Skip to content

Add nil safe operator #173

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 7, 2021
Merged

Conversation

zachbadgett
Copy link
Contributor

Takes care of #147

Only thing this doesn't work on is when the env struct is predefined before compiling due to OpEqualString and OpEqualInt optimizations doing type casts.

Not completely happy with the way it's implemented, but it was the easiest to do without changing much core code.

@antonmedv
Copy link
Member

Awesome work! 👏 I'll try to take a closes look at this before merging.

@guillaume-merindol-cko
Copy link

Any news on this PR? :)

@antonmedv
Copy link
Member

Sorry for delay! @davidklassen can you also take a look? Thanks.

@davidklassen
Copy link
Contributor

LGTM

@antonmedv antonmedv merged commit baae791 into expr-lang:master Jun 7, 2021
@antonmedv
Copy link
Member

Found an error. In this case error should be returned, but instead is returned.

package main

import (
	"fmt"

	"github.com/antonmedv/expr"
)

type S struct {
	S *S
	A string
}

func main() {
	env := map[string]interface{}{
		"s": S{S: &S{S: &S{A: "OK"}}},
	}

	code := `s?.S.NOPE.A`

	program, err := expr.Compile(code, expr.Env(env))
	if err != nil {
		panic(err)
	}

	output, err := expr.Run(program, env)
	if err != nil {
		panic(err)
	}

	fmt.Println(output)
}

Part with S.NOPE should throw error as not nil safe.

@zachbadgett can you take a look? Thanks.

@antonmedv
Copy link
Member

The problem is with parser:

go run github.com/antonmedv/expr/cmd/exe -ast <<< 'a?.b.c'
PropertyNode{
	Node: PropertyNode{
		Node: IdentifierNode{
			Value: "a",
			NilSafe: true,
		},
		Property: "b",
-		NilSafe: true,
	},
	Property: "c",
-	NilSafe: true,
}

@antonmedv
Copy link
Member

One year later, I think this is not error any more)))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants