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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build/PHPCS: update/improve the PHPCS configuration #1975

Merged
merged 6 commits into from
Sep 5, 2018
Merged

Build/PHPCS: update/improve the PHPCS configuration #1975

merged 6 commits into from
Sep 5, 2018

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Sep 5, 2018

馃憠 IMPORTANT: there seems to be something seriously wrong with the Travis configuration of the repo which prevents the PHPCS check from running.
If/when that issue is fixed, the build will start to fail as the current codebase contains numerous violations against the ruleset used.

馃憠 I would strongly recommend for the ruleset to be updated with the recommended custom properties as outlined on the WPCS Customizable sniff properties wiki page.

Commit summary

Build/PHPCS: rename ruleset

If the PHPCS ruleset is named .phpcs.xml, phpcs.xml, .phpcs.xml.dist or phpcs.xml.dist, it will automatically be picked up by PHPCS and you don't need to pass the ruleset name anymore.

PHPCS ruleset: fix up the ruleset

  • Update the ruleset name and description to be more accurate.
  • Update the WP ruleset inclusion inline doc + xml element.

Build/PHPCS: move command line arguments to the ruleset

... and clean up the command in the Travis script.

This will also ensure that devs will run the same command without having to remember the exact settings.

Build/PHPCS: clean up the file paths PHPCS shows

This will strip the paths down to the relative paths from the project root directory instead of showing the full file paths.

Build/PHPCS: don't ignore warnings

... but don't fail Travis builds on them either.

The -n flag will hide warnings, the -runtime-set ignore_warnings_on_exit setting will show warnings, but will prevent a Travis build from failing when only warnings are found.

Build/PHPCS: move file directive to the ruleset

Includes updating the documentation about the PHPCS run.

Summary of violations detected based on the codebase as it is/was at the time of submitting this PR:

----------------------------------------------------------------------------
A TOTAL OF 81 ERRORS AND 117 WARNINGS WERE FOUND IN 37 FILES
----------------------------------------------------------------------------

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
------------------------------------------------------------------------------------------
    STANDARD  CATEGORY            SNIFF                                              COUNT
------------------------------------------------------------------------------------------
[x] WordPres  Arrays              Multiple statement alignment double arrow not ali  64
[x] PEAR      Functions           Function call signature multiple arguments         32
[ ] Squiz     PHP                 Commented out code found                           20
[x] PEAR      Functions           Function call signature content after open bracke  15
[x] PEAR      Functions           Function call signature close bracket line         14
[ ] WordPres  PHP                 Strict comparisons loose comparison                9
[x] Generic   Formatting          Multiple statement alignment not same warning      6
[ ] WordPres  PHP                 Strict in array missing true strict                6
[ ] WordPres  WP                  Enqueued resource parameters not in footer         6
[ ] WordPres  WP                  Enqueued resource parameters missing version       3
[x] WordPres  White space         Disallow inline tabs non indent tabs used          3
[x] Generic   Formatting          Multiple statement alignment incorrect warning     2
[ ] Squiz     Commenting          Function comment missing                           2
[x] Squiz     Commenting          Function comment spacing after param type          2
[x] WordPres  Arrays              Array declaration spacing associative array found  2
[x] WordPres  Arrays              Comma after array item no comma                    2
[ ] Generic   Classes             Duplicate class name found                         1
[ ] Generic   Files               One class per file multiple found                  1
[ ] Generic   Strings             Unnecessary string concat found                    1
[ ] Squiz     Commenting          File comment missing                               1
[ ] Squiz     Commenting          File comment spacing after comment                 1
[ ] Squiz     Commenting          Function comment missing param tag                 1
[ ] Squiz     Commenting          Inline comment invalid end char                    1
[x] Squiz     Strings             Concatenation spacing padding found                1
[ ] WordPres  DB                  Direct database query direct query                 1
[ ] WordPres  WP                  Alternative functions json_encode_json_encode      1
------------------------------------------------------------------------------------------
A TOTAL OF 198 SNIFF VIOLATIONS WERE FOUND IN 26 SOURCES
------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 11 MARKED SOURCES AUTOMATICALLY (143 VIOLATIONS IN TOTAL)
------------------------------------------------------------------------------------------

@codecov
Copy link

codecov bot commented Sep 5, 2018

Codecov Report

Merging #1975 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             develop   #1975   +/-   ##
=========================================
  Coverage       5.38%   5.38%           
  Complexity      1802    1802           
=========================================
  Files            143     143           
  Lines           6257    6257           
=========================================
  Hits             337     337           
  Misses          5920    5920

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update 27c384e...a76f0f6. Read the comment docs.

