At the moment, static tests allow to use or operator which, however, is bad coding practice.
There was a PR related to this problem: https://github.com/magento/magento2/pull/20628
I believe ot would be nice to include this check in the static tests.
Update static tests rules to cover this case
Hi @novikor. Thank you for your report.
To help us process this issue please make sure that you provided the following information:
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:
@magento-engcom-team give me 2.3-develop instance - upcoming 2.3.x release
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.
In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:
Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: Format is valid appears.[x] 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add Issue: Clear Description label to the issue by yourself.
[x] 3. Add Component: XXXXX label(s) to the ticket, indicating the components it may be related to.
[ ] 4. Verify that the issue is reproducible on 2.3-develop branchDetails
- Add the comment @magento-engcom-team give me 2.3-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.3-develop branch, please, add the label Reproduced on 2.3.x.
- If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and _stop verification process here_!
[ ] 5. Verify that the issue is reproducible on 2.2-develop branch. Details
- Add the comment @magento-engcom-team give me 2.2-develop instance to deploy test instance on Magento infrastructure.
- If the issue is reproducible on 2.2-develop branch, please add the label Reproduced on 2.2.x
_Next steps are available in case you are a member of Community Maintainers._
[ ] 6. Add label Issue: Confirmed once verification is complete.
[ ] 7. Make sure that automatic system confirms that report has been added to the backlog.
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 php-cs-fixer as it has a rule
logical_operators [@PhpCsFixer:risky]
Use && and || logical operators instead of and and or.
Risky rule: risky, because you must double-check if using and/or with lower precedence was intentional.
Oh, there is existing phpcs sniff actually, thanks @lenaorobei for finding this out.
Closing in favor of https://github.com/magento/magento-coding-standard/issues/23.
@novikor please consider php-cs-fixer integration anyway ;)
@orlangur , I have found a solution how to apply this rule to phpcs and static tests.
Ruleset.xml is present in magento/magento-coding-standard, but .php_cs.dist is not.
May I create a PR in this repository?
@novikor sure thing! Are you going to implement a proper php-cs-fixer integration?
@novikor sure thing! Are you going to implement a proper
php-cs-fixerintegration?
Yes, I will try
Hi @novikor. Thank you for your report.
The issue has been fixed in magento/magento2#21275 by @novikor in 2.3-develop branch
Related commit(s):
The fix will be available with the upcoming 2.3.2 release.
Hi @novikor. Thank you for your report.
The issue has been fixed in magento/magento2#21543 by @novikor in 2.2-develop branch
Related commit(s):
The fix will be available with the upcoming 2.2.9 release.