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

feature: tighten return types of config helper by using dynamic analysis #229

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 105 additions & 0 deletions src/Handlers/Helpers/ConfigHandler.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
<?php

namespace Psalm\LaravelPlugin\Handlers\Helpers;

use Illuminate\Config\Repository;
use Psalm\LaravelPlugin\Providers\ApplicationProvider;
use Psalm\Plugin\EventHandler\Event\FunctionReturnTypeProviderEvent;
use Psalm\Plugin\EventHandler\FunctionReturnTypeProviderInterface;
use Psalm\Type\Atomic\TArray;
use Psalm\Type\Atomic\TArrayKey;
use Psalm\Type\Atomic\TBool;
use Psalm\Type\Atomic\TClosedResource;
use Psalm\Type\Atomic\TFloat;
use Psalm\Type\Atomic\TLiteralFloat;
use Psalm\Type\Atomic\TLiteralInt;
use Psalm\Type\Atomic\TLiteralString;
use Psalm\Type\Atomic\TMixed;
use Psalm\Type\Atomic\TNamedObject;
use Psalm\Type\Atomic\TNull;
use Psalm\Type\Atomic\TResource;
use Psalm\Type\Union;

use function gettype;
use function get_class;

class ConfigHandler implements FunctionReturnTypeProviderInterface
{
public static function getFunctionIds(): array
{
return ['config'];
}

public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $event): ?Union
{
// we're going to attempt some dynamic analysis to tighten the actual return type here.
// this could be done statically, but it's quicker + easier to do this dynamically.
// PRs to make this static in the future more than welcome!
$call_args = $event->getCallArgs();
if (!isset($call_args[0])) {
return new Union([
new TNamedObject(Repository::class),
]);
}

$argumentType = $call_args[0]->value;

if (!isset($argumentType->value)) {
return null;
}

$argumentValue = $argumentType->value;

Check failure on line 51 in src/Handlers/Helpers/ConfigHandler.php

View workflow job for this annotation

GitHub Actions / Psalm

MixedAssignment

src/Handlers/Helpers/ConfigHandler.php:51:9: MixedAssignment: Unable to determine the type that $argumentValue is being assigned to (see https://psalm.dev/032)

try {
// dynamic analysis
$returnValue = ApplicationProvider::getApp()->make('config')->get($argumentValue);

Check failure on line 55 in src/Handlers/Helpers/ConfigHandler.php

View workflow job for this annotation

GitHub Actions / Psalm

MixedAssignment

src/Handlers/Helpers/ConfigHandler.php:55:13: MixedAssignment: Unable to determine the type that $returnValue is being assigned to (see https://psalm.dev/032)

Check failure on line 55 in src/Handlers/Helpers/ConfigHandler.php

View workflow job for this annotation

GitHub Actions / Psalm

MixedMethodCall

src/Handlers/Helpers/ConfigHandler.php:55:75: MixedMethodCall: Cannot determine the type of the object on the left hand side of this expression (see https://psalm.dev/015)
} catch (\Throwable $t) {
return null;
}

// turn actual return value into a psalm type. there's probably a helper in psalm to do this, but i couldn't find one
switch (gettype($returnValue)) {
case 'boolean':
$type = new TBool();
break;
case 'integer':
$type = new TLiteralInt($returnValue);
break;
case 'double':
$type = new TLiteralFloat($returnValue);
break;
case 'string':
$type = new TLiteralString($returnValue);

Check failure on line 72 in src/Handlers/Helpers/ConfigHandler.php

View workflow job for this annotation

GitHub Actions / Psalm

InternalMethod

src/Handlers/Helpers/ConfigHandler.php:72:25: InternalMethod: Constructor Psalm\Type\Atomic\TLiteralString::__construct is internal to Psalm\Type::getAtomicStringFromLiteral, Psalm\Type\Atomic\TLiteralClassString::__construct, and Psalm\Type\Atomic\TLiteralString::make but called from Psalm\LaravelPlugin\Handlers\Helpers\ConfigHandler::getFunctionReturnType (see https://psalm.dev/175)
break;
case 'array':
$type = new TArray([
new Union([new TArrayKey()]),
new Union([new TMixed()]),
]);
break;
case 'object':
$type = new TNamedObject(get_class($returnValue));
break;
case 'resource':
$type = new TResource();
break;
case 'resource (closed)':
$type = new TClosedResource();
break;
case 'NULL':
if (isset($call_args[1])) {
return $event->getStatementsSource()->getNodeTypeProvider()->getType($call_args[1]->value);
}
$type = new TNull();
break;
case 'unknown type':
default:
$type = new TMixed();
break;
}

return new Union([
$type,
]);
}
}
3 changes: 3 additions & 0 deletions src/Plugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Psalm\LaravelPlugin\Handlers\Eloquent\ModelPropertyAccessorHandler;
use Psalm\LaravelPlugin\Handlers\Eloquent\ModelRelationshipPropertyHandler;
use Psalm\LaravelPlugin\Handlers\Eloquent\RelationsMethodHandler;
use Psalm\LaravelPlugin\Handlers\Helpers\ConfigHandler;
use Psalm\LaravelPlugin\Handlers\Helpers\CacheHandler;
use Psalm\LaravelPlugin\Handlers\Helpers\PathHandler;
use Psalm\LaravelPlugin\Handlers\Helpers\TransHandler;
Expand Down Expand Up @@ -135,6 +136,8 @@ private function registerHandlers(RegistrationInterface $registration): void

require_once 'Handlers/SuppressHandler.php';
$registration->registerHooksFromClass(SuppressHandler::class);
require_once 'Handlers/Helpers/ConfigHandler.php';
$registration->registerHooksFromClass(ConfigHandler::class);
}

private function generateStubFiles(): void
Expand Down
54 changes: 54 additions & 0 deletions tests/acceptance/ConfigTypes.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
Feature: Config helper
The global config helper will return a strict type

Background:
Given I have the following config
"""
<?xml version="1.0"?>
<psalm errorLevel="1">
<projectFiles>
<directory name="."/>
<ignoreFiles> <directory name="../../vendor"/> </ignoreFiles>
</projectFiles>
<plugins>
<pluginClass class="Psalm\LaravelPlugin\Plugin"/>
</plugins>
</psalm>
"""
And I have the following code preamble
"""
<?php declare(strict_types=1);

"""

Scenario: config with no arguments returns a repository instance
Given I have the following code
"""
function test(): \Illuminate\Config\Repository {
return config();
}
"""
When I run Psalm
Then I see no errors

Scenario: config with one argument
Given I have the following code
"""
function test(): string
{
return config('app.name');
}
"""
When I run Psalm
Then I see no errors

Scenario: config with first null argument and second argument provided
Given I have the following code
"""
function test(): bool
{
return config('app.non-existent', false);
}
"""
When I run Psalm
Then I see no errors
Loading