Made the UnifiedHighlighter's hasUnrecognizedQuery function processes FunctionQuery the same way as MatchAllDocsQuery and MatchNoDocsQuery queries for performance reasons.

[vletard]

Description

This pull request is a follow up to #12938. I took on the task of complying with @romseygeek's comment on behalf of Lexum.
I also rebased to the latest commit of apache:main.

Let me know if any other adjustment is needed.

[romseygeek]

This would be simpler as QUERIES_WITH_NO_HL_EFFECT.contains(query.getClass())

[vletard]

Would you say that there is no need to capture hypothetical subclasses?

[romseygeek]

MatchAllDocsQuery is already final, but the other two aren't. Maybe this would be better done as a new protected method, isIgnorableQuery(Query q) with a default implementation of instanceof checks, and then if somebody has their own extension (or another query implementation that will do weird things here) they can override the method?

[vletard]

I just edited my branch with your suggestion.

[github-actions]

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!

[romseygeek]

Can you add a test to TestUnifiedHighlighterExtensibility that checks that overrides to isIgnorableQuery work here as well? I think it will, but to be perfectly honest I can never remember the inheritance rules around class-specific this calls...

[romseygeek]

Thanks @vletard! This looks great, I'd just like to add one more test to ensure that inheritance works in the way we expect.

[vletard]

I added a test in TestUnifiedHighlighterExtensibility, probably too simple for what you asked though.
From my understanding, performing a more realistic test with adequate UH and query setup would require one of the following:

• cross-package import some code from TestUnifiedHighlighter to TestUnifiedHighlighterExtensibility
• copy this code instead of importing it (we probably want to avoid this as it would duplicate much of the randomized generation process)
• create an extensibility test in the uhighlight package instead of uhighlight.visibility

Options 1 and 3 that I tend to prefer would require not-so-light edits, so I'd like your opinion.
What is the expected way to do this in Lucene unit testing?

Thanks!

[github-actions]

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!