You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "JarvisCraft (via GitHub)" <gi...@apache.org> on 2023/05/15 22:05:26 UTC

[GitHub] [lucene] JarvisCraft opened a new pull request, #12295: Use `instanceof` pattern-matching where possible

JarvisCraft opened a new pull request, #12295:
URL: https://github.com/apache/lucene/pull/12295

   ### Description
   
   This PR enables the usage of `instanceof` pattern matching wherever possible (without changing semantics) reducing error-proneness and potentially enhancing readability.
   


-- 
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] Use `instanceof` pattern-matching where possible [lucene]

Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on code in PR #12295:
URL: https://github.com/apache/lucene/pull/12295#discussion_r1391562831


##########
lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90PointsWriter.java:
##########
@@ -182,7 +181,7 @@ public void merge(MergeState mergeState) throws IOException {
      * a bulk merge of the points.
      */
     for (PointsReader reader : mergeState.pointsReaders) {
-      if (reader instanceof Lucene90PointsReader == false) {
+      if (!(reader instanceof Lucene90PointsReader)) {

Review Comment:
   This change is not needed. Some people here prefer the `== false` so let's keep it at least here.



##########
lucene/codecs/src/java/org/apache/lucene/codecs/memory/DirectPostingsFormat.java:
##########
@@ -1470,20 +1475,26 @@ public PostingsEnum postings(PostingsEnum reuse, int flags) {
 
         // TODO: the logic of which enum impl to choose should be refactored to be simpler...
         if (hasPos && PostingsEnum.featureRequested(flags, PostingsEnum.POSITIONS)) {
-          if (terms[termOrd] instanceof LowFreqTerm) {
-            final LowFreqTerm term = ((LowFreqTerm) terms[termOrd]);
-            final int[] postings = term.postings;
-            final byte[] payloads = term.payloads;
+          var term = terms[termOrd];

Review Comment:
   I would revert this whole cleanup here. The code was much more readable before. The new syntax is fine if we only have one branch using the auto-casted instance. Here we have if/else now using different syntax, so please revert!



##########
lucene/core/src/java/org/apache/lucene/util/RamUsageEstimator.java:
##########
@@ -448,38 +448,38 @@ private static long sizeOfObject(Object o, int depth, long defSize) {
       return 0;
     }
     long size;
-    if (o instanceof Accountable) {
-      size = ((Accountable) o).ramBytesUsed();
-    } else if (o instanceof String) {
-      size = sizeOf((String) o);
-    } else if (o instanceof boolean[]) {
-      size = sizeOf((boolean[]) o);
-    } else if (o instanceof byte[]) {
-      size = sizeOf((byte[]) o);
-    } else if (o instanceof char[]) {
-      size = sizeOf((char[]) o);
-    } else if (o instanceof double[]) {
-      size = sizeOf((double[]) o);
-    } else if (o instanceof float[]) {
-      size = sizeOf((float[]) o);
-    } else if (o instanceof int[]) {
-      size = sizeOf((int[]) o);
-    } else if (o instanceof Integer) {
-      size = sizeOf((Integer) o);
-    } else if (o instanceof Long) {
-      size = sizeOf((Long) o);
-    } else if (o instanceof long[]) {
-      size = sizeOf((long[]) o);
-    } else if (o instanceof short[]) {
-      size = sizeOf((short[]) o);
-    } else if (o instanceof String[]) {
-      size = sizeOf((String[]) o);
-    } else if (o instanceof Query) {
-      size = sizeOf((Query) o, defSize);
-    } else if (o instanceof Map) {
-      size = sizeOfMap((Map<?, ?>) o, ++depth, defSize);
-    } else if (o instanceof Collection) {
-      size = sizeOfCollection((Collection<?>) o, ++depth, defSize);
+    if (o instanceof Accountable accountable) {

Review Comment:
   Rewrite this to pattern-matching (new) switch statement.



##########
lucene/codecs/src/java/org/apache/lucene/codecs/memory/DirectPostingsFormat.java:
##########
@@ -1470,20 +1475,26 @@ public PostingsEnum postings(PostingsEnum reuse, int flags) {
 
         // TODO: the logic of which enum impl to choose should be refactored to be simpler...
         if (hasPos && PostingsEnum.featureRequested(flags, PostingsEnum.POSITIONS)) {
-          if (terms[termOrd] instanceof LowFreqTerm) {
-            final LowFreqTerm term = ((LowFreqTerm) terms[termOrd]);
-            final int[] postings = term.postings;
-            final byte[] payloads = term.payloads;
+          var term = terms[termOrd];
+          if (term instanceof LowFreqTerm) {
+            final LowFreqTerm lowFreqTerm = (LowFreqTerm) terms[termOrd];
+            final int[] postings = lowFreqTerm.postings;
+            final byte[] payloads = lowFreqTerm.payloads;
             return new LowFreqPostingsEnum(hasOffsets, hasPayloads).reset(postings, payloads);
           } else {
-            final HighFreqTerm term = (HighFreqTerm) terms[termOrd];
+            final HighFreqTerm highFreqTerm = (HighFreqTerm) terms[termOrd];
             return new HighFreqPostingsEnum(hasOffsets)
-                .reset(term.docIDs, term.freqs, term.positions, term.payloads);
+                .reset(
+                    highFreqTerm.docIDs,
+                    highFreqTerm.freqs,
+                    highFreqTerm.positions,
+                    highFreqTerm.payloads);
           }
         }
 
-        if (terms[termOrd] instanceof LowFreqTerm) {
-          final int[] postings = ((LowFreqTerm) terms[termOrd]).postings;
+        var term = terms[termOrd];

Review Comment:
   same here.



##########
lucene/core/src/java/org/apache/lucene/util/IOUtils.java:
##########
@@ -390,16 +390,16 @@ public static Error rethrowAlways(Throwable th) throws IOException, RuntimeExcep
       throw new AssertionError("rethrow argument must not be null.");
     }
 
-    if (th instanceof IOException) {
-      throw (IOException) th;
+    if (th instanceof IOException ioException) {

Review Comment:
   Here we could also use pattern-matching (new) switch.



##########
lucene/highlighter/src/java/org/apache/lucene/search/highlight/WeightedSpanTermExtractor.java:
##########
@@ -607,23 +604,22 @@ protected void collectSpanQueryFields(SpanQuery spanQuery, Set<String> fieldName
   protected boolean mustRewriteQuery(SpanQuery spanQuery) {
     if (!expandMultiTermQuery) {
       return false; // Will throw UnsupportedOperationException in case of a SpanRegexQuery.
-    } else if (spanQuery instanceof FieldMaskingSpanQuery) {

Review Comment:
   and here.



##########
lucene/core/src/java/org/apache/lucene/util/bkd/OfflinePointReader.java:
##########
@@ -144,10 +144,10 @@ public PointValue pointValue() {
   @Override
   public void close() throws IOException {
     try {
-      if (countLeft == 0 && in instanceof ChecksumIndexInput && checked == false) {
+      if (countLeft == 0 && in instanceof ChecksumIndexInput checksumIn && !checked) {

Review Comment:
   does the `== false` work if we reorder the clauses and move `checked == false` before the instanceof check?



##########
lucene/core/src/java/org/apache/lucene/index/SoftDeletesDirectoryReaderWrapper.java:
##########
@@ -69,13 +69,13 @@ protected DirectoryReader doWrapDirectoryReader(DirectoryReader in) throws IOExc
     Map<CacheKey, LeafReader> readerCache = new HashMap<>();
     for (LeafReader reader : getSequentialSubReaders()) {
       // we try to reuse the life docs instances here if the reader cache key didn't change
-      if (reader instanceof SoftDeletesFilterLeafReader && reader.getReaderCacheHelper() != null) {
+      if (reader instanceof SoftDeletesFilterLeafReader softDeletesFilterLeafReader

Review Comment:
   here we could use a pattern matching (new) switch statement!



##########
lucene/analysis/morfologik/src/java/org/apache/lucene/analysis/morfologik/MorphosyntacticTagsAttributeImpl.java:
##########
@@ -52,8 +52,8 @@ public void clear() {
 
   @Override
   public boolean equals(Object other) {
-    if (other instanceof MorphosyntacticTagsAttribute) {
-      return equal(this.getTags(), ((MorphosyntacticTagsAttribute) other).getTags());
+    if (other instanceof MorphosyntacticTagsAttribute morphosyntacticTagsAttribute) {

Review Comment:
   Yes, but it should be cleaned up later when we standardize equals/hashCode.



##########
lucene/highlighter/src/java/org/apache/lucene/search/highlight/WeightedSpanTermExtractor.java:
##########
@@ -127,17 +127,15 @@ public WeightedSpanTermExtractor(String defaultField) {
    */
   protected void extract(Query query, float boost, Map<String, WeightedSpanTerm> terms)
       throws IOException {
-    if (query instanceof BoostQuery) {
-      BoostQuery boostQuery = (BoostQuery) query;
+    if (query instanceof BoostQuery boostQuery) {

Review Comment:
   rewrite the whole thing as a pattern-matching (new) switch statement.



##########
lucene/highlighter/src/java/org/apache/lucene/search/highlight/WeightedSpanTermExtractor.java:
##########
@@ -585,18 +582,18 @@ public Map<String, WeightedSpanTerm> getWeightedSpanTermsWithScores(
   }
 
   protected void collectSpanQueryFields(SpanQuery spanQuery, Set<String> fieldNames) {
-    if (spanQuery instanceof FieldMaskingSpanQuery) {
-      collectSpanQueryFields(((FieldMaskingSpanQuery) spanQuery).getMaskedQuery(), fieldNames);
-    } else if (spanQuery instanceof SpanFirstQuery) {
-      collectSpanQueryFields(((SpanFirstQuery) spanQuery).getMatch(), fieldNames);
-    } else if (spanQuery instanceof SpanNearQuery) {
-      for (final SpanQuery clause : ((SpanNearQuery) spanQuery).getClauses()) {
+    if (spanQuery instanceof FieldMaskingSpanQuery fieldMaskingSpanQuery) {

Review Comment:
   same here.



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


[GitHub] [lucene] JarvisCraft commented on pull request #12295: Use `instanceof` pattern-matching where possible

Posted by "JarvisCraft (via GitHub)" <gi...@apache.org>.
JarvisCraft commented on PR #12295:
URL: https://github.com/apache/lucene/pull/12295#issuecomment-1551711212

   > So I tend to change the equals methods with instanceof to follow the more modern approach consistently.
   
   In this case, I may roll back changes to `equals` methods and create a separate PR replacing their `equals` implementations where it is backwards-compatible (i.e. on final classes).


-- 
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] Use `instanceof` pattern-matching where possible [lucene]

Posted by "iverase (via GitHub)" <gi...@apache.org>.
iverase commented on PR #12295:
URL: https://github.com/apache/lucene/pull/12295#issuecomment-1791268281

   can we keep explicit  `== false` checks instead of less readable `!`?


-- 
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] Use `instanceof` pattern-matching where possible [lucene]

Posted by "mikemccand (via GitHub)" <gi...@apache.org>.
mikemccand commented on PR #12295:
URL: https://github.com/apache/lucene/pull/12295#issuecomment-1790653367

   In general it's great for Lucene devs to use the new language features we gain by setting a minimum Java version.  This is (part of?) why we have such minimums!
   
   This nice `instanceof` pattern matching was [introduced in Java 14](https://blogs.oracle.com/javamagazine/post/pattern-matching-for-instanceof-in-java-14), and since `main` (to be 10.0) requires Java 17 or 18, we can use nice new feature.
   
   But we cannot backport to 9.x since it supports Java 11+, but I think that added burden (on backport) is OK.  We accumulate such burden as the source code diverges anyways...
   
   Thanks for persisting here @JarvisCraft -- +1 to the plan to revert the `equals` method changes (and maybe do a follow-on PR to "standardize" those), and merge the rest of this PR.


-- 
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] Use `instanceof` pattern-matching where possible [lucene]

Posted by "JarvisCraft (via GitHub)" <gi...@apache.org>.
JarvisCraft commented on PR #12295:
URL: https://github.com/apache/lucene/pull/12295#issuecomment-1881153765

   Apologies for the late reply, I've lost the track of the message in the thread. I will soon coma back to it and see what should be changed.


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


[GitHub] [lucene] uschindler commented on pull request #12295: Use `instanceof` pattern-matching where possible

Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #12295:
URL: https://github.com/apache/lucene/pull/12295#issuecomment-1549864245

   I already assigned the Lucene 10 milestone, not lucene 9.x


-- 
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] Use `instanceof` pattern-matching where possible [lucene]

Posted by "JarvisCraft (via GitHub)" <gi...@apache.org>.
JarvisCraft commented on PR #12295:
URL: https://github.com/apache/lucene/pull/12295#issuecomment-1791256716

   @mikemccand, thanks for the comments!
   
   I've undone the changes to `equals()` methods and applied the fix to the remaining fixable occurrences of the pattern.


-- 
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] Use `instanceof` pattern-matching where possible [lucene]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12295:
URL: https://github.com/apache/lucene/pull/12295#issuecomment-1880903091

   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] Use `instanceof` pattern-matching where possible [lucene]

Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #12295:
URL: https://github.com/apache/lucene/pull/12295#issuecomment-1808887454

   P.S.: When pushing more changes, please **do not squash and force-push**! This makes reviewing not working easy. We squash later when merging, it is not needed to do this while developing a PR.
   
   When You add separate commits I can verify what you have changed, when its squashed I have to do all the work again.


-- 
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] Use `instanceof` pattern-matching where possible [lucene]

Posted by "JarvisCraft (via GitHub)" <gi...@apache.org>.
JarvisCraft commented on PR #12295:
URL: https://github.com/apache/lucene/pull/12295#issuecomment-1791269608

   > can we keep explicit `== false` checks instead of less readable `!`?
   
   No, since javac only recognizes `(!(EXPR instanceof TYPE NAME))`


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


[GitHub] [lucene] uschindler commented on pull request #12295: Use `instanceof` pattern-matching where possible

Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #12295:
URL: https://github.com/apache/lucene/pull/12295#issuecomment-1549756344

   I am fine with this, unless it makes backports harder. So basically I use this for *new* code in new 10.x features only, but I'd like to prevent this from happing in code that is possibly backported.


-- 
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] Use `instanceof` pattern-matching where possible [lucene]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12295:
URL: https://github.com/apache/lucene/pull/12295#issuecomment-1907130198

   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


[GitHub] [lucene] uschindler commented on pull request #12295: Use `instanceof` pattern-matching where possible

Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #12295:
URL: https://github.com/apache/lucene/pull/12295#issuecomment-1550852181

   I tend to leave out the `equals()` methods. Many people in Lucene prefer to have a "step-by-step equals", as it is easier to read.
   
   The general problem is that we add "modern" equals and hashCode methods that do not use instanceof but compare classes directly. The examples you changed are old-style instanceof versions, which are buggy in class hiererachies (unless it is a final class).
   
   So I tend to change the `equals` methods with `instanceof` to follow the more modern approach consistently. This would be a separate PR. The problem of all those methods is that they were created by some IDE without human intervention (and all IDEs use different patterns, although modern ones use class comparisons instead of instanceof).


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


[GitHub] [lucene] JarvisCraft commented on pull request #12295: Use `instanceof` pattern-matching where possible

Posted by "JarvisCraft (via GitHub)" <gi...@apache.org>.
JarvisCraft commented on PR #12295:
URL: https://github.com/apache/lucene/pull/12295#issuecomment-1549837524

   I've fixed the issues and applied `./gradlew tidy`.
   
   As for the backporting burden, I, unfortunately, lack the expertise in providing Lucene backports, so I guess that I'd better leave it up to you to decide which of these are okay to be applied and which ones are in "hot code" (although I possitively expect this specific lines to be rarely visited).
   
   Considering that this changes also require Java version higher than 11, this seems to be a Lucene 10 change, so I guess there is plenty of room for discussion here whether specific files should be changed.


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


[GitHub] [lucene] uschindler commented on pull request #12295: Use `instanceof` pattern-matching where possible

Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #12295:
URL: https://github.com/apache/lucene/pull/12295#issuecomment-1549769045

   The precommit checks already answer most of your questions: Only use that feature for now if it is easy to apply. :-)


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


[GitHub] [lucene] JarvisCraft commented on pull request #12295: Use `instanceof` pattern-matching where possible

Posted by "JarvisCraft (via GitHub)" <gi...@apache.org>.
JarvisCraft commented on PR #12295:
URL: https://github.com/apache/lucene/pull/12295#issuecomment-1549821494

   > The precommit checks already answer most of your questions: Only use that feature for now if it is easy to apply. :-)
   
   Fair point! As for now, I will run them locally (my bad here) and update the code accordingly


-- 
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] Use `instanceof` pattern-matching where possible [lucene]

Posted by "mikemccand (via GitHub)" <gi...@apache.org>.
mikemccand commented on code in PR #12295:
URL: https://github.com/apache/lucene/pull/12295#discussion_r1391341171


##########
lucene/analysis/morfologik/src/java/org/apache/lucene/analysis/morfologik/MorphosyntacticTagsAttributeImpl.java:
##########
@@ -52,8 +52,8 @@ public void clear() {
 
   @Override
   public boolean equals(Object other) {
-    if (other instanceof MorphosyntacticTagsAttribute) {
-      return equal(this.getTags(), ((MorphosyntacticTagsAttribute) other).getTags());
+    if (other instanceof MorphosyntacticTagsAttribute morphosyntacticTagsAttribute) {

Review Comment:
   I guess we keep this `equals` since it was already using `instanceof`.



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