Api2: cleanup getNode#5607
Merged
Merged
Conversation
|
sreichel
approved these changes
May 22, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refines API2 helper logic around reading XML config nodes (getNode()), aiming to make return types predictable and remove PHPStan baseline suppressions related to those paths.
Changes:
- Tightened type handling in
Mage_Api2_Helper_Datawhen reading auth adapter and user type configuration nodes, plus stricterin_array()usage. - Updated an ACL resource method docblock return type to be more precise (
falseinstead ofbool). - Removed multiple PHPStan baseline entries that should no longer be needed after the helper cleanup.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| app/code/core/Mage/Api2/Model/Resource/Acl/Filter/Attribute.php | Narrows the documented return type of getAllowedAttributes() to match actual DB adapter behavior. |
| app/code/core/Mage/Api2/Helper/Data.php | Adds stronger type checks around getNode()/asArray() results and strict comparisons in in_array(). |
| .phpstan.dist.baselines/variable.undefined.php | Removes an ignore that was masking $adapters undefined cases. |
| .phpstan.dist.baselines/return.type.php | Removes an ignore for getAuthAdapters() return-type mismatch. |
| .phpstan.dist.baselines/method.nonObject.php | Removes an ignore for calling getNode() on possibly-null config. |
| .phpstan.dist.baselines/if.condNotBoolean.php | Removes an ignore related to non-boolean if conditions in the helper. |
| .phpstan.dist.baselines/function.strict.php | Removes an ignore requiring strict in_array() usage. |
| .phpstan.dist.baselines/foreach.nonIterable.php | Removes an ignore for iterating over a non-iterable return from config parsing. |
| .phpstan.dist.baselines/booleanNot.exprNotBoolean.php | Removes an ignore for negating a non-boolean expression from config parsing. |
| .phpstan.dist.baselines/argument.type.php | Removes ignores for invalid argument types reaching uasort() / other functions. |
Comment on lines
+56
to
60
| $adapters = Mage::getConfig()?->getNode(self::XML_PATH_AUTH_ADAPTERS); | ||
|
|
||
| if (!$adapters) { | ||
| if (!$adapters instanceof Varien_Simplexml_Element) { | ||
| return []; | ||
| } |
Contributor
Author
There was a problem hiding this comment.
@sreichel i'm pretty sure copilot is wrong there
Comment on lines
+84
to
+88
| $types = Mage::getConfig()?->getNode(self::XML_PATH_USER_TYPES); | ||
|
|
||
| if (!$types instanceof Varien_Simplexml_Element) { | ||
| return []; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description (*)
Related Pull Requests
Fixed Issues (if relevant)
Manual testing scenarios (*)
Questions or comments
composer phpstan:baselinedoesn't work for me, so i need to ask github againContribution checklist (*)