[GitHub] [lucene] rmuir commented on a change in pull request #44: LUCENE-9878: enable redundantNullCheck in ecjLint

[GitBox <gi...@apache.org>]

rmuir commented on a change in pull request #44:
URL: https://github.com/apache/lucene/pull/44#discussion_r601934375



##########
File path: lucene/grouping/src/java/org/apache/lucene/search/grouping/SearchGroup.java
##########
@@ -143,9 +143,9 @@ public boolean equals(Object _other) {
       if (_other instanceof MergedGroup) {
         MergedGroup<?> other = (MergedGroup<?>) _other;
         if (groupValue == null) {
-          return other == null;
+          return other.groupValue == null;

Review comment:
       I think this stuff was a real bug, causing `equals` to basically always return false. I don't know how it is used by this thing (caching or performance or whatever). Detector knows that `other == _other` and that `_other instanceof MergedGroup`, hence it simply cannot be null here.
   
   But the bigger issue is that we should be comparing against `other.groupValue` in this method.




-- 
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.

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