Improve PHPCS configuration

[swissspidy]

Task Description

We discussed potentially enabling many more rules from https://github.com/slevomat/coding-standard

Let's go through them to see which ones are useful for the project

[spacedmonkey]

https://github.com/slevomat/coding-standard/blob/master/doc/variables.md#slevomatcodingstandardvariablesunusedvariable

https://github.com/slevomat/coding-standard/blob/master/doc/variables.md#slevomatcodingstandardvariablesuselessvariable-

https://github.com/slevomat/coding-standard/blob/master/doc/namespaces.md#slevomatcodingstandardnamespacesreferenceusednamesonly-

https://github.com/slevomat/coding-standard/blob/master/doc/namespaces.md#slevomatcodingstandardnamespacesuselessalias-

https://github.com/slevomat/coding-standard/blob/master/doc/namespaces.md#slevomatcodingstandardnamespacesunuseduses-

https://github.com/slevomat/coding-standard/blob/master/doc/namespaces.md#slevomatcodingstandardnamespacesusedoesnotstartwithbackslash-

https://github.com/slevomat/coding-standard/blob/master/doc/classes.md#slevomatcodingstandardclassesclassconstantvisibility-

https://github.com/slevomat/coding-standard/blob/master/doc/complexity.md#slevomatcodingstandardcomplexitycognitive

https://github.com/slevomat/coding-standard/blob/master/doc/php.md#slevomatcodingstandardphpshortlist-

https://github.com/slevomat/coding-standard/blob/master/doc/php.md#slevomatcodingstandardphprequirenowdoc-

https://github.com/slevomat/coding-standard/blob/master/doc/operators.md#slevomatcodingstandardoperatorsspreadoperatorspacing-

The following are some of the rules I would look at implementing.

Some good examples to follow

https://github.com/wearerequired/coding-standards/blob/master/Required/ruleset.xml

[spacedmonkey]

Also for reference. #12760

[swissspidy]

Soo many rules!

From the above we already use a few, so that's good.

The cognitive complexity rule definitely needs tweaking

There's even more I find useful. For example:

• SlevomatCodingStandard.Classes.ClassStructure - Checks that class/trait/interface members are in the correct order.
• SlevomatCodingStandard.Files.LineLength - Enforces maximum length of a single line of code.
  SlevomatCodingStandard.Functions.StaticClosure - Reports closures not using $this that are not declared static.
• SlevomatCodingStandard.TypeHints.ParameterTypeHint
  Checks for missing parameter typehints in case they can be declared natively. This sniff causes errors when overriding or implementing parent methods from a WP class which does not have typehints, like WP_Widget or WP_REST_Controller. So for these cases we need lots of overrides.
• SlevomatCodingStandard.Numbers.RequireNumericLiteralSeparator - Requires use of numeric literal separators.

I have some WIP config locally including a mix of these rules, will refine it in the new year.

[spacedmonkey]

SlevomatCodingStandard.Files.LineLength

I have tried to implement this rule, but it resulted in many many false positive and added more overhead than value. I would recommend not using this lint rule.

[spacedmonkey]

SlevomatCodingStandard.Numbers.RequireNumericLiteralSeparator - Requires use of numeric literal separators.

#12911

[spacedmonkey]

SlevomatCodingStandard.Classes.ClassStructure - Checks that class/trait/interface members are in the correct order

I really like this rule. I have put together a demo PR - #12912

[spacedmonkey]

Complexity Cognitive seems very similar to Cyclomatic Complexity.

After setting up the rule, there are lots of places where the rule would have to be ignored. Examples of code that is marked as complex.

• prepare_item_for_response - 28
• safecss_filter_attr - 53
• ensure_required_markup - 44

The lint has some use. If you set the value to say 25. This results in the following lints errors.

FILE: includes/REST_API/Embed_Controller.php
--------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------------------------------------
 258 | ERROR | Cognitive complexity for "url_to_post" is 36 but has to be less than or equal to 25.
     |       | (SlevomatCodingStandard.Complexity.Cognitive.ComplexityTooHigh)
--------------------------------------------------------------------------------------------------------------------------------------------------------------------


FILE: includes/KSES.php
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 257 | ERROR | Cognitive complexity for "safecss_filter_attr" is 53 but has to be less than or equal to 25.
     |       | (SlevomatCodingStandard.Complexity.Cognitive.ComplexityTooHigh)
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------


FILE: includes/REST_API/Products_Controller.php
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 270 | ERROR | Cognitive complexity for "prepare_item_for_response" is 26 but has to be less than or equal to 25.
     |       | (SlevomatCodingStandard.Complexity.Cognitive.ComplexityTooHigh)
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------


FILE: includes/AMP/Sanitization.php
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 106 | ERROR | Cognitive complexity for "ensure_required_markup" is 44 but has to be less than or equal to 25.
     |       | (SlevomatCodingStandard.Complexity.Cognitive.ComplexityTooHigh)
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------


FILE: includes/REST_API/Hotlinking_Controller.php
---------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------------------------------------------------------------
 624 | ERROR | Cognitive complexity for "validate_url" is 34 but has to be less than or equal to 25.
     |       | (SlevomatCodingStandard.Complexity.Cognitive.ComplexityTooHigh)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------


FILE: includes/Admin/Cross_Origin_Isolation.php
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------
 177 | ERROR | Cognitive complexity for "replace_in_dom" is 27 but has to be less than or equal to 25.
     |       | (SlevomatCodingStandard.Complexity.Cognitive.ComplexityTooHigh)
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------


FILE: includes/REST_API/Stories_Controller.php
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 83 | ERROR | Cognitive complexity for "prepare_item_for_response" is 28 but has to be less than or equal to 25.
    |       | (SlevomatCodingStandard.Complexity.Cognitive.ComplexityTooHigh)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Time: 6.89 secs; Memory: 20MB

These are seems valid, as they are all complex.

[swissspidy]

Complexity Cognitive seems very similar to Cyclomatic Complexity.

Yes, they're similar.

After setting up the rule, there are lots of places where the rule would have to be ignored. Examples of code that is marked as complex.

Yeah I noticed the same in my WIP config I mentioned further above.

[spacedmonkey]

I think ticket is good to be closed now.

[spacedmonkey]

My bad, there is still #12911 and #12760 need to be merged.