Add nullability annotations to IndexSearcher APIs#14132
Add nullability annotations to IndexSearcher APIs#14132Edarke wants to merge 2 commits intoapache:mainfrom
Conversation
|
This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution! |
|
This seems reasonable to me, but it implies a future promise to maintain it, and I don't know how we would ever know if we added some new usage that isn't properly annotated |
|
For the java language: Specifying that something is nullable doesn't provide any value: all types are nullable by default already. Specifying that something is NOT-nullable would allow for additional smarts: it is the only possible way to do "gradual nullness typing". I agree too, that we would need linter to verify this stuff in builds. Maybe the errorprone could do it. |
I guess these null-checking systems impose their own assumptions and assume that anything not annotated with @nullable is not-nullable. Clearly the language doesn't enforce that, but maybe some build-time processor can check for it? Also, the proposal here seems to have been to annotate everything all at once, not gradual, but I agree it seems more sensible to assume everything can be null, indicate @NotNull and then enforce that. |
|
One option we could do for correctness is to turn on ecj's null analysis. ecj is pretty fast and runs as part of gradle checks already, and it is the compiler often using this feature in the IDEs anyway. https://github.com/apache/lucene/blob/main/gradle/validation/ecj-lint/ecj.javadocs.prefs#L2-L10 |
|
I will look in on the ECJ side, last time I used it, their null analysis had issues on lucene's enormous codebase. It was many years ago though, maybe it has solidified. error-prone seems to only support Nullable-style vs NotNullable-style annotations, but maybe just reflects google's choices. It does have some good checks though, I esp like the ones where Nullable is interpreted differently by different tools, e.g. |
|
This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution! |
|
This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution! |
3 participants
Annotate public APIs with an internal definition of
@Nullablewhere appropriate.This proposal is a follow up from the discussion here. This change adds an internal copy of JSpecify's
@Nullableto lucene-util and uses Intellij's "infer nullability" feature to automatically apply the annotation toIndexSearcherand its dependencies. If this looks good, there will be a follow up PR to automatically annotate other APIs.The motivation for this change is to increase the null safety of downstream projects using Lucene by allowing users to configure a javac plugin to lint their build. This change was tested with NullAway, which recognizes any annotation with the simple class name
@Nullable