The patch

Note that this patch also solves XRP-99 .

Replaces previous patch.

Fixed FormulaParser.jj so that also bracketed variables (those variables with exotic names, written like {this.is.an.exotic.named.variable}) are added to the parser variables list.

Changed the DivideFunction so that Math.max(a.scale,b.scale) is used, and 5/2 = 3 while 5/2.0 = 2.5 and 5/2.00 = 2.50 etc..

Simone,

Thanks for your patch, though a few remarks on the function parser changes.

From a quick look at the changes, I was a bit suprised by the simplicity of the change for adding and/or infix notation. I think with this approach you'll have an unexpected evaluation-order. Preferably, OR should have a greater precendence than AND, followed by the comparison operators.

I'm also wondering (havn't tested) if it can handle more complex expressions such as:

3 < 5 and 7 < 9

I also don't see where the newly added not() rule is called.

BTW, the example you've given earlier won't really work either I guess:

(country = "italy" or country = "sanmarino" or country = "vatican") and age > 18

since brackets are not supported?

(Not that you claimed it would be either, it's just that with infix notation it becomes more important)

Yes Bruno, you are right, operands precedence is wrong, right now OR and AND have same priority of relational and equality. I'm writing some junit tests to make sure I manage to get it right.

So, the common java order is :
postfix : expr++ expr--
unary : ++expr --expr +expr -expr ! ~
multiplicative : * / %
additive : + -
shift : << >> >>>
relational : < <= > >= instanceof
equality : == !=
bitwise AND : &
bitwise exclusive OR : ^
bitwise inclusive OR : |
logical AND : &&
logical OR : ||
ternary : ?:
assignment : = "op="

This means that "a = b and c = d" is "(a = b) and (c = d)" . So, the right precedence order should be :

unary : -expr !
multiplicative : * / %
additive : + -
relational : < <= > >=
equality : == !=
logical AND : &&
logical OR : ||

This way "5/2.0+2 = 4.5 and 6+2=8" is "(((5/2.0)+2) = 4.5) and ((6+2)=08)" right?
Where should function be placed? I don't think it's a big problem since their grammar is extremely simple.

functions come at the lowest level I think, just like literal values.

Wow, it has been a hard task. I had to change some stuff to make it work as expected. I also moved blocks in FormulaParser.jj to make it more readable, from the lowest priority to the higher priority, with exception of "element" which is a kind of special block since it routes everything back to the lowest priority when brackets are used.

Also, all the code calls FormulaParser.sum() to actually parse the expression. This is because "sum" was the lower precedence block before this refactoring. Anyway, to make formula parse hide it's internal implementation, i added a parse() block but kept the old sum() block not to break everything.

I dropped the "!" not, since it's a unary operator it has only meaning in !boolean and !(boolean expression), where Not() is perfectly suitable and even clearer (who haven't wrote "!a instanceof Something" at least once in java?).

I added a test for complex expressions with and/or infixed, it passes 5/2*2 = 6 and var1 > 24 .

Please note that this patch does not involve floating point division as the previous ones, another issue will be opened for it so that mailing list can express opinions on that subject.

Looks good, though I'd split the logical() rule in an 'or' and 'and':

orExpr() = andExpr() (<OR> andExpr()) *

andExpr = compare() (<AND> compare())*

so that the OR has a higher precedence than the AND.

In the compare() there's also unused checks for <AND> and <OR>, which confused me at first. So better remove them and replace them with:

        else
            throw new ParseException("Unexpected token kind: " + x.kind);

The change to remove the direct call to sum() is a good move.

If you can make these last changes I'm ready to commit your patch.

Latest patch :
- Or is now differentiated from And in FormulaParser.jj
- Added two test cases in ExpressionTest, one to check for operators precedence with infixed and/or, another to check for variables name extraction during parsing.

All test passed successfully.

Again, no division floating point is taken in consideration in this patch, I'm going to open a new bug about it.

Patch is applied.

Thanks for your contribution!

WARNING : I unfortunately introduced a small, untested bug with these modifications. Currently a function taking a boolean parameter is not working anymore (for example, the common Or and And functions).

This is really just a silly problem. In the FormulaParser.jj where functions are parsed, there is still a reference to realsum() (it was sum() before) so it expects a mathematical expression and does not accept a boolean expression.

This can be fixed simply replacing line 309 for FormulaParser.jj from this :

    <ID> { functionName = token.image; } "(" [ realsum() {paramCount++;} ( "," realsum() {paramCount++;})* ] ")"

to this

    <ID> { functionName = token.image; } "(" [ parse() {paramCount++;} ( "," parse() {paramCount++;})* ] ")"

I can propduce a patch if you want, but maybe a simple cut 'n paste could be enought :)

Applied the suggested fix to svn (revision 674)
http://svn.cocoondev.org/viewsvn?rev=674&root=xreporter&view=rev

Thx for reporting!