[ https://issues.apache.org/jira/browse/JEXL-384?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17629006#comment-17629006 ]
Henri Biestro commented on JEXL-384: ------------------------------------ I agree the current behaviour is flimsy. Since we now a way to describe whether an operator should be strict or not, there is no reason for the default to treat the add(String, null)/add(null,String) as a special case. It also seems the null operand handling could be performed in a more generic manner, ie before calling the operator if it is strict. > 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 > Assignee: Henri Biestro > 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)