Skip to content
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

Automatic PHP version compatibility check for php-cs #6

Open
thomasnordquist opened this issue Sep 5, 2017 · 12 comments
Open

Automatic PHP version compatibility check for php-cs #6

thomasnordquist opened this issue Sep 5, 2017 · 12 comments
Labels
enhancement Issues that describe new features or improvements to existing features.

Comments

@thomasnordquist
Copy link
Contributor

thomasnordquist commented Sep 5, 2017

We could easily test our Plugins for php version compatibility:
https://github.com/wimg/PHPCompatibility

Some small effort is necessary to have a "per plugin" mapping for minimal php versions, this could be done via the plugin.json minimal compatibility.

Example:
1474907900shot

@thomasnordquist
Copy link
Contributor Author

What do you think ?
@svenmuennich @fixpunkt @samuelvogel

@fixpunkt
Copy link
Member

fixpunkt commented Sep 5, 2017

In which way is this different from php -l?

@samuelvogel
Copy link
Member

The minimum PHP version is dictated to us from Shopware and also checked when uploading to the store. So I'm not sure about the benefit of this (except seeing errors earlier).

@fixpunkt
Copy link
Member

fixpunkt commented Sep 5, 2017

It's only checked when requesting the plugin review in the store. I still make these types of mistake occasionally.

Getting this info a lot earlier (i.e. as part of the pre-commit hook or at the start of the CI run) and thus catching this kind of mistake before putting in all the work leading up to a release should be a valuable tool.

@svenmuennich
Copy link
Member

I think a php -l on the CI for release builds should be sufficient to catch these types of errors.

@fixpunkt
Copy link
Member

fixpunkt commented Sep 7, 2017

I'd prefer to run them earlier, as I explained, because we want to catch these before we've made the release commit. Since running the linter is cheap, I also don't see a benefit in restricting this to release builds.

@svenmuennich
Copy link
Member

I don't see a way of doing it with php -l before the CI, because that would require to have the min. required PHP version running on your dev machine.

@fixpunkt
Copy link
Member

fixpunkt commented Sep 7, 2017

On the CI is fine, I'd just prefer to run it for every commit.

@svenmuennich
Copy link
Member

We'll have to see how we can that on the CI, since AFAIK we were planning to run CI commit builds only on PHP 7.

@mihaiplasoianu Is it possible to change the PHP 7 version in TravisCI dynamically without using a build matrix?

@fixpunkt
Copy link
Member

fixpunkt commented Sep 7, 2017

So maybe we can just use https://github.com/wimg/PHPCompatibility after all, since that allows setting the target PHP version at runtime.

@svenmuennich
Copy link
Member

Although we run the tests etc. in a matrix of different PHP versions, we only have to lint the code with the minimum required PHP version. But using the package mentioned above seems to be the easier and more portable option.

@thomasnordquist
Copy link
Contributor Author

thomasnordquist commented Sep 25, 2017

PHPCompatibility does not require a PHP change, it applies it's own compatibility rules set.
It can detect incompatibilities even though the php linter is not aware of those incompatibilities.

The tests don't take up to much time if the are limited on the changed files only.
(Testing complete projects takes a few seconds)

All tests were executed with PHP 5.6

PHP 5.3

bildschirmfoto 2017-09-25 um 15 54 15

PHP 5.4

bildschirmfoto 2017-09-25 um 15 54 28

PHP 5.6

bildschirmfoto 2017-09-25 um 15 54 37

PHP 7

bildschirmfoto 2017-09-25 um 15 57 31

The file in question

<?php

if (empty(trim(" "))) {
	echo "empty() works\n";
}

if (empty([])) {
	echo "short array sytax works\n";
}

echo sys_get_temp_dir(). "\n";

throw new Exception("Bier");

// Deprecation of static calls on non static methods
class foo {
    function bar() {
        echo 'I am not static!';
    }
}

foo::bar();

mcrypt_cbc("asd");

$bar = &new foo();

Fieldtest ViisonDPDAdapter (PHP 5.3)

As expected, there are a few things off with ViisonCommon and 5.3, but far more interesting is that there are incompatibilities in ViisonShippingCommon

