Smarthome: Save Actions: final modifiers

[sjsf]

As just pointed out in https://github.com/eclipse/smarthome/pull/3650#pullrequestreview-43702946, there are several mistakes which could not happen if we were using final modifiers for method arguments and private fields (where applicable), ideally supported by the IDE:

So far, we treat method arguments as "virtually final" in reviews, so it would just formalize what we are doing anyway. On the other side, visually it adds some bloat. As will #3624.

As I could not find any public discussion related to this, I think it makes sense to have it here.

[maggu2810]

The last time we talked about (IIRC it has been at the time of switching from Yoxos to Oomph) I would like to use it (also for local variables) but Kai don't like it.

I am still a friend of using final whenever possible, so from my side :+1:

[sjsf]

In my opinion, having it everywhere totally obfuscates the code. Therefore I tend to agree with @kaikreuzer about this point. Java is broken there from a language perspective: "final" should have been the default.

We use it for private fields manually already wherever possible, so I seems pretty clear that it would not hurt having this as a save action.

Remains the method parameters where we as of today "enforce" it only by best-effort-review. Which is error prone, as we have seen.

[martinvw]

Sonar is very capable of detecting this and other problems a version which I scanned on 2 april had 90 occurrences where the rule _"Method parameters, caught exceptions and foreach variables should not be reassigned"_ was broken.

As far as I can see we can even enable a warning in Eclipse for this, which I would prefer over adding the distraction to the code.

My major personal objection is that the above (always add final) configuration will make a lot of object-references to mutable values final. In that finalgives a false sense of security and I'm really against using final for example for mutable Lists, arrays, Date's, Calendars etc.

[martinvw]

Sonar is very capable of detecting this and other problems a version which I scanned on 2 april had 90 occurrences where the rule "Method parameters, caught exceptions and foreach variables should not be reassigned" was broken.

I once had the intention to fix all of these observations (this one but also comparisons which are always false, deprecated methods / c;lasses being used, use of StringBuffervs StringBuilder, == where equalsshould be used etc) for both openHAB and Smarthome but got sucked up in other tasks very quickly. Maybe I have more time of all openHAB PR's are processed 😉

[htreu]

How do we proceed here? I like @martinvw proposal the best with adding a warning (might be even an error) for assignment of parameters. This will not result in final keyword overload and enforces a rule we all can agree an I guess. In combination with a save action adding final to private fields where applicable should do a good job (like @sjka proposed).
Ideally a PR for this does address at least all the parameter assignments and set the rule to error.

[maggu2810]

I would prefer an error for a parameter assignment.
There are such a lot of warnings in the IDE that they got ignored easily.

[sjsf]

I have added the WARNINGs for method parameters with #4223. Errors are a little tricky because of the generated code (see #4223).

As there was no objection to letting the auto-formatter change private fields to final, I created #4258.