pestle icon indicating copy to clipboard operation
pestle copied to clipboard

M2 Crud Model: Inspection info: invocation parameter types are not compatible with declared

Open sallaberry opened this issue 5 years ago • 7 comments

pestle magento2:generate:crud-model

Which module? (Pulsestorm_HelloGenerate)] Namespace_Module
What model name? (Thing)] Thing
Creating: /[..]/app/code/Namespace/Module/Api/ThingRepositoryInterface.php
Creating: /[..]/app/code/Namespace/Module/Model/ThingRepository.php
Creating: /[..]/app/code/Namespace/Module/Api/Data/ThingInterface.php
Creating: /[..]/app/code/Namespace/Module/Model/ResourceModel/Thing/Collection.php
Creating: /[..]/app/code/Namespace/Module/Model/ResourceModel/Thing.php
Creating: /[..]/app/code/Namespace/Module/Model/Thing.php
Creating: /[..]/app/code/Namespace/Module/Setup/InstallSchema.php
Creating: /[..]/app/code/Namespace/Module/Setup/InstallData.php

I fixed all the issues detected by phpcs / phpmd (using Magento 2 provided rulesets) like code style, missing phpdocs and other easy to fix stuff, but I have two remaining warnings in the file : /[..]/app/code/Namespace/Module/Model/ThingRepository.php

In the save and delete methods when calling respectively :

$this->objectResourceModel->save($object);

or

$this->objectResourceModel->delete($object);

Warning on $object :

Expected \Magento\Framework\Model\AbstractModel, got \Namespace\Module\Api\Data\ThingInterface
Inspection info: invocation parameter types are not compatible with declared

sallaberry avatar Aug 07 '19 18:08 sallaberry

@sallaberry Thank you!

Three quick questions

  1. What version of Magento are you running against?
  2. Does this only happen with the changes you made, or is it a problem with the current version of pestle.phar as well?
  3. Do you have you changes in a branch somewhere where I could look at them?

astorm avatar Aug 08 '19 02:08 astorm

  1. Magento 2.3.1
  2. Yes it happens without my changes
  3. I forked the repo and worked on a special branch if that's what you wanted : https://github.com/sallaberry/pestle/tree/%23476-Crud_Improvements

sallaberry avatar Aug 09 '19 00:08 sallaberry

sigh -- sounds like Magento's tightened-up/change their interface/contract assumptions in 2.3.1

@sallaberry Were you planning on submitting a PR for you phpcs / phpmd changes? If so, what would you be easier

  1. You submit your PR, we confirm it works with older versions of Magento, and then I (or we?) attack the problem of making sure the model's passed to the repository are able to pass all their type hint checks

  2. First I (or we?) attack the problem of making sure the model's passed to the repository are able to pass all their type hint checks, and then once we figure that out we make it work with what you've got on your branch?

astorm avatar Aug 09 '19 00:08 astorm

  1. Sounds good, it should still work even though I had to edit some functions in Pulsestorm\Cli\Code_Generation

I'll open the PR right now

sallaberry avatar Aug 09 '19 01:08 sallaberry

It appears the warning actually come from one of PhpStorm basic php rules, not from the official ruleset. It also complains about SortOrder being undefined in the same file ( Model/<Model>Repository.php ).

Maybe we can ignore the warnings but PhpStorm is usually right so I'm not sure...

sallaberry avatar Aug 09 '19 03:08 sallaberry

@sallaberry A few more questions

  1. I think I may have misunderstood -- I thought the warning you were still seeing were generated via PHP. It sounds like you're saying that they're coming from ??some-system?? used by PHP Storm itself?

  2. Which Magento coding standards are you applying? Link/composer package?

astorm avatar Aug 09 '19 14:08 astorm

I'm sorry if I wasn't clear enough, this is just an inspection problem.

The warnings appears directly in my IDE Phpstorm which comes with his own coding standards, but if you install phpcs globally, you can point Phpstorm to the phpcs binary and use it directly from Phpstorm. (I use squizlabs/php_codesniffer installed globally)

I configured phpcs inside phpstorm to use a ruleset located in dev/tests/static/framework/Magento/ruleset.xml and phpmd to use dev/tests/static/testsuite/Magento/Test/Php/_files/phpmd/ruleset.xml

Now if there's something wrong in my code and the warning comes from phpcs or phpmd, the error message is prefixed with 'phpcs:' or 'phpmd:'. It's not the case with the two messages I have. I'm sorry I didn't notice that earlier.

Inside Phpstorm native code inspection configuration, I can disable specific rules, set their warning level etc. If I disable the rule: Type Compatibility > Parameter Type (Invocation parameter types are not compatible with declared.) then the warning on ->save($object) disappears. What I find weird is Phpstorm usually doesn't complain for nothing and I always had this rule on.

sallaberry avatar Aug 09 '19 17:08 sallaberry