➜ thomas-mbp:php-compatibility ./vendor/bin/phpcs --standard=PHPCompatibility --runtime-set testVersion 5.3 ~/projects/ShopwareDPDAdapter

FILE: /Users/thomasnordquist/projects/ShopwareDPDAdapter/Views/backend/_resources/viison_dpd/css/dpd-icon-set.css
-----------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------
 1 | WARNING | File appears to be minified and cannot be processed
-----------------------------------------------------------------------------------------------------------------


FILE: /Users/thomasnordquist/projects/ShopwareDPDAdapter/ViisonCommon/Classes/ApiException.php
----------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------
 30 | ERROR | The built-in interface Throwable is not present in PHP version 5.6 or earlier
----------------------------------------------------------------------------------------------


FILE: ...rs/thomasnordquist/projects/ShopwareDPDAdapter/ViisonCommon/Classes/Installation/AclResource/AclResourceCreator.php
-------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------
 79 | ERROR | Closures / anonymous functions did not have access to $this in PHP 5.3 or earlier
-------------------------------------------------------------------------------------------------------------------------


FILE: /Users/thomasnordquist/projects/ShopwareDPDAdapter/ViisonCommon/Classes/Installation/Menu/InstallationHelper.php
----------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------
 37 | ERROR | "callable" keyword is not present in PHP version 5.3 or earlier
 37 | ERROR | 'callable' type declaration is not present in PHP version 5.3 or earlier
----------------------------------------------------------------------------------------------------------------------


FILE: /Users/thomasnordquist/projects/ShopwareDPDAdapter/ViisonCommon/Classes/Installation/Schema/SchemaCreator.php
-------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------
 42 | ERROR | Closures / anonymous functions did not have access to $this in PHP 5.3 or earlier
-------------------------------------------------------------------------------------------------------------------


FILE: /Users/thomasnordquist/projects/ShopwareDPDAdapter/ViisonCommon/Classes/Subscribers/HideDocumentTypeSubscriber.php
------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------------
 68 | ERROR | Closures / anonymous functions did not have access to $this in PHP 5.3 or earlier
------------------------------------------------------------------------------------------------------------------------


FILE: /Users/thomasnordquist/projects/ShopwareDPDAdapter/ViisonCommon/Classes/Subscribers/ViewLoading.php
---------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------
 96 | ERROR | Closures / anonymous functions did not have access to $this in PHP 5.3 or earlier
---------------------------------------------------------------------------------------------------------


FILE: /Users/thomasnordquist/projects/ShopwareDPDAdapter/ViisonCommon/Classes/TryWithFinally.php
------------------------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 1 LINE
------------------------------------------------------------------------------------------------
 16 | ERROR | "callable" keyword is not present in PHP version 5.3 or earlier
 16 | ERROR | 'callable' type declaration is not present in PHP version 5.3 or earlier
 16 | ERROR | "callable" keyword is not present in PHP version 5.3 or earlier
 16 | ERROR | 'callable' type declaration is not present in PHP version 5.3 or earlier
------------------------------------------------------------------------------------------------


FILE: .../thomasnordquist/projects/ShopwareDPDAdapter/ViisonShippingCommon/Controllers/Backend/ViisonShippingCommonOrder.php
-------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------
 1298 | ERROR | The function array_column() is not present in PHP version 5.4 or earlier
-------------------------------------------------------------------------------------------------------------------------


FILE: /Users/thomasnordquist/projects/ShopwareDPDAdapter/ViisonShippingCommon/Util.php
-------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
-------------------------------------------------------------------------------------------------------------
 617 | ERROR | The function openssl_encrypt() does not have a parameter "iv" in PHP version 5.3.2 or earlier
 637 | ERROR | The function openssl_decrypt() does not have a parameter "iv" in PHP version 5.3.2 or earlier
-------------------------------------------------------------------------------------------------------------

Time: 5.79 secs; Memory: 33Mb

@svenmuennich svenmuennich added the enhancement Issues that describe new features or improvements to existing features. label Feb 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues that describe new features or improvements to existing features.
Projects
None yet
Development

No branches or pull requests

4 participants