Skip to content

null-safe operator should detect target type as primitive #65098

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jdconrad opened this issue Nov 16, 2020 · 4 comments
Closed

null-safe operator should detect target type as primitive #65098

jdconrad opened this issue Nov 16, 2020 · 4 comments
Labels
>bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache Team:Core/Infra Meta label for core/infra team

Comments

@jdconrad
Copy link
Contributor

When we use the null-safe operator in Painless, we ignore the target type. This means that we may not know a null-safe operator is invalid until run-time when we could instead know at compile-time.

Take the following scripts:

<1> params?.test && ...
<2> params?.test.value && ...

<1> We know test must be a boolean primitive, so in this case the error should occur at compile-time to inform the script author of the invalid use of a null-safe operator.
<2> We know value must be a boolean primitive, so in this case the error should occur at compile-time to inform the script author of the invalid use of a null-safe operator as well. However, to handle this case appropriately information must be returned during the semantic phase to inform the node handling value there is at least one null-safe operator used as part of this variable/field chain.

@jdconrad jdconrad added >bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache labels Nov 16, 2020
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Nov 16, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@jdconrad
Copy link
Contributor Author

I've added this issue under discuss because I'd like to think through whether or not the second case is worth handling at compile-time.

jdconrad added a commit that referenced this issue Nov 16, 2020
…65099)

This reverts a change where null-safe was enhanced to cause a compile-time error instead of a run-
time error when the target value was a primitive type. The reason for the reversion is consistency 
across def/non-def types and versions. I've added a follow up issue to fix this behavior in general 
(#65098).
jdconrad added a commit that referenced this issue Nov 16, 2020
…65099)

This reverts a change where null-safe was enhanced to cause a compile-time error instead of a run-
time error when the target value was a primitive type. The reason for the reversion is consistency 
across def/non-def types and versions. I've added a follow up issue to fix this behavior in general 
(#65098).
jdconrad added a commit that referenced this issue Nov 16, 2020
…65099)

This reverts a change where null-safe was enhanced to cause a compile-time error instead of a run-
time error when the target value was a primitive type. The reason for the reversion is consistency 
across def/non-def types and versions. I've added a follow up issue to fix this behavior in general 
(#65098).
@rjernst rjernst added the needs:triage Requires assignment of a team area label label Dec 3, 2020
@stu-elastic stu-elastic removed the needs:triage Requires assignment of a team area label label Dec 9, 2020
@stu-elastic
Copy link
Contributor

We should do this after #61388

@rjernst
Copy link
Member

rjernst commented Apr 4, 2025

After considering this for many years, the benefits don't outweigh the large time investment that would be necessary, so closing.

@rjernst rjernst closed this as not planned Won't fix, can't repro, duplicate, stale Apr 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache Team:Core/Infra Meta label for core/infra team
Projects
None yet
Development

No branches or pull requests

4 participants