Can we remove unnecessary 'final's?

[devycarol]

I'm not sure how you feel about project-wide style revisions, but could we remove all uses of the 'final' keyword in local contexts in the Java code? That's method parameters and local variables.

We obviously wouldn't do this for fields, methods, or classes, as defaulting those final really is best practice for safety (a point I really agree with Kotlin on).

I can do this with inspection profiles, it would just modify a lot of files.

There's no performance gain that I'm aware of. The JVM can discern "effectively final" variables, which is most of them. If we're concerned about safety, modern code vision tools are a much more effective to that end. I personally color my mutated local variables yellow in my IDE, since mutation should always be done with care.

I don't think it's our job to remedy the design mistake Java made in 1939 or whatever having variables mutable by default by making our code 35% wider. We can save the bytes, the screen space, and the readability. This would allow more space for things that are actually helpful, like nullity annotations and code that actually does stuff. We wouldn't have to type unnecessary letters just to stay consistent with surrounding style.

(And yes, I'm aware most such declarations come from the AOSP era.)

[eranl]

Couldn't agree with you more.

[Helium314]

I finally (well...) had a look at the inspections mentioned in #2252.
As said, for me this is fine as long as it's consistently done. Do "default" inspections (test branch) work for you, or are they overridden by your IDE settings?

Need to make some plan on dealing with open PRs (and the experience that usually when I merge a bunch of PRs, roughly the same number of new PR pops up within a few days...).
So it will take a while until changes are actually in the code.

[eranl]

One possible approach, which is used by many open source projects, is enforced, automated style rules. They are applied by a build step, and PRs that don't follow them fail verification.

[Helium314]

So this is a workflow that runs on each PR, right? That sounds good.
Is it possible to check only what was changed in the PR? The results depending on changes coming from somewhere else were one of the reasons I disabled the tests-workflow.

[Helium314]

@eranl @devycarol any other inspections you would like to adjust?

[eranl]

So this is a workflow that runs on each PR, right? That sounds good. Is it possible to check only what was changed in the PR? The results depending on changes coming from somewhere else were one of the reasons I disabled the tests-workflow.

I think we should first fix the whole code base, and then start enforcing.

[eranl]

@eranl @devycarol any other inspections you would like to adjust?

Use var.

[Helium314]

I added the final thing now. Not sure about var, I don't like it for whatever reason.

[devycarol]

My take on var is that is should always be used for the result 'new' expressions (constructors and arrays) or typecast expressions, since those are the 2 cases where the syntax guarantees the exact type of the reference is what it already says on the right hand side. It should especially be used on constructors with type parameters, var list = new ArrayList<String>(); is a whole lot less obnoxious than List<String> list = new ArrayList<>(); Pointless type-weakening is a huge pet peeve of mine, and var makes it so people don't arbitrarily do that to save characters.

I think it's acceptable to use for factory functions whose return type is the given class. So var set = Set.<String>of() would be acceptable whereas var number = Math.max(a, b) would not be. Here there's some risk of confusion if the return type were to change for some reason, but it's a rare enough case and Kotlin's type inference is no better about this.

Unfortunately JetBrains has no inspections for this, it's just "always use var" or "never use var". Wish there were "obvious explicit type" or "vague inferred type".

[eranl]

My take is:

1. type info is mostly noise for humans. It's mainly needed for IDEs and compilers. If they can infer it and keep my code safe, I'm happy to not have to provide it or know it.
2. the IDE shows you the inferred type info if you choose.

So if we can make the code cleaner and more concise by using var, I think it's useful everywhere.

[devycarol]

I care about types but maybe I'm just weird 🤷