NullAway icon indicating copy to clipboard operation
NullAway copied to clipboard

Invalid `@Contract` declaration reported for code that doesn't exist

Open odrotbohm opened this issue 7 months ago • 3 comments

Running NullAway on this class:

import java.time.Instant;

import org.jspecify.annotations.Nullable;

class Foo {

	@Nullable Instant instant;

	Foo(@Nullable Instant instant) {
		this.instant = instant;
	}
}

results in

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.13.0:compile (java-compile) on project …: Compilation failure: Compilation failure: 
[ERROR] …/src/main/java/Foo.java:[13] [NullAway] Invalid @Contract annotation detected for method isNull(). It contains the following uparseable clause: _ -> this (incorrect number of arguments in the clause's antecedent [1], should be the same as the number of arguments in for the method [0]).
[ERROR]     (see http://t.uber.com/nullaway )

Maven: 3.9.9 NullAway: 0.12.7 Java: OpenJDK Runtime Environment Temurin-24.0.1+9 (build 24.0.1+9)

odrotbohm avatar May 18 '25 20:05 odrotbohm

Hrm, I can't repro this one; tried a couple different ways. Any way you could make a standalone github project with the same issue?

msridhar avatar May 18 '25 20:05 msridhar

I'm sorry about that. The class fails in a project I deducted the reproducer from but the core issue actually is caused by the class referenced by some other code calling a method that has an invalid @Contract annotation declared. The error message is quite misleading as it doesn't point to the actual source of the problem but a totally unrelated class.

It looks like the verification of @Contract annotations is only triggered if some code is compiled that calls a method annotated with @Contract. In other words:

class WithInvalidContract {

	@Contract("_ -> this")
	WithInvalidContract isNull() {
		return this;
	}
}

This compiles unless you have some additional code like this:

class Client {

	Object someMethod() {
		var foo = new WithInvalidContract();
		return foo.isNull();
	}
}

Note how the call to isNull() has to be part of the chain producing a return value. If it's not, the problem is not reported either.

Does it make sense to verify the methods using @Contract on their declarations? I can see that the scenario might be an edge case, but especially library authors might expose methods that, within their own code base, might solely be used from their own test cases, which in turn - especially for projects having been around for a long time - might not have the null verification enabled because it would just take too much time fixing all the warnings.

odrotbohm avatar May 20 '25 10:05 odrotbohm

Can I ask where the error gets reported? If it's getting reported at the foo.isNull() call then that is definitely a bug. I agree with you that if/when we do @Contract validation we should do it at the declaration of the contract, not at a call site of the method.

msridhar avatar May 20 '25 14:05 msridhar