Contrib.social

Add return type hints

Open on Drupal.org →
Created on 10 August 2022, over 2 years ago
Updated 7 October 2024, 3 months ago

Problem/Motivation

I have run PHPStan with the following phpstan.neon to get an overview about the overall code quality.
As of official minimum version of PHP 8.0 or 7.4 we can add all related type hints and return types and fix the rest of all.

PHPStan results

 ------ ---------------------------------------------------------------
  Line   etracker.install
 ------ ---------------------------------------------------------------
  13     Function etracker_update_8101() has no return type specified.
  31     Function etracker_update_8102() has no return type specified.
 ------ ---------------------------------------------------------------

 ------ ---------------------------------------------------------------------------------------------------------------------------------
  Line   etracker.module
 ------ ---------------------------------------------------------------------------------------------------------------------------------
  21     Function etracker_help() has no return type specified.
  21     Function etracker_help() has parameter $route_name with no type specified.
  31     Function etracker_library_info_build() has no return type specified.
  94     Function etracker_preprocess_page() has no return type specified.
  94     Function etracker_preprocess_page() has parameter $variables with no type specified.
  148    Function etracker_page_attachments_alter() has no return type specified.
  193    Instanceof between array|string|null and Drupal\Core\StringTranslation\TranslatableMarkup will always evaluate to false.
  195    PHPDoc tag @var has invalid value ($titleObject Drupal\Core\StringTranslation\TranslatableMarkup): Unexpected token
         "$titleObject", expected type at offset 9
  223    Parameter #1 $str of function rawurlencode expects string, mixed given.
  248    Function etracker_form_user_form_alter() has no return type specified.
  248    Function etracker_form_user_form_alter() has parameter $form with no type specified.
  251    PHPDoc tag @var has invalid value ($interface \Drupal\Core\Entity\EntityFormInterface): Unexpected token "$interface", expected
         type at offset 9
  252    Call to an undefined method Drupal\Core\Form\FormInterface::getEntity().
  304    Function etracker_form_user_form_submit() has no return type specified.
  304    Function etracker_form_user_form_submit() has parameter $form with no type specified.
  328    PHPDoc tag @var has invalid value ($breadcrumb Drupal\Core\Breadcrumb\Breadcrumb): Unexpected token "$breadcrumb", expected
         type at offset 9
  339    Parameter #2 $array of function implode expects array<string>, array<int, array|Drupal\Component\Render\MarkupInterface|string>
         given.
  369    Result of && is always false.
  369    Variable $key in isset() always exists and is always null.
  369    Variable $value in isset() always exists and is always null.
 ------ --------------------------------------------------------------------------------------------------------------------------------- 
 ------ -----------------------------------------------------------------------
  Line   modules\cookies_etracker\cookies_etracker.install
 ------ -----------------------------------------------------------------------
  13     Function cookies_etracker_install() has no return type specified.
  36     Function cookies_etracker_update_8001() has no return type specified.
  50     Function cookies_etracker_update_8002() has no return type specified.
 ------ -----------------------------------------------------------------------

 ------ ---------------------------------------------------------------------------------------------------------------
  Line   modules\cookies_etracker\cookies_etracker.module
 ------ ---------------------------------------------------------------------------------------------------------------
  16     Function cookies_etracker_help() has no return type specified.
  16     Function cookies_etracker_help() has parameter $route_name with no type specified.
  46     Function cookies_etracker_library_info_alter() has no return type specified.
  46     Function cookies_etracker_library_info_alter() has parameter $extension with no type specified.
  46     Function cookies_etracker_library_info_alter() has parameter $libraries with no type specified.
  100    Function cookies_etracker_page_attachments() has no return type specified.
  100    Function cookies_etracker_page_attachments() has parameter $page with no type specified.
  151    Function cookies_etracker_form_etracker_admin_settings_alter() has no return type specified.
  151    Function cookies_etracker_form_etracker_admin_settings_alter() has parameter $form with no type specified.
  151    Function cookies_etracker_form_etracker_admin_settings_alter() has parameter $form_id with no type specified.
  214    Function _cookies_etracker_form_etracker_admin_settings_save() has no return type specified.
 ------ ---------------------------------------------------------------------------------------------------------------

 ------ --------------------------------------------------------------------------------------------------------------------------------
  Line   modules\cookies_etracker\tests\src\FunctionalJavascript\CookieTrackingTest.php
 ------ --------------------------------------------------------------------------------------------------------------------------------
  84     Method Drupal\Tests\cookies_etracker\FunctionalJavascript\CookieTrackingTest::testCookieEtrackerBlockingModeDefault() has no
         return type specified.
  116    Method Drupal\Tests\cookies_etracker\FunctionalJavascript\CookieTrackingTest::testCookieEtrackerBlockingModeKnockedOutCookie()
         has no return type specified.
  152    Method Drupal\Tests\cookies_etracker\FunctionalJavascript\CookieTrackingTest::testCookieEtrackerBlockingModeKnockedOutScript()
         has no return type specified.
  186    Method
         Drupal\Tests\cookies_etracker\FunctionalJavascript\CookieTrackingTest::testCookieEtrackerBlockingModeIgnoreConsentScript() has
         no return type specified.
 ------ --------------------------------------------------------------------------------------------------------------------------------

 ------ ---------------------------------------------------------------------------------------------------------------------------------
  Line   modules\cookies_etracker\tests\src\Functional\EtrackerCookiesFunctionalTest.php
 ------ ---------------------------------------------------------------------------------------------------------------------------------
  78     Method Drupal\Tests\cookies_etracker\Functional\EtrackerCookiesFunctionalTest::testLibraryExists() has no return type
         specified.
  91     Method Drupal\Tests\cookies_etracker\Functional\EtrackerCookiesFunctionalTest::testScriptIsTextPlainBeforeConsentInHeader() has
         no return type specified.
  119    Method Drupal\Tests\cookies_etracker\Functional\EtrackerCookiesFunctionalTest::testScriptIsTextPlainBeforeConsentInFooter() has
         no return type specified.
 ------ ---------------------------------------------------------------------------------------------------------------------------------

 ------ --------------------------------------------------------------------------------------------------------------------
  Line   src\EventSubscriber\CspSubscriber.php
 ------ --------------------------------------------------------------------------------------------------------------------
  35     Method Drupal\etracker\EventSubscriber\CspSubscriber::onCspPolicyAlter() has no return type specified.
  35     Parameter $alterEvent of method Drupal\etracker\EventSubscriber\CspSubscriber::onCspPolicyAlter() has invalid type
         Drupal\csp\Event\PolicyAlterEvent.
  35     Parameter $alterEvent of method Drupal\etracker\EventSubscriber\CspSubscriber::onCspPolicyAlter() has invalid type
         Drupal\csp\Event\PolicyAlterEvent.
  36     Call to method getPolicy() on an unknown class Drupal\csp\Event\PolicyAlterEvent.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
 ------ --------------------------------------------------------------------------------------------------------------------

 ------ -----------------------------------------------------------------------------------------------------
  Line   src\Form\EtrackerAdminSettingsForm.php
 ------ -----------------------------------------------------------------------------------------------------
  262    Method Drupal\etracker\Form\EtrackerAdminSettingsForm::validateForm() has no return type specified.
  293    Method Drupal\etracker\Form\EtrackerAdminSettingsForm::submitForm() has no return type specified.
 ------ -----------------------------------------------------------------------------------------------------

 ------ ------------------------------------------------------------------------------------------------------------------
  Line   tests\src\Functional\AdminSettingsFormTests.php
 ------ ------------------------------------------------------------------------------------------------------------------
  61     Method Drupal\Tests\etracker\Functional\AdminSettingsFormTests::testPageTracking() has no return type specified.
 ------ ------------------------------------------------------------------------------------------------------------------

 ------ ---------------------------------------------------------------------------------------------------------------------------------
  Line   tests\src\Functional\ETrackerFunctionalTests.php
 ------ ---------------------------------------------------------------------------------------------------------------------------------
  77     Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testEtrackerAccessFormAsAnonymous() has no return type
         specified.
  89     Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testEtrackerAccessFormAsAdmin() has no return type specified.
  102    Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testEtrackerFormAccountKey() has no return type specified.
  123    Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testEtrackerInHeaderExists() has no return type specified.
  149    Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testEtrackerInFooterExists() has no return type specified.
  170    Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testDataBlockCookieValueEnabled() has no return type
         specified.
  190    Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testDataRespectDntValueEnabled() has no return type specified.
  210    Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testExclusionByPathWithSlashUrl() has no return type
         specified.
  240    Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testInclusionByPathWithSlashUrl() has no return type
         specified.
  270    Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testExclusionByPathWithoutSlashUrl() has no return type
         specified.
  301    Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testInclusionByPathWithoutSlashUrl() has no return type
         specified.
  332    Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testExclusionByRoleAuthenticatedTest() has no return type
         specified.
  362    Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testInclusionByRoleAuthenticatedTest() has no return type
         specified.
  392    Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testExclusionByRoleAnonymousTest() has no return type
         specified.
  423    Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testInclusionByRoleAnonymousTest() has no return type
         specified.
  455    Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testExclusionByRoleAdminTest() has no return type specified.
  496    Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testInclusionByRoleAdminTest() has no return type specified.
  539    Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testExclusionByRoleMultipleRoleTest() has no return type
         specified.
  569    Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testInclusionByRoledMultipleRoleTest() has no return type
         specified.
  600    Method Drupal\Tests\etracker\Functional\ETrackerFunctionalTests::testInclusionByRoleAuthenticatedInheritTest() has no return
         type specified.
 ------ ---------------------------------------------------------------------------------------------------------------------------------


 [ERROR] Found 70 errors
📌 Task
Status

Active

Version

3.0

Component

Code

Created by

🇩🇪Germany sunlix Wesel

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

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment 3 months ago →
    🇩🇪Germany sunlix Wesel

    sunlix → changed the visibility of the branch 3302882-review-and-fix to hidden.

  • Comment 3 months ago →
    🇩🇪Germany sunlix Wesel

    sunlix → changed the visibility of the branch 3302882-review-and-fix to active.

  • Comment 3 months ago →
    🇩🇪Germany sunlix Wesel
  • Merge request !44Resolve #3302882 "Review and fix" → (Merged) created by sunlix
  • Comment 3 months ago →
    🇩🇪Germany sunlix Wesel

    Finished some refactoring and solving the phpstan, phpcs hints.
    eslint> is the only one with some complainings but from my point of view this looks like a false-positive from eslint.
    These are all multiline querySelector calls and for me it makes no sense to a comma at the end of the parameter list, because there is no further parameters to be set.

    Maybe @Grevil and @Anybody has a opinion on that?

    /builds/project/etracker/web/modules/custom/etracker/js/etracker.admin.js
  19:59  error  Insert `,`  prettier/prettier
  47:62  error  Insert `,`  prettier/prettier
  58:54  error  Insert `,`  prettier/prettier
  74:80  error  Insert `,`  prettier/prettier
  81:90  error  Insert `,`  prettier/prettier
/builds/project/etracker/web/modules/custom/etracker/js/etracker.js
  61:75  error  Insert `,`  prettier/prettier
  • Comment 3 months ago →
    🇩🇪Germany Anybody Porta Westfalica

    Thanks @sunlix, totally agree with #6 - still we could add it to mute the warnings. Did you try inlining the string? (All into 1 line)?

    LGTM, did you test the changes manually afterwards? The largest risk I can see is in JS refactoring.

  • Comment 3 months ago →
    🇩🇪Germany sunlix Wesel

    I did a manual test on the admin-js. The cookies related is covered by the fantastic functional test suite from @Grevil.
    So I think this should be good. But we can leave it here for feedback from @Grevil.

    If I lint the silly string in to one line eslint want to make it multiline because of the line length. :D
    The only one we can do is to introduce constants for the long selectors.

  • Comment 3 months ago →
    🇩🇪Germany sunlix Wesel

    I added that last trailing comma because I wanted to see the test green.
    It feels unusual for me. But it is allowed: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Traili...

    I will find my way dealing with this :D

    Ready for a merge if no one disagree. :)

  • Pipeline finished with Skipped
    3 months ago
    #303367
  • Comment 3 months ago →
    System Message
    • sunlix → committed 2228276f on 8.x-3.x 
      Issue #3302882 by sunlix, grevil, anybody: Improve overall code quality...
  • Comment 3 months ago →
    🇩🇪Germany sunlix Wesel
  • Comment 2 months ago →
    🇩🇪Germany Anybody Porta Westfalica

    Great! Yes I think a new release is fine.

  • Comment 2 months ago →
    System Message

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

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024