Clean Up: Add final modifier to private fields

[jukzi]

No description provided.

[github-actions]

Test Results

     246 files  ±0       246 suites  ±0   59m 57s ⏱️ - 4m 30s
  3 299 tests ±0    3 271 ✔️  - 1  24 💤 ±0  0 ❌ ±0  4 🔥 +1 
10 194 runs  ±0  10 118 ✔️  - 1  72 💤 ±0  0 ❌ ±0  4 🔥 +1 

For more details on these errors, see this check.

Results for commit ed65987. ± Comparison against base commit d880fec.

♻️ This comment has been updated with latest results.

[vogella]

I personally see no value in adding more words (final) to the code and in the past we have actually reduced the usage of final. WDYT @HannesWell?

[iloveeclipse]

I personally see no value in adding more words (final) to the code

Final fields are good, because they avoid possibly improper field initialization at runtime.

and in the past we have actually reduced the usage of final.

Not sure who is "we" and why removing final would be desired?

[merks]

I think final fields make it more clear that one doesn't not intend subsequent assignment. With implicit final variables in newer Javas, the use of final in the body of a method is kind of noisy...

[gireeshpunathil]

does this compile? AFAIK, this produces unitialized final field error.

[merks]

It's assigned in the constructor, so yes it compiles without errors:

[jukzi]

"final" on local variables is bad boiler plate and should be removed. But on fields it is mandatory for proper multi threading. I am tired to solve single missing finals, so i just added all missing. May be it adds some keywords to structures that are not used concurrently but it's to much work for me to figure out where it is needed and where it could be skipped.
see
https://stackoverflow.com/questions/27254152/what-exactly-does-the-final-keyword-guarantee-regarding-concurrency
https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.5

[merks]

Not to hijack this topic...

I wonder if there is anything that finds pointless initialization of fields to their default value, e.g.,

This actually generates byte code to do that initialization so is actually worse than pointless...

[vogella]

"final" on local variables is bad boiler plate and should be removed. But on fields it is mandatory for proper multi threading. I am tired to solve single missing finals, so i just added all missing. May be it adds some keywords to structures that are not used concurrently but it's to much work for me to figure out where it is needed and where it could be skipped. see https://stackoverflow.com/questions/27254152/what-exactly-does-the-final-keyword-guarantee-regarding-concurrency https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.5

Thanks for clarification, change is fine for me in this case.

[jukzi]

I wonder if there is anything that finds pointless initialization of fields to their default value, e.g.,

ask for a refactoring in jdt

[iloveeclipse]

I wonder if there is anything that finds pointless initialization of fields to their default value, e.g.,

The compiler knows that and could emit a warning.

[HannesWell]

I personally see no value in adding more words (final) to the code and in the past we have actually reduced the usage of final. WDYT @HannesWell?

I have the same opinion as the others for the mentioned reasons. Additionally it also makes the code easier to follow because one can be sure that the field's reference is not changed as a side-effect of any method call.

[jukzi]

15:05:27 [ERROR] Failed to execute goal org.eclipse.tycho.extras:tycho-p2-extras-plugin:4.0.2:compare-version-with-baselines (compare-attached-artifacts-with-release) on project org.eclipse.pde.unittest.junit: Only qualifier changed for (org.eclipse.pde.unittest.junit.feature.jar/1.0.500.v20230912-1247). Expected to have bigger x.y.z than what is available in baseline (1.0.500.v20230728-0611) -> [Help 1]

[jukzi]

15:57:01 [API ERROR] File MANIFEST.MF at line 6: The service version is increased unnecessarily since either the major or minor or service version is already increased (location: /home/jenkins/agent/workspace/eclipse.pde_PR-739/ui/org.eclipse.pde.unittest.junit/META-INF/MANIFEST.MF)

[jukzi]

I need help with the version bump here: org.eclipse.pde.unittest.junit - if i increase i get an error, if i do not increase i get an another error :(

[jukzi]

15:05:27 [ERROR] Failed to execute goal org.eclipse.tycho.extras:tycho-p2-extras-plugin:4.0.2:compare-version-with-baselines (compare-attached-artifacts-with-release) on project org.eclipse.pde.unittest.junit: Only qualifier changed for

i guess that error message is wrong and it should say project org.eclipse.pde.unittest.junit-feature

[jukzi]

ignoring unrelated BundleMergeSplitTests #261