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.

Sign up for GitHub

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

Jump to bottom

Require constructor property promotion #283

Merged
merged 2 commits into from Aug 26, 2022
Merged

Require constructor property promotion #283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e0e1eed
Require constructor property promotion
greg0ire Jul 20, 2022
b7e1a4b
Update compatibility patch
greg0ire Jul 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/Doctrine/ruleset.xml
View file
Open in desktop
Expand Up @@ -144,6 +144,8 @@
<property name="enableMultipleSpacesBetweenModifiersCheck" value="true"/>
</properties>
</rule>
<!-- Require usage of constructor property promotion -->
<rule ref="SlevomatCodingStandard.Classes.RequireConstructorPropertyPromotion"/>
<!-- Forbid uses of multiple traits separated by comma -->
<rule ref="SlevomatCodingStandard.Classes.TraitUseDeclaration"/>
<!-- Require no spaces before trait use, between trait uses and one space after trait uses -->
Expand Down
6 changes: 3 additions & 3 deletions tests/expected_report.txt
View file
Open in desktop
Expand Up @@ -15,7 +15,7 @@ tests/input/ControlStructures.php 28 0
tests/input/doc-comment-spacing.php 11 0
tests/input/duplicate-assignment-variable.php 1 0
tests/input/EarlyReturn.php 7 0
tests/input/example-class.php 43 0
tests/input/example-class.php 44 0
tests/input/ExampleBackedEnum.php 3 0
tests/input/Exceptions.php 1 0
tests/input/forbidden-comments.php 14 0
Expand Down Expand Up @@ -50,9 +50,9 @@ tests/input/use-ordering.php 1 0
tests/input/useless-semicolon.php 2 0
tests/input/UselessConditions.php 21 0
----------------------------------------------------------------------
A TOTAL OF 428 ERRORS AND 0 WARNINGS WERE FOUND IN 46 FILES
A TOTAL OF 429 ERRORS AND 0 WARNINGS WERE FOUND IN 46 FILES
----------------------------------------------------------------------
PHPCBF CAN FIX 363 OF THESE SNIFF VIOLATIONS AUTOMATICALLY
PHPCBF CAN FIX 364 OF THESE SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


12 changes: 1 addition & 11 deletions tests/fixed/example-class.php
View file
Open in desktop
Expand Up @@ -25,20 +25,10 @@ class Example implements IteratorAggregate
{
private const VERSION = PHP_VERSION - (PHP_MINOR_VERSION * 100) - PHP_PATCH_VERSION;

private int|null $foo = null;

/** @var string[] */
Copy link
Member Author

Choose a reason for hiding this comment

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

This comment is lost. Should I report a bug about it? I've tried adding an @param phpdoc to the constructor, and it stays.

In your projects, when you use CPP, do you use @param, or do you annotate properties, like this:

public function __construct(
    /** @var list<string> */
    private array $someProperty,
   …
) {

Copy link
Member

Choose a reason for hiding this comment

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

I use the following:

/** @param list<EntitySlot> $slots */
public function __construct(
    private readonly array $slots,
) {
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Same as @morozov

Copy link
Contributor

Choose a reason for hiding this comment

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

@greg0ire I can add some options to the sniff if needed (in August).

Copy link
Member

Choose a reason for hiding this comment

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

I think, this is fine. We have other sniffs plus static analysis in place that should already complain about the missing @param annotation.

Copy link
Member Author

@greg0ire greg0ire Jul 21, 2022 •
edited

Choose a reason for hiding this comment

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

There seems to be an agreement. What should I do then? Use @param in tests/fixed/example-class.php and fix the patches so that they apply cleanly? Or just keep as is so that this disappearance stays documented?

Copy link
Member

Choose a reason for hiding this comment

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

Or just keep as is so that this disappearance stays documented?

Keeping it as it is, is fine I guess.

private array $bar;

private bool $baz;

private ControlStructureSniff|int|string|null $baxBax = null;

public function __construct(int|null $foo = null, array $bar = [], bool $baz = false, $baxBax = 'unused')
public function __construct(private int|null $foo = null, private array $bar = [], private bool $baz = false, $baxBax = 'unused')
{
$this->foo = $foo;
$this->bar = $bar;
$this->baz = $baz;
$this->baxBax = $baxBax;
}

Expand Down
84 changes: 42 additions & 42 deletions tests/php72-compatibility.patch
View file
Open in desktop
@@ -1,13 +1,13 @@
diff --git a/tests/expected_report.txt b/tests/expected_report.txt
index d53fd48..53dada5 100644
index 5110131..53dada5 100644
--- a/tests/expected_report.txt
+++ b/tests/expected_report.txt
@@ -14,26 +14,23 @@ tests/input/constants-var.php 7 0
tests/input/ControlStructures.php 28 0
tests/input/doc-comment-spacing.php 11 0
tests/input/duplicate-assignment-variable.php 1 0
-tests/input/EarlyReturn.php 7 0
-tests/input/example-class.php 43 0
-tests/input/example-class.php 44 0
-tests/input/ExampleBackedEnum.php 3 0
-tests/input/Exceptions.php 1 0
+tests/input/EarlyReturn.php 6 0
Expand Down Expand Up @@ -51,10 +51,10 @@ index d53fd48..53dada5 100644
-tests/input/UselessConditions.php 21 0
+tests/input/UselessConditions.php 20 0
----------------------------------------------------------------------
-A TOTAL OF 428 ERRORS AND 0 WARNINGS WERE FOUND IN 46 FILES
-A TOTAL OF 429 ERRORS AND 0 WARNINGS WERE FOUND IN 46 FILES
+A TOTAL OF 382 ERRORS AND 0 WARNINGS WERE FOUND IN 42 FILES
----------------------------------------------------------------------
-PHPCBF CAN FIX 363 OF THESE SNIFF VIOLATIONS AUTOMATICALLY
-PHPCBF CAN FIX 364 OF THESE SNIFF VIOLATIONS AUTOMATICALLY
+PHPCBF CAN FIX 317 OF THESE SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Expand All @@ -63,7 +63,7 @@ diff --git a/tests/fixed/ControlStructures.php b/tests/fixed/ControlStructures.p
index f8f7f65..a653086 100644
--- a/tests/fixed/ControlStructures.php
+++ b/tests/fixed/ControlStructures.php
@@ -104,7 +104,7 @@ class ControlStructures
@@ -104,7 +104,7 @@ public function spaceBelowBlocks(): void

try {
echo 4;
Expand All @@ -76,7 +76,7 @@ diff --git a/tests/fixed/EarlyReturn.php b/tests/fixed/EarlyReturn.php
index fc734db..caf1dbb 100644
--- a/tests/fixed/EarlyReturn.php
+++ b/tests/fixed/EarlyReturn.php
@@ -11,7 +11,7 @@ class EarlyReturn
@@ -11,7 +11,7 @@ public function bar(): bool
return $bar === 'bar';
}

Expand Down Expand Up @@ -117,7 +117,7 @@ diff --git a/tests/fixed/NamingCamelCase.php b/tests/fixed/NamingCamelCase.php
index 5493471..57d9f2b 100644
--- a/tests/fixed/NamingCamelCase.php
+++ b/tests/fixed/NamingCamelCase.php
@@ -6,11 +6,14 @@ namespace Example;
@@ -6,11 +6,14 @@

class NamingCamelCase
{
Expand Down Expand Up @@ -155,7 +155,7 @@ diff --git a/tests/fixed/TrailingCommaOnFunctions.php b/tests/fixed/TrailingComm
index 4248238..f3ffa91 100644
--- a/tests/fixed/TrailingCommaOnFunctions.php
+++ b/tests/fixed/TrailingCommaOnFunctions.php
@@ -15,7 +15,7 @@ class TrailingCommaOnFunctions
@@ -15,7 +15,7 @@ public function a(int $arg): void
}

public function b(
Expand All @@ -164,7 +164,7 @@ index 4248238..f3ffa91 100644
): void {
}

@@ -28,7 +28,7 @@ class TrailingCommaOnFunctions
@@ -28,7 +28,7 @@ public function uses(): void
};

$multiLine = static function (int $arg) use (
Expand All @@ -173,7 +173,7 @@ index 4248238..f3ffa91 100644
): void {
var_dump($var);
};
@@ -37,9 +37,8 @@ class TrailingCommaOnFunctions
@@ -37,9 +37,8 @@ public function uses(): void

$class = new TrailingCommaOnFunctions();

Expand All @@ -188,7 +188,7 @@ diff --git a/tests/fixed/UselessConditions.php b/tests/fixed/UselessConditions.p
index 71e0cfb..2151b17 100644
--- a/tests/fixed/UselessConditions.php
+++ b/tests/fixed/UselessConditions.php
@@ -95,7 +95,7 @@ class UselessConditions
@@ -95,7 +95,7 @@ public function necessaryIfConditionWithMethodCall(): bool
return false;
}

Expand All @@ -201,7 +201,7 @@ diff --git a/tests/fixed/arrow-functions-format.php b/tests/fixed/arrow-function
index 4da39b8..a45074f 100644
--- a/tests/fixed/arrow-functions-format.php
+++ b/tests/fixed/arrow-functions-format.php
@@ -18,10 +18,10 @@ $returningObject = static fn () => new stdClass();
@@ -18,10 +18,10 @@

$multiLineArrowFunctions = Collection::from([1, 2])
->map(
Expand All @@ -215,35 +215,35 @@ index 4da39b8..a45074f 100644

$thisIsNotAnArrowFunction = [$this->fn => 'value'];
diff --git a/tests/fixed/example-class.php b/tests/fixed/example-class.php
index 56cd902..998e51d 100644
index 7d27825..998e51d 100644
--- a/tests/fixed/example-class.php
+++ b/tests/fixed/example-class.php
@@ -25,16 +25,19 @@ class Example implements IteratorAggregate
@@ -25,17 +25,30 @@ class Example implements IteratorAggregate
{
private const VERSION = PHP_VERSION - (PHP_MINOR_VERSION * 100) - PHP_PATCH_VERSION;

- private int|null $foo = null;
- private ControlStructureSniff|int|string|null $baxBax = null;
+ /** @var int|null */
+ private $foo;

/** @var string[] */
- private array $bar;
- public function __construct(private int|null $foo = null, private array $bar = [], private bool $baz = false, $baxBax = 'unused')
+ /** @var string[] */
+ private $bar;

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

- private ControlStructureSniff|int|string|null $baxBax = null;
+
+ /** @var ControlStructureSniff|int|string|null */
+ private $baxBax;

- public function __construct(int|null $foo = null, array $bar = [], bool $baz = false, $baxBax = 'unused')
+
+ public function __construct(?int $foo = null, array $bar = [], bool $baz = false, $baxBax = 'unused')
{
$this->foo = $foo;
$this->bar = $bar;
@@ -45,7 +48,7 @@ class Example implements IteratorAggregate
+ $this->foo = $foo;
+ $this->bar = $bar;
+ $this->baz = $baz;
$this->baxBax = $baxBax;
}

/**
* Description
*/
Expand All @@ -256,7 +256,7 @@ diff --git a/tests/fixed/namespaces-spacing.php b/tests/fixed/namespaces-spacing
index 36cbae2..d42bbfe 100644
--- a/tests/fixed/namespaces-spacing.php
+++ b/tests/fixed/namespaces-spacing.php
@@ -16,5 +16,5 @@ use const DATE_RFC3339;
@@ -16,5 +16,5 @@
strrev(
(new DateTimeImmutable('@' . time(), new DateTimeZone('UTC')))
->sub(new DateInterval('P1D'))
Expand All @@ -267,7 +267,7 @@ diff --git a/tests/fixed/new_with_parentheses.php b/tests/fixed/new_with_parenth
index 47a06ec..6e81bbe 100644
--- a/tests/fixed/new_with_parentheses.php
+++ b/tests/fixed/new_with_parentheses.php
@@ -24,5 +24,5 @@ $y = [new stdClass()];
@@ -24,5 +24,5 @@

$z = new stdClass() ? new stdClass() : new stdClass();

Expand Down Expand Up @@ -301,7 +301,7 @@ diff --git a/tests/fixed/null_coalesce_operator.php b/tests/fixed/null_coalesce_
index 51c361c..8846dd1 100644
--- a/tests/fixed/null_coalesce_operator.php
+++ b/tests/fixed/null_coalesce_operator.php
@@ -4,7 +4,7 @@ declare(strict_types=1);
@@ -4,7 +4,7 @@

$foo = $_GET['foo'] ?? 'foo';

Expand Down Expand Up @@ -372,7 +372,7 @@ diff --git a/tests/fixed/return_type_on_methods.php b/tests/fixed/return_type_on
index 0c897ae..8e2c6f7 100644
--- a/tests/fixed/return_type_on_methods.php
+++ b/tests/fixed/return_type_on_methods.php
@@ -31,7 +31,7 @@ class Test
@@ -31,7 +31,7 @@ public function f(
int $c,
int $d,
int $e,
Expand All @@ -381,7 +381,7 @@ index 0c897ae..8e2c6f7 100644
): void {
}

@@ -40,7 +40,7 @@ class Test
@@ -40,7 +40,7 @@ public function g(
int $c,
int $d,
int $e,
Expand All @@ -390,7 +390,7 @@ index 0c897ae..8e2c6f7 100644
): void {
}

@@ -49,7 +49,7 @@ class Test
@@ -49,7 +49,7 @@ public function h(
int $c,
int $d,
int $e,
Expand All @@ -399,7 +399,7 @@ index 0c897ae..8e2c6f7 100644
): void {
}

@@ -58,7 +58,7 @@ class Test
@@ -58,7 +58,7 @@ public function i(
int $c,
int $d,
int $e,
Expand All @@ -408,7 +408,7 @@ index 0c897ae..8e2c6f7 100644
): void {
}

@@ -67,7 +67,7 @@ class Test
@@ -67,7 +67,7 @@ public function j(
int $c,
int $d,
int $e,
Expand All @@ -421,7 +421,7 @@ diff --git a/tests/fixed/type-hints.php b/tests/fixed/type-hints.php
index 5e26ed8..10e6f34 100644
--- a/tests/fixed/type-hints.php
+++ b/tests/fixed/type-hints.php
@@ -10,7 +10,7 @@ use Traversable;
@@ -10,7 +10,7 @@
class TraversableTypeHints
{
/** @var Traversable */
Expand All @@ -430,7 +430,7 @@ index 5e26ed8..10e6f34 100644

/**
* @param Iterator $iterator
@@ -25,5 +25,6 @@ class TraversableTypeHints
@@ -25,5 +25,6 @@ public function get(Iterator $iterator): Traversable

class UnionTypeHints
{
Expand All @@ -442,7 +442,7 @@ diff --git a/tests/input/ControlStructures.php b/tests/input/ControlStructures.p
index 73944e3..a0e0b2e 100644
--- a/tests/input/ControlStructures.php
+++ b/tests/input/ControlStructures.php
@@ -93,7 +93,7 @@ class ControlStructures
@@ -93,7 +93,7 @@ public function spaceBelowBlocks(): void
}
try {
echo 4;
Expand Down Expand Up @@ -497,7 +497,7 @@ diff --git a/tests/input/TrailingCommaOnFunctions.php b/tests/input/TrailingComm
index fc5480e..f3ffa91 100644
--- a/tests/input/TrailingCommaOnFunctions.php
+++ b/tests/input/TrailingCommaOnFunctions.php
@@ -10,7 +10,7 @@ use function var_dump;
@@ -10,7 +10,7 @@

class TrailingCommaOnFunctions
{
Expand All @@ -506,7 +506,7 @@ index fc5480e..f3ffa91 100644
{
}

@@ -23,7 +23,7 @@ class TrailingCommaOnFunctions
@@ -23,7 +23,7 @@ public function uses(): void
{
$var = null;

Expand All @@ -515,7 +515,7 @@ index fc5480e..f3ffa91 100644
var_dump($var);
};

@@ -37,8 +37,7 @@ class TrailingCommaOnFunctions
@@ -37,8 +37,7 @@ public function uses(): void

$class = new TrailingCommaOnFunctions();

Expand All @@ -529,7 +529,7 @@ diff --git a/tests/input/arrow-functions-format.php b/tests/input/arrow-function
index d3903ff..8a358e8 100644
--- a/tests/input/arrow-functions-format.php
+++ b/tests/input/arrow-functions-format.php
@@ -18,10 +18,10 @@ $returningObject = static fn () => new stdClass();
@@ -18,10 +18,10 @@

$multiLineArrowFunctions = Collection::from([1, 2])
->map(
Expand All @@ -546,7 +546,7 @@ diff --git a/tests/input/namespaces-spacing.php b/tests/input/namespaces-spacing
index e7be018..e1ab639 100644
--- a/tests/input/namespaces-spacing.php
+++ b/tests/input/namespaces-spacing.php
@@ -11,5 +11,5 @@ use const DATE_RFC3339;
@@ -11,5 +11,5 @@
strrev(
(new DateTimeImmutable('@' . time(), new DateTimeZone('UTC')))
->sub(new DateInterval('P1D'))
Expand Down