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.

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

fix: make, makeWith and resolve methods not resolving to correct type on Application and Container. #1451

Merged
merged 3 commits into from Dec 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

- fix: changed dependency from `Illuminate\Config\Repository` to Config interface by @nai4rus
- feat: assert `view-string` when using `view()->exists()` by @mad-briller
- feat: Add return-type extension for Application / Container `make` `makeWith` and `resolve` methods by @mad-briller.
- feat: freed `joinSub` to allow models to join other models by @harmenjanssen in https://github.com/nunomaduro/larastan/pull/1352
- feat: updated return type of the Request::header method by @mad-briller
- feat: Added stub for `optional()` helper and class by @mad-briller in https://github.com/nunomaduro/larastan/pull/1344
Expand Down
13 changes: 13 additions & 0 deletions extension.neon
Expand Up @@ -482,6 +482,19 @@ services:
class: NunoMaduro\Larastan\Support\ViewFileHelper
arguments:
viewDirectories: %viewDirectories%

-
class: NunoMaduro\Larastan\ReturnTypes\ApplicationMakeDynamicReturnTypeExtension
tags:
- phpstan.broker.dynamicMethodReturnTypeExtension

-
class: NunoMaduro\Larastan\ReturnTypes\ContainerMakeDynamicReturnTypeExtension
tags:
- phpstan.broker.dynamicMethodReturnTypeExtension

- NunoMaduro\Larastan\ReturnTypes\AppMakeHelper

rules:
- NunoMaduro\Larastan\Rules\UselessConstructs\NoUselessWithFunctionCallsRule
- NunoMaduro\Larastan\Rules\UselessConstructs\NoUselessValueFunctionCallsRule
Expand Down
39 changes: 5 additions & 34 deletions src/ReturnTypes/AppMakeDynamicReturnTypeExtension.php
Expand Up @@ -5,23 +5,18 @@
namespace NunoMaduro\Larastan\ReturnTypes;

use Illuminate\Support\Facades\App;
use NunoMaduro\Larastan\Concerns\HasContainer;
use PhpParser\Node\Expr\ClassConstFetch;
use PhpParser\Node\Expr\StaticCall;
use PhpParser\Node\Name\FullyQualified;
use PhpParser\Node\Scalar\String_;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\MethodReflection;
use PHPStan\Type\DynamicStaticMethodReturnTypeExtension;
use PHPStan\Type\ErrorType;
use PHPStan\Type\NeverType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use Throwable;

final class AppMakeDynamicReturnTypeExtension implements DynamicStaticMethodReturnTypeExtension
{
use HasContainer;
public function __construct(
private AppMakeHelper $appMakeHelper
) {
}

public function getClass(): string
{
Expand All @@ -35,30 +30,6 @@ public function isStaticMethodSupported(MethodReflection $methodReflection): boo

public function getTypeFromStaticMethodCall(MethodReflection $methodReflection, StaticCall $methodCall, Scope $scope): ?Type
{
if (count($methodCall->getArgs()) === 0) {
return new ErrorType();
}

$expr = $methodCall->getArgs()[0]->value;
if ($expr instanceof String_) {
try {
/** @var object|null $resolved */
$resolved = $this->resolve($expr->value);

if ($resolved === null) {
return new ErrorType();
}

return new ObjectType(get_class($resolved));
} catch (Throwable $exception) {
return new ErrorType();
}
}

if ($expr instanceof ClassConstFetch && $expr->class instanceof FullyQualified) {
return new ObjectType($expr->class->toString());
}

return new NeverType();
return $this->appMakeHelper->resolveTypeFromCall($methodCall);
}
}
50 changes: 50 additions & 0 deletions src/ReturnTypes/AppMakeHelper.php
@@ -0,0 +1,50 @@
<?php

namespace NunoMaduro\Larastan\ReturnTypes;

use NunoMaduro\Larastan\Concerns\HasContainer;
use PhpParser\Node\Expr\ClassConstFetch;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\StaticCall;
use PhpParser\Node\Name\FullyQualified;
use PhpParser\Node\Scalar\String_;
use PHPStan\Type\ErrorType;
use PHPStan\Type\NeverType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use Throwable;

final class AppMakeHelper
{
use HasContainer;

public function resolveTypeFromCall(FuncCall|MethodCall|StaticCall $call): Type
{
if (count($call->getArgs()) === 0) {
return new ErrorType();
}

$expr = $call->getArgs()[0]->value;
if ($expr instanceof String_) {
try {
/** @var object|null $resolved */
$resolved = $this->resolve($expr->value);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's not really guaranteed that resolve will return an object, scalars can be bound into the container using constant string names too and i've seen that in the wild, so it might be worth expanding this code to anticipate more types to be returned in the future


if ($resolved === null) {
return new ErrorType();
}

return new ObjectType(get_class($resolved));
} catch (Throwable $exception) {
return new ErrorType();
}
}

if ($expr instanceof ClassConstFetch && $expr->class instanceof FullyQualified) {
return new ObjectType($expr->class->toString());
}

return new NeverType();
}
}
35 changes: 35 additions & 0 deletions src/ReturnTypes/ApplicationMakeDynamicReturnTypeExtension.php
@@ -0,0 +1,35 @@
<?php

declare(strict_types=1);

namespace NunoMaduro\Larastan\ReturnTypes;

use Illuminate\Contracts\Foundation\Application;
use PhpParser\Node\Expr\MethodCall;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\MethodReflection;
use PHPStan\Type\DynamicMethodReturnTypeExtension;
use PHPStan\Type\Type;

final class ApplicationMakeDynamicReturnTypeExtension implements DynamicMethodReturnTypeExtension
{
public function __construct(
private AppMakeHelper $appMakeHelper
) {
}

public function getClass(): string
{
return Application::class;
}

public function isMethodSupported(MethodReflection $methodReflection): bool
{
return in_array($methodReflection->getName(), ['make', 'makeWith', 'resolve'], true);
}

public function getTypeFromMethodCall(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope): ?Type
{
return $this->appMakeHelper->resolveTypeFromCall($methodCall);
}
}
35 changes: 35 additions & 0 deletions src/ReturnTypes/ContainerMakeDynamicReturnTypeExtension.php
@@ -0,0 +1,35 @@
<?php

declare(strict_types=1);

namespace NunoMaduro\Larastan\ReturnTypes;

use Illuminate\Contracts\Container\Container;
use PhpParser\Node\Expr\MethodCall;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\MethodReflection;
use PHPStan\Type\DynamicMethodReturnTypeExtension;
use PHPStan\Type\Type;

final class ContainerMakeDynamicReturnTypeExtension implements DynamicMethodReturnTypeExtension
{
public function __construct(
private AppMakeHelper $appMakeHelper
) {
}

public function getClass(): string
{
return Container::class;
}

public function isMethodSupported(MethodReflection $methodReflection): bool
{
return in_array($methodReflection->getName(), ['make', 'makeWith', 'resolve'], true);
}

public function getTypeFromMethodCall(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope): ?Type
{
return $this->appMakeHelper->resolveTypeFromCall($methodCall);
}
}
38 changes: 6 additions & 32 deletions src/ReturnTypes/Helpers/AppExtension.php
Expand Up @@ -5,24 +5,20 @@
namespace NunoMaduro\Larastan\ReturnTypes\Helpers;

use Illuminate\Foundation\Application;
use NunoMaduro\Larastan\Concerns\HasContainer;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\ClassConstFetch;
use NunoMaduro\Larastan\ReturnTypes\AppMakeHelper;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Name\FullyQualified;
use PhpParser\Node\Scalar\String_;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\FunctionReflection;
use PHPStan\Type\DynamicFunctionReturnTypeExtension;
use PHPStan\Type\ErrorType;
use PHPStan\Type\NeverType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use Throwable;

class AppExtension implements DynamicFunctionReturnTypeExtension
{
use HasContainer;
public function __construct(
private AppMakeHelper $appMakeHelper
) {
}

public function isFunctionSupported(FunctionReflection $functionReflection): bool
{
Expand All @@ -38,28 +34,6 @@ public function getTypeFromFunctionCall(
return new ObjectType(Application::class);
}

/** @var Expr $expr */
$expr = $functionCall->getArgs()[0]->value;

if ($expr instanceof String_) {
try {
/** @var object|null $resolved */
$resolved = $this->resolve($expr->value);

if ($resolved === null) {
return new ErrorType();
}

return new ObjectType(get_class($resolved));
} catch (Throwable $exception) {
return new ErrorType();
}
}

if ($expr instanceof ClassConstFetch && $expr->class instanceof FullyQualified) {
return new ObjectType($expr->class->toString());
}

return new NeverType();
return $this->appMakeHelper->resolveTypeFromCall($functionCall);
}
}
2 changes: 2 additions & 0 deletions tests/Type/GeneralTypeTest.php
Expand Up @@ -35,6 +35,8 @@ public function dataFileAsserts(): iterable
yield from $this->gatherAssertTypes(__DIR__.'/data/database-transaction.php');
yield from $this->gatherAssertTypes(__DIR__.'/data/container-array-access.php');
yield from $this->gatherAssertTypes(__DIR__.'/data/view-exists.php');
yield from $this->gatherAssertTypes(__DIR__.'/data/application-make.php');
yield from $this->gatherAssertTypes(__DIR__.'/data/container-make.php');
}

/**
Expand Down
16 changes: 16 additions & 0 deletions tests/Type/data/application-make.php
@@ -0,0 +1,16 @@
<?php

namespace ApplicationMake;

use Illuminate\Contracts\Config\Repository;
use function PHPStan\Testing\assertType;

/** @var \Illuminate\Foundation\Application $app */
assertType('Illuminate\Contracts\Config\Repository', $app->make(Repository::class));
assertType('Illuminate\Contracts\Config\Repository', $app->makeWith(Repository::class));
assertType('Illuminate\Contracts\Config\Repository', $app->resolve(Repository::class));

/** @var \Illuminate\Contracts\Foundation\Application $app */
assertType('Illuminate\Contracts\Config\Repository', $app->make(Repository::class));
assertType('Illuminate\Contracts\Config\Repository', $app->makeWith(Repository::class));
assertType('Illuminate\Contracts\Config\Repository', $app->resolve(Repository::class));
16 changes: 16 additions & 0 deletions tests/Type/data/container-make.php
@@ -0,0 +1,16 @@
<?php

namespace ContainerMake;

use Illuminate\Contracts\Config\Repository;
use function PHPStan\Testing\assertType;

/** @var \Illuminate\Container\Container $container */
assertType('Illuminate\Contracts\Config\Repository', $container->make(Repository::class));
assertType('Illuminate\Contracts\Config\Repository', $container->makeWith(Repository::class));
assertType('Illuminate\Contracts\Config\Repository', $container->resolve(Repository::class));

/** @var \Illuminate\Contracts\Container\Container $container */
assertType('Illuminate\Contracts\Config\Repository', $container->make(Repository::class));
assertType('Illuminate\Contracts\Config\Repository', $container->makeWith(Repository::class));
assertType('Illuminate\Contracts\Config\Repository', $container->resolve(Repository::class));