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

Add a PHPCompatibility ruleset to PHPCS #1651

Closed
wants to merge 3 commits into from
Closed

Conversation

Luc45
Copy link
Contributor

@Luc45 Luc45 commented May 1, 2020

This PR adds a PHPCompatibility ruleset to the existing PHPCS rules.

It is configured to check compatibility with PHP 5.6+.

For instance, if there's a: public function foo(): void in the code being checked by PHPCS, it will come back with: void return type is not present in PHP version 7.0 or earlier

It will also warn about deprecations. For instance, using create_function() gives me: create_function() is deprecated as of PHP 7.2, please use full fledged functions or anonymous functions instead.

The compatibility coverage is not 100%, but close to 95%. You can see the 5% it doesn't cover yet, here:

Please, do not merge yet, I'll push some commits to this PR to test that PHPCompatibility is working alright. Done.

@Luc45 Luc45 added code review Status: requires a code review. hold Status: on hold–do not proceed with other status items. needs changelog Needs a changelog entry before merging. labels May 1, 2020
@Luc45 Luc45 requested a review from sc0ttkclark as a code owner May 1, 2020 00:21
@Luc45 Luc45 self-assigned this May 1, 2020
Copy link
Contributor

@tr1b0t tr1b0t left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 17 error(s)
⚠️ 2 warning(s)

Temporary_PHPCompatibility_Test.php Outdated Show resolved Hide resolved

// Should tell me return void is not available in PHP 5.6
class Foo {
public function do_nothing(): void {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 You must use "/**" style comments for a class comment

hash: 3fda944041338d326807f99503c2d731

// Should tell me return void is not available in PHP 5.6
class Foo {
public function do_nothing(): void {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 🚫 Missing doc comment for function do_nothing()
  • 🚫 void return type is not present in PHP version 7.0 or earlier

hash: 3d3b3964061b197aba8324456a398b48

}

// Should tell me anonymous class are not available in PHP 5.6
$a = new class {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 Inline comments must end in full-stops, exclamation marks, or question marks

hash: f0e90559592e54bbe82db68036616ab0


// Should tell me anonymous class are not available in PHP 5.6
$a = new class {}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 🚫 Anonymous classes are not supported in PHP 5.6 or earlier
  • 🚫 Parenthesis should always be used when instantiating a new object.

hash: 36fa514a1d31a8bbfe4ec88f51f1d1e9

$a = new class {}

// Should tell me "create_function()" has been deprecated on PHP 7.2
create_function('', '');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 Inline comments must end in full-stops, exclamation marks, or question marks

hash: 1b23e12727e46bff82706f1484d9b863

$a = new class {}

// Should tell me "create_function()" has been deprecated on PHP 7.2
create_function('', ''); No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • ⚠️ create_function() is highly discouraged, as it can execute arbritary code (additionally, it's deprecated as of PHP 7.2): https://wpvip.com/documentation/vip-go/code-review-blockers-warnings-notices/#eval-and-create_function. )
  • ⚠️ Function create_function() is deprecated since PHP 7.2; Use an anonymous function instead
  • 🚫 PHP syntax error: syntax error, unexpected 'create_function' (T_STRING)
  • 🚫 create_function() is deprecated as of PHP 7.2, please use full fledged functions or anonymous functions instead.
  • 🚫 Expected 1 spaces after opening parenthesis; 0 found
  • 🚫 Expected 1 spaces before closing parenthesis; 0 found
  • 🚫 Function create_function() has been deprecated
  • 🚫 There must be a single space after the first parenthesis
  • 🚫 There must be a single space before the last parenthesis
  • 🚫 File must end with a newline character

hash: 5768da684300b0e405a37232e76f4b56

@bordoni
Copy link
Member

bordoni commented May 1, 2020

We might need to look into modifications to https://github.com/moderntribe/tribalscents as well.

@Luc45
Copy link
Contributor Author

Luc45 commented May 4, 2020

We might need to look into modifications to https://github.com/moderntribe/tribalscents as well.

Hey Gustavo, what kind of modifications? Like, porting this to tribalscents?

Copy link
Contributor

@sc0ttkclark sc0ttkclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's explore putting this into tribalscents so it can apply to multiple repos more easily.

@sc0ttkclark sc0ttkclark marked this pull request as draft June 6, 2020 17:14
@sc0ttkclark sc0ttkclark added needs ticket Needs an associated Jira ticket before merging. and removed code review Status: requires a code review. needs changelog Needs a changelog entry before merging. labels Jun 6, 2020
@bordoni bordoni closed this Jun 19, 2020
@bordoni bordoni deleted the phpcompatibility branch June 19, 2020 03:33
@@ -10,6 +10,9 @@
</rule>
<rule ref="TribalScents"/>

<rule ref="PHPCompatibility"/>
Copy link

@jrfnl jrfnl Jul 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<rule ref="PHPCompatibility"/>
<rule ref="PHPCompatibilityWP"/>

(Otherwise you still won't benefit from the PHPCompatibilityWP ruleset taking polyfilled PHP features in WP into account)

@jrfnl
Copy link

jrfnl commented Jul 11, 2020

The compatibility coverage is not 100%, but close to 95%. You can see the 5% it doesn't cover yet

Actually, there are also things which static analysis just can't detect which aren't covered, but we try.

@Luc45
Copy link
Contributor Author

Luc45 commented Jul 14, 2020

Thanks a lot for dropping by, @jrfnl! I'm no longer involved in the development of this project.

Best regards,
Lucas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hold Status: on hold–do not proceed with other status items. needs ticket Needs an associated Jira ticket before merging.
Projects
None yet
5 participants