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

Upgrade slevomat/coding-standard to v6 #144

Merged
merged 8 commits into from
Dec 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ install: travis_retry composer update --prefer-dist

script:
- vendor/bin/phpcs
- vendor/bin/phpcs $(find tests/input/* | sort) --report=summary --report-file=phpcs.log; diff tests/expected_report.txt phpcs.log
- make test-report

stages:
- Validate against schema
Expand All @@ -41,6 +41,8 @@ jobs:
- xmllint --noout --schema vendor/squizlabs/php_codesniffer/phpcs.xsd lib/Doctrine/ruleset.xml

- stage: Apply fixes
before_script:
- cp -R tests/input/ tests/input2/
script: vendor/bin/phpcbf tests/input2; [ $? -ne 2 ] && diff tests/input2 tests/fixed
script: make test-fix

- stage: Apply fixes
php: 7.4snapshot
script: make test-fix
26 changes: 26 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
.PHONY: test test-report test-fix update-compatibility-patch

PHP_74_OR_NEWER=`php -r "echo (int) version_compare(PHP_VERSION, '7.4', '>=');"`

test: test-report test-fix

test-report: vendor
@if [ $(PHP_74_OR_NEWER) -eq 1 ]; then git apply tests/php-compatibility.patch; fi
@vendor/bin/phpcs `find tests/input/* | sort` --report=summary --report-file=phpcs.log; diff tests/expected_report.txt phpcs.log; if [ $$? -ne 0 ] && [ $(PHP_74_OR_NEWER) -eq 1 ]; then git apply -R tests/php-compatibility.patch; exit 1; fi
@if [ $(PHP_74_OR_NEWER) -eq 1 ]; then git apply -R tests/php-compatibility.patch; fi

test-fix: vendor
@if [ $(PHP_74_OR_NEWER) -eq 1 ]; then git apply tests/php-compatibility.patch; fi
@cp -R tests/input/ tests/input2/
@vendor/bin/phpcbf tests/input2; diff tests/input2 tests/fixed; if [ $$? -ne 0 ]; then rm -rf tests/input2/ && if [ $(PHP_74_OR_NEWER) -eq 1 ]; then git apply -R tests/php-compatibility.patch; fi; exit 1; fi
@rm -rf tests/input2/ && if [ $(PHP_74_OR_NEWER) -eq 1 ]; then git apply -R tests/php-compatibility.patch; fi

update-compatibility-patch:
@git apply tests/php-compatibility.patch
@echo -e "Please open your editor and apply your changes\n"
@until [ "$${compatibility_resolved}" == "y" ]; do read -p "Have finished your changes (y|n)? " compatibility_resolved; done && compatibility_resolved=
@git diff -- tests/expected_report.txt tests/fixed > .tmp-patch && mv .tmp-patch tests/php-compatibility.patch && git apply -R tests/php-compatibility.patch
@git commit -m 'Update compatibility patch' tests/php-compatibility.patch

vendor: composer.json
composer update
4 changes: 0 additions & 4 deletions bin/test.sh

This file was deleted.

4 changes: 2 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@
"require": {
"php": "^7.2",
"dealerdirect/phpcodesniffer-composer-installer": "^0.5.0",
"slevomat/coding-standard": "dev-master#469368cc6d8fe83aceba259ef65f1c0a2f63055b",
"squizlabs/php_codesniffer": "^3.5.0"
"slevomat/coding-standard": "^6.0",
"squizlabs/php_codesniffer": "^3.5.3"
},
"config": {
"sort-packages": true
Expand Down
18 changes: 16 additions & 2 deletions docs/en/reference/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,22 @@ Testing
=======

If you are contributing to the Doctrine Coding Standard and want to test your contribution, you just
need to execute PHP_CodeSniffer with the tests folder and ensure it matches the expected report:
need to execute the ``test`` GNU make target:

.. code-block:: bash

$ ./vendor/bin/phpcs tests/input --report=summary --report-file=phpcs.log; diff tests/expected_report.txt phpcs.log
$ make test

Sometimes you are enabling a sniff that enforces some features for a specific PHP version, that will
probably cause our build pipeline to fail when running on such versions.

We have created the ``update-compatibility-patch`` GNU make target to help you with that:

.. code-block:: bash

$ make update-compatibility-patch

That target will apply the latest version of our compatibility patch and ask you to adjust the code in ``tests/fixed`` and
``tests/expected_report.txt`` using the editor of your preference.
Once you are done, please type "y" to resume the command, which will update the compatibility patch and create a commit
with your changes for you to push to your remote branch.
45 changes: 41 additions & 4 deletions lib/Doctrine/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,10 @@
<!-- Forbid assignments in conditions -->
<rule ref="SlevomatCodingStandard.ControlStructures.AssignmentInCondition"/>
<!-- Require consistent spacing for block structures -->
<rule ref="SlevomatCodingStandard.ControlStructures.BlockControlStructureSpacing"/>
<rule ref="SlevomatCodingStandard.ControlStructures.BlockControlStructureSpacing">
<exclude name="SlevomatCodingStandard.ControlStructures.BlockControlStructureSpacing.IncorrectLinesCountBeforeControlStructure" />
<exclude name="SlevomatCodingStandard.ControlStructures.BlockControlStructureSpacing.IncorrectLinesCountBeforeFirstControlStructure" />
</rule>
<!-- Forbid fancy yoda conditions -->
<rule ref="SlevomatCodingStandard.ControlStructures.DisallowYodaComparison"/>
<!-- Require usage of early exit -->
Expand Down Expand Up @@ -334,17 +337,51 @@
<!-- Require types to be written as natively if possible;
require iterable types to specify phpDoc with their content;
forbid useless/duplicated information in phpDoc -->
<rule ref="SlevomatCodingStandard.TypeHints.TypeHintDeclaration">
<rule ref="SlevomatCodingStandard.TypeHints.ParameterTypeHint">
<properties>
<property name="traversableTypeHints" type="array">
<element value="Traversable"/>
<element value="Iterator"/>
<element value="IteratorAggregate"/>
<element value="Doctrine\Common\Collections\Collection"/>
</property>
</properties>
</rule>
<rule ref="SlevomatCodingStandard.TypeHints.PropertyTypeHint">
<properties>
<property name="allAnnotationsAreUseful" value="true"/>
<property name="enableEachParameterAndReturnInspection" value="true"/>
<property name="traversableTypeHints" type="array">
<element value="Traversable"/>
<element value="Iterator"/>
<element value="IteratorAggregate"/>
<element value="Doctrine\Common\Collections\Collection"/>
</property>
</properties>
</rule>
<rule ref="SlevomatCodingStandard.TypeHints.ReturnTypeHint">
<properties>
<property name="traversableTypeHints" type="array">
<element value="Traversable"/>
<element value="Iterator"/>
<element value="IteratorAggregate"/>
<element value="Doctrine\Common\Collections\Collection"/>
</property>
</properties>
</rule>
<!-- Forbid useless @var for constants -->
<rule ref="SlevomatCodingStandard.TypeHints.UselessConstantTypeHint"/>
<!-- Forbid useless phpDocs for functions -->
<rule ref="SlevomatCodingStandard.Commenting.UselessFunctionDocComment">
<properties>
<property name="traversableTypeHints" type="array">
<element value="Traversable"/>
<element value="Iterator"/>
<element value="IteratorAggregate"/>
<element value="Doctrine\Common\Collections\Collection"/>
</property>
</properties>
</rule>
<!-- Forbid useless inherit doc comment -->
<rule ref="SlevomatCodingStandard.Commenting.UselessInheritDocComment"/>
<!-- Forbid duplicated variables assignments -->
<rule ref="SlevomatCodingStandard.Variables.DuplicateAssignmentToVariable"/>
<!-- Forbid useless variables -->
Expand Down
7 changes: 4 additions & 3 deletions tests/expected_report.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ tests/input/constants-var.php 4 0
tests/input/doc-comment-spacing.php 10 0
tests/input/duplicate-assignment-variable.php 1 0
tests/input/EarlyReturn.php 6 0
tests/input/example-class.php 48 0
tests/input/example-class.php 45 0
tests/input/forbidden-comments.php 8 0
tests/input/forbidden-functions.php 6 0
tests/input/inline_type_hint_assertions.php 7 0
Expand All @@ -32,14 +32,15 @@ tests/input/superfluous-naming.php 11 0
tests/input/test-case.php 8 0
tests/input/trailing_comma_on_array.php 1 0
tests/input/traits-uses.php 11 0
tests/input/type-hints.php 4 0
tests/input/UnusedVariables.php 1 0
tests/input/use-ordering.php 1 0
tests/input/useless-semicolon.php 2 0
tests/input/UselessConditions.php 20 0
----------------------------------------------------------------------
A TOTAL OF 289 ERRORS AND 0 WARNINGS WERE FOUND IN 33 FILES
A TOTAL OF 290 ERRORS AND 0 WARNINGS WERE FOUND IN 34 FILES
----------------------------------------------------------------------
PHPCBF CAN FIX 232 OF THESE SNIFF VIOLATIONS AUTOMATICALLY
PHPCBF CAN FIX 229 OF THESE SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


1 change: 1 addition & 0 deletions tests/fixed/example-class.php
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ public function throwWhenInvalid() : void
public function trySwitchSpace() : void
{
try {
$var = 1;
switch (self::VERSION) {
default:
}
Expand Down
24 changes: 24 additions & 0 deletions tests/fixed/type-hints.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

declare(strict_types=1);

namespace TypeHints;

use Iterator;
use Traversable;

class TraversableTypeHints
{
/** @var Traversable */
private $parameter;

/**
* @param Iterator $iterator
*
* @return Traversable
alcaeus marked this conversation as resolved.
Show resolved Hide resolved
*/
public function get(Iterator $iterator) : Traversable
{
return $this->parameter;
}
}
1 change: 1 addition & 0 deletions tests/input/example-class.php
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ public function throwWhenInvalid() : void
public function trySwitchSpace() : void
{
try {
$var = 1;
Copy link
Member

Choose a reason for hiding this comment

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

This kinda snuck in: not a problem for me as long as it doesn't remove newlines before control structures, but seems unrelated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just ensured the settings for space before block control structures is not modified from current behaviour. The test for it was missing.

switch (self::VERSION) {
default:
}
Expand Down
24 changes: 24 additions & 0 deletions tests/input/type-hints.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

declare(strict_types=1);

namespace TypeHints;

use Iterator;
use Traversable;

class TraversableTypeHints
{
/** @var Traversable */
private $parameter;

/**
* @param Iterator $iterator
*
* @return Traversable
*/
public function get(Iterator $iterator) : Traversable
{
return $this->parameter;
}
}
67 changes: 67 additions & 0 deletions tests/php-compatibility.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
diff --git a/tests/expected_report.txt b/tests/expected_report.txt
index 855edfd..5653a39 100644
--- a/tests/expected_report.txt
+++ b/tests/expected_report.txt
@@ -12,7 +12,7 @@ tests/input/constants-var.php 4 0
tests/input/doc-comment-spacing.php 10 0
tests/input/duplicate-assignment-variable.php 1 0
tests/input/EarlyReturn.php 6 0
-tests/input/example-class.php 45 0
+tests/input/example-class.php 48 0
tests/input/forbidden-comments.php 8 0
tests/input/forbidden-functions.php 6 0
tests/input/inline_type_hint_assertions.php 7 0
@@ -32,15 +32,15 @@ tests/input/superfluous-naming.php 11 0
tests/input/test-case.php 8 0
tests/input/trailing_comma_on_array.php 1 0
tests/input/traits-uses.php 11 0
-tests/input/type-hints.php 4 0
+tests/input/type-hints.php 5 0
tests/input/UnusedVariables.php 1 0
tests/input/use-ordering.php 1 0
tests/input/useless-semicolon.php 2 0
tests/input/UselessConditions.php 20 0
----------------------------------------------------------------------
-A TOTAL OF 290 ERRORS AND 0 WARNINGS WERE FOUND IN 34 FILES
+A TOTAL OF 294 ERRORS AND 0 WARNINGS WERE FOUND IN 34 FILES
----------------------------------------------------------------------
-PHPCBF CAN FIX 229 OF THESE SNIFF VIOLATIONS AUTOMATICALLY
+PHPCBF CAN FIX 233 OF THESE SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


diff --git a/tests/fixed/example-class.php b/tests/fixed/example-class.php
index 195d963..a2ee5c8 100644
--- a/tests/fixed/example-class.php
+++ b/tests/fixed/example-class.php
@@ -24,14 +24,12 @@ class Example implements IteratorAggregate
{
private const VERSION = PHP_VERSION - (PHP_MINOR_VERSION * 100) - PHP_PATCH_VERSION;

- /** @var int|null */
- private $foo;
+ private ?int $foo = null;

/** @var string[] */
- private $bar;
+ private array $bar;

- /** @var bool */
- private $baz;
+ private bool $baz;

/** @var ControlStructureSniff|int|string|null */
private $baxBax;
diff --git a/tests/fixed/type-hints.php b/tests/fixed/type-hints.php
index a1b1827..fb7d406 100644
--- a/tests/fixed/type-hints.php
+++ b/tests/fixed/type-hints.php
@@ -10,7 +10,7 @@ use Traversable;
class TraversableTypeHints
{
/** @var Traversable */
- private $parameter;
+ private Traversable $parameter;

/**
* @param Iterator $iterator