[
https://issues.apache.org/jira/browse/JEXL-384?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Hussachai Puripunpinyo updated JEXL-384:
----------------------------------------
Description:
https://issues.apache.org/jira/browse/JEXL-359
{quote}A typical case is '+' for string and null where one would like to
consider null as a valid argument even if arithmetic is strict.
{quote}
While the above reason is valid, the code introduced in JEXL-359 causes more
confusion, and the behavior is now inconsistent.
Also, I'd like to give some counter arguments about not supporting null +
string in the strict mode. If we want to make null + string works even if
arithmetic is strict, there will be no difference between "" + "ABC" and null +
"ABC" because both cases yield "ABC" in both strict and non-strict mode. In our
code base, we set the engine to be strict, so users have to be careful about
null since it can give an undesirable result. In a non-strict mode, we don't
have a way to distinguish whether the result is the combination of null or
empty string and a string. Users need to explicitly check for null, but there
will be no enforcement
which means nobody will do that check. That's why we prefer it to be strict and
a user has to check null before using or use some namespace functions that we
provide where null will be explicitly handled. Otherwise, the exception will be
thrown.
Let me elaborate why some part of JEXL-359 causes the behavior to be
inconsistent.
*Strict Mode*
{code:java}
var i = null;
i + 'ABC'; // This will throw an exception.
null + 'ABC'; // This yields 'ABC' - the same as non-strict mode.
{code}
*Non Strict Mode*
{code:java}
var i = null;
i + 'ABC'; // This yields 'ABC';
null + 'ABC'; // This also yields 'ABC';
{code}
You can see that the behavior of null + 'ABC' in the strict mode is not
consistent with the null variable.
Also, there is a way to allow string concatenation with null using a namespace
function in a strict mode, and I think JEXL shouldn't make an exception for
this one particular case.
I'd like to propose the PR with some regression tests that applies to only null
(literal) + string case.
[https://github.com/apache/commons-jexl/pull/136]
*Note:* I ignored one test *testNullArgs* because the feature doesn't seem to
be defined well. It won't work with numeric types.
I can pass around the JexlOperator to isStrict function everywhere to address
that, but I'm trying to refrain from changing things too much since you may
have some ideas and my code might get in the way.
Please let me know if I can help with that, or you can merge my PR, take part
of it or ignore it completely :)
My idea about fixing *testNullArgs* properly is that all toString, toDouble,
toBigDecimal functions should take an additional argument which is JexlOperator
where a user can override for specific operator as stated in testNullArgs test.
If that sounds right to you, I can help creating another Jira and PR to tackle
this. Thank you.
was:
https://issues.apache.org/jira/browse/JEXL-359
{quote}A typical case is '+' for string and null where one would like to
consider null as a valid argument even if arithmetic is strict.
{quote}
While the above reason is valid, the code introduced in JEXL-359 causes more
confusion, and the behavior is now inconsistent.
Also, I'd like to give some counter arguments about not supporting null +
string in the strict mode. If we want to make null + string works even if
arithmetic is strict, there will be no difference between "" + "ABC" and null +
"ABC" because both cases yield "ABC" in both strict and non-strict mode. In our
code base, we set the engine to be strict, so users have to be careful about
null since it can give an undesirable result. In a non-strict mode, we don't
have a way to distinguish whether the result is the combination of null or
empty string and a string. Users need to explicitly check for null, but there
will be no enforcement
which means nobody will do that check. That's why we prefer it to be strict and
a user has to check null before using or use some namespace functions that we
provide where null will be explicitly handled. Otherwise, the exception will be
thrown.
Let me elaborate why some part of JEXL-359 causes the behavior to be
inconsistent.
*Strict Mode*
{code:java}
var i = null;
i + 'ABC'; // This will throw an exception.
null + 'ABC'; // This yields 'ABC' - the same as non-strict mode.
{code}
*Non Strict Mode*
{code:java}
var i = null;
i + 'ABC'; // This yields 'ABC';
null + 'ABC'; // This also yields 'ABC';
{code}
You can see that the behavior of null + 'ABC' in the strict mode is not
consistent with the null variable.
Also, there is a way to allow string concatenation with null using a namespace
function in a strict mode, and I think JEXL shouldn't make an exception for
this one particular case.
I'd like to propose the PR with some regression tests that applies to only null
(literal) + string case.
[https://github.com/apache/commons-jexl/pull/136]
*Note:* I ignored one test *testNullArgs* because the feature doesn't seem to
be defined well. It won't work with numeric types.
I can pass around the JexlOperator to isStrict function everywhere to address
that, but I'm trying to refrain from changing things too much since you may
have some ideas and my code might get in the way.
Please let me know if I can help with that, or you can merge my PR, take part
of it or ignore it completely :)
> Revert null + String behavior
> -----------------------------
>
> Key: JEXL-384
> URL: https://issues.apache.org/jira/browse/JEXL-384
> Project: Commons JEXL
> Issue Type: Task
> Affects Versions: 3.3
> Reporter: Hussachai Puripunpinyo
> Priority: Major
>
> https://issues.apache.org/jira/browse/JEXL-359
> {quote}A typical case is '+' for string and null where one would like to
> consider null as a valid argument even if arithmetic is strict.
> {quote}
> While the above reason is valid, the code introduced in JEXL-359 causes more
> confusion, and the behavior is now inconsistent.
> Also, I'd like to give some counter arguments about not supporting null +
> string in the strict mode. If we want to make null + string works even if
> arithmetic is strict, there will be no difference between "" + "ABC" and null
> + "ABC" because both cases yield "ABC" in both strict and non-strict mode. In
> our code base, we set the engine to be strict, so users have to be careful
> about null since it can give an undesirable result. In a non-strict mode, we
> don't have a way to distinguish whether the result is the combination of null
> or empty string and a string. Users need to explicitly check for null, but
> there will be no enforcement
> which means nobody will do that check. That's why we prefer it to be strict
> and a user has to check null before using or use some namespace functions
> that we provide where null will be explicitly handled. Otherwise, the
> exception will be thrown.
> Let me elaborate why some part of JEXL-359 causes the behavior to be
> inconsistent.
> *Strict Mode*
> {code:java}
> var i = null;
> i + 'ABC'; // This will throw an exception.
> null + 'ABC'; // This yields 'ABC' - the same as non-strict mode.
> {code}
>
> *Non Strict Mode*
> {code:java}
> var i = null;
> i + 'ABC'; // This yields 'ABC';
> null + 'ABC'; // This also yields 'ABC';
> {code}
>
> You can see that the behavior of null + 'ABC' in the strict mode is not
> consistent with the null variable.
> Also, there is a way to allow string concatenation with null using a
> namespace function in a strict mode, and I think JEXL shouldn't make an
> exception for this one particular case.
> I'd like to propose the PR with some regression tests that applies to only
> null (literal) + string case.
> [https://github.com/apache/commons-jexl/pull/136]
> *Note:* I ignored one test *testNullArgs* because the feature doesn't seem to
> be defined well. It won't work with numeric types.
> I can pass around the JexlOperator to isStrict function everywhere to address
> that, but I'm trying to refrain from changing things too much since you may
> have some ideas and my code might get in the way.
> Please let me know if I can help with that, or you can merge my PR, take part
> of it or ignore it completely :)
> My idea about fixing *testNullArgs* properly is that all toString, toDouble,
> toBigDecimal functions should take an additional argument which is
> JexlOperator where a user can override for specific operator as stated in
> testNullArgs test. If that sounds right to you, I can help creating another
> Jira and PR to tackle this. Thank you.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)