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

Guard against crashes when only an entity's interface is known #155

Open
wants to merge 6 commits into
base: 1.3.x
Choose a base branch
from

Conversation

Firehed
Copy link

@Firehed Firehed commented Oct 22, 2020

I got a "please report this stack trace" error immediately following the extended configuration of this extension.

After a bit of digging while writing up the issue, including recovering a truncated stack trace, I found that this extension was trying to call $objectManager->getClassMetadata() with one of the interfaces that one of my entities implements, and Doctrine doesn't much care for you doing that. This change checks whether the class referenced is actually an interface and filters out further analysis since it a) crashes and b) isn't relevant for determining which fields are present on a Doctrine entity.

# phpstan.neon
includes:
    - phpstan-baseline.neon
    - vendor/phpstan/phpstan-deprecation-rules/rules.neon
    - vendor/phpstan/phpstan-doctrine/extension.neon
    - vendor/phpstan/phpstan-doctrine/rules.neon
    - vendor/phpstan/phpstan-phpunit/extension.neon
parameters:
    doctrine:
        objectManagerLoader: tests/phpstanObjectManagerLoader.php
# tests/phpstanObjectManagerLoader.php
<?php

use Doctrine\ORM\EntityManagerInterface;

require 'vendor/autoload.php';

$config = require 'config.php';
return $config->get(EntityManagerInterface::class);

This is a summary of the class hierarchy that triggered the error, though of course each interface and class is in its own autoloadable file:

namespace X\Y\G {
  interface NodeInterface { ... }
}
namespace X\Y\Z {
  interface ProductInterface extends \X\Y\G\NodeInterface
  {
    /* no new methods */ 
  }
  /**
   * @Entity
   * @Table(name="products")
   */
  class Product implements ProductInterface
  {
    // typical class definition, with doctrine php annotations
  }
}

While by the look of the stack trace it's some sort of Doctrine-generated issue, it seems like the debug output is being truncated.

This prevents a call to Doctrine's metadata factory with an interface, which throws an exception since the class in question is not an entity.
@ondrejmirtes
Copy link
Member

Hi,

  1. please show me the stack trace
  2. reproduce this problem in a test

@Firehed
Copy link
Author

Firehed commented Oct 22, 2020

Here's the stack trace:

Doctrine\Persistence\Mapping\MappingException: Class 'Slant\Normalizer\Model\Products\ProductInterface' does not exist in /repo/vendor/doctrine/persistence/lib/Doctrine/Persistence/Mapping/MappingException.php:93
Stack trace:
#0 /repo/vendor/doctrine/persistence/lib/Doctrine/Persistence/Mapping/RuntimeReflectionService.php(36): Doctrine\Persistence\Mapping\MappingException::nonExistingClass('Slant\\Normalize...')
#1 /repo/vendor/doctrine/persistence/lib/Doctrine/Persistence/Mapping/AbstractClassMetadataFactory.php(250): Doctrine\Persistence\Mapping\RuntimeReflectionService->getParentClasses('Slant\\Normalize...')
#2 /repo/vendor/doctrine/persistence/lib/Doctrine/Persistence/Mapping/AbstractClassMetadataFactory.php(286): Doctrine\Persistence\Mapping\AbstractClassMetadataFactory->getParentClasses('Slant\\Normalize...')
#3 /repo/vendor/doctrine/orm/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php(81): Doctrine\Persistence\Mapping\AbstractClassMetadataFactory->loadMetadata('Slant\\Normalize...')
#4 /repo/vendor/doctrine/persistence/lib/Doctrine/Persistence/Mapping/AbstractClassMetadataFactory.php(183): Doctrine\ORM\Mapping\ClassMetadataFactory->loadMetadata('Slant\\Normalize...')
#5 /repo/vendor/doctrine/orm/lib/Doctrine/ORM/EntityManager.php(287): Doctrine\Persistence\Mapping\AbstractClassMetadataFactory->getMetadataFor('Slant\\Normalize...')
#6 /repo/vendor/phpstan/phpstan-doctrine/src/Rules/Doctrine/ORM/RepositoryMethodCallRule.php(86): Doctrine\ORM\EntityManager->getClassMetadata('Slant\\Normalize...')
#7 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Analyser/FileAnalyser.php(71): PHPStan\Rules\Doctrine\ORM\RepositoryMethodCallRule->processNode(Object(PhpParser\Node\Expr\MethodCall), Object(PHPStan\Analyser\MutatingScope))
#8 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Analyser/NodeScopeResolver.php(423): PHPStan\Analyser\FileAnalyser->PHPStan\Analyser\{closure}(Object(PhpParser\Node\Expr\MethodCall), Object(PHPStan\Analyser\MutatingScope))
#9 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Analyser/NodeScopeResolver.php(340): PHPStan\Analyser\NodeScopeResolver->PHPStan\Analyser\{closure}(Object(PhpParser\Node\Expr\MethodCall), Object(PHPStan\Analyser\MutatingScope))
#10 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Analyser/NodeScopeResolver.php(1626): PHPStan\Analyser\NodeScopeResolver::PHPStan\Analyser\{closure}(Object(PhpParser\Node\Expr\MethodCall), Object(PHPStan\Analyser\MutatingScope))
#11 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Analyser/NodeScopeResolver.php(1114): PHPStan\Analyser\NodeScopeResolver->callNodeCallbackWithExpression(Object(Closure), Object(PhpParser\Node\Expr\MethodCall), Object(PHPStan\Analyser\MutatingScope), Object(PHPStan\Analyser\ExpressionContext))
#12 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Analyser/NodeScopeResolver.php(1824): PHPStan\Analyser\NodeScopeResolver->processExprNode(Object(PhpParser\Node\Expr\MethodCall), Object(PHPStan\Analyser\MutatingScope), Object(Closure), Object(PHPStan\Analyser\ExpressionContext))
#13 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Analyser/NodeScopeResolver.php(1297): PHPStan\Analyser\NodeScopeResolver->processArgs(Object(PHPStan\Reflection\ObjectTypeMethodReflection), Object(PHPStan\Reflection\Generic\ResolvedFunctionVariant), Array, Object(PHPStan\Analyser\MutatingScope), Object(Closure), Object(PHPStan\Analyser\ExpressionContext))
#14 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Analyser/NodeScopeResolver.php(375): PHPStan\Analyser\NodeScopeResolver->processExprNode(Object(PhpParser\Node\Expr\MethodCall), Object(PHPStan\Analyser\MutatingScope), Object(Closure), Object(PHPStan\Analyser\ExpressionContext))
#15 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Analyser/NodeScopeResolver.php(227): PHPStan\Analyser\NodeScopeResolver->processStmtNode(Object(PhpParser\Node\Stmt\Expression), Object(PHPStan\Analyser\MutatingScope), Object(Closure))
#16 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Analyser/NodeScopeResolver.php(345): PHPStan\Analyser\NodeScopeResolver->processStmtNodes(Object(PhpParser\Node\Stmt\ClassMethod), Array, Object(PHPStan\Analyser\MutatingScope), Object(Closure))
#17 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Analyser/NodeScopeResolver.php(227): PHPStan\Analyser\NodeScopeResolver->processStmtNode(Object(PhpParser\Node\Stmt\ClassMethod), Object(PHPStan\Analyser\MutatingScope), Object(Closure))
#18 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Analyser/NodeScopeResolver.php(483): PHPStan\Analyser\NodeScopeResolver->processStmtNodes(Object(PhpParser\Node\Stmt\Class_), Array, Object(PHPStan\Analyser\MutatingScope), Object(Closure))
#19 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Analyser/NodeScopeResolver.php(227): PHPStan\Analyser\NodeScopeResolver->processStmtNode(Object(PhpParser\Node\Stmt\Class_), Object(PHPStan\Analyser\MutatingScope), Object(Closure))
#20 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Analyser/NodeScopeResolver.php(386): PHPStan\Analyser\NodeScopeResolver->processStmtNodes(Object(PhpParser\Node\Stmt\Namespace_), Array, Object(PHPStan\Analyser\MutatingScope), Object(Closure))
#21 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Analyser/NodeScopeResolver.php(196): PHPStan\Analyser\NodeScopeResolver->processStmtNode(Object(PhpParser\Node\Stmt\Namespace_), Object(PHPStan\Analyser\MutatingScope), Object(Closure))
#22 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Analyser/FileAnalyser.php(147): PHPStan\Analyser\NodeScopeResolver->processNodes(Array, Object(PHPStan\Analyser\MutatingScope), Object(Closure))
#23 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Analyser/Analyser.php(51): PHPStan\Analyser\FileAnalyser->analyseFile('/Users/firehed/...', Array, Object(PHPStan\Rules\Registry), NULL)
#24 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Command/AnalyseApplication.php(155): PHPStan\Analyser\Analyser->analyse(Array, Object(Closure), NULL, true, Array)
#25 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Command/AnalyseApplication.php(79): PHPStan\Command\AnalyseApplication->runAnalyser(Array, Array, true, '/Users/firehed/...', Object(PHPStan\Command\Symfony\SymfonyOutput), Object(PHPStan\Command\Symfony\SymfonyOutput), Object(_HumbugBox3b5d526a7336\Symfony\Component\Console\Input\ArgvInput))
#26 phar:///repo/vendor/phpstan/phpstan/phpstan/src/Command/AnalyseCommand.php(141): PHPStan\Command\AnalyseApplication->analyse(Array, true, Object(PHPStan\Command\Symfony\SymfonyOutput), Object(PHPStan\Command\Symfony\SymfonyOutput), false, true, '/Users/firehed/...', Object(_HumbugBox3b5d526a7336\Symfony\Component\Console\Input\ArgvInput))
#27 phar:///repo/vendor/phpstan/phpstan/phpstan/vendor/symfony/console/Command/Command.php(228): PHPStan\Command\AnalyseCommand->execute(Object(_HumbugBox3b5d526a7336\Symfony\Component\Console\Input\ArgvInput), Object(_HumbugBox3b5d526a7336\Symfony\Component\Console\Output\ConsoleOutput))
#28 phar:///repo/vendor/phpstan/phpstan/phpstan/vendor/symfony/console/Application.php(848): _HumbugBox3b5d526a7336\Symfony\Component\Console\Command\Command->run(Object(_HumbugBox3b5d526a7336\Symfony\Component\Console\Input\ArgvInput), Object(_HumbugBox3b5d526a7336\Symfony\Component\Console\Output\ConsoleOutput))
#29 phar:///repo/vendor/phpstan/phpstan/phpstan/vendor/symfony/console/Application.php(235): _HumbugBox3b5d526a7336\Symfony\Component\Console\Application->doRunCommand(Object(PHPStan\Command\AnalyseCommand), Object(_HumbugBox3b5d526a7336\Symfony\Component\Console\Input\ArgvInput), Object(_HumbugBox3b5d526a7336\Symfony\Component\Console\Output\ConsoleOutput))
#30 phar:///repo/vendor/phpstan/phpstan/phpstan/vendor/symfony/console/Application.php(136): _HumbugBox3b5d526a7336\Symfony\Component\Console\Application->doRun(Object(_HumbugBox3b5d526a7336\Symfony\Component\Console\Input\ArgvInput), Object(_HumbugBox3b5d526a7336\Symfony\Component\Console\Output\ConsoleOutput))
#31 phar:///repo/vendor/phpstan/phpstan/phpstan/bin/phpstan(72): _HumbugBox3b5d526a7336\Symfony\Component\Console\Application->run()
#32 phar:///repo/vendor/phpstan/phpstan/phpstan/bin/phpstan(73): _HumbugBox3b5d526a7336\{closure}()
#33 /repo/vendor/phpstan/phpstan/phpstan(6): require('phar:///Users/f...')
#34 {main}

I'll try to reproduce in a test.

@Firehed Firehed changed the title Guard against crashes for entities which implement interfaces Guard against crashes when only an entity's interface is known Oct 22, 2020
@Firehed
Copy link
Author

Firehed commented Oct 22, 2020

I've added test cases which reproduce the original issue, which was slightly different from what I originally thought - the example in doFindByWithInterface is where the crash occurred prior to the change.

The issue was analyzing a repository of an unknown concrete class which implements a known interface, which works just fine at runtime but relies somewhat on convention (and perhaps the implied availability of column mapping from presence of getters or setters) rather than being provably correct via @Column mappings. The analysis tried to pull metadata for the interface rather than any/all of the implementations of it.

Unfortunately this implementation does mean there's some unknown state, which arguably could be a warning, but that's still better than outright crashing.

@Marmelatze
Copy link

This should be addressed by my PR in #141, right?

@Firehed
Copy link
Author

Firehed commented Nov 11, 2020

Unfortunately, it does not: I applied your patch to my working copy where I first encountered the issue, and the crash persists.

When I add some basic debugging output to your code, I never even reach the change you made - even when combining both patches. Something as simple as /** @return ObjectRepository<SomeInterface> */ will reproduce the crash.

I think our patches are solving slightly different (though both valid) issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants