Support empty string for safe access operator Currently only an empty json string, i.e. a string in a string was supported: '""'.

[ModProg]

Please follow this template, if applicable.

Description

Fixes ?. for empty strings

Usage

{""?.test == "null"}

Additional Notes

TBD: should a json string i.e. '""' be considered null?

Checklist

Please make sure you can check all the boxes that apply to this PR.

• I added my changes to CHANGELOG.md, if appropriate.

• I used cargo fmt to automatically format all code before committing

[oldwomanjosiah]

What was the error you were seeing when indexing an empty string?

[oldwomanjosiah]

This isn't for this PR, but should we be throwing an error here? It doesn't seem like we'd want to silently ignore out of bounds accesses

[ModProg]

What was the error you were seeing when indexing an empty string?

@oldwomanjosiah

When I add the test to master I get:

thread 'eval::tests::safe_access_to_empty' panicked at 'Output was not Ok(_): ConversionError(ConversionError { value: "", target_type: "json-value", source: Some(Error("EOF while parsing a value", line: 1, column: 0)) })', crates/simplexpr/src/eval.rs:392:5

[elkowar]

I'm not sure if this is actually desirable behavior, as empty strings will be valid values in many situations -- having a differentiation between empty strings and null does seem quite useful. However, I guess it also depends on how empty strings are treated elsewhere, which to be honest, I'm not 100% sure at this point -- and it miiiiiight be useful to then provide some easier way to convert empty strings into null... easier than (foo == "") ? "null" : foo

[ModProg]

I'm not sure if this is actually desirable behavior, as empty strings will be valid values in many situations

yes, but ""?.test would be an error otherwise so this is not breaking anything.

the only situation where behavior wouldn't be unambiguous would be ""?:.

[ModProg]

I think we should only make an empty string "" and not an empty json string '""' null.

If you use a ? operator you actively decide to use the value as json, so as long as it is documented it is fine to add this behavior.

[elkowar]

That i'd be more fine with, sure! Although this behavior should probably be documented very well

[w-lfchen]

i've adopted this pr in #1176 (i want to finally get this merged), this pr can therefore be closed.
thank you for your work!

[elkowar]

Closing, as this has been merged as part of #1176. Thanks a ton for your work!