Problem/Motivation

3 of 5 jobs in the "Validate" step in our GitLab CI pipeline are failing:
eslint
phpcs
phpstan

Steps to reproduce

https://git.drupalcode.org/project/address/-/pipelines/74589

Proposed resolution

Fix them.

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork address-3413859

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dww created an issue. See original summary.

dww opened merge request !43

dww’s picture

dww
we/he/they
CreditAttribution: dww at 3281d Consulting for SMAI commented
Assigned: Unassigned » bojanz
Status: Active » Needs review

Whoot, https://git.drupalcode.org/project/address/-/pipelines/75265 is finally all green. 😅

I punted on a couple of the errors since they were a bit more complicated to fix than I care to spend the time on right now. Should probably open a follow-up for them. For now, put them in phpstan.neon:

 ------ -------------------------------------------------------------------------- 
  Line   src/Plugin/Field/FieldType/AddressItem.php                                
 ------ -------------------------------------------------------------------------- 
  357    Access to an undefined property                                           
         Drupal\address\Plugin\Field\FieldType\AddressItem::$langcode.             
         💡 Learn more:                                                            
            https://phpstan.org/blog/solving-phpstan-access-to-undefined-property  
  421    Access to an undefined property                                           
         Drupal\address\Plugin\Field\FieldType\AddressItem::$country_code.         
         💡 Learn more:                                                            
            https://phpstan.org/blog/solving-phpstan-access-to-undefined-property  
  429    Access to an undefined property                                           
         Drupal\address\Plugin\Field\FieldType\AddressItem::$langcode.             
         💡 Learn more:                                                            
            https://phpstan.org/blog/solving-phpstan-access-to-undefined-property  
 ------ -------------------------------------------------------------------------- 
 ------ -------------------------------------------------------------------------- 
  Line   src/Plugin/Field/FieldType/ZoneItem.php                                   
 ------ -------------------------------------------------------------------------- 
  75     Access to an undefined property                                           
         Drupal\address\Plugin\Field\FieldType\ZoneItem::$value.                   
         💡 Learn more:                                                            
            https://phpstan.org/blog/solving-phpstan-access-to-undefined-property  
 ------ --------------------------------------------------------------------------

I'd like to backport this to 8.x-1.x, and I believe the changes are all safe (enough).

Curious to see what you think.

Thanks!
-Derek

dww’s picture

dww
we/he/they
CreditAttribution: dww at 3281d Consulting for SMAI commented

Opened #3413883: Cleanup access to undefined properties in FieldType classes and pushed a @todo comment to phpstan.neon pointing there.

Nikolay Shapovalov’s picture

Nikolay Shapovalov
Primary language Russian
CreditAttribution: Nikolay Shapovalov as a volunteer commented
Status: Needs review » Needs work

Thanks for MR, I provide feedback. Please check.

dww’s picture

dww
we/he/they
CreditAttribution: dww at 3281d Consulting for SMAI commented
Status: Needs work » Needs review

Thanks for the review! Addressed/resolved the threads. Yeah, it's much safer to deal with #3414148: Use DI not \Drupal in src/Plugin/views/filter/AdministrativeArea.php separately to not introduce disruptions.

@bojanz: Any final concerns before I merge?

bojanz’s picture

bojanz CreditAttribution: bojanz commented
Status: Needs review » Reviewed & tested by the community

Looks good at a glance!

  • dww committed 2b8df398 on 2.0.x 
    Issue #3413859 by dww, Nikolay Shapovalov, bojanz: Fix all code validate...

  • dww committed ceb1012e on 8.x-1.x 
    Issue #3413859 by dww, Nikolay Shapovalov, bojanz: Fix all code validate...
dww’s picture

dww
we/he/they
CreditAttribution: dww at 3281d Consulting for SMAI commented
Assigned: bojanz » Unassigned
Status: Reviewed & tested by the community » Fixed

Great, thanks! Merged to 2.0.x and cherry picked to 8.x-1.x.

dww closed merge request !43

  • dww committed bf8901a9 on 8.x-1.x 
    Issue #3413859 by dww: Follow-up to expand phpstan-baseline.php in 8.x-1...

  • dww committed f4ba2aa6 on 8.x-1.x 
    Issue #3413859 by dww: Follow-up to expand phpstan.neon to ignore a...

Status: Fixed » Closed (fixed)

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