If the PHPCS ruleset is named `.phpcs.xml`, `phpcs.xml`, `.phpcs.xml.dist` or `phpcs.xml.dist`, it will automatically be picked up by PHPCS and you don't need to pass the ruleset name anymore.

Additionally, using a `.dist` file for a repo ruleset allows for individual developers to overrule the ruleset with a custom version (without the `.dist` file extension).
This makes testing of new additions/changes to the ruleset easier.
The master ruleset can be imported into a custom ruleset by using `<rule ref="./.phpcs.xml.dist"/>`.

Since PHPCS 3.1.0, it is also possible to use dot-prefixed files for the PHPCS config, allowing these files to be sorted with other configuration related files.

The loading order of the ruleset files in PHPCS, as of version 3.1.1., is:
1. `.phpcs.xml`
2. `phpcs.xml`
3. `.phpcs.xml.dist`
4. `phpcs.xml.dist`

References:
* https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage#using-a-default-configuration-file
* https://github.com/squizlabs/PHP_CodeSniffer/releases/tag/3.1.0
* https://github.com/squizlabs/PHP_CodeSniffer/releases/tag/3.1.1 (change in the file loading order)
* Update the ruleset name and description to be more accurate.
* Update the WP ruleset inclusion inline doc + xml element.
... and clean up the command in the Travis script.

This will also ensure that devs will run the same command without having to remember the exact settings.
This will strip the paths down to the relative paths from the project root directory instead of showing the full file paths.
... but don't fail Travis builds on them either.

The `-n` flag will **hide** warnings, the `-runtime-set ignore_warnings_on_exit` setting will **show** warnings, but will prevent a Travis build from failing when only warnings are found.
Includes updating the documentation about the PHPCS run.

### Summary of violations detected:
```
----------------------------------------------------------------------------
A TOTAL OF 81 ERRORS AND 117 WARNINGS WERE FOUND IN 37 FILES
----------------------------------------------------------------------------

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
------------------------------------------------------------------------------------------
    STANDARD  CATEGORY            SNIFF                                              COUNT
------------------------------------------------------------------------------------------
[x] WordPres  Arrays              Multiple statement alignment double arrow not ali  64
[x] PEAR      Functions           Function call signature multiple arguments         32
[ ] Squiz     PHP                 Commented out code found                           20
[x] PEAR      Functions           Function call signature content after open bracke  15
[x] PEAR      Functions           Function call signature close bracket line         14
[ ] WordPres  PHP                 Strict comparisons loose comparison                9
[x] Generic   Formatting          Multiple statement alignment not same warning      6
[ ] WordPres  PHP                 Strict in array missing true strict                6
[ ] WordPres  WP                  Enqueued resource parameters not in footer         6
[ ] WordPres  WP                  Enqueued resource parameters missing version       3
[x] WordPres  White space         Disallow inline tabs non indent tabs used          3
[x] Generic   Formatting          Multiple statement alignment incorrect warning     2
[ ] Squiz     Commenting          Function comment missing                           2
[x] Squiz     Commenting          Function comment spacing after param type          2
[x] WordPres  Arrays              Array declaration spacing associative array found  2
[x] WordPres  Arrays              Comma after array item no comma                    2
[ ] Generic   Classes             Duplicate class name found                         1
[ ] Generic   Files               One class per file multiple found                  1
[ ] Generic   Strings             Unnecessary string concat found                    1
[ ] Squiz     Commenting          File comment missing                               1
[ ] Squiz     Commenting          File comment spacing after comment                 1
[ ] Squiz     Commenting          Function comment missing param tag                 1
[ ] Squiz     Commenting          Inline comment invalid end char                    1
[x] Squiz     Strings             Concatenation spacing padding found                1
[ ] WordPres  DB                  Direct database query direct query                 1
[ ] WordPres  WP                  Alternative functions json_encode_json_encode      1
------------------------------------------------------------------------------------------
A TOTAL OF 198 SNIFF VIOLATIONS WERE FOUND IN 26 SOURCES
------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 11 MARKED SOURCES AUTOMATICALLY (143 VIOLATIONS IN TOTAL)
------------------------------------------------------------------------------------------
```
@aristath aristath merged commit a76f0f6 into themeum:develop Sep 5, 2018
@aristath
Copy link
Contributor

aristath commented Sep 5, 2018

@jrfnl Thank you very much for both this one and #1974
Both were merged and I'm proceeding to fix any issues in the files. 鉂わ笍

@jrfnl jrfnl deleted the feature/rename-phpcs-ruleset branch September 5, 2018 18:30
@jrfnl
Copy link
Contributor Author

jrfnl commented Sep 5, 2018

@aristath You're welcome and I hope both WPCS as well as PHPCompatibility will serve you well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants