You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "Edarke (via GitHub)" <gi...@apache.org> on 2025/01/10 17:26:03 UTC
[PR] Add nullability annotations to IndexSearcher APIs [lucene]
Edarke opened a new pull request, #14132:
URL: https://github.com/apache/lucene/pull/14132
Annotate public APIs with an internal definition of `@Nullable` where appropriate.
This proposal is a follow up from the discussion [here](https://lists.apache.org/thread/oh09k7nlvntzf49gx7d399d1chdk1z0n). This change adds an internal copy of JSpecify's `@Nullable` to lucene-util and uses Intellij's "infer nullability" feature to automatically apply the annotation to `IndexSearcher` and 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`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add nullability annotations to IndexSearcher APIs [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on PR #14132:
URL: https://github.com/apache/lucene/pull/14132#issuecomment-2616629072
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.
https://errorprone.info/bugpattern/NullableWildcard
https://errorprone.info/bugpattern/NullableTypeParameter
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add nullability annotations to IndexSearcher APIs [lucene]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #14132:
URL: https://github.com/apache/lucene/pull/14132#issuecomment-3408722556
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 is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add nullability annotations to IndexSearcher APIs [lucene]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #14132:
URL: https://github.com/apache/lucene/pull/14132#issuecomment-2613644568
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 is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add nullability annotations to IndexSearcher APIs [lucene]
Posted by "msokolov (via GitHub)" <gi...@apache.org>.
msokolov commented on PR #14132:
URL: https://github.com/apache/lucene/pull/14132#issuecomment-2615754639
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add nullability annotations to IndexSearcher APIs [lucene]
Posted by "msokolov (via GitHub)" <gi...@apache.org>.
msokolov commented on PR #14132:
URL: https://github.com/apache/lucene/pull/14132#issuecomment-2616198970
> Specifying that something is nullable doesn't provide any value: all types are nullable by default already.
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add nullability annotations to IndexSearcher APIs [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on PR #14132:
URL: https://github.com/apache/lucene/pull/14132#issuecomment-2616174314
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add nullability annotations to IndexSearcher APIs [lucene]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #14132:
URL: https://github.com/apache/lucene/pull/14132#issuecomment-2649549901
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 is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Add nullability annotations to IndexSearcher APIs [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on PR #14132:
URL: https://github.com/apache/lucene/pull/14132#issuecomment-2616604765
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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org