Use 'static' as return for fluent interfaces of Facets

[thePanz]

This example code:

        $facet = (new JsonTerms())
            ->setKey('my-facet')
            ->setField('field_name');

Reports an error on PHPStan: Call to an undefined method Solarium\Component\Facet\AbstractFacet::setField()

The reason is:

• the class JsonTerms has the setField() method, but ..
• the setKey() method is implemented in the AbstractFacet as public function setKey(string $key): self

This is handled as that the returned object from setKey() is an instance of AbstractFacet not the current instance of JsonTerms.

This also solves a PHPStan error (when running at level=2)

- tests/Component/FacetSetTest.php
  Call to an undefined method Solarium\\Component\\Facet\\AbstractFacet::setQuery()            

Note: this might be a BC-Break, sadly

[thePanz]

Note: the failure is related to https://github.com/solariumphp/solarium/pull/1151, which fixes it Thanks to @thomascorthals

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.15%. Comparing base (9cdf3dd) to head (d229710). Report is 19 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1152      +/-   ##
==========================================
+ Coverage   97.75%   98.15%   +0.39%     
==========================================
  Files         400      404       +4     
  Lines       10524    10663     +139     
==========================================
+ Hits        10288    10466     +178     
+ Misses        236      197      -39     

Flag	Coverage Δ
unittests	98.15% <100.00%> (+0.39%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[thomascorthals]

This also solves a PHPStan error (when running at level=2)

FYI, I have been working on putting Solarium at level=2 but it's quite an undertaking. As a first step, I have an almost functional branch in my fork where I have explicitly typed almost the entire codebase: https://github.com/thomascorthals/solarium/tree/types. There's one bug I'm going to fix separately first holding this back, and that is held back by the StyleCI issue because of an update on their end.

If the explicit typing can be merged (with hopefully no real BC breaks), I'll continue work on getting the codebase at level 2. A good chunk of that was already done, but since it involved so many type hinting I decided to split that off because it became an unreviewable mess. At this point I'm still confident I can pull it off.

Can I ping you for input when I open those large PRs? It's the kind of change that might be sensitive to blind spots. I can do real-life testing on our dev environment but of course we don't use every last bit of Solr/Solarium functionality.

[thePanz]

@thomascorthals thanks for your pointer! the set of changes in your PR are quite long, you're right!

Did you manually alter them, or used rector-php?

I might be in favor of adding small PRs to achieve covering the code-base with full types, but that's just my 2¢

About PHPStan, we could consider going to an higher level (5 maybe?) and add the current issues into a phpstan-baseline.php file as done in other projects (IIRC Doctrine is doing that). WDYT?

[thomascorthals]

@thePanz I did it manually. My experiences with automated code changes are mixed.

Haven't personally worked with a baseline before. When we started using it at work we just upped the level one step at a time and reworked the entire codebase. We're up to level 5 on most projects. (With the luxury of not having to worry about BC breaks.) I would like to try and get Solarium at level 2 like that. Then look if level 3 is feasible across the codebase or if we should introduce a baseline.

[thePanz]

Thanks @thomascorthals

Maybe those changes should be added into a next major branch? WDYT @mkalkbrenner?

[mkalkbrenner]

Maybe those changes should be added into a next major branch? WDYT @mkalkbrenner?

Do you think it will break something? I would prefer another minor release.

[thomascorthals]

Do you think it will break something?

I think he meant upping Solarium to level 2 in its entirety. @thePanz?

[thePanz]

Do you think it will break something?

Not sure, we should test it further IMO

I think he meant upping Solarium to level 2 in its entirety. @thePanz?

I meant this PR :) @thomascorthals

I also read your PR to bring phpstan to level 2, some of those changes (adding :void to methods for example) could be added in a minor release. Would you like to add those (just :void returns) to a PR, or should I?