[5.3] Drone: Adding check for PHP version #38471
Conversation
….2-phpcs-phpversion # Conflicts: # composer.lock
…into 4.2-phpcs-phpversion
|
@Hackwar what can we do here? clean the error or mark as exception for phpcs? |
|
Most of these should be fixed. Some would have to be marked as exclusions. Will look at that soon. |
….2-phpcs-phpversion
…into 4.2-phpcs-phpversion
….2-phpcs-phpversion # Conflicts: # .drone.yml
joomla-cms-bot
removed
Language Change
|
I've split this PR up into 5 PRs, see above. When those are merged and they are upmerged into this branch, that should leave only the code to introduce the version check in phpcs. I don't think it is usefull to solve the conflicts before those are merged, since each will introduce new conflicts... |
| <rule ref="PHPCompatibility.Keywords.ForbiddenNamesAsDeclared"> | ||
| <exclude-pattern type="relative">libraries/src/Object/CMSObject\.php</exclude-pattern> | ||
| <exclude-pattern type="relative">libraries/src/String/PunycodeHelper\.php</exclude-pattern> | ||
| <exclude-pattern type="relative">tests/Unit/Libraries/Cms/Object/CMSObjectTest\.php</exclude-pattern> | ||
| </rule> | ||
|
|
||
| <rule ref="PHPCompatibility.Constants.NewConstants.password_argon2idFound"> | ||
| <exclude-pattern type="relative">libraries/src/Authentication/Password/Argon2idHandler\.php</exclude-pattern> | ||
| </rule> | ||
|
|
||
| <rule ref="PHPCompatibility.FunctionUse.ArgumentFunctionsReportCurrentValue"> | ||
| <exclude-pattern type="relative">libraries/src/Date/Date\.php</exclude-pattern> | ||
| <exclude-pattern type="relative">libraries/src/Application/CliApplication\.php</exclude-pattern> | ||
| <exclude-pattern type="relative">libraries/src/Application/ConsoleApplication\.php</exclude-pattern> | ||
| <exclude-pattern type="relative">libraries/src/Document/HtmlDocument\.php</exclude-pattern> | ||
| </rule> | ||
|
|
||
| <rule ref="PHPCompatibility.FunctionNameRestrictions.NewMagicMethods"> | ||
| <exclude-pattern type="relative">administrator/components/com_finder/src/Indexer/Result\.php</exclude-pattern> | ||
| </rule> |
There was a problem hiding this comment.
Please be aware that these excludes will exclude the issues for the complete file, which may hide new issues being introduced in the future.
If you want more selective "ignores", you can use the PHPCS native inline ignore annotations in the files themselves, typically ignoring only one particular line or a small group of lines.
This will more effectively prevent people from accidentally introducing new issues.
| @@ -21,6 +21,32 @@ | |||
|
|
|||
There was a problem hiding this comment.
☝🏻 As a general remark - I see top-level exclude-patterns in this config file, but no <file> include ? That also may make the type="relative" directives unstable as it is unclear what this should be relative to...
| <rule ref="PHPCompatibilityJoomla"> | ||
| <exclude-pattern type="relative">libraries/src/Encrypt/AES/Mcrypt\.php</exclude-pattern> | ||
| </rule> |
There was a problem hiding this comment.
| <rule ref="PHPCompatibilityJoomla"> | |
| <exclude-pattern type="relative">libraries/src/Encrypt/AES/Mcrypt\.php</exclude-pattern> | |
| </rule> | |
| <rule ref="PHPCompatibilityJoomla"> | |
| <include-pattern>*\.php$</include-pattern> | |
| <exclude-pattern type="relative">libraries/src/Encrypt/AES/Mcrypt\.php</exclude-pattern> | |
| </rule> |
PHPCS will scan PHP, JS and CSS files by default. As there is no top-level extensions setting included in the ruleset to change this, you may want to ensure that the PHPCompatibility sniffs are only run against PHP files.
There was a problem hiding this comment.
Also: as the Mcrypt file is excluded completely - is that a polyfill for the Mcrypt extension ? And if so, should the PHPCompatibilityJoomla ruleset be updated to exclude errors and warnings from the polyfilled functionality completely ?
There was a problem hiding this comment.
The Mcrypt file is a wrapper and basically currently broken in 4.x. We are working on removing it...
joomla-cms-bot
added
PR-5.0-dev
and removed
Language Change
Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
HLeithner
changed the title
|
This pull request has been automatically rebased to 5.1-dev. |
|
This pull request has been automatically rebased to 5.2-dev. |
HLeithner
changed the title
|
Is anything happening with this? It has not been touched for over a year and it is unclear if anything will ever happen. It refers to it being partialy split into 5 pr but some of those have been closed as abandoned. |
|
This pull request has been automatically rebased to 5.3-dev. |
HLeithner
changed the title
Right now we actually aren't checking the compatibility of our code for a certain PHP version statically. This PR should change that.