Skip to content

Conversation

tufitko
Copy link
Contributor

@tufitko tufitko commented Dec 4, 2021

#173 (comment)
fix for that just change scope for nilsafe variable :)

also seems IdentifierNode shouldn't have NilSafe option

require.Equal(t, false, output)
}

func TestExpr_nil_safe_first_ident(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still a valid use case. If foo? is nil the entire chain should be considered safe

Copy link
Contributor Author

@tufitko tufitko Dec 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really, just type foo and you will get an error unknown name foo, to avoid this error you can use AllowUndefinedVariables option. (or we can support something like ?.foo)

foo?.bar - here operator ?. should be affect bar property only
foo.bar?.baz - and here only on baz, bar (like foo above) isn't nilsafe

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not about foo being the first variable, it's about the rest of the chain being safe after the nil operator.

If you have foo.missing?.test.err having missing be nil should keep the rest of the chain safe, a nil test shouldn't cause the compiler to error on err unless missing is not nil

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO not supporting the nil operator on the first identifier would be a weird inconsistency but at the very least you need to support this use case: https://github.com/antonmedv/expr/pull/211/files#diff-c7a89724039da4beba41207c0d28a3d27c3474ceee39acedf5ff5eb3207691e4R997

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, maybe we need to add option to js-like fetch from objects?
in go getting an unknown property is forbidden and returned error
in js getting an unknown property returned undefined

@antonmedv antonmedv closed this Oct 6, 2022
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.

3 participants