Support for nikic/php-parser 5 and psalm 6 #563
Conversation
tugmaks
force-pushed
the
parser5-and-psalm6
branch
from
28270e2 to
de6aa37
Compare
| @@ -35,6 +35,7 @@ public function __invoke(Traversable $files): Traversable | |||
| } | |||
|
|
|||
| try { | |||
| /** @psalm-suppress PossiblyFalseArgument */ | |||
There was a problem hiding this comment.
We should not suppress here, but either expand baseline, or add a specific exception
| @@ -273,7 +272,7 @@ private function getComposerData(string $jsonFile): array | |||
| private function getASTFromFilesLocator(InputInterface $input): LocateASTFromFiles | |||
| { | |||
| $errorHandler = $input->getOption('ignore-parse-errors') ? new CollectingErrorHandler() : null; | |||
| $parser = (new ParserFactory())->create(ParserFactory::PREFER_PHP7, new Lexer()); | |||
| $parser = (new ParserFactory())->createForHostVersion(); | |||
There was a problem hiding this comment.
We should use the latest supported syntax, rather than the host syntax
| /** @var array<string, string> $requires */ | ||
| /** | ||
| * @var array<string, string> $requires | ||
| * @psalm-suppress PossiblyFalseArgument |
There was a problem hiding this comment.
We should not suppress: instead either add to baseline or add assertions
| if (property_exists($node, 'namespacedName')) { | ||
| $namespacedName = $node->namespacedName; | ||
| } | ||
| $isNamespacedNameInitialized = (new ReflectionProperty($node, 'namespacedName'))->isInitialized($node); |
There was a problem hiding this comment.
Accessing via reflection here is a no-go: we'll get immediate hidden BC breaks
There was a problem hiding this comment.
I do not know how to check here without reflection. Do you have any idea ?
There was a problem hiding this comment.
I would probably check if there's a better way to access this property: reading an uninitialized property indicates (to me) that we are doing something wrong when talking to the PHP-Parser API
There was a problem hiding this comment.
I'll try a solution locally and then merge, no need to touch this further, @tugmaks!
Thanks meanwhile for all the work done thus far! 💪
There was a problem hiding this comment.
After much digging, I 100% verified that this is the "API", which kinda sucks, but is an acceptable tradeoff, for now.
...serRequireCheckerTest/FileLocator/LocateComposerPackageDirectDependenciesSourceFilesTest.php
Outdated
Show resolved
Hide resolved
test/ComposerRequireCheckerTest/NodeVisitor/DefinedSymbolCollectorFunctionalTest.php
Outdated
Show resolved
Hide resolved
| if (property_exists($node, 'namespacedName')) { | ||
| $namespacedName = $node->namespacedName; | ||
| } | ||
| $isNamespacedNameInitialized = (new ReflectionProperty($node, 'namespacedName'))->isInitialized($node); |
There was a problem hiding this comment.
I would probably check if there's a better way to access this property: reading an uninitialized property indicates (to me) that we are doing something wrong when talking to the PHP-Parser API
| if (property_exists($node, 'namespacedName')) { | ||
| $namespacedName = $node->namespacedName; | ||
| } | ||
| $isNamespacedNameInitialized = (new ReflectionProperty($node, 'namespacedName'))->isInitialized($node); |
There was a problem hiding this comment.
I'll try a solution locally and then merge, no need to touch this further, @tugmaks!
Thanks meanwhile for all the work done thus far! 💪
Ocramius
added
enhancement
dependencies
|
Thanks @tugmaks! |
See https://github.com/nikic/PHP-Parser/blob/master/UPGRADE-5.0.md