Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PHPStan error in Acl component #2401

Closed
wants to merge 1 commit into from
Closed

Conversation

myfluxi
Copy link
Contributor

@myfluxi myfluxi commented Dec 12, 2020

We must add integer to the hinted types of optional $pluginID arg.
Else PHPstan level 5 check fails with:


Line Bootstrap/Acl.php


53 Parameter #4 $pluginID of method
Shopware_Components_Acl::createResource() expects null, int given.


1. Why is this change necessary?

PHPStan level 5 check in our plugin fails.

2. What does this change do, exactly?

We just add int to the hinted types of the optional $pluginID argument to createResource().

3. Describe each step to reproduce the issue or behaviour.

Use createResource(), run a PHPStan check level 5 to trigger an error like:


Line Bootstrap/Acl.php


53 Parameter #4 $pluginID of method
Shopware_Components_Acl::createResource() expects null, int given.


4. Please link to the relevant issues (if any).

5. Which documentation changes (if any) need to be made because of this PR?

6. Checklist

  • I have written tests and verified that they fail without my change
  • I have squashed any insignificant commits
  • This change has comments for package types, values, functions, and non-obvious lines of code
  • I have read the contribution requirements and fulfil them.

@CLAassistant
Copy link

CLAassistant commented Dec 12, 2020

CLA assistant check
All committers have signed the CLA.

We must add integer to the allowed types of optional $pluginID arg.
Else PHPstan level 5 check fails with:

------ --------------------------------------------------------------------
  Line   Bootstrap/Acl.php
 ------ --------------------------------------------------------------------
  53     Parameter #4 $pluginID of method
         Shopware_Components_Acl::createResource() expects null, int given.
 ------ --------------------------------------------------------------------
public function createResource(
$resourceName,
array $privileges = null,
/* @noinspection PhpUnusedParameterInspection */ $menuItemName = null,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/* @noinspection PhpUnusedParameterInspection */ $menuItemName = null,
$menuItemName = null,

array $privileges = null,
/* @noinspection PhpUnusedParameterInspection */ $menuItemName = null,
$pluginID = null
){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
){
) {

@@ -144,12 +144,14 @@ public function hasResourceInDatabase($resourceName)
* @param string $resourceName - unique identifier or resource key
* @param array|null $privileges - optionally array [a,b,c] of new privileges
* @param null $menuItemName - optionally s_core_menu.name item to link to this resource
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param null $menuItemName - optionally s_core_menu.name item to link to this resource
* @param string|null $menuItemName - optionally s_core_menu.name item to link to this resource

@shopwareBot
Copy link

This PR was closed due to inactivity. If this change is still important to you, feel free to create a new pull request.

For more information about our contribution guidelines, see https://developers.shopware.com/contributing/contribution-guideline/.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants