Hi Pedro,
very interesting work, thank you very much.
I have a little nitpick though:
The expression "addressList.1" is parsed into a PropertyExpression with
JavaProperty-"person" followed by BeanProperty-"1".
When PropertyResolver converts the parsed expression into
{"addressList", "1"}, the syntactic analysis is lost though.
With the lexical analysis remaining only, the expression results in
invocation of #getAdressList() followed by #get(1) instead,
i.e. "1" is interpreted as a list index.
It seems our current grammar is more lenient than what your parser is
expecting.
Regards
Sven
On 25.09.2016 09:40, Pedro Santos wrote:
Hi devs,
Currently PropertyResolver is doing all the tokenizer plus property
resolution logic to resolve property expressions to IGetAndSet interfaces.
To improve its cohesion and to add new messages explaining syntax error in
the property expression entered by users, I propose us to use a parser for
the grammar below.
I created the branch [1] to show the parser and, if the team agrees with
the proposal, I will move to the next step that is to change the
PropertyResolver to use the generated syntax tree object instead of the
current lexemes. I'm using the ticket WICKET-4008 to track the work.
Proposed grammar in an EBNF like description:
java letter = "_" | "$" | "A" | "a" | "B" | "b" | (...);
java letter or digit = java letter | "0" | "1" | (...) ;
char = java letter or digit | "." | "(" | ")" | "[" | "]" | "!" | "@" |
"#" | (...);
index char = char - "]";
empty space = { " " };
java identifier = java letter , {java letter or digit};
property name = java letter or digit , {java letter or digit};
method sign = "(" , empty space , ")";
index = "[" , index char , { index char } , "]";
bean property = property name, [ index ];
java property = java identifier , [ index | method sign ];
property expression = [ bean property | java property | index ] , { "." ,
property expression };
Main goals:
- To remove from PropertyResolver its current tokenizer code
- To validate property expressions syntax
To exemplify the new messages explaining syntax errors:
- Given the expression: "bean.method(parameter)", we can explain that no
parameter is allowed inside the brackets
- Given the expression "array[]", we can explain that the empty brackets
aren't allowed
- Given the expression "bean.prop#name", we can explain that
"bean.prop#name" isn't a valid bean property name
- Given the expression "bean.0method()", we can explain that "0method"
isn't a valid java identifier, thus the method call is invalid
Collateral benefits:
- We will be able to map keys in map expressions using the '[' character
like "may[key[weird]"
Obs. 1: even having the open square bracket '[', the parser is able to
understand the key because of grammar's production rule: index char = char
- "]";
Obs. 2: a better solution was proposed in the thread
http://wicket-dev.markmail.org/thread/unwdqpxulw7tcd5l
- Allows empty spaces inside method call brackets ( )
Obs.: because of grammar's production rule: method sign = "(" , empty
space, ")";
- Improves Wicket's performance since every detected syntax error prevents
PropertyResolver from to test bean's classes via reflection, know to add
performance overhead [2].
Future benefits:
- The parser enables us to easily enforce that expressions like
"bean.map[key]" will have the key tested only against a map, and won't be
resolved as to object's get/set methods.
- PropertyResolver resolves expressions like "bean.attributeAt.1" to the
methods: bean.getAttributeAt(1) and bean.setAttributeAt(1, value). This
resolution logic can be improved to only test for this kind of method
signatures if the analyzed input allows such usage (currently the resolver
always test for such signatures).
1 -
https://github.com/apache/wicket/commits/WICKET-4008-property-expression-parser
2 - http://docs.oracle.com/javase/tutorial/reflect/index.html
Cheers
Pedro Santos
