Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mutators cause false positives in the nullness checker #4689

Closed
sxlijin opened this issue May 26, 2021 · 3 comments
Closed

Mutators cause false positives in the nullness checker #4689

sxlijin opened this issue May 26, 2021 · 3 comments
Assignees

Comments

@sxlijin
Copy link

sxlijin commented May 26, 2021

The nullness checker appears to be unable to determine that mutators do not modify a given variable. A minimal example that can be used to prove this:

import org.checkerframework.checker.nullness.qual.Nullable;
import java.util.ArrayList;

class Foo {
    static class Bar {
        Object a = new Object();
        void mutate() { a = new Object(); }
    }
    
    Bar b = new Bar();
    @Nullable Object o = null;
    
    void foo() {
        if (o == null) return;
        b.mutate();
        o.hashCode();
    }
}

The real-world example where this came up was something like:

List<Node> nodes = new ArrayList<>();
InterestingType a = null;
void loop() {
  ...
  if (a == null) {
    a = nonNullThing;
    nodes.add(buildNode());
    a.doThing();
  }
  ...
}
@mernst mernst self-assigned this Jun 1, 2021
@mernst
Copy link
Member

mernst commented Jul 20, 2021

The nullness checker appears to be unable to determine that mutators do not modify a given variable.

While checking one method, the Nullness Checker does not do inference of side effects for other methods.
Rather, the Nullness Checker verifies programmer-written annotations.

If you use a fact when reasoning about your code, you need to express that fact as an annotation.

The manual section "Side effects" describes several ways to express that a mutator does not set a variable to null, including:

It seems that @MonotonicNonNull might be appropriate for your real-world example.
I'm closing this issue, but let us know if the manual is not clear.

@mernst mernst closed this as completed Jul 20, 2021
@sxlijin
Copy link
Author

sxlijin commented Jul 21, 2021

That is... very surprising.

I remember that it took me a long time to track this down. Is there any way to perhaps propagate this information up to the resulting error, so that users at least know where to look?

@mernst
Copy link
Member

mernst commented Jul 21, 2021

Could you clarify what is the very surprising part?

Issue #982 is a proposal to give users more information about the fact that side-effecting methods may set fields. Would that have helped you?

There is also #984, but that assumes the user is thinking about side effects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants