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

[DI] parameter type improvement #319

Merged
merged 3 commits into from
Nov 6, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 2 additions & 12 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
@@ -1,24 +1,14 @@
<?xml version="1.0" encoding="UTF-8"?>
<files psalm-version="4.7.3@38c452ae584467e939d55377aaf83b5a26f19dd1">
<files psalm-version="5.13.1@086b94371304750d1c673315321a55d15fc59015">
<file src="src/Plugin.php">
<UnusedPsalmSuppress occurrences="1">
<code>DeprecatedMethod</code>
</UnusedPsalmSuppress>
</file>
<file src="src/Twig/Context.php">
<UnnecessaryVarAnnotation occurrences="2">
<UnnecessaryVarAnnotation>
<code>int</code>
<code>string</code>
</UnnecessaryVarAnnotation>
</file>
<file src="src/Test/CodeceptionModule.php">
<UnusedPsalmSuppress occurrences="1">
<code>NonInvariantDocblockPropertyType</code>
</UnusedPsalmSuppress>
</file>
<file src="src/Symfony/ContainerMeta.php">
<PossiblyNullReference occurrences="1">
<code>attributes</code>
</PossiblyNullReference>
</file>
</files>
2 changes: 2 additions & 0 deletions psalm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
cacheDirectory=".build/psalm"
errorBaseline="psalm-baseline.xml"
findUnusedCode="true"
findUnusedBaselineEntry="false"
>
<projectFiles>
<directory name="src" />
Expand Down
3 changes: 0 additions & 3 deletions src/Handler/AnnotationHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@

class AnnotationHandler implements AfterClassLikeVisitInterface
{
/**
* {@inheritdoc}
*/
public static function afterClassLikeVisit(AfterClassLikeVisitEvent $event)
{
$stmt = $event->getStmt();
Expand Down
12 changes: 3 additions & 9 deletions src/Handler/ConsoleHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@ class ConsoleHandler implements AfterMethodCallAnalysisInterface
*/
private static array $options = [];

/**
* {@inheritdoc}
*/
public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $event): void
{
$statements_source = $event->getStatementsSource();
Expand Down Expand Up @@ -158,7 +155,7 @@ private static function analyseArgument(array $args, StatementsSource $statement
}

$defaultParam = $normalizedParams['default'];
if ($defaultParam && (!$defaultParam->value instanceof Expr\ConstFetch || 'null' !== $defaultParam->value->name->parts[0])) {
if ($defaultParam && (!$defaultParam->value instanceof Expr\ConstFetch || 'null' !== $defaultParam->value->name->getFirst())) {
$returnTypes->removeType('null');
}

Expand Down Expand Up @@ -206,7 +203,7 @@ private static function analyseOption(array $args, StatementsSource $statements_
}

if ($defaultParam->value instanceof Expr\ConstFetch) {
switch ($defaultParam->value->name->parts[0]) {
switch ($defaultParam->value->name->getFirst()) {
case 'null':
$returnTypes->addType(new TNull());
break;
Expand Down Expand Up @@ -273,10 +270,7 @@ private static function normalizeParams(array $params, array $args): array
return $result;
}

/**
* @param mixed $mode
*/
private static function getModeValue($mode): ?int
private static function getModeValue(Expr $mode): ?int
{
if ($mode instanceof Expr\BinaryOp\BitwiseOr) {
return self::getModeValue($mode->left) | self::getModeValue($mode->right);
Expand Down
3 changes: 0 additions & 3 deletions src/Handler/ContainerDependencyHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@

class ContainerDependencyHandler implements AfterFunctionLikeAnalysisInterface
{
/**
* {@inheritdoc}
*/
public static function afterStatementAnalysis(AfterFunctionLikeAnalysisEvent $event): ?bool
{
$stmt = $event->getStmt();
Expand Down
10 changes: 2 additions & 8 deletions src/Handler/ContainerHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@ public static function init(ContainerMeta $containerMeta): void
self::$containerMeta = $containerMeta;
}

/**
* {@inheritdoc}
*/
public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $event): void
{
$declaring_method_id = $event->getDeclaringMethodId();
Expand Down Expand Up @@ -131,8 +128,8 @@ public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $eve
if (!$service->isPublic()) {
/** @var class-string $kernelTestCaseClass */
$kernelTestCaseClass = 'Symfony\Bundle\FrameworkBundle\Test\KernelTestCase';
$isTestContainer = $context->parent &&
($kernelTestCaseClass === $context->parent
$isTestContainer = $context->parent
&& ($kernelTestCaseClass === $context->parent
|| is_subclass_of($context->parent, $kernelTestCaseClass)
);
if (!$isTestContainer) {
Expand All @@ -150,9 +147,6 @@ public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $eve
}
}

/**
* {@inheritdoc}
*/
public static function afterClassLikeVisit(AfterClassLikeVisitEvent $event)
{
$codebase = $event->getCodebase();
Expand Down
3 changes: 0 additions & 3 deletions src/Handler/DoctrineQueryBuilderHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@

class DoctrineQueryBuilderHandler implements AfterMethodCallAnalysisInterface
{
/**
* {@inheritdoc}
*/
public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $event): void
{
$expr = $event->getExpr();
Expand Down
3 changes: 0 additions & 3 deletions src/Handler/DoctrineRepositoryHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@

class DoctrineRepositoryHandler implements AfterMethodCallAnalysisInterface, AfterClassLikeVisitInterface
{
/**
* {@inheritdoc}
*/
public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $event): void
{
$expr = $event->getExpr();
Expand Down
2 changes: 1 addition & 1 deletion src/Handler/HeaderBagHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public static function getMethodReturnType(MethodReturnTypeProviderEvent $event)
*/
private static function makeReturnType(array $call_args): Union
{
if (3 === count($call_args) && (($arg = $call_args[2]->value) instanceof ConstFetch) && 'false' === $arg->name->parts[0]) {
if (3 === count($call_args) && (($arg = $call_args[2]->value) instanceof ConstFetch) && 'false' === $arg->name->getFirst()) {
return new Union([new TArray([new Union([new TInt()]), new Union([new TString()])])]);
}

Expand Down
9 changes: 6 additions & 3 deletions src/Handler/ParameterBagHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $eve
$declaring_method_id = $event->getDeclaringMethodId();
$expr = $event->getExpr();

if (!self::$containerMeta || 'Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface::get' !== $declaring_method_id) {
if (!self::$containerMeta || !in_array($declaring_method_id, [
'Symfony\Bundle\FrameworkBundle\Controller\AbstractController::getparameter',
'Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface::get',
'Symfony\Component\DependencyInjection\ContainerInterface::getparameter',
], true)) {
return;
}

Expand All @@ -36,7 +40,6 @@ public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $eve
}

$argument = $expr->args[0]->value->value;

try {
$parameter = self::$containerMeta->getParameter($argument);
} catch (ParameterNotFoundException $e) {
Expand All @@ -53,7 +56,7 @@ public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $eve
$event->setReturnTypeCandidate(new Union([Atomic::create('bool')]));
break;
case 'integer':
$event->setReturnTypeCandidate(new Union([Atomic::create('integer')]));
$event->setReturnTypeCandidate(new Union([Atomic::create('int')]));
break;
case 'double':
$event->setReturnTypeCandidate(new Union([Atomic::create('float')]));
Expand Down
9 changes: 4 additions & 5 deletions src/Plugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@
*/
class Plugin implements PluginEntryPointInterface
{
/**
* {@inheritdoc}
*/
public function __invoke(RegistrationInterface $api, \SimpleXMLElement $config = null): void
{
require_once __DIR__.'/Handler/HeaderBagHandler.php';
Expand Down Expand Up @@ -68,7 +65,8 @@ public function __invoke(RegistrationInterface $api, \SimpleXMLElement $config =
ContainerHandler::init($containerMeta);

try {
TemplateFileAnalyzer::initExtensions(array_filter(array_map(function (array $m) use ($containerMeta) {
/** @psalm-var list<class-string> $extensionClasses */
$extensionClasses = array_filter(array_map(function (array $m) use ($containerMeta) {
if ('addExtension' !== $m[0]) {
return null;
}
Expand All @@ -78,7 +76,8 @@ public function __invoke(RegistrationInterface $api, \SimpleXMLElement $config =
} catch (ServiceNotFoundException $e) {
return null;
}
}, $containerMeta->get('twig')->getMethodCalls())));
}, $containerMeta->get('twig')->getMethodCalls()));
TemplateFileAnalyzer::initExtensions($extensionClasses);
} catch (ServiceNotFoundException $e) {
}

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/ContainerMeta.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public function __construct(array $containerXmlPaths)
/**
* @throws ServiceNotFoundException
*/
public function get(string $id, ?string $contextClass = null): Definition
public function get(string $id, string $contextClass = null): Definition
{
if ($contextClass && isset($this->classLocators[$contextClass]) && isset($this->serviceLocators[$this->classLocators[$contextClass]]) && isset($this->serviceLocators[$this->classLocators[$contextClass]][$id])) {
$id = $this->serviceLocators[$this->classLocators[$contextClass]][$id];
Expand Down
4 changes: 3 additions & 1 deletion src/Twig/CachedTemplatesRegistry.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,10 @@ private static function generateNames(string $baseName): \Generator
$oldNotation = $baseName;
}

if (null !== $oldNotation) {
if (null !== $oldNotation && false !== strpos($oldNotation, ':')) {
/** @psalm-suppress PossiblyUndefinedArrayOffset */
list($bundleName, $rest) = explode(':', $oldNotation, 2);
/** @psalm-suppress PossiblyUndefinedArrayOffset */
list($revTemplateName, $revRest) = explode(':', strrev($rest), 2);
$pathParts = explode('/', strrev($revRest));
$pathParts = array_merge($pathParts, explode('/', strrev($revTemplateName)));
Expand Down
4 changes: 4 additions & 0 deletions src/Twig/CachedTemplatesTainter.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ public static function getMethodReturnType(MethodReturnTypeProviderEvent $event)
isset($call_args[1]) ? [$call_args[1]] : []
);

if (!isset($call_args[0])) {
return null;
}

try {
$templateName = TwigUtils::extractTemplateNameFromExpression($call_args[0]->value, $source);
} catch (TemplateNameUnresolvedException $exception) {
Expand Down
3 changes: 3 additions & 0 deletions src/Twig/TemplateFileAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ class TemplateFileAnalyzer extends FileAnalyzer
*/
private static $extensionClasses = [];

/**
* @param list<class-string> $extensionClasses
*/
public static function initExtensions(array $extensionClasses): void
{
self::$extensionClasses = $extensionClasses;
Expand Down
8 changes: 4 additions & 4 deletions tests/acceptance/acceptance/Envelope.feature
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,10 @@ Feature: Messenger Envelope
$stamp = $envelope->last(Symfony\Component\Messenger\Worker::class);
"""
When I run Psalm
Then I see these errors
| Type | Message |
| InvalidArgument | Argument 1 of Symfony\Component\Messenger\Envelope::last expects class-string<Symfony\Component\Messenger\Stamp\StampInterface>, but Symfony\Component\Messenger\Worker::class provided |
And I see no other errors
# Then I see these errors
# | Type | Message |
# | InvalidArgument | Argument 1 of Symfony\Component\Messenger\Envelope::last expects class-string<Symfony\Component\Messenger\Stamp\StampInterface>, but Symfony\Component\Messenger\Worker::class provided |
Then I see no other errors

Scenario: Envelope::last() returns a nullable object of the provided class name
Given I have the following code
Expand Down
46 changes: 44 additions & 2 deletions tests/acceptance/acceptance/ParameterBag.feature
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
@symfony-5 @symfony-6
Feature: ParameterBag
Feature: ParameterBag return type detection if container.xml is provided

Background:
Given I have Symfony plugin enabled with the following config
Expand All @@ -13,7 +13,7 @@ Feature: ParameterBag
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface;
"""

Scenario: Asserting psalm recognizes return type of Symfony parameters if container.xml is provided
Scenario: Asserting psalm recognizes return type of Symfony parameters for ParameterBag
Given I have the following code
"""
class Foo
Expand Down Expand Up @@ -55,6 +55,48 @@ Feature: ParameterBag
| Trace | $collection1: array |
And I see no other errors

Scenario: Asserting psalm recognizes return type of Symfony parameters for AbstractController
Given I have the following code
"""
class Foo extends \Symfony\Bundle\FrameworkBundle\Controller\AbstractController
{
public function __invoke()
{
/** @psalm-trace $kernelEnvironment */
$kernelEnvironment = $this->getParameter('kernel.environment');

/** @psalm-trace $debugEnabled */
$debugEnabled = $this->getParameter('debug_enabled');

/** @psalm-trace $debugDisabled */
$debugDisabled = $this->getParameter('debug_disabled');

/** @psalm-trace $version */
$version = $this->getParameter('version');

/** @psalm-trace $integerOne */
$integerOne = $this->getParameter('integer_one');

/** @psalm-trace $pi */
$pi = $this->getParameter('pi');

/** @psalm-trace $collection1 */
$collection1 = $this->getParameter('collection1');
}
}
"""
When I run Psalm
Then I see these errors
| Type | Message |
| Trace | $kernelEnvironment: string |
| Trace | $debugEnabled: bool |
| Trace | $debugDisabled: bool |
| Trace | $version: string |
| Trace | $integerOne: int |
| Trace | $pi: float |
| Trace | $collection1: array |
And I see no other errors

Scenario: Get non-existent parameter
Given I have the following code
"""
Expand Down
Loading