highlight decapsulation in base code

[mosconi]

It would be nice, if occurrences of decapsulation could be highlighted directly in the base code.

Two proposals:

1. Similar to callin markers, an additional marker could indicate decapsulation at the base side.
2. Decapsulation could be shown as a warning at the base side. Together with the existing decapsulation warnings, this could lead to redundant information in the list of warnings.

Anyway, the JDT warning about base features "never used locally", which are bound by a decapsulating callout could be adapted to reflect the decapsulation.

[stephan]

I would prefer option (a.), which could mean to simply introduce callout markers very similar to callin markers. At type level I'd consider the existing role marker sufficient, OK?

Thinking about the "never used locally" warning, a trade-off may have to be made: compilation is performed "locally", i.e., one class at a time. For the proposed feature the compiler would have to search the whole workspace just to find whether a single warning should be suppressed. Given how frequent the compiler is called, this might degrade our performance.

By contrast, callin markers are handled at the UI so when activating a Java editor references to the contained java class are searched. This might suggest, that suppressing "unused"-warnings should, in this particular case, also be handled at the UI? This is OK for the editor, but the warning would perhaps still be shown in the problem view, hm...

A mid-term solution might just go for the callout-markers (including callout to public?) and the user would have to figure out the semantics of two markers: "never used locally" and "bound by callout". How does this sound?

[mosconi]

I also prefer option (a.) as addition to existing callin and role markers. But I would restrict it to actual decapsulation as this is a deviation from plain Java. Callouts to visible fields are more or less normal control flows, that are not highlighted in plain Java. Otherwise we would replicate the functionality of the call hierarchy ;-)

Regarding the "never used locally" warning, it would be great to disable it completely, but I see the issues behind this. A partial solution could be to just disable the quickfix for removing the method and perhaps modifying the explanation text in the editor.

[stephan]

Replying to mosconi:

I also prefer option (a.) as addition to existing callin and role markers. But I would restrict it to actual decapsulation as this is a deviation from plain Java.

Agree.

Regarding the "never used locally" warning, it would be great to disable it completely, but I see the issues behind this. A partial solution could be to just disable the quickfix for removing the method and perhaps modifying the explanation text in the editor.

It should be checked, whether quickfixes can access the information from sibling markers, or require a search of their own.

We might even think of a more general approach to warn about any modifications (quickfix, refactoring) of elements bound by callin/callout.

[stephan]

Option (a.) has been implemented in r19463.

For now I leave this issue open awaiting feedback.

[stephan]

In r19465 the callin marker creater removes the annotations/markers for "never used/read locally" warnings.

This seems to work fairly well, yet now we have a racing/updating issue: Any compilation performed after the callin marker creator has finished will re-insert the markers. Closing/opening the editor refreshes the markers and all is fine.

We could now hook into the builder, too, and block the markers in question if a corresponding callout marker already exists (thus avoiding to re-compute callout bindings). I'm just not sure if we really want to burden the builder (frequent operation) with the additional effort (searching existing markers) for such an unusual(?) situation: editing a base class that defines a private unused yet callout bound feature.

Comments?

[stephan]

Implementation from r19463 works fine in unstable builds.

Remaining issues regarding "unused" warnings should be opened in a different ticket, if bad effects are observed.

Closing as fixed.