Prohibit static, non-final fields

[leventov]

This can be done with IntelliJ structural search.

1. Open Structural Search.
2. Press gear button -> "Existing Templates...".
3. Choose "static fields that are not final".
4. Choose Scope -> "NonGeneratedFiles".

There are 332 occurrences.

[SandishKumarHN]

@leventov so all public static (excluding public static final ) fields should be just public?

[leventov]

Visibility modifier doesn't matter. static, non-final fields are a global mutable state, which is bad. Most often it's just an omission of the final modifier.

[SandishKumarHN]

@leventov then what happens to non final static fields used in static blocks?

[leventov]

Then what happens to non final static fields used in static blocks?

What do you mean? Nothing relevant to this issue happens to such fields.

[SandishKumarHN]

@leventov isn't this about removing all "static fields that are not final" fields? and adding structural search rule.

What happens to the below code in the case of Prohibit "static fields that are not final"?

private static final Logger log = new Logger(AllocationMetricCollectors.class);
 private static Method getThreadAllocatedBytes;
 private static ThreadMXBean threadMXBean;
 private static boolean initialized = false;

 static {
   try {
     // classes in the sun.* packages are not part of the public/supported Java API and should not be used directly.
     threadMXBean = ManagementFactory.getThreadMXBean();
     getThreadAllocatedBytes = threadMXBean.getClass().getMethod("getThreadAllocatedBytes", long[].class);
     getThreadAllocatedBytes.setAccessible(true);
     getThreadAllocatedBytes.invoke(threadMXBean, (Object) threadMXBean.getAllThreadIds());
     initialized = true;
   }
   catch (Exception e) {
     log.warn(e, "Cannot initialize %s", AllocationMetricCollector.class.getName());
   }
 }

[leventov]

There are three types of static, non-final fields in the project:

1. Fields which should properly be final, just the final modifier was missed out. This is the wast majority of the 295 total occurrences in the project. The goal of this inspection check is to fix these omissions and prevent them in the future.
2. Fields which are effectively final, but cannot formally be final because the Java compiler complains. Example: threadMXBean and initialized from your comment. What we should do about these fields: suppress the annotation using @SuppressWarnings("SSBasedInspection") // effectively final field; suppressing for "static, non final field" pattern with these fields.
3. Static, truly mutable fields. This is an anti-pattern and should be avoided. If there is a good reason to use static mutable fields, these should also be @SuppressWarnings("SSBasedInspection")