Skip to content

Commit 44a3d27

Browse files
gmazzapgmazzap
committed
Properly handle Generators and return types
Fix #10
1 parent cb8f340 commit 44a3d27

File tree

2 files changed

+218
-20
lines changed

2 files changed

+218
-20
lines changed

‎Inpsyde/Sniffs/CodeQuality/ReturnTypeDeclarationSniff.php‎

Lines changed: 128 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -47,23 +47,39 @@ public function process(File $file, $position)
4747
}
4848

4949
list($functionStart, $functionEnd) = PhpcsHelpers::functionBoundaries($file, $position);
50-
if (!$functionStart < 0 || $functionEnd <= 0) {
50+
51+
if (($functionStart < 0) || ($functionEnd <= 0)) {
5152
return;
5253
}
5354

5455
list(
5556
$hasNonVoidReturnType,
5657
$hasVoidReturnType,
5758
$hasNoReturnType,
58-
$hasNullable
59+
$hasNullableReturn,
60+
$returnsGenerator
5961
) = $this->returnTypeInfo($file, $position);
6062

6163
list($nonVoidReturnCount, $voidReturnCount, $nullReturnCount) = PhpcsHelpers::countReturns(
6264
$file,
6365
$position
6466
);
6567

66-
if ($hasNullable) {
68+
$yieldCount = $this->countYield($functionStart, $functionEnd, $file);
69+
70+
if ($yieldCount || $returnsGenerator) {
71+
$this->maybeGeneratorErrors(
72+
$yieldCount,
73+
$returnsGenerator,
74+
$nonVoidReturnCount,
75+
$file,
76+
$position
77+
);
78+
79+
return;
80+
}
81+
82+
if ($hasNullableReturn) {
6783
$voidReturnCount -= $nullReturnCount;
6884
}
6985

@@ -96,6 +112,7 @@ private function maybeErrors(
96112
File $file,
97113
int $position
98114
) {
115+
99116
if ($hasNonVoidReturnType && ($nonVoidReturnCount === 0 || $voidReturnCount > 0)) {
100117
$msg = 'Return type with';
101118
$file->addError(
@@ -134,25 +151,100 @@ private function maybeErrors(
134151
}
135152
}
136153

154+
/**
155+
* @param int $yieldCount
156+
* @param bool $returnsGenerator
157+
* @param int $nonVoidReturnCount
158+
* @param File $file
159+
* @param int $position
160+
*/
161+
private function maybeGeneratorErrors(
162+
int $yieldCount,
163+
bool $returnsGenerator,
164+
int $nonVoidReturnCount,
165+
File $file,
166+
int $position
167+
) {
168+
169+
if ($nonVoidReturnCount > 1) {
170+
$file->addWarning(
171+
'A generator should only contain a single return point.',
172+
$position,
173+
'InvalidGeneratorManyReturns'
174+
);
175+
}
176+
177+
if ($yieldCount && $returnsGenerator) {
178+
return;
179+
}
180+
181+
if (!$yieldCount) {
182+
$file->addError(
183+
'Found a generator return type in non-yielding function.',
184+
$position,
185+
'GeneratorReturnTypeWithoutYield'
186+
);
187+
188+
return;
189+
}
190+
191+
if (!$nonVoidReturnCount) {
192+
$file->addWarning(
193+
'Found a function that yield values but missing Generator return type.',
194+
$position,
195+
'NoGeneratorReturnType'
196+
);
197+
198+
return;
199+
}
200+
201+
$returnType = $this->returnTypeContent($file, $position);
202+
if ($returnType === 'Traversable' || $returnType === 'Iterator') {
203+
return;
204+
}
205+
206+
$file->addError(
207+
'Found a function that yield values but declare a return type different than Generator.',
208+
$position,
209+
'IncorrectReturnTypeForGenerator'
210+
);
211+
}
212+
137213
/**
138214
* @param File $file
139215
* @param int $functionPosition
140-
* @return array
216+
* @return string
141217
*/
142-
private function returnTypeInfo(File $file, int $functionPosition): array
218+
private function returnTypeContent(File $file, int $functionPosition): string
143219
{
144220
$tokens = $file->getTokens();
145-
$functionToken = $tokens[$functionPosition];
146-
147221
$returnTypeToken = $file->findNext(
148222
[T_RETURN_TYPE],
149223
$functionPosition + 3, // 3: open parenthesis, close parenthesis, colon
150-
($functionToken['scope_opener'] ?? 0) - 1
224+
($tokens[$functionPosition]['scope_opener'] ?? 0) - 1
151225
);
152226

153227
$returnType = $tokens[$returnTypeToken] ?? null;
154-
if ($returnType && $returnType['code'] !== T_RETURN_TYPE) {
155-
return [false, false, true, false];
228+
if (!$returnType || $returnType['code'] !== T_RETURN_TYPE) {
229+
return '';
230+
}
231+
232+
return ltrim($returnType['content'] ?? '', '\\');
233+
}
234+
235+
/**
236+
* @param File $file
237+
* @param int $functionPosition
238+
* @return array
239+
*/
240+
private function returnTypeInfo(File $file, int $functionPosition): array
241+
{
242+
$tokens = $file->getTokens();
243+
244+
$returnTypeContent = $this->returnTypeContent($file, $functionPosition);
245+
246+
if (!$returnTypeContent) {
247+
return [false, false, true, false, false];
156248
}
157249

158250
$start = $tokens[$functionPosition]['parenthesis_closer'] + 1;
@@ -168,10 +260,11 @@ private function returnTypeInfo(File $file, int $functionPosition): array
168260
}
169261
}
170262

171-
$hasNonVoidReturnType = $returnType['content'] !== 'void';
172-
$hasVoidReturnType = $returnType['content'] === 'void';
263+
$hasNonVoidReturnType = $returnTypeContent !== 'void';
264+
$hasVoidReturnType = $returnTypeContent === 'void';
265+
$returnsGenerator = $returnTypeContent === 'Generator';
173266

174-
return [$hasNonVoidReturnType, $hasVoidReturnType, false, $hasNullable];
267+
return [$hasNonVoidReturnType, $hasVoidReturnType, false, $hasNullable, $returnsGenerator];
175268
}
176269

177270
/**
@@ -214,4 +307,26 @@ private function areNullableReturnTypesSupported(): bool
214307

215308
return $min && version_compare($min, '7.1', '>=');
216309
}
310+
311+
/**
312+
* @param int $functionStart
313+
* @param int $functionEnd
314+
* @return int
315+
*/
316+
private function countYield(int $functionStart, int $functionEnd, File $file): int
317+
{
318+
$count = 0;
319+
$tokens = $file->getTokens();
320+
for ($i = $functionStart + 1; $i < $functionEnd; $i++) {
321+
if ($tokens[$i]['code'] === T_CLOSURE) {
322+
$i = $tokens[$i]['scope_closer'];
323+
continue;
324+
}
325+
if ($tokens[$i]['code'] === T_YIELD || $tokens[$i]['code'] === T_YIELD_FROM) {
326+
$count++;
327+
}
328+
}
329+
330+
return $count;
331+
}
217332
}

‎tests/fixtures/return-type-declaration.php‎

Lines changed: 90 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
<?php
22
// @phpcsSniff CodeQuality.ReturnTypeDeclaration
33

4-
// @phpcsWarningCodeOnNextLine NoReturnType
5-
function a()
6-
{
7-
return 'x';
8-
}
9-
104
// @phpcsErrorCodeOnNextLine IncorrectVoidReturnType
115
function c(): void
126
{
137
return true;
148
}
159

10+
// @phpcsWarningCodeOnNextLine NoReturnType
11+
function a()
12+
{
13+
return 'x';
14+
}
15+
1616
function aa($foo)
1717
{
1818
return;
@@ -57,6 +57,89 @@ function g(): bool
5757
return false;
5858
}
5959

60+
function gen(string $content): \Generator {
61+
$line = strtok($content, "\n");
62+
while ($line !== false) {
63+
$line = strtok("\n");
64+
yield is_string($line) ? trim($line) : '';
65+
}
66+
}
67+
68+
// @phpcsErrorCodeOnNextLine GeneratorReturnTypeWithoutYield
69+
function genNoYield(string $content): \Generator {
70+
$line = strtok($content, "\n");
71+
while ($line !== false) {
72+
$line = strtok("\n");
73+
is_string($line) ? trim($line) : '';
74+
}
75+
76+
return true;
77+
}
78+
79+
// @phpcsWarningCodeOnNextLine NoGeneratorReturnType
80+
function yieldNoGen(string $content) {
81+
$line = strtok($content, "\n");
82+
while ($line !== false) {
83+
$line = strtok("\n");
84+
yield is_string($line) ? trim($line) : '';
85+
}
86+
}
87+
88+
// @phpcsErrorCodeOnNextLine IncorrectReturnTypeForGenerator
89+
function yieldWrongReturn(string $content): int {
90+
$line = strtok($content, "\n");
91+
while ($line !== false) {
92+
$line = strtok("\n");
93+
yield is_string($line) ? trim($line) : '';
94+
}
95+
96+
return 1;
97+
}
98+
99+
function yieldIteratorReturn(string $content): \Iterator {
100+
$line = strtok($content, "\n");
101+
while ($line !== false) {
102+
$line = strtok("\n");
103+
yield is_string($line) ? trim($line) : '';
104+
}
105+
106+
return 1;
107+
}
108+
109+
110+
function genFrom(): \Generator {
111+
112+
$gen = function(int $x): int {
113+
if ($x < 0) {
114+
return 0;
115+
}
116+
117+
if ($x > 100) {
118+
return 100;
119+
}
120+
121+
return $x;
122+
};
123+
124+
$data = array_map($gen, range(-100, 100));
125+
yield from $data;
126+
}
127+
128+
// @phpcsWarningCodeOnNextLine InvalidGeneratorManyReturns
129+
function genMultiReturn(): \Generator {
130+
if (defined('MEH_MEH')) {
131+
return 1;
132+
}
133+
134+
yield from [1,2];
135+
136+
if (defined('MEH')) {
137+
return 1;
138+
}
139+
140+
return 2;
141+
}
142+
60143
add_filter('x', function () {
61144
return '';
62145
});
@@ -294,4 +377,4 @@ public function iReturnWrongNull() : \ArrayAccess
294377

295378
return new \ArrayObject();
296379
}
297-
}
380+
}

0 commit comments

Comments
 (0)