Skip to content

Commit bd46957

Browse files
committed
fix: no model forwarding to builder
This fixes two issues: - the rule now correctly handles nullable union types - the rule now correctly handles the `with()` method
1 parent 1872da9 commit bd46957

File tree

3 files changed

+30
-9
lines changed

3 files changed

+30
-9
lines changed

src/Rules/NoModelForwardingToBuilder.php

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use PHPStan\Analyser\Scope;
1414
use PHPStan\Rules\Rule;
1515
use PHPStan\Rules\RuleErrorBuilder;
16+
use PHPStan\Type\TypeCombinator;
1617

1718
use function array_map;
1819
use function sprintf;
@@ -37,7 +38,7 @@ public function processNode(Node $node, Scope $scope): array
3738
$scope->getType($node->name)->getConstantStrings(),
3839
);
3940

40-
$calledOnType = $scope->getType($node->var);
41+
$calledOnType = TypeCombinator::removeNull($scope->getType($node->var));
4142

4243
foreach ($calledOnType->getObjectClassReflections() as $classReflection) {
4344
if (! $classReflection->is(Model::class)) {
@@ -52,7 +53,14 @@ public function processNode(Node $node, Scope $scope): array
5253
$methodReflection = $classReflection->getMethod($method, $scope);
5354
$declaringClass = $methodReflection->getDeclaringClass();
5455

55-
if (! $declaringClass->is(QueryBuilder::class) && ! $declaringClass->is(EloquentBuilder::class)) {
56+
if (
57+
! $declaringClass->is(QueryBuilder::class)
58+
&& ! $declaringClass->is(EloquentBuilder::class)
59+
// Override for the with() method, which is also a static method
60+
// on the Model class, so is not caught by the above check.
61+
// Should not be called on a model instance when this rule is enabled.
62+
&& $method !== 'with'
63+
) {
5664
continue;
5765
}
5866

tests/Rules/NoModelForwardingToBuilderTest.php

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
use PHPStan\Rules\Rule;
99
use PHPStan\Testing\RuleTestCase;
1010

11+
use function strtr;
12+
1113
/** @extends RuleTestCase<NoModelForwardingToBuilder> */
1214
class NoModelForwardingToBuilderTest extends RuleTestCase
1315
{
@@ -18,14 +20,22 @@ protected function getRule(): Rule
1820

1921
public function testRule(): void
2022
{
23+
$message = static fn (string $name) => strtr(
24+
"Method [:name] is forwarded to a Builder instance, which is not allowed.\n 💡 Use [:::name()], [::query()->:name()] or [->newQuery()->:name()] instead.",
25+
[':name' => $name],
26+
);
27+
2128
$this->analyse([__DIR__ . '/data/NoModelForwardingToBuilderInstance.php'], [
22-
["Method [first] is forwarded to a Builder instance, which is not allowed.\n 💡 Use [::first()], [::query()->first()] or [->newQuery()->first()] instead.", 5],
23-
["Method [get] is forwarded to a Builder instance, which is not allowed.\n 💡 Use [::get()], [::query()->get()] or [->newQuery()->get()] instead.", 6],
24-
["Method [find] is forwarded to a Builder instance, which is not allowed.\n 💡 Use [::find()], [::query()->find()] or [->newQuery()->find()] instead.", 7],
25-
["Method [paginate] is forwarded to a Builder instance, which is not allowed.\n 💡 Use [::paginate()], [::query()->paginate()] or [->newQuery()->paginate()] instead.", 8],
26-
["Method [where] is forwarded to a Builder instance, which is not allowed.\n 💡 Use [::where()], [::query()->where()] or [->newQuery()->where()] instead.", 9],
27-
["Method [take] is forwarded to a Builder instance, which is not allowed.\n 💡 Use [::take()], [::query()->take()] or [->newQuery()->take()] instead.", 10],
28-
["Method [max] is forwarded to a Builder instance, which is not allowed.\n 💡 Use [::max()], [::query()->max()] or [->newQuery()->max()] instead.", 11],
29+
[$message('first'), 5],
30+
[$message('get'), 6],
31+
[$message('find'), 7],
32+
[$message('paginate'), 8],
33+
[$message('where'), 9],
34+
[$message('take'), 10],
35+
[$message('max'), 11],
36+
[$message('with'), 12],
37+
[$message('first'), 14],
38+
[$message('with'), 15],
2939
]);
3040
}
3141

tests/Rules/data/NoModelForwardingToBuilderInstance.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,7 @@
99
$user->where('id', 1);
1010
$user->take(1);
1111
$user->max('foo');
12+
$user->with('foo');
1213

14+
$user->accounts()->first()->first();
15+
$user->accounts()->first()->with('foo');

0 commit comments

Comments
 (0)