-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Fix accepted types for escaper methods #40114
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
base: 2.4-develop
Are you sure you want to change the base?
Fix accepted types for escaper methods #40114
Conversation
…types are allowed.
… knows it'll be a string or an array depending on the type of the input.
Hi @hostep. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
@magento run all tests |
@magento run all tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -1,7 +1,7 @@ | |||
<?php | |||
/** | |||
* Copyright © Magento, Inc. All rights reserved. | |||
* See COPYING.txt for license details. | |||
* Copyright 2013 Adobe |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Copyright 2013 Adobe | |
* Copyright 2011 Adobe |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@engcom-Hotel: it should be 2013, not 2011. Looking at the git history, github is guessing it got renamed from app/code/core/Mage/Adminhtml/Model/System/Config/Backend/Price/Scope.php
to app/code/core/Mage/Adminhtml/Block/Tax/Rate/ImportExportHeader.php
in 2011 and then to app/code/Magento/Adminhtml/Block/Sales/Order/View/History.php
in 2013
But going from a PriceScopeBackendConfig to a TaxRateImportExportHeader to a SalesOrderViewHistoryBlock makes no sense to me. So in my opinion it should be 2013.
Can you double check this again?
I didn't check the other files, only this one...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you really believe it should be 2011, go ahead and change it yourself because I'm unavailable for the next 2 weeks, since I'll go on holiday. Thanks!
@@ -1,7 +1,7 @@ | |||
<?php | |||
/** | |||
* Copyright © Magento, Inc. All rights reserved. | |||
* See COPYING.txt for license details. | |||
* Copyright 2013 Adobe |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Copyright 2013 Adobe | |
* Copyright 2011 Adobe |
@@ -1,7 +1,7 @@ | |||
<?php | |||
/** | |||
* Copyright © Magento, Inc. All rights reserved. | |||
* See COPYING.txt for license details. | |||
* Copyright 2014 Adobe |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Copyright 2014 Adobe | |
* Copyright 2013 Adobe |
@@ -1,7 +1,7 @@ | |||
<?php | |||
/** | |||
* Copyright © Magento, Inc. All rights reserved. | |||
* See COPYING.txt for license details. | |||
* Copyright 2014 Adobe |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Copyright 2014 Adobe | |
* Copyright 2011 Adobe |
@magento run Functional Tests EE, Functional Tests B2B |
Description (*)
When performing static analysis using phpstan on level 5 or higher on (custom) phtml files, we often run into false positives that have to do with the kind of data we send to one of the many
escape
methods.Also the return type of
escapeHtml
was not clear as it can return astring
orarray
depending on the type you send to the method.We fix that in this PR.
Many thanks to @navarr who came up this suggested solution in scope of bitExpert/phpstan-magento#346
Related Pull Requests
Fixed Issues (if relevant)
Manual testing scenarios (*)
See #40012 & bitExpert/phpstan-magento#346
Maybe I'll find more time later to work out a good example, but I don't have the time at the moment.
Questions or comments
Contribution checklist (*)