Skip to content

Commit 4f3cdbd

Browse files
kubawerloskubawerlos
authored
ReadonlyPromotedPropertiesFixer - analyse property usage (#1039)
1 parent 8477333 commit 4f3cdbd

File tree

3 files changed

+274
-23
lines changed

3 files changed

+274
-23
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
[![Latest stable version](https://img.shields.io/packagist/v/kubawerlos/php-cs-fixer-custom-fixers.svg?label=current%20version)](https://packagist.org/packages/kubawerlos/php-cs-fixer-custom-fixers)
66
[![PHP version](https://img.shields.io/packagist/php-v/kubawerlos/php-cs-fixer-custom-fixers.svg)](https://php.net)
77
[![License](https://img.shields.io/github/license/kubawerlos/php-cs-fixer-custom-fixers.svg)](LICENSE)
8-
![Tests](https://img.shields.io/badge/tests-3788-brightgreen.svg)
8+
![Tests](https://img.shields.io/badge/tests-3795-brightgreen.svg)
99
[![Downloads](https://img.shields.io/packagist/dt/kubawerlos/php-cs-fixer-custom-fixers.svg)](https://packagist.org/packages/kubawerlos/php-cs-fixer-custom-fixers)
1010

1111
[![CI status](https://github.com/kubawerlos/php-cs-fixer-custom-fixers/actions/workflows/ci.yaml/badge.svg)](https://github.com/kubawerlos/php-cs-fixer-custom-fixers/actions/workflows/ci.yaml)

‎src/Fixer/ReadonlyPromotedPropertiesFixer.php‎

Lines changed: 80 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,25 @@
2525
*/
2626
final class ReadonlyPromotedPropertiesFixer extends AbstractFixer
2727
{
28+
private const ASSIGNMENT_KINDS = [
29+
'=',
30+
[\T_INC, '++'],
31+
[\T_DEC, '--'],
32+
[\T_PLUS_EQUAL, '+='],
33+
[\T_MINUS_EQUAL, '-='],
34+
[\T_MUL_EQUAL, '*='],
35+
[\T_DIV_EQUAL, '/='],
36+
[\T_MOD_EQUAL, '%='],
37+
[\T_POW_EQUAL, '**='],
38+
[\T_AND_EQUAL, '&='],
39+
[\T_OR_EQUAL, '|='],
40+
[\T_XOR_EQUAL, '^='],
41+
[\T_SL_EQUAL, '<<='],
42+
[\T_SR_EQUAL, '>>='],
43+
[\T_COALESCE_EQUAL, '??='],
44+
[\T_CONCAT_EQUAL, '.='],
45+
];
46+
2847
/** @var list<int> */
2948
private array $promotedPropertyVisibilityKinds;
3049

@@ -144,16 +163,7 @@ private function fixParameters(
144163
continue;
145164
}
146165

147-
$propertyAssignment = $tokens->findSequence(
148-
[
149-
[\T_VARIABLE, '$this'],
150-
[\T_OBJECT_OPERATOR],
151-
[\T_STRING, \substr($tokens[$index]->getContent(), 1)],
152-
],
153-
$classOpenBraceIndex,
154-
$classCloseBraceIndex,
155-
);
156-
if ($propertyAssignment !== null) {
166+
if (!self::isUsedAsReadonly($tokens, \substr($tokens[$index]->getContent(), 1), $classOpenBraceIndex, $classCloseBraceIndex)) {
157167
continue;
158168
}
159169

@@ -186,4 +196,64 @@ private function getInsertIndex(Tokens $tokens, int $index): ?int
186196

187197
return $insertIndex;
188198
}
199+
200+
private static function isUsedAsReadonly(Tokens $tokens, string $name, int $index, int $endIndex): bool
201+
{
202+
$sequence = [
203+
[\T_VARIABLE, '$this'],
204+
[\T_OBJECT_OPERATOR],
205+
[\T_STRING, $name],
206+
];
207+
208+
while ($index < $endIndex) {
209+
$propertyAssignment = $tokens->findSequence($sequence, $index, $endIndex);
210+
if ($propertyAssignment === null) {
211+
break;
212+
}
213+
214+
$index = \array_key_last($propertyAssignment);
215+
216+
if (!self::isReadonlyUsage($tokens, $index)) {
217+
return false;
218+
}
219+
}
220+
221+
return true;
222+
}
223+
224+
private static function isReadonlyUsage(Tokens $tokens, int $index): bool
225+
{
226+
$index = $tokens->getPrevMeaningfulToken($index);
227+
\assert(\is_int($index));
228+
229+
while (!$tokens[$index]->equalsAny(self::ASSIGNMENT_KINDS)) {
230+
if ($tokens[$index]->isObjectOperator()) {
231+
$methodOrPropertyIndex = $tokens->getNextMeaningfulToken($index);
232+
\assert(\is_int($methodOrPropertyIndex));
233+
234+
$afterMethodOrPropertyIndex = $tokens->getNextMeaningfulToken($methodOrPropertyIndex);
235+
\assert(\is_int($afterMethodOrPropertyIndex));
236+
237+
if ($tokens[$afterMethodOrPropertyIndex]->equals('(') || $tokens[$afterMethodOrPropertyIndex]->isObjectOperator()) {
238+
return true;
239+
}
240+
241+
$index = $afterMethodOrPropertyIndex;
242+
continue;
243+
}
244+
245+
$blockType = Tokens::detectBlockType($tokens[$index]);
246+
if ($blockType !== null && $blockType['isStart']) {
247+
$index = $tokens->findBlockEnd($blockType['type'], $index);
248+
249+
$index = $tokens->getNextMeaningfulToken($index);
250+
\assert(\is_int($index));
251+
continue;
252+
}
253+
254+
return true;
255+
}
256+
257+
return false;
258+
}
189259
}

‎tests/Fixer/ReadonlyPromotedPropertiesFixerTest.php‎

Lines changed: 193 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,19 +34,19 @@ public function testFix(string $expected, ?string $input = null): void
3434
}
3535

3636
/**
37-
* @return iterable<list<string>>
37+
* @return iterable<array{string, 1?: string}>
3838
*/
3939
public static function provideFixCases(): iterable
4040
{
41-
yield [
41+
yield 'not promoted property' => [
4242
'<?php class Foo {
4343
public function __construct(
4444
int $x
4545
) {}
4646
}',
4747
];
4848

49-
yield [
49+
yield 'multiple promoted properties' => [
5050
'<?php class Foo {
5151
public function __construct(
5252
public readonly int $a,
@@ -63,7 +63,7 @@ public function __construct(
6363
}',
6464
];
6565

66-
yield [
66+
yield 'already readonly properties' => [
6767
'<?php class Foo {
6868
public function __construct(
6969
public readonly int $a,
@@ -88,7 +88,7 @@ public function __construct(
8888
}',
8989
];
9090

91-
yield [
91+
yield 'not in constructor' => [
9292
'<?php
9393
class Foo { public function __construct(public readonly int $x) {} }
9494
class Bar { public function notConstruct(int $x) {} }
@@ -101,14 +101,52 @@ class Baz { public function __construct(public int $x) {} }
101101
',
102102
];
103103

104-
yield [
105-
'<?php class Foo {
106-
public function __construct(public int $x) {}
107-
public function doSomething() { $this->x = 42; }
108-
}',
104+
yield 'property used in assignment' => [
105+
<<<'PHP'
106+
<?php class Foo {
107+
public function __construct(
108+
public int $p1,
109+
public int $p2,
110+
public int $p3,
111+
public int $p4,
112+
public int $p5,
113+
public int $p6,
114+
public int $p7,
115+
public int $p8,
116+
public int $p9,
117+
public bool $p10,
118+
public bool $p11,
119+
public bool $p12,
120+
public int $p13,
121+
public int $p14,
122+
public int $p15,
123+
public string $p16,
124+
public array $p17,
125+
) {}
126+
public function f() {
127+
$this->p1 = 1;
128+
$this->p2++;
129+
$this->p3--;
130+
$this->p4 += 4;
131+
$this->p5 -= 5;
132+
$this->p6 *= 6;
133+
$this->p7 /= 7;
134+
$this->p8 %= 8;
135+
$this->p9 **= 9;
136+
$this->p10 &= true;
137+
$this->p11 |= true;
138+
$this->p12 ^= true;
139+
$this->p13 <<= 1;
140+
$this->p14 >>= 1;
141+
$this->p15 ??= 15;
142+
$this->p16 .= '16';
143+
$this->p17[0][0][0][0][0][0][0][0] = 17;
144+
}
145+
}
146+
PHP,
109147
];
110148

111-
yield [
149+
yield 'property of other object used in assignment' => [
112150
'<?php class Foo {
113151
public function __construct(public readonly int $x) {}
114152
public function doSomething() { $object->x = 42; }
@@ -119,7 +157,7 @@ public function doSomething() { $object->x = 42; }
119157
}',
120158
];
121159

122-
yield [
160+
yield 'multiple properties with assignments' => [
123161
'<?php class Foo {
124162
public function __construct(public int $x, public readonly int $y, public int $z) {}
125163
public function doSomething() { $this->x = 42; $this->z = 10; }
@@ -129,6 +167,149 @@ public function __construct(public int $x, public int $y, public int $z) {}
129167
public function doSomething() { $this->x = 42; $this->z = 10; }
130168
}',
131169
];
170+
171+
yield 'property used in return statement' => [
172+
<<<'PHP'
173+
<?php class Foo {
174+
public function __construct(public readonly object $p) {}
175+
public function f() {
176+
return $this->p;
177+
}
178+
}
179+
PHP,
180+
<<<'PHP'
181+
<?php class Foo {
182+
public function __construct(public object $p) {}
183+
public function f() {
184+
return $this->p;
185+
}
186+
}
187+
PHP,
188+
];
189+
190+
yield 'property used as argument' => [
191+
<<<'PHP'
192+
<?php class Foo {
193+
public function __construct(
194+
public readonly array $array,
195+
public readonly object $object,
196+
) {}
197+
public function f() {
198+
bar($this->array[4], $this->object);
199+
baz(1, $this->array, 2);
200+
baz(3, $this->object, 4);
201+
}
202+
}
203+
PHP,
204+
<<<'PHP'
205+
<?php class Foo {
206+
public function __construct(
207+
public array $array,
208+
public object $object,
209+
) {}
210+
public function f() {
211+
bar($this->array[4], $this->object);
212+
baz(1, $this->array, 2);
213+
baz(3, $this->object, 4);
214+
}
215+
}
216+
PHP,
217+
];
218+
219+
yield 'property used in method call' => [
220+
<<<'PHP'
221+
<?php class Foo {
222+
public function __construct(public readonly object $p) {}
223+
public function f() {
224+
$this->p->method();
225+
}
226+
}
227+
PHP,
228+
<<<'PHP'
229+
<?php class Foo {
230+
public function __construct(public object $p) {}
231+
public function f() {
232+
$this->p->method();
233+
}
234+
}
235+
PHP,
236+
];
237+
238+
yield 'property used in null-safe method call' => [
239+
<<<'PHP'
240+
<?php class Foo {
241+
public function __construct(public readonly object $p) {}
242+
public function f() {
243+
$this->p?->method();
244+
}
245+
}
246+
PHP,
247+
<<<'PHP'
248+
<?php class Foo {
249+
public function __construct(public object $p) {}
250+
public function f() {
251+
$this->p?->method();
252+
}
253+
}
254+
PHP,
255+
];
256+
257+
yield 'property of property' => [
258+
<<<'PHP'
259+
<?php class Foo {
260+
public function __construct(public readonly object $object) {}
261+
public function f() {
262+
$this->object->property = 3;
263+
}
264+
}
265+
PHP,
266+
<<<'PHP'
267+
<?php class Foo {
268+
public function __construct(public object $object) {}
269+
public function f() {
270+
$this->object->property = 3;
271+
}
272+
}
273+
PHP,
274+
];
275+
276+
yield 'property used multiple times' => [
277+
<<<'PHP'
278+
<?php class Foo {
279+
public function __construct(public readonly object $p) {}
280+
public function f() {
281+
$this->value1 = $this->p->method1();
282+
bar($this->p, $this->p->property1);
283+
$this->value2 = $this->p->property2;
284+
$this->p->method2();
285+
}
286+
}
287+
PHP,
288+
<<<'PHP'
289+
<?php class Foo {
290+
public function __construct(public object $p) {}
291+
public function f() {
292+
$this->value1 = $this->p->method1();
293+
bar($this->p, $this->p->property1);
294+
$this->value2 = $this->p->property2;
295+
$this->p->method2();
296+
}
297+
}
298+
PHP,
299+
];
300+
301+
yield 'property used multiple times, including assignment' => [
302+
<<<'PHP'
303+
<?php class Foo {
304+
public function __construct(public object $p) {}
305+
public function f() {
306+
$this->p->method1();
307+
$this->p = new Bar();
308+
$this->p->method2();
309+
}
310+
}
311+
PHP,
312+
];
132313
}
133314

134315
/**

0 commit comments

Comments
 (0)