This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 269188 - "initializer can be static" hint does not account for side-effects
Summary: "initializer can be static" hint does not account for side-effects
Status: RESOLVED INVALID
Alias: None
Product: java
Classification: Unclassified
Component: Hints (show other bugs)
Version: Dev
Hardware: All All
: P2 normal with 1 vote (vote)
Assignee: Svata Dedic
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-30 19:07 UTC by athompson
Modified: 2017-05-17 09:29 UTC (History)
1 user (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description athompson 2016-11-30 19:07:32 UTC
The "initializer may be static" hint does not account for side-effects and is therefore more misleading than useful. Additionally, accepting the hint may have a harmful affect on the intended execution of the code. For example:

public class MyClass {
  private static final Logger logger = Logger.getLogger(MyClass.class.getName());

  {
    logger.info("New MyClass instance created!");
  }

  ...
}

In this case the initializer is used to do something (log a message) whenever a new instance of the class is created, regardless of which constructor is used. Making this initializer static clearly alters the code's intent in an undesirable way.
Comment 1 athompson 2016-11-30 19:11:56 UTC
correction: hint name: "initializer can be static"
Comment 2 Svata Dedic 2017-01-12 11:58:45 UTC
The purpose of the hint is to mark such initializers, which are not static, but eventually could be.

Whether the init code should be executed once, or for every instance creation cannot be decided, it's up to the developer.

The same holds for side effects; it's perfectly valid for a static init to invoke (static) methods with side effects (updating static fields or calling out to other mutating methods). The developer must decide, if change to static init is appropriate for the code.

Places where the hint is inapropriate can suppress the int (@SuppressWarnings(InitializerMayBeStatic))
Comment 3 athompson 2017-01-13 20:40:00 UTC
Sorry for reopening, but please read my reasoning below.

I agree with some of what you say, but that isn't the issue as I see it. The problem is that both the hint and quick fix break the "implied contract" between NetBeans and the user, and the result is unexpected behavior.

Most if not all other quick fixes in Netbeans either fixes an error or tells the user of a better/different way to accomplish *exactly the original task that the user set out to do*. Because of this the user generally feels it's "safe" and even desired to use a quick fix, even if the user doesn't quite understand why the suggested fix is needed, because the code will still do what they wanted. This hint/quick fix makes no such guarantee, and from my quick analysis breaks this promise on most static blocks.

Additionally, the hint wording is clearly misleading and feeds into the expectation mentioned above. The text reads, "initializer can be static" which the user invariably interprets as "the initializer can safely be made static", especially in light of the description (below). But in the case given in the initial comment the initializer absolutely *can't* be static.

This is the hint's description:

"(A) The initializer does not access any instance variables or methods, (B) it can be static and execute just once, not during each instance creation."

Whoever authored this incorrectly assumed that A implies B, but it absolutely does not.
Comment 4 athompson 2017-01-13 21:27:12 UTC
correction:

...

This hint/quick fix makes no such guarantee, and from my quick analysis breaks this promise on *initializers*.
Comment 5 athompson 2017-01-13 21:28:55 UTC
Ack! Correction of correction:

This hint/quick fix makes no such guarantee, and from my quick analysis breaks this promise on *most initializers*.
Comment 6 Svata Dedic 2017-05-17 09:29:27 UTC
We are not in disagreement re. what the hint AND the fix does. 

And there are quite a few other hints which, if unsed without care, could affect computation results - even such simple thing as "Assign result to a local variable" alters execution order and may, potentially, break the code.

It is not computable whether even your logging example is intended to log "hey, this class was just loaded" and the user just forgot the static or the semantic indeed matches the example's logging message :)

We'll follow the crowd; IntelliJ has the same hint with the same semantics, sorry :)