Resolving checker conflicts
From Arnout Engelen
Traditionally, CANAPA blindly applied whatever suggestion ESC/Java provided. Though this is a reasonable initial approach, we think it can be improved upon.
Contents |
[edit] General considerations
[edit] what is 'correct'?
An annotation is 'correct' if it correctly describes the application's design - this might still mean it yields a lot of errors.
[edit] information flow
ESC/Java tends to suggest adding non-nullness annotations rather than removing them. This gives the traditional CANAPA approach a clear non-null-bias. Two measures can be taken to provide some counterweight to this:
- if 'null' is literally assigned to a position, never mark that position non_null. This does not seem to happen often (only 3 times for the DigiD gateway).
- if a method returns 'null' literally anywhere, its returnvalue cannot be non_null. This happens regularly, but not often (12 times for the Digid gateway).
- for class fields, always allow removing the non_null annotation, except if it was (re-)added by the programmer.
[edit] avoiding adding incorrect annotations
An inferrer that infers too many incorrect annotations is not useful. For that reason, one should be conservative about adding possibly incorrect annotations that have a big impact on later iterations - such as for example non-nullness of public class fields.
[edit] Specific choices
[edit] Fields
A field should get a /*@non_null*/ annotation if that is assumed anywhere, but it should be removed again if it turns out to be problematic somewhere else.
ESC/Java can see the difference between a nonnull field not being initialized (NonNullInit) and a nonnull field being assigned null (NonNull, in this case 'no suggestion is being made'. We do want to revoke this annotation though.
In this case we want to remove the nonnull annotation, if it wasn't manually added:
test.java:5: Warning: Possible assignment of null to variable declared non_null (NonNull)
o = null;
^
Associated declaration is "test.java", line 2, col 15:
/*CANAPA*//*@non_null*/Object o;
^
Suggestion [5,6]: none <null>
In this case we also want to remove our nonnull annotation, if it wasn't manually added:
test.java:1: Warning: Field declared non_null possibly not initialized (NonNullInit)
class test {
^
Associated declaration is "test.java", line 2, col 15:
/*CANAPA*//*@non_null*/Object o;
^
Suggestion [1,11]: add an initializer or initialize the field in constructor
[edit] Practical issues
- how to represent a change robustly?
