-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Static tests: forbid 'or' instead of '||' #21062
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
Comments
Hi @novikor. Thank you for your report.
Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:
For more details, please, review the Magento Contributor Assistant documentation. @novikor do you confirm that you was able to reproduce the issue on vanilla Magento instance following steps to reproduce?
|
Hi @engcom-backlog-nazar. Thank you for working on this issue.
|
Hi @novikor thank you for you report, It will be processed faster if you move this to https://github.com/magento/community-features |
@engcom-backlog-nazar , OK, I`ll do it. However, I can try to create a PR to this repository with this improvement by myself. Am I right? |
@novikor surely you can ;) |
@novikor Yes feel fre to create a Pull Request. |
@novikor the best way to introduce such check would be to implement a long-awaited static test involving
|
Oh, there is existing Closing in favor of magento/magento-coding-standard#23. @novikor please consider |
@orlangur , I have found a solution how to apply this rule to phpcs and static tests. May I create a PR in this repository? |
@novikor sure thing! Are you going to implement a proper |
Yes, I will try |
Added Squiz.Operators.ValidLogicalOperators rule
Added Squiz.Operators.ValidLogicalOperators rule
Summary (*)
At the moment, static tests allow to use
or
operator which, however, is bad coding practice.There was a PR related to this problem: #20628
I believe ot would be nice to include this check in the static tests.
Proposed solution
Update static tests rules to cover this case
Examples (*)
The text was updated successfully, but these errors were encountered: