Skip to content

Commit dd54cea

Browse files
committed
Adjust name to reflect also assignment as OK.
1 parent 95154b8 commit dd54cea

File tree

5 files changed

+56
-23
lines changed

5 files changed

+56
-23
lines changed

README.md

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,8 @@ This rule check if the options (args) passed to Table::find and SelectQuery are
148148
### TableGetMatchOptionsTypesRule
149149
This rule check if the options (args) passed to Table::get are valid find options types.
150150

151-
### ControllerMethodMustReturnRule
152-
This rule enforces that controller methods like `render()` and `redirect()` must be returned to prevent unreachable code. These methods should always be used with a `return` statement to make the control flow explicit.
151+
### ControllerMethodMustBeUsedRule
152+
This rule enforces that controller methods like `render()` and `redirect()` must be used (returned or assigned) to prevent unreachable code. These methods should not be called in void context - use them with a `return` statement or assign them to a variable to make the control flow explicit.
153153

154154
<details>
155155
<summary>Examples:</summary>
@@ -168,6 +168,14 @@ public function myAction()
168168
return $this->render('edit');
169169
}
170170

171+
// Also good - assignment is valid
172+
public function myAction()
173+
{
174+
$response = $this->render('edit');
175+
176+
return $response;
177+
}
178+
171179
// Bad - code after redirect() is unreachable
172180
public function myAction()
173181
{

rules.neon

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ parameters:
88
disallowEntityArrayAccessRule: false
99
getMailerExistsClassRule: true
1010
loadComponentExistsClassRule: true
11-
controllerMethodMustReturnRule: true
11+
controllerMethodMustBeUsedRule: true
1212
parametersSchema:
1313
cakeDC: structure([
1414
addAssociationExistsTableClassRule: anyOf(bool(), arrayOf(bool()))
@@ -19,16 +19,16 @@ parametersSchema:
1919
disallowEntityArrayAccessRule: anyOf(bool(), arrayOf(bool()))
2020
getMailerExistsClassRule: anyOf(bool(), arrayOf(bool()))
2121
loadComponentExistsClassRule: anyOf(bool(), arrayOf(bool()))
22-
controllerMethodMustReturnRule: anyOf(bool(), arrayOf(bool()))
22+
controllerMethodMustBeUsedRule: anyOf(bool(), arrayOf(bool()))
2323
])
2424

2525
conditionalTags:
2626
CakeDC\PHPStan\Visitor\AddAssociationSetClassNameVisitor:
2727
phpstan.parser.richParserNodeVisitor: %cakeDC.addAssociationExistsTableClassRule%
2828
CakeDC\PHPStan\Rule\Controller\LoadComponentExistsClassRule:
2929
phpstan.rules.rule: %cakeDC.loadComponentExistsClassRule%
30-
CakeDC\PHPStan\Rule\Controller\ControllerMethodMustReturnRule:
31-
phpstan.rules.rule: %cakeDC.controllerMethodMustReturnRule%
30+
CakeDC\PHPStan\Rule\Controller\ControllerMethodMustBeUsedRule:
31+
phpstan.rules.rule: %cakeDC.controllerMethodMustBeUsedRule%
3232
CakeDC\PHPStan\Rule\Model\AddAssociationExistsTableClassRule:
3333
phpstan.rules.rule: %cakeDC.addAssociationExistsTableClassRule%
3434
CakeDC\PHPStan\Rule\Model\AddAssociationMatchOptionsTypesRule:
@@ -50,7 +50,7 @@ services:
5050
-
5151
class: CakeDC\PHPStan\Rule\Controller\LoadComponentExistsClassRule
5252
-
53-
class: CakeDC\PHPStan\Rule\Controller\ControllerMethodMustReturnRule
53+
class: CakeDC\PHPStan\Rule\Controller\ControllerMethodMustBeUsedRule
5454
-
5555
class: CakeDC\PHPStan\Rule\Model\AddAssociationExistsTableClassRule
5656
-

src/Rule/Controller/ControllerMethodMustReturnRule.php renamed to src/Rule/Controller/ControllerMethodMustBeUsedRule.php

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,17 @@
2222
use PHPStan\Rules\RuleErrorBuilder;
2323

2424
/**
25-
* Rule to enforce that certain controller methods must be returned
26-
* to prevent unreachable code.
25+
* Rule to enforce that certain controller methods must be used
26+
* (returned or assigned) to prevent unreachable code.
2727
*/
28-
class ControllerMethodMustReturnRule implements Rule
28+
class ControllerMethodMustBeUsedRule implements Rule
2929
{
3030
/**
31-
* Methods that must be returned
31+
* Methods that must be used (returned or assigned)
3232
*
3333
* @var array<string>
3434
*/
35-
protected array $methodsRequiringReturn = [
35+
protected array $methodsRequiringUsage = [
3636
'render',
3737
'redirect',
3838
];
@@ -67,7 +67,7 @@ public function processNode(Node $node, Scope $scope): array
6767
}
6868

6969
$methodName = $methodCall->name->toString();
70-
if (!in_array($methodName, $this->methodsRequiringReturn, true)) {
70+
if (!in_array($methodName, $this->methodsRequiringUsage, true)) {
7171
return [];
7272
}
7373

@@ -92,14 +92,15 @@ public function processNode(Node $node, Scope $scope): array
9292
}
9393

9494
// If we reach here, it means the method call is wrapped in an Expression node
95-
// which means it's used as a statement (not returned)
95+
// which means it's used as a statement (not returned or assigned)
9696
return [
9797
RuleErrorBuilder::message(sprintf(
98-
'Method %s() must be returned to prevent unreachable code. Use "return $this->%s()" instead.',
98+
'Method `%s()` must be used to prevent unreachable code. ' .
99+
'Use `return $this->%s()` or assign it to a variable.',
99100
$methodName,
100101
$methodName,
101102
))
102-
->identifier('cake.controller.' . $methodName . 'MustReturn')
103+
->identifier('cake.controller.' . $methodName . 'MustBeUsed')
103104
->build(),
104105
];
105106
}

tests/TestCase/Rule/Controller/ControllerMethodMustReturnRuleTest.php renamed to tests/TestCase/Rule/Controller/ControllerMethodMustBeUsedRuleTest.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,18 @@
1313

1414
namespace CakeDC\PHPStan\Test\TestCase\Rule\Controller;
1515

16-
use CakeDC\PHPStan\Rule\Controller\ControllerMethodMustReturnRule;
16+
use CakeDC\PHPStan\Rule\Controller\ControllerMethodMustBeUsedRule;
1717
use PHPStan\Rules\Rule;
1818
use PHPStan\Testing\RuleTestCase;
1919

20-
class ControllerMethodMustReturnRuleTest extends RuleTestCase
20+
class ControllerMethodMustBeUsedRuleTest extends RuleTestCase
2121
{
2222
/**
2323
* @return \PHPStan\Rules\Rule
2424
*/
2525
protected function getRule(): Rule
2626
{
27-
return new ControllerMethodMustReturnRule();
27+
return new ControllerMethodMustBeUsedRule();
2828
}
2929

3030
/**
@@ -34,19 +34,19 @@ public function testRule(): void
3434
{
3535
$this->analyse([__DIR__ . '/Fake/FailingControllerMethodReturnLogic.php'], [
3636
[
37-
'Method render() must be returned to prevent unreachable code. Use "return $this->render()" instead.',
37+
'Method `render()` must be used to prevent unreachable code. Use `return $this->render()` or assign it to a variable.',
3838
17,
3939
],
4040
[
41-
'Method redirect() must be returned to prevent unreachable code. Use "return $this->redirect()" instead.',
41+
'Method `redirect()` must be used to prevent unreachable code. Use `return $this->redirect()` or assign it to a variable.',
4242
29,
4343
],
4444
[
45-
'Method render() must be returned to prevent unreachable code. Use "return $this->render()" instead.',
45+
'Method `render()` must be used to prevent unreachable code. Use `return $this->render()` or assign it to a variable.',
4646
62,
4747
],
4848
[
49-
'Method redirect() must be returned to prevent unreachable code. Use "return $this->redirect()" instead.',
49+
'Method `redirect()` must be used to prevent unreachable code. Use `return $this->redirect()` or assign it to a variable.',
5050
74,
5151
],
5252
]);

tests/TestCase/Rule/Controller/Fake/FailingControllerMethodReturnLogic.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,4 +74,28 @@ public function actionWithConditionalRedirectNoReturn()
7474
$this->redirect(['action' => 'add']);
7575
}
7676
}
77+
78+
/**
79+
* Action with render() assigned to variable (valid usage)
80+
*
81+
* @return \Cake\Http\Response|null
82+
*/
83+
public function actionWithRenderAssigned()
84+
{
85+
$response = $this->render('edit');
86+
87+
return $response;
88+
}
89+
90+
/**
91+
* Action with redirect() assigned to variable (valid usage)
92+
*
93+
* @return \Cake\Http\Response|null
94+
*/
95+
public function actionWithRedirectAssigned()
96+
{
97+
$response = $this->redirect(['action' => 'index']);
98+
99+
return $response;
100+
}
77101
}

0 commit comments

Comments
 (0)