Postponed until #3270464: Investigate search_api_opensearch as base for elasticsearch_connector has been committed.
PHPStan statically analyzes the code in the module to help identify bugs/issues early.
By increasing the PHPStan level, we increase the number of tests that it runs, and can therefore identify more issues.
Increase the PHPStan level to rule level "2".
Fix any major issues (ignore other issues by adding to phpstan-baseline.neon
Explain why we're ignoring certain rules in the phpstan-baseline.neon
None.
None.
None.
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
sokru commentedComment #5
bramdriesenComment #6
sokru commentedIncreasing the PHPStan level creates 48 new errors (see https://git.drupalcode.org/issue/elasticsearch_connector-3426825/-/jobs/...), we should fix them in this issue.
Comment #7
bramdriesen3 errors remaining. Not sure what the easiest way to fix those is.
Comment #9
mparker17@BramDriesen, thank you very much for your hard work!
The remaining issues in
src/Plugin/search_api/backend/ElasticSearchBackend.php...... are because we were checking if
$connector instanceof PluginFormInterface; andPluginFormInterfacewhich doesn't definegetLabel()norgetDescription()methods (and it doesn't inherit from any interfaces that define these methods either).However, if we instead check whether
$connector instanceof ElasticSearchConnectorInterface, the PHPStan lints will go away, becauseElasticSearchConnectorInterfaceextendsPluginFormInterface(i.e.: so this check is equivalent to the previous check) AND it defines both agetLabel()method and agetDescription()method.There is one more lint in
src/SearchAPI/Query/FilterBuilder.php, "Call to an undefined method Drupal\search_api\Query\ConditionGroupInterface::__toString()." - I will check this out in a bit.Comment #10
mparker17Regarding the
Call to an undefined method Drupal\search_api\Query\ConditionGroupInterface::__toString().lint insrc/SearchAPI/Query/FilterBuilder.php, seems like Search API's API could use an improvement, so I've filed #3428603: Declare that ConditionGroupInterface or ConditionGroup implements \Stringable, and we can add this lint to the baseline for now.Comment #11
mparker17Alright! PHPStan is passing. Moving to Needs Review.
@sokru, did you also want @BramDriesen and/or myself or to address some of the existing items in
phpstan-baseline.neon?Comment #12
mparker17I was able to fix a deprecated function call, but the other lints in
phpstan-baseline.neonare hard to figure out how to change safely, so I'm leaving them here. We should collaborate with the search_api_opensearch maintainers to figure out whether we can delete the code causing those errors.For now, though, this is RTBC from me, and I'd like another maintainer to review (and ask questions), instead of me merging directly.
Comment #14
sokru commentedThanks! I agree with decision to leave existing lints to
phpstan-baseline.neon, we could research it on separate issue and collaborate with search_api_opensearch module maintainers, and maybe doing it before increasing the level further. It seems we get related errors when reaching level 5.Also thanks for creating #3428603: Declare that ConditionGroupInterface or ConditionGroup implements \Stringable, maybe we should ask feedback from #search or #contribute on Drupal.org's Slack if we don't get any replies in few weeks.
Comment #16
mparker17Update the issue summary to document changes made in this ticket.
Comment #17
mparker17