test-fullautomation / python-jsonpreprocessor

A preprocessor for json files
Apache License 2.0
2 stars 2 forks source link

Error handling improvements (2) #208

Open HolQue opened 4 months ago

HolQue commented 4 months ago

I have some doubts about the outcome and the error messages caused by the following code examples:

This works:

"param1" : "value",
"param2" : ${param1}[0]

Result:

{'param1': 'value', 'param2': 'v'}

Also this works:

"param1" : "value",
"index"  : 0,
"param2" : ${param1}[${index}]

Result:

{'index': 0, 'param1': 'value', 'param2': 'v'}

But how to deal with this?

"param1" : "value",
"param2" : ${param1}[10]

Result:

Error: 'The variable '${param1}[10]' is not available!'!

Like in

https://github.com/test-fullautomation/python-jsonpreprocessor/issues/207

the square brackets are part of a dollar operator expression containing an existing parameter.

The question is: Do we want to allow the users to apply indices to strings, or not?

If not allowed, a possible error message could be:

Expression '${param1}[0]' cannot be evaluated. Reason: It is not allowed to apply indices to string parameters.

If allowed, the first results are OK. But in this case I would also expect that ${param1}[10] causes an "index out of range" error (because the maximum index in "value" is 4).

Finally: The current error message

'The variable '${param1}[10]' is not available!'!

should only be thrown in case of

"param1" : "value",

is not defined. Because if the parameter does not exist, we do not know the data type of this parameter. If we do not know the data type of this parameter, we cannot define what we do expect to find inside the square brackets. And then we only can tell that the entire thing does not exist.

In general:

Taking a look at error messages also mentioned in other tickets, it seems to me that the JsonPreprocessor in general is not really able to properly compute the relationship between X and Y in such expressions : ${X}[Y]. But there is a strong dependency: Y depends on the data type of X!

This dependency should be considered in error messages.

But this requires:

In this case we are able to make a very detailed check of the content of Y. And therefore we are also able to provide a very detailed error message.

Only in case of X or Y do not exist, we are lost. In this case we can fall back to the more common error message about an expression, that cannot be evaluated, or a not existing variable.

Please give this way a chance. It would improve a lot.

HolQue commented 4 months ago

A possible syntax convention could be:

Expression: ${X}[Y]

X is of type str => [Y] not allowed => expression ${X}[Y] is invalid

X is of type list => Y = 0 or positive integer within range(len(list)); not one of these characters: '+', '-', ':'

X is of type dict => Y must be of type str (following the naming convention for key names) Python 3 also allows integers as key names. I would prefer not to allow this.

namsonx commented 4 weeks ago

Hello Holger,

Until we finalize the question Do we want to allow the users to apply indices to strings, or not?, I created a small change 5cf9fc55bba to forward the error message from Python to our JsonPreprocessor in this case. Now, the error message in this case is Could not resolve expression '${param1}[10]'. Reason: string index out of range

Thank you, Son