Checkstyle: Support lombok `val` keyword for/instead of FinalLocalVariableCheck

[rikoe]

I would like to use FinalLocalVariableCheck to ensure my local variables are compiled as final, but ideally I don't want to type final every time I declare a local variable. Lombok's val keyword is great for this, but it isn't recognised by FinalLocalVariableCheck and still gives me warnings about adding the final keyword.

Is there a way I can get around this without disabling the entire rule for my whole project, or is there a way I can create an alternative rule with the same effect that checks if val was used for local variables?

[romani]

Val is not a keyword, it is a type - https://projectlombok.org/features/val
It will never be recognized by Checkstyle , as we do not know what this type is doing for you. We can not analyze classes in class path.
There is also WARNING: This feature does not currently work in NetBeans. ... so level of generated code magic is significant.... we will never support all magic.

Ones we complete http://checkstyle.sourceforge.net/config_filters.html#SuppressionXpathFilter to support all checks, you can use it to suppress.

If project use lombok extensively , you should suppress this Check on files or packages ...You need to review supported suppressions - http://checkstyle.sourceforge.net/config_filters.html#Content

You can create a custom rule , host it in your own project and use your rule instead, there numerous custom Checks in web, there are a lot of examples on how to do this and run custom Checks.

[romani]

from https://projectlombok.org/features/val, there is The local variable will also be made final. , so it might be

example from https://projectlombok.org/features/val , but I added final keyword:

$ wget http://central.maven.org/maven2/org/projectlombok/lombok/1.16.20/lombok-1.16.20.jar
$ cat ValExample.java 
import java.util.ArrayList;
import java.util.HashMap;
import lombok.val;

public class ValExample {
  public String example() {
    final val example = new ArrayList<String>(); //FINAL!!!!!
    example.add("Hello, World!");
    val foo = example.get(0);
    return foo.toLowerCase();
  }

  public void example2() {
    val map = new HashMap<Integer, String>();
    map.put(0, "zero");
    map.put(5, "five");
    for (val entry : map.entrySet()) {
      System.out.printf("%d: %s\n", entry.getKey(), entry.getValue());
    }
  }
}

$ javac -cp lombok-1.16.20.jar ValExample.java

# if change file to make assignments:
#     val example = new ArrayList<String>();
#     example = new ArrayList<String>();


$ javac -cp lombok-1.16.20.jar ValExample.java 
ValExample.java:8: error: cannot assign a value to final variable example
    example = new ArrayList<String>();
    ^
1 error

there is no error to use final in val. But lombok somehow make it final in binaries or ....
@rikoe , can you comment on what level lombok do this magic ? Or I miss smth ....

[romani]

So lombok do change source code , explanation from their developer - https://stackoverflow.com/a/42760020/1015848

[rikoe]

Thanks for all the help and advice. If true that val is simply a type (which is replaced later on), it should be possible to write a check that is similar to FinalLocalVariableCheck, but checks if a local variable is either val, in which case it doesn't need to be final, or not val in which case it does.

Is my assumption correct? Let's call it for example FinalOrValLocalVariableCheck...

[romani]

Such Check is very complicated, creation of new copy of it and support of both will be nightmare.
Usage of lombok is rare in who Java community. We do not do hacks just in favor of some library magic.
Such cases should be suppressed by xpath suppression filter - http://checkstyle.sourceforge.net/config_filters.html#SuppressionXpathFilter . It is not possible now, but one day in future it will be possible.

As for now we do not have other way, but use some other filter/suppressions to avoid violations. Or disable this Check completely, as you use some library that make code generation base on what is given to Checkstyle for processing, it will never work reliable.

Last resort is to add in this Check new property to skip some types but will add extra penalty in performance and complications to logic to implement it reliability. To make this there should be more reasons and demands from other libraries or .... .

[romani]

If really want it work in such Check, you can implement my "last resort" option at our experimental project https://github.com/sevntu-checkstyle/sevntu.checkstyle , as it becomes popular or more reasons commeup, we can move such features to main project.

[rikoe]

Fair enough, although I don't like the tone of your reply, referring to lombok as "some library magic", and referring as support for it as "hacks" and then just closing my request. I think you are letting your own prejudices cloud your judgement - lombok is widely used by many Java teams that I have worked on, regardless of what your view about it may be.

I think it is not an unreasonable request to add a simple property to the check in question to ignore certain types. I don't know the architecture of checkstyle very well, but having looked at the code, I don't see why it would necessarily introduce a performance penalty or impact reliability, especially if it is opt-in. It would be similar to e.g. IllegalTypeCheck.

I thought it might have been useful/more flexible to contribute to the repository a PR that introduce a general "ignore certain types" option to this check, but now I am not sure this will be accepted. I may just look into implementing my own custom rule based on a copy of the existing one, as you suggest.

Thanks for considering my request, anyway.

[romani]

although I don't like the tone of your reply, referring to lombok as "some library magic", and referring as support for it as "hacks" and then just closing my request.

I did not mean to be rude, but If left this issue without resolution, it will be even worse.
I know how it looks like, when you start to use library, it looks like all other start to use it. From my perspective I receive requests from whole world to their customs needs and nuances then well known libraries make false-positives in some rules, only because they do special processing of them if some annotation exists. Some recent example is springboot . I can allow adjust Check for each very custom usage/library as Check is used in whole java world.

I think it is not an unreasonable request to add a simple property

:) , ok, please be welcome to sevntu project with PR, it always looks simple to fix, but after contributors start to do fixes they got understanding that it demands big effort to make implementation reliable. I had a lot of abandoned PRs.

I don't see why it would necessarily introduce a performance penalty or impact reliability, especially if it is opt-in.

Just step in and you will see nuances, we will guide you in development, and in testing. We have several Checks that already have ability to skip by type, there are limitations and now all cases could be properly detected.

I thought it might have been useful/more flexible to contribute to the repository a PR that introduce a general "ignore certain types" option to this check, but now I am not sure this will be accepted

For now, I am against this option. So issue is not marked as approved. I spent a lot of time to think about this new option. So I have doubts, so I gave you alternative way out. This will resolve problem on your side and may be in future I will change my mind.

[rikoe]

Okay, thanks for the considered response, I am more than happy to do it in the sevntu repo if that offers me a route to attempt this. I appreciate that the difficulty and nuances of what I am suggesting will be much clearer to yourself than to my uninitiated eyes...