Problem/Motivation

PHPStan is currently skipping multiple #should return .* but return statement is missing# errors.

Steps to reproduce

Fix the errors in test code, clean up the baseline.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3309047

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:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mondrake created an issue. See original summary.

mondrake’s picture

Status: Active » Needs review
Spokje’s picture

Status: Needs review » Needs work

Looks like you missed three removals from the baseline?

mondrake’s picture

Status: Needs work » Needs review

#4 thanks! Actually, the three misses were due to allowing a null return type (consistently with what the docs were saying)

diff --git a/core/lib/Drupal/Component/Plugin/Derivative/DeriverInterface.php b/core/lib/Drupal/Component/Plugin/Derivative/DeriverInterface.php
index 32b59ef7484f48a943a7f153bf179f8680f859bc..ff13b0fbc698a71af227db2314ec2d154772f035 100644
--- a/core/lib/Drupal/Component/Plugin/Derivative/DeriverInterface.php
+++ b/core/lib/Drupal/Component/Plugin/Derivative/DeriverInterface.php
@@ -20,7 +20,7 @@ interface DeriverInterface {
    *   is derived. It is maybe an entire object or just some array, depending
    *   on the discovery mechanism.
    *
-   * @return array
+   * @return array|null
    *   The full definition array of the derivative plugin, typically a merge of
    *   $base_plugin_definition with extra derivative-specific information. NULL
    *   if the derivative doesn't exist.

This mere addition meant the PHPStan error not to appear any longer. I have anyway also adjusted the reported methods to explicitly return NULL in case the derivative does not exist, to imporve readibility.

Spokje’s picture

Status: Needs review » Needs work

I've added the "per usual" annoying nitpicks, as before: Not blocking at all, close them if you don't agree.
Change them if you agree.

In both cases: I'm happy to RTBC this.

mondrake’s picture

Status: Needs work » Needs review

Thanks @Spokje

Spokje’s picture

Status: Needs review » Reviewed & tested by the community

- All my nitpicks are addressed (or refused for a very good reason).
- Code changes make sense (see also #5)

RTBC if TestBot agrees.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

Added some review comments to gitlab.

mondrake’s picture

Status: Needs work » Needs review
mondrake’s picture

rebased

Spokje’s picture

Status: Needs review » Reviewed & tested by the community

- Random JS test failures that are not related to this MR
- All concerns of both @longwave and @alexpott were addressed.

RTBC for me.

alexpott credited longwave.

alexpott’s picture

Version: 10.0.x-dev » 9.4.x-dev
Status: Reviewed & tested by the community » Fixed

Backported the changes to 9.4.x (apart from the phpstan baseline) because this is test code or docs.

  • alexpott committed 830646e on 10.1.x
    Issue #3309047 by mondrake: Fix 'should return {type} but return...

  • alexpott committed 381ed6f on 10.0.x
    Issue #3309047 by mondrake: Fix 'should return {type} but return...

  • alexpott committed 13c8eaf on 9.5.x
    Issue #3309047 by mondrake: Fix 'should return {type} but return...

  • alexpott committed 09daf7c on 9.4.x
    Issue #3309047 by mondrake: Fix 'should return {type} but return...

Status: Fixed » Closed (fixed)

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