Problem/Motivation

PHPStan performs static analysis on a codebase to find bugs. PHPStan is run in our CI pipeline. PHPStan groups its lints into rule levels.

At time-of-writing, the 9.0.x and 8.0.x branches are at Rule Level 2. As of commit 1965641 in 9.0.x and commit 848acd3, when I test the module at level 3 (i.e.: by running ddev phpstan --level=3, it shows only 7 lints.

6 of these lints are in test code. The only lint that shows up in production code can be fixed by changing a comment. Thus, these changes are very low risk.

Proposed resolution

Fix the lints, then update phpstan.neon so new merge requests must conform to at least level 3.

Remaining tasks

  1. Merge request 9.0.x - merge request !207 created by @mparker17 in #3
  2. Merge request 8.0.x - merge request !208 created by @mparker17 in #5
  3. Merge to 9.0.x - merged by @mparker17 in #7
  4. Merge to 8.0.x - merged by @mparker17 in #9
  5. Release 9.0.x - released in 9.0.0-alpha3 by @mparker17
  6. Release 8.0.x - released in 8.0.0-alpha7 by @mparker17

User interface changes

None.

API changes

None.

Data model changes

None.

Issue fork elasticsearch_connector-3586383

Command icon Show commands

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

About issue forks

Comments

mparker17 created an issue. See original summary.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Title: PHPStan level 4 (9.0.x and 8.0.x) » PHPStan level 3 (9.0.x and 8.0.x)

Title/Summary mismatch (summary was correct, we want to be at level 3 when this issue is done).

For reference, here is the list of errors (output from ddev phpstan --level=3 --error-format=raw but slightly formatted)...

  1. src/Plugin/search_api/backend/ElasticSearchBackend.php:354:Property Drupal\elasticsearch_connector\Plugin\search_api\backend\ElasticSearchBackend::$backendClient (Drupal\elasticsearch_connector\SearchAPI\BackendClient) does not accept Drupal\elasticsearch_connector\SearchAPI\BackendClientInterface.
  2. tests/modules/elasticsearch_connector_test/src/Drush/Commands/ElasticsearchConnectorTestCommands.php:170:Method Drupal\elasticsearch_connector_test\Drush\Commands\ElasticsearchConnectorTestCommands::addTestEntity() should return Drupal\entity_test\Entity\EntityTestMulRevChanged but returns Drupal\Core\Entity\EntityInterface.
  3. tests/src/Functional/IndexConfigFunctionalTestTrait.php (in context of class Drupal\Tests\elasticsearch_connector\Functional\ConfigImportIndexTest):48:Property Drupal\Tests\elasticsearch_connector\Functional\ConfigImportIndexTest::$entities (array) does not accept array.
  4. tests/src/Functional/IndexConfigFunctionalTestTrait.php (in context of class Drupal\Tests\elasticsearch_connector\Functional\ConfigImportIndexTest):51:Method Drupal\Tests\elasticsearch_connector\Functional\ConfigImportIndexTest::addTestEntity() should return Drupal\entity_test\Entity\EntityTestMulRevChanged but returns Drupal\Core\Entity\EntityInterface.
  5. tests/src/Functional/IndexConfigFunctionalTestTrait.php (in context of class Drupal\Tests\elasticsearch_connector\Functional\ManualConfigIndexTest):48:Property Drupal\Tests\elasticsearch_connector\Functional\ManualConfigIndexTest::$entities (array) does not accept array.
  6. tests/src/Functional/IndexConfigFunctionalTestTrait.php (in context of class Drupal\Tests\elasticsearch_connector\Functional\ManualConfigIndexTest):51:Method Drupal\Tests\elasticsearch_connector\Functional\ManualConfigIndexTest::addTestEntity() should return Drupal\entity_test\Entity\EntityTestMulRevChanged but returns Drupal\Core\Entity\EntityInterface.
  7. tests/src/Kernel/ElasticSearchBackendTest.php:90:Property Drupal\KernelTests\KernelTestBase::$container (Drupal\Core\DependencyInjection\ContainerBuilder) does not accept null.

mparker17 opened merge request !207

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes
Status: Active » Needs review

Created merge request !207 for the 9.0.x branch; let's see what testbot thinks.

mparker17 opened merge request !208

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Tests passed for 9.0.x; so I've created a merge request for 8.0.x

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Version: 9.0.x-dev » 8.0.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Merging to 8.0.x next

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes
Status: Patch (to be ported) » Fixed

Merged to 8.0.x now. I'll update this issue when these change are released.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes

The changes in this issue were released in elasticsearch_connector-9.0.0-alpha3, and elasticsearch_connector-8.0.0-alpha7

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.