test-fullautomation / python-jsonpreprocessor

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

Mistakable error message: Variable not available #186

Open HolQue opened 4 months ago

HolQue commented 4 months ago

The JSON code:

"intval" : 1,
"testlist" : ["B", 2],
"param_${testlist}['${intval}']}" : 3

causes:

Error: 'The variable '${testlist}['1']' is not available!'!

Here I have doubts about the meaning of the single quotes inside the square brackets. Are they allowed/required or not? The requirements are unclear to me.

But I expect that expressions are resolved from innermost to outermost. What should be the order of computation in the code example above?

I expect:

  1. Detect parameter intval
  2. Parameter intval does exist
  3. Parameter intval is of type int
  4. Parameter intval is wrapped in single quotes, therefore will be interpreted as string
  5. This is found inside square brackets, therefore can either be an index or a key name
  6. The parameter, the square bracket expression belongs to, is of type list, therefore the content of the square brackets is expected to be of type int (but is of type str, and this is an error).

In my opinion it should be possible to provide a corresponding error message, telling that list indices are expected to be of type int.

To tell only that a variable does not exist, does not really help the users.

And independent from this: Quotes should not be accepted as part of parameter names (or dictionary key names). Seems that the JsonPreprocessor still has a common issue with proper handling of quotes inside expressions.

Reference: JPP_0268

test-fullautomation commented 4 months ago

Hi Son,

a possible speaking error messge would be: ${testlist} expects integer as index. Got string instead in 'param_${testlist}['${intval}']}'.

if above is not possible, then a generic message could be: Expression '"param_${testlist}['${intval}']}" : 3' can't be evaluated.

Thank you, Thomas

test-fullautomation commented 4 months ago

Hi Son, what is the status of this ticket? Thank you, Thomas

namsonx commented 4 months ago

Hello Thomas,

I will implement this ticket in next 0.11.0 release.

Thank you, Son

HolQue commented 2 months ago

Update:

The following line from code example above, is invalid in between:

"param_${testlist}['${intval}']}" : 3

Because of the decision, not to create parameter names dynamically.

Therefore the code example needs to be adapted:

"intval" : 1,
"testlist" : ["B", 2],
"param" : ${testlist}['${intval}']

Result:

Error: 'The variable '${testlist}['1']' is not available!'!

That's wrong. This is still a type issue, and not a missing variable issue.

Expected error message: List indices have to be of type int.

namsonx commented 1 month ago

Hello Thomas,

I implemented this ticket and pushed the change to stabi branch.

Thank you, Son

HolQue commented 3 weeks ago
"intval" : 1,
"testlist" : ["B", 2],
"param" : ${testlist}['${intval}']

causes:

Error: '${testlist} expects integer as index. Got string instead in ${testlist}['${intval}']'!

This is like expected now.

But the same with a string parameter:

"strval" : "ABC",
"testlist" : ["B", 2],
"param" : ${testlist}[${strval}]

causes:

Error: 'The parameter '${testlist}['ABC']' is not available!'!

But expected is the same error message like above.

namsonx commented 2 weeks ago

Hello Holger,

Thank you for your finding! I'm going to forward the exception from Python the error message for both cases like below: Exception: Could not resolve expression '${testlist}['1']'. Reason: list indices must be integers, not str Exception: Could not resolve expression '${testlist}['ABC']'. Reason: list indices must be integers, not str If the new error message is fine, I will push the change to stabi branch. Please give me your feedback?

Thank you, Son

HolQue commented 2 weeks ago

Hi Son,

that's fine to me. And you should adapt the wording a little bit.

Either:

list indices must be of type 'int', not 'str'

or:

list indices must be integers, not strings

HolQue commented 2 weeks ago

Hi Son,

see also https://github.com/test-fullautomation/python-jsonpreprocessor/issues/306

Should be aligned.

namsonx commented 2 weeks ago

Hi Son,

that's fine to me. And you should adapt the wording a little bit.

Either:

list indices must be of type 'int', not 'str'

or:

list indices must be integers, not strings

Hello Holger,

The reason list indices must be integers, not str is forwarded from Python's exception. Do we need to modify the exception message from Python?

Thank you, Son

HolQue commented 2 weeks ago

Hi Son,

oops - I was not aware of that. No modification please.

namsonx commented 2 weeks ago

Hello Holger,

I push commit f3df857af39 to solve your review comment.

Thank you, Son

HolQue commented 1 day ago

Retest successful. Issue can be closed.