Check returns values added in #3283794

Created on 9 October 2023, 9 months ago

Problem/Motivation

In πŸ“Œ [Meta] Fix Drupal.Commenting.FunctionComment.InvalidNoReturn Active it was discovered that an incorrect return value was added in #3283794: Fix 'should return {type} but return statement is missing' PHPStan L0 errors in test code β†’ . In the issue to fix that, πŸ“Œ Fix return type in \Drupal\Tests\rest\Functional\ResourceTestBase::recursiveKSort Active , xjm asked for a followup to review that commit to see if they are other errors. The followup is largely because the size of the change, xjm calculated +192, -85 -- 277 total lines to review,, is larger than the recommendation of 100-200 lines.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

πŸ“Œ Task
Status

Active

Version

11.0 πŸ”₯

Component
BaseΒ  β†’

Last updated about 10 hours ago

Created by

πŸ‡³πŸ‡ΏNew Zealand quietone New Zealand

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @quietone
  • πŸ‡³πŸ‡ΏNew Zealand quietone New Zealand

    There were 92 changed files.

    64 files were changed to add return []; to empty getNormalizedPostEntity() and empty getPostDocument() in various tests to match the relevant interface.

    4 migrate_drupal_ui files were changed to add return []; to empty methods to match the definition of the method defined in the base class.

    2 migration test bases classes changed to conform to relevant interface. Including add the throwing of a RuntimeException for methods not implemented.
    \Drupal\Tests\migrate_drupal\Unit\source\TestDrupalSqlBase::query now throws a RuntimeException not implemented

    1 file changed to fix switch statement
    1 file incorrectly changed for recursiveKSort
    1 file changed to removed useless return statements from assert helper methods
    AssertContentTraitchanged to return void for assert helper methods
    EntityResolverManagerTest, FormStateTest so test class matches FormInterface
    FieldDefinitionTest add a throw for the error case

  • πŸ‡³πŸ‡ΏNew Zealand quietone New Zealand

    These are require a bit more thought, not low hanging fruit.

    These were fixed to match the base method @return. However, the base method returns NULL but the doc is '@return string'.

    • \Drupal\Tests\jsonapi\Functional\FileUploadTest::getExpectedUnauthorizedAccessMessage
    • \Drupal\Tests\jsonapi\Functional\NodeTest::getExpectedUnauthorizedAccessMessage
    • \Drupal\Tests\jsonapi\Functional\RestJsonApiUnsupported::getExpectedUnauthorizedAccessMessage
    • \Drupal\Tests\user\Functional\UserRegistrationRestTest::getExpectedUnauthorizedAccessMessage

    These changed to return new CacheableMetadata(); instead of void.

    • \Drupal\Tests\jsonapi\Functional\FileUploadTest::getExpectedUnauthorizedAccessCacheability.
    • \Drupal\Tests\jsonapi\Functional\NodeTest::getExpectedUnauthorizedAccessMessage
    • \Drupal\Tests\jsonapi\Functional\RestJsonApiUnsupported::getExpectedUnauthorizedAccessCacheability
    • \Drupal\Tests\layout_builder\Functional\Rest\LayoutRestTestBase::getExpectedUnauthorizedAccessCacheability
    • \Drupal\Tests\rest\Functional\FileUploadResourceTestBase::getExpectedUnauthorizedAccessCacheability
    • \Drupal\Tests\user\Functional\UserRegistrationRestTest::getExpectedUnauthorizedAccessCacheability

    These are still todo

    • core/modules/comment/tests/src/Functional/CommentNonNodeTest.php
    • core/modules/layout_builder/tests/modules/layout_builder_test/src/Plugin/SectionStorage/TestStateBasedSectionStorage.php
    • core/modules/migrate/tests/src/Unit/TestMigrateExecutable.php
    • core/modules/system/tests/src/Functional/Form/StubForm.php
    • core/modules/system/tests/src/Unit/Breadcrumbs/PathBasedBreadcrumbBuilderTest.php
    • core/tests/Drupal/Tests/Core/Image/ImageTest.php
    • core/tests/Drupal/Tests/Core/Plugin/LazyPluginCollectionTestBase.php
Production build 0.69.0 2024