Skip to content

[2.x] fix: widen Extend\SearchDriver class-string types to SearcherInterface#4668

Merged
imorland merged 2 commits into
2.xfrom
im/search-driver-typehint
May 21, 2026
Merged

[2.x] fix: widen Extend\SearchDriver class-string types to SearcherInterface#4668
imorland merged 2 commits into
2.xfrom
im/search-driver-typehint

Conversation

@imorland
Copy link
Copy Markdown
Member

Summary

The docblock annotations on Extend\SearchDriver::addSearcher() / addFilter() / replaceFilter() / setFulltext() / addMutator() were typed class-string<AbstractSearcher>, but the prose right next to them already said:

This searcher must implement \Flarum\Search\SearcherInterface.
Or extend \Flarum\Search\Database\AbstractSearcher if using the default driver.

The type annotation was strictly narrower than the prose. Custom non-DB search drivers — the whole reason SearcherInterface exists separately from AbstractSearcher — would trip phpstan with a false positive on every single addSearcher() / addFilter() call in their extend.php.

This PR widens the annotations to class-string<SearcherInterface>, which is what the runtime actually checks against (line 50/66/etc. just assign the string into an array) and what the docblocks have always described. AbstractSearcher extends/implements SearcherInterface, so this is a strictly looser type — no breakage for the default driver, and custom-driver extensions get a phpstan-clean run.

Also drops the now-unused use Flarum\Search\Database\AbstractSearcher; import.

Changes

  • framework/core/src/Extend/SearchDriver.php: 5 docblock annotations widened, 1 import removed, 1 import added.

Test plan

  • No behavioural change — all 5 affected methods just assign their string param into an array. The widening only affects what static analysis tools accept.
  • Existing tests pass.
  • Extensions that pass AbstractSearcher subclasses still type-check (covariance — narrower is fine).
  • Extensions that pass plain SearcherInterface implementers now type-check (this is the bug being fixed).

…terface

The docblock annotations on addSearcher/addFilter/replaceFilter/setFulltext/
addMutator were typed `class-string<AbstractSearcher>`, but the surrounding
comments already say "must implement SearcherInterface" (and "or extend
AbstractSearcher if using the default driver"). Custom non-DB drivers —
the whole reason SearcherInterface exists separately from AbstractSearcher —
were tripping phpstan with a false positive on every addSearcher/addFilter
call in their extend.php.

Widen the annotations to `class-string<SearcherInterface>`, which is what
the runtime actually checks against, and which the docblocks have always
described.

No behavioural change. Drops the now-unused AbstractSearcher import.
@imorland imorland requested a review from a team as a code owner May 21, 2026 17:54
@imorland imorland added this to the 2.0.0-rc.2 milestone May 21, 2026
@imorland imorland merged commit f03954b into 2.x May 21, 2026
25 checks passed
@imorland imorland deleted the im/search-driver-typehint branch May 21, 2026 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants