Increase PHPStan level

amanmansuri72 → made their first commit to this issue’s fork.

@amanmansuri72 this issue is for phpstan, not phpcs. That effort belongs on 🐛 Drupal Coding Standards Issues | phpcs Needs work .

Hello,
I fixed some errors shown at level 2 to level 6 can u check that ?

For the error below, I have checked the interface containing the signature of the function "get" we have only one param "CountryCode", I think we should remove the second param in the class

------ ------------------------------------------------------------------------------- 
  Line   src/Plugin/GraphQL/DataProducer/AddressFormatRepository.php                    
 ------ ------------------------------------------------------------------------------- 
  98     Method                                                                         
         CommerceGuys\Addressing\AddressFormat\AddressFormatRepositoryInterface::get()  
         invoked with 2 parameters, 1 required.                                         
 ------ ------------------------------------------------------------------------------- 

What do you think ?

Hi h_kac, if you would like work to be reviewed, please open a MR and set the issue to 'Needs Review' :)

Changes in the MR aren't seems to be phpstan related.

cc: @darvanen

Yeah that would be from #2, feel free to revert that commit, it belongs in another issue per #3.

It looks like the MR doesn't contain any change to set the PHPStan level higher for the test runner too.

Still all the changes are related to phpcs.

@h_kac you may find it easier to make a new fork rather than manually trying to revert changes.

#12 is right, many phpcs related changes remain on the current MR.

I'll check for phpstan error.

ankitv18 → changed the visibility of the branch 3440876-increase-phpstan-level to hidden.

@darvanen MR!12 is ready for review.

@ankitv18 I'm not really keen on duplicating method comments from parent classes just for the sake of passing higher levels of PHPStan. If we want to pass at that level we should see that the parent passes first (in the graphql module) and leave ours as @inheritdoc in my opinion.

Perhaps we should postpone this on that work? Otherwise I'm happy to commit a version with a lower level.

We will also need to comment out the various OPT_IN variables at the end of this as they are not to be run on every test in every issue. Advice I've recieved from several senior maintainers is to save those for final runs.

Thoughts?

I'm also not committed with all these changes as I just pushed the commits to show the higher level of phpstan changes.

Anyways I'm reverting the changes and put the phpstan level to 2 and left the other required changes for phpstan apart from docblock.

5 is great, let's leave it there for now. Thanks for this, it was quite the education for me.

@h_kac I'm going to give you a credit because you're quite new and I can see you were trying. Keep going! There are many ways to contribute. Also join Drupal Slack → , it's a great way to see what's happening in the community.

@Darvanen thank you

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