-
-
Notifications
You must be signed in to change notification settings - Fork 6
Fix CI pipeline: phpunit.xml validation warning, php-parser v4 test skip, and comprehensive type-analysis regression coverage #83
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
Changes from 23 commits
cdeace8
1fae5be
f4f4072
d48c560
d22f7dd
5180aa4
6bc14a7
75671f9
1d1d4fa
9d4fe95
5c3e936
e466f5f
d034f80
35bf8b8
3833dc7
37b9ace
f315024
6fed600
72f1c78
8f45db2
d5eb4ca
cfd4ad0
d01b6b8
8b03fb1
9433ca1
e95fe6f
29aa9bd
00ed1ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,12 +58,9 @@ | |
| $this->attributes = Utils::extractAttributesFromAstNode($node->attrGroups); | ||
| } | ||
|
|
||
| // PHP < 8.2 raises an uncatchable E_COMPILE_ERROR for certain PHP 8.2+ syntax | ||
| // (standalone true/false/null types, DNF types, readonly class). Similarly, | ||
| // PHP < 8.3 raises an error for PHP 8.3+ syntax (typed class constants). | ||
| // Skip autoloading in those cases; AST data is still read from the node below. | ||
| $canAutoload = (\PHP_VERSION_ID >= 80200 || !self::nodeUsesPHP82PlusSyntax($node)) | ||
| && (\PHP_VERSION_ID >= 80300 || !self::nodeUsesPHP83PlusSyntax($node)); | ||
| // Skip autoloading when the current runtime cannot safely compile newer syntax; | ||
| // AST data is still read from the node below. | ||
| $canAutoload = self::canAutoloadFromPhpNode($node); | ||
| $classExists = false; | ||
| if ($canAutoload) { | ||
| try { | ||
|
|
@@ -122,6 +119,8 @@ | |
| } | ||
| } | ||
|
|
||
| $this->addPromotedPropertiesFromConstructor($node); | ||
|
|
||
| if (!empty($node->implements)) { | ||
| foreach ($node->implements as $interfaceObject) { | ||
| $interfaceFQN = $interfaceObject->toString(); | ||
|
|
@@ -462,101 +461,80 @@ | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Returns true if the class node uses syntax that requires PHP 8.2+ and would | ||
| * cause an uncatchable E_COMPILE_ERROR when autoloaded on PHP < 8.2. | ||
| * | ||
| * @param Class_ $node | ||
| * | ||
| * @return bool | ||
| */ | ||
| private static function nodeUsesPHP82PlusSyntax(Class_ $node): bool | ||
| private function addPromotedPropertiesFromConstructor(Class_ $node): void | ||
| { | ||
| // readonly class is PHP 8.2+ | ||
| if ($node->isReadonly()) { | ||
| return true; | ||
| } | ||
| foreach ($node->getMethods() as $method) { | ||
| if ($method->name->name !== '__construct') { | ||
| continue; | ||
| } | ||
|
|
||
| foreach ($node->stmts as $stmt) { | ||
| if ($stmt instanceof \PhpParser\Node\Stmt\ClassMethod) { | ||
| if (self::containsPHP82PlusType($stmt->returnType)) { | ||
| return true; | ||
| foreach ($method->params as $parameter) { | ||
| if (!self::isPromotedParameter($parameter)) { | ||
| continue; | ||
| } | ||
| foreach ($stmt->params as $param) { | ||
| if (self::containsPHP82PlusType($param->type)) { | ||
| return true; | ||
| } | ||
|
|
||
| $parameterVar = $parameter->var; | ||
| if ( | ||
| !($parameterVar instanceof \PhpParser\Node\Expr\Variable) | ||
| || !\is_string($parameterVar->name) | ||
| ) { | ||
| continue; | ||
| } | ||
| } elseif ($stmt instanceof \PhpParser\Node\Stmt\Property) { | ||
| if (self::containsPHP82PlusType($stmt->type)) { | ||
| return true; | ||
|
|
||
| $promotedProperty = (new PHPProperty($this->parserContainer)) | ||
| ->readObjectFromPromotedParam($parameter, $this->name); | ||
|
|
||
| $propertyName = $parameterVar->name; | ||
| $existingProperty = $this->properties[$propertyName] ?? null; | ||
| if ($existingProperty !== null) { | ||
| $this->mergePromotedPropertyData($existingProperty, $promotedProperty, $parameter); | ||
|
|
||
| continue; | ||
| } | ||
|
|
||
| $this->properties[$propertyName] = $promotedProperty; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| break; | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of iterating through all methods to find the constructor, you can use the $method = $node->getMethod('__construct');
if ($method === null) {
return;
}
foreach ($method->params as $parameter) {
if (!self::isPromotedParameter($parameter)) {
continue;
}
$parameterVar = $parameter->var;
if (
!($parameterVar instanceof \PhpParser\Node\Expr\Variable)
|| !\is_string($parameterVar->name)
) {
continue;
}
$promotedProperty = (new PHPProperty($this->parserContainer))
->readObjectFromPromotedParam($parameter, $this->name);
$propertyName = $parameterVar->name;
$existingProperty = $this->properties[$propertyName] ?? null;
if ($existingProperty !== null) {
$this->mergePromotedPropertyData($existingProperty, $promotedProperty, $parameter);
continue;
}
$this->properties[$propertyName] = $promotedProperty;
} |
||
| } | ||
|
|
||
| /** | ||
| * Returns true if the class node uses syntax that requires PHP 8.3+ and would | ||
| * cause an uncatchable E_COMPILE_ERROR when autoloaded on PHP < 8.3. | ||
| * | ||
| * Covers: typed class constants (Stmt\ClassConst with a non-null type). | ||
| * | ||
| * @param Class_ $node | ||
| * | ||
| * @return bool | ||
| */ | ||
| private static function nodeUsesPHP83PlusSyntax(Class_ $node): bool | ||
| { | ||
| foreach ($node->stmts as $stmt) { | ||
| // Typed class constants are PHP 8.3+ | ||
| if ($stmt instanceof \PhpParser\Node\Stmt\ClassConst && $stmt->type !== null) { | ||
| return true; | ||
| } | ||
| private function mergePromotedPropertyData( | ||
|
Check failure on line 502 in src/voku/SimplePhpParser/Model/PHPClass.php
|
||
| PHPProperty $existingProperty, | ||
| PHPProperty $promotedProperty, | ||
| \PhpParser\Node\Param $parameter | ||
| ): void { | ||
| if ($existingProperty->access === '' && $promotedProperty->access !== '') { | ||
| $existingProperty->access = $promotedProperty->access; | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The condition if ($promotedProperty->access !== '') {
$existingProperty->access = $promotedProperty->access;
} |
||
|
|
||
| return false; | ||
| } | ||
| if ($existingProperty->type === null && $promotedProperty->type !== null) { | ||
| $existingProperty->type = $promotedProperty->type; | ||
| } | ||
|
|
||
| /** | ||
| * Returns true if the given type node is a PHP 8.2+ type that causes an | ||
| * uncatchable E_COMPILE_ERROR when loaded on PHP < 8.2. | ||
| * | ||
| * Covers: standalone true/false/null types and DNF types (union of intersections). | ||
| * | ||
| * @param \PhpParser\Node|null $typeNode | ||
| * | ||
| * @return bool | ||
| */ | ||
| private static function containsPHP82PlusType($typeNode): bool | ||
| { | ||
| if ($typeNode === null) { | ||
| return false; | ||
| if ($existingProperty->is_readonly === null && $promotedProperty->is_readonly !== null) { | ||
| $existingProperty->is_readonly = $promotedProperty->is_readonly; | ||
| } | ||
|
|
||
| // Standalone true, false, null as the *sole* type (not in a nullable like ?string) | ||
| // are PHP 8.2+ only. PHP-Parser represents these as Identifier nodes (not Name). | ||
| // Nullable null (?null) is syntactically invalid; NullableType wraps the inner type. | ||
| if ($typeNode instanceof \PhpParser\Node\Identifier) { | ||
| $name = \strtolower($typeNode->name); | ||
| return $name === 'true' || $name === 'false' || $name === 'null'; | ||
| if ($existingProperty->is_final === null && $promotedProperty->is_final !== null) { | ||
| $existingProperty->is_final = $promotedProperty->is_final; | ||
| } | ||
|
|
||
| // DNF types: union type containing an intersection type (PHP 8.2+) | ||
| if ($typeNode instanceof \PhpParser\Node\UnionType) { | ||
| foreach ($typeNode->types as $t) { | ||
| if ($t instanceof \PhpParser\Node\IntersectionType || self::containsPHP82PlusType($t)) { | ||
| return true; | ||
| } | ||
| } | ||
| if ($existingProperty->access_set === '' && $promotedProperty->access_set !== '') { | ||
| $existingProperty->access_set = $promotedProperty->access_set; | ||
| } | ||
|
|
||
| // Recurse into nullable type | ||
| if ($typeNode instanceof \PhpParser\Node\NullableType) { | ||
| return self::containsPHP82PlusType($typeNode->type); | ||
| if ($existingProperty->hooks === [] && $promotedProperty->hooks !== []) { | ||
| $existingProperty->hooks = $promotedProperty->hooks; | ||
| } | ||
|
|
||
| return false; | ||
| if ($existingProperty->attributes === [] && $promotedProperty->attributes !== []) { | ||
| $existingProperty->attributes = $promotedProperty->attributes; | ||
| } | ||
|
Comment on lines
+512
to
+530
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When merging promoted property data, the if ($existingProperty->is_readonly === null && $promotedProperty->is_readonly !== null) {
$existingProperty->is_readonly = $promotedProperty->is_readonly;
}
if ($existingProperty->is_final === null && $promotedProperty->is_final !== null) {
$existingProperty->is_final = $promotedProperty->is_final;
}
if ($existingProperty->access_set === '' && $promotedProperty->access_set !== '') {
$existingProperty->access_set = $promotedProperty->access_set;
}
if ($existingProperty->hooks === [] && $promotedProperty->hooks !== []) {
$existingProperty->hooks = $promotedProperty->hooks;
}
if ($existingProperty->attributes === [] && $promotedProperty->attributes !== []) {
$existingProperty->attributes = $promotedProperty->attributes;
} |
||
|
|
||
| if ($parameter->default !== null && $promotedProperty->typeFromDefaultValue !== null) { | ||
| $existingProperty->defaultValue = $promotedProperty->defaultValue; | ||
| $existingProperty->typeFromDefaultValue = $promotedProperty->typeFromDefaultValue; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -325,7 +325,7 @@ | |
| /** | ||
| * @throws \PHPStan\PhpDocParser\Parser\ParserException | ||
| */ | ||
| private function readPhpDocByTokens(string $docComment, string $parameterName): void | ||
|
Check failure on line 328 in src/voku/SimplePhpParser/Model/PHPParameter.php
|
||
| { | ||
| $tokens = Utils::modernPhpdocTokens($docComment); | ||
|
|
||
|
|
@@ -355,7 +355,19 @@ | |
| if (!$this->phpDocRaw) { | ||
| $this->phpDocRaw = $paramContent . ' ' . '$' . $parameterName; | ||
| } | ||
| $this->typeFromPhpDocExtended = Utils::modernPhpdoc($paramContent); | ||
| try { | ||
| $this->typeFromPhpDocExtended = Utils::modernPhpdoc($paramContent); | ||
| } catch (\PHPStan\PhpDocParser\Parser\ParserException $e) { | ||
| $recoveredType = Utils::recoverBrokenPhpdocType($paramContent); | ||
| if ($recoveredType !== null) { | ||
| $normalizedRecoveredType = Utils::normalizePhpType($recoveredType); | ||
| $this->typeFromPhpDoc = $this->typeFromPhpDoc ?? $normalizedRecoveredType; | ||
| $this->typeFromPhpDocSimple = $this->typeFromPhpDocSimple ?? $normalizedRecoveredType; | ||
| $this->typeFromPhpDocExtended = $recoveredType; | ||
| } | ||
|
|
||
| $this->addParseError($e); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The method $tmpErrorMessage = $parameterName . ':' . ($this->line ?? '?') . ' | ' . $e->getMessage();
$this->parseError[\md5($tmpErrorMessage)] = $tmpErrorMessage; |
||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a helper method to check for the
finalmodifier flag, as properties (including promoted ones) can now be marked as final in PHP 8.4.