Expression.parse() returns OK with undefined variables

[lostminds]

Tested versions

4.2.1.stable

System information

macOS 14.2.1

Issue description

The Expression class allows parsing of mathematical expressions, allowing optionally to include variables that can be used. Using variables in the expression that are not defined will cause the expression to fail on expression.execute() as expected. However, using expression.parse(), which I understand should evaluate if the expression is before executing, returns OK even if undefined variables are present.

This mean that it's difficult to check if an expression includes undefined variables (for example based on user input) before calling expression.execute(), and execute will throw an error if there are such undefined variables:

E 0:00:08:0338   Script.gd:93 @ expression_test(): self can't be used because instance is null (not passed)
  <C++ Error>    Condition "p_show_error" is true. Returning: Variant()
  <C++ Source>   core/math/expression.cpp:1507 @ execute()
  <Stack Trace>  Script.gd:93 @ expression_test()
                 Script.gd:101 @ parse_value()
                 ValueField.gd:60 @ on_value_field_text_submitted()

This same error "self can't be used because instance is null (not passed)" is also returned in expression.get_error_text() and is not very helpful in my mind, as it does not mention the offending undefined variable, and instead makes it seem like you've referenced self, which is not the case and may also be a bug?

Steps to reproduce

Set up the following function:

func expression_test(string:String):
	var expression:Expression = Expression.new()
	print("expression test with string: '"+string+"'")
	var parse_error = expression.parse(string)
	print("parse_error: "+str(parse_error))
	if (parse_error == OK):
		var parse_result = expression.execute()
		print("parse_result: "+str(parse_result))
		print("has_execute_failed: "+str(expression.has_execute_failed()))
	print("get_error_text: "+expression.get_error_text())

And then test it with:

expression_test("1+1") #simple test expression
expression_test("1+a") #expression with undefined variable!

Generates output:

expression test with string: '1+1'
parse_error: 0
parse_result: 2
has_execute_failed: false
get_error_text: 
expression test with string: '1+a'
parse_error: 0
parse_result: <null>
has_execute_failed: true
get_error_text: self can't be used because instance is null (not passed)

Along with the error above being thrown when calling expression.execute() for "1+a"

Expected outcome for me would be that expression.parse("1+a") would return an error as it contains an undefined variable, and then that expression.get_error_text() would return something like "undefined variable 'a'"

Minimal reproduction project (MRP)

See code above

[xorblo-doitus]

I think parse() assumes that undefined variables will be attributes of the base_instance object passed to execute().

If you pass a base_instance the error message is a bit more helpful: Invalid named index 'a' for base type Object.

So parse() should have a base_instance parameter too to be able to check for undefined variables/attributes/methods.

[lostminds]

So parse() should have a base_instance parameter too to be able to check for undefined variables/attributes/methods.

Yes, that could be a good solution. And critically allow the (default) case of passing null as the base_instance object in parse() to give correct parsing errors for the case of not passing a base_instance object in execute().

[Tomas-Domas]

It's definitely intended that unknown identifiers be kept for execute to take care of. Unless you pass the base_instance to parse as well, it's fine as is.