Syntax check for Annotations with Spring Expression Language

[MahatmaFatalError]

Annotations like e.g. @Cacheable, @EventListener or @PreAuthorize take a String literal containing logic in form of SpEL. In general this is error prone since there is no compile-time check.

It would be nice to be warned earlier that the expression is erroneous. It seems this is a feature of IntelliJ:

[martinlippert]

This would indeed be a super awesome enhancement for the Spring Tools 4. I will definitely put this on the list of things we should take a look at for future releases.

[martinlippert]

I am working on this at the moment and I think I got pretty far along with the basic validation of the syntax of a SpEL expression. No content-assist or anything like that, but it flags the expression with an error if the syntax is not correct.

Of course there are other things that could go wrong inside of SpEL expressions, especially referencing non-existing elements, etc, but that isn't checked yet. But I hope that the basic syntax validation is already a great step forward.

However, looking at your example above I am wondering where the CustomEventListener annotation with the customCondition is coming from. At the moment I can identify existing Spring annotations that use SpEL expressions and take sub-interfaces into account automatically, but the validation has no idea about user-defined attributes of a user-defined annotation that use SpEL.

I can imagine having special metadata around for the tooling to identify those annotations/params, but wanted to check back with you about the example that you provided first. What is your CustomEventListener and the customCondition about?

[MahatmaFatalError]

I am working on this at the moment and I think I got pretty far along with the basic validation of the syntax of a SpEL expression. No content-assist or anything like that, but it flags the expression with an error if the syntax is not correct.

Of course there are other things that could go wrong inside of SpEL expressions, especially referencing non-existing elements, etc, but that isn't checked yet. But I hope that the basic syntax validation is already a great step forward.

Awesome, great news!

However, looking at your example above I am wondering where the CustomEventListener annotation with the customCondition is coming from. At the moment I can identify existing Spring annotations that use SpEL expressions and take sub-interfaces into account automatically, but the validation has no idea about user-defined attributes of a user-defined annotation that use SpEL.

I can imagine having special metadata around for the tooling to identify those annotations/params, but wanted to check back with you about the example that you provided first. What is your CustomEventListener and the customCondition about?

I took the screenshot from https://blog.jetbrains.com/idea/2016/07/whats-new-in-intellij-idea-2016-2-for-spring-developers/ where they introduced functionality to assist even custom @eventlistener annotations defined with @AliasFor in addition to the standard annotations. However, I think for STS it is sufficient to start with common standard Spring annotations and further enhance the functionality over time.

[martinlippert]

As mentioned, there is a basic check for the syntax of the SpEL expression in place now. It works across Java annotations as well as Spring XML config files. It does not yet take the @AliasFor information into account, that is something for future work in this area.

Nevertheless, I mark this issue as resolved and will deliver this as part of the upcoming Spring Tools 4.7.1 update.

[MahatmaFatalError]

I updated to STS 4.7.1 to try this out, however, no warning appears for nonexisting parameters as cache key:

What am I doing wrong?

[martinlippert]

@MahatmaFatalError I think you are doing nothing wrong. The implemented validation checks for the syntax of the SpEL expression. It doesn't check anything beyond the pure syntax.

Feel free to open an enhancement request for this. I would especially be interested to read which pieces you are missing the most and which would have helped you the most in practice.

[MahatmaFatalError]

Ah, now I understand.
Well, one concrete issue we had was something like this situation:

@CacheEvict(key = "#customer.customerId")
public void insert(Customer customer) {
	//some logic
}

Then assume to refactor rename the field customerId of the Customer class to e.g. just id. Since the cache key String literal is not considered by the refactoring action it is hard to notice that the @CacheEvict does not work anymore as expected.

So, what's needed here is a deeper validation at compile time to check if the expression is sound. In this case, if the referenced field of the parameter's type exists.

There might be similar situations for @PreAuthorize and other annotations.

So, shall I create new issue tickets? One for the validation and one for considering the literals of the annotations for refactor rename action?

[martinlippert]

@MahatmaFatalError Yes, please create those new enhancement requests, sounds like a very good thing to do. Especially the refactoring support would indeed be awesome.