Add @NonNullIfReturn: parameter-level conditional postcondition for nullness#1530
Add @NonNullIfReturn: parameter-level conditional postcondition for nullness#1530HenryXi1 wants to merge 7 commits intoeisop:masterfrom
Conversation
|
The Once we've settled on the annotation names in this PR, open a corresponding PR in https://github.com/eisop/jdk/ with the same branch name as what you're using in this PR. The build system should then pick up the corresponding changes and be happy again. |
| * each invocation. | ||
| */ | ||
| private final Map<ExecutableElement, Set<Contract.ConditionalPostcondition>> | ||
| conditionalPostconditionCache = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
See how we do caching elsewhere:
We want a maximum size and LRU policy, to avoid using up too much memory.
Concurrency is not a concern, everything runs in one thread.
We can pick our own cache size, if we can't access the methods from the ATF.
| @Retention(RetentionPolicy.RUNTIME) | ||
| @Target({ElementType.PARAMETER}) | ||
| @ParameterConditionalPostconditionAnnotation(qualifier = NonNull.class) | ||
| public @interface NotNullIfReturns { |
There was a problem hiding this comment.
For consistency with the qualifier, how about calling this NonNullIfReturns?
| @Documented | ||
| @Retention(RetentionPolicy.RUNTIME) | ||
| @Target({ElementType.ANNOTATION_TYPE}) | ||
| public @interface ParameterConditionalPostconditionAnnotation { |
There was a problem hiding this comment.
We might want NonNullIfReturnsTrue and NonNullIfReturnsFalse as concrete annotations.
How could we re-model this to support a fixed truth value?
| // Example 2: equals — param is non-null when method returns true | ||
| @Override | ||
| public boolean equals(@NotNullIfReturns(true) @Nullable Object other) { | ||
| return this == other; |
There was a problem hiding this comment.
It should already come from the existing logic, but for completeness, add a test where the logic is incorrect - e.g. always return true, without checking the parameter.
| } | ||
|
|
||
| // Example 3: isNull — param is non-null when method returns false | ||
| public static boolean isNull(@NotNullIfReturns(false) @Nullable Object obj) { |
There was a problem hiding this comment.
Should there be a well-formedness check that the parameter is @Nullable?
Should this be expressible in the Parameter conditional... Annotation?
| * Maximum size for the conditional postcondition cache. Uses LRU eviction to avoid unbounded | ||
| * memory use. Concurrency is not a concern (single-threaded). | ||
| */ | ||
| private static final int CACHE_SIZE = 300; |
There was a problem hiding this comment.
Instead of our own size, can you just use atypeFactory.getCacheSize()? That also allows adapting the size with a command line flag.
| AnnotationMirror metaAnnotation = r.second; | ||
| AnnotationMirror enforcedQualifier = | ||
| getQualifierEnforcedByContractAnnotation(metaAnnotation, null, null); | ||
| if (enforcedQualifier == null) { |
There was a problem hiding this comment.
Raise an error if the qualifier is used incorrectly.
HenryXi1
force-pushed
the
add-parameter-annotation-for-conditional-nullness
branch
from
e1923a9 to
1eecfca
Compare
2 participants
Goal
Enable expressing "parameter is non-null when the method returns true/false" by annotating the parameter directly (e.g.,
@NonNullIfReturn(true)) instead of the method-level@EnsuresNonNullIf(expression="#1", result=true).Changes
checker-qual
@ParameterConditionalPostconditionAnnotationtoframework.qual.value()of typeboolean.#1,#2, etc.).@NonNullIfReturn.PARAMETERboolean@ParameterConditionalPostconditionAnnotation(qualifier = NonNull.class)Framework
CONDITIONALPOSTCONDITION.ParameterConditionalPostconditionAnnotation.Contract.ConditionalPostconditionusing the parameter index for the expression and the annotation’svalue()for the result.System.err.printlndebug logging has been removed.Tests
@NonNullIfReturn(true)and@NonNullIfReturn(false)behaviors.