Skip to content

Commit

Permalink
Rewrote TypeGenerator to not convert to string and back for each type
Browse files Browse the repository at this point in the history 
This massively reduces the internal complexity
of the `TypeGenerator`, since we convert
`ReflectionType` symbols **directly** to our `@internal`
data structures, ready to be rendered.

Before this, we would cast the whole `ReflectionType` to a `string` that
fits our need, and then we would go back to parsing that string, with
a substantial overhead (especially considering the newly introduced
validation rules around DNF types).

Note that this introduces new Psalm violations that were added to `psalm-baseline.xml`,
but which are solved by my work @ vimeo/psalm#8722

Signed-off-by: Marco Pivetta <ocramius@gmail.com>
  • Loading branch information
@Ocramius
Ocramius committed Dec 8, 2022
1 parent aa62c5f commit 83916dd
Show file tree
Hide file tree
Showing 8 changed files with 125 additions and 181 deletions.
82 changes: 27 additions & 55 deletions psalm-baseline.xml
View file
Expand Up @@ -308,6 +308,9 @@
</UnsafeInstantiation>
</file>
<file src="src/Generator/MethodGenerator.php">
<ArgumentTypeCoercion occurrences="1">
<code>$reflectionMethod-&gt;getReturnType()</code>
</ArgumentTypeCoercion>
<DeprecatedMethod occurrences="2">
<code>DocBlockGenerator::fromArray($value)</code>
<code>ParameterGenerator::fromArray($parameter)</code>
Expand Down Expand Up @@ -346,6 +349,9 @@
</UnsafeInstantiation>
</file>
<file src="src/Generator/ParameterGenerator.php">
<ArgumentTypeCoercion occurrences="1">
<code>$reflectionParameter-&gt;getType()</code>
</ArgumentTypeCoercion>
<MixedArgument occurrences="8">
<code>$array['name']</code>
<code>$value</code>
Expand Down Expand Up @@ -373,6 +379,9 @@
</UnsafeInstantiation>
</file>
<file src="src/Generator/PropertyGenerator.php">
<ArgumentTypeCoercion occurrences="1">
<code>$reflectionProperty-&gt;getType()</code>
</ArgumentTypeCoercion>
<DeprecatedMethod occurrences="1">
<code>DocBlockGenerator::fromArray($value)</code>
</DeprecatedMethod>
Expand Down Expand Up @@ -491,19 +500,29 @@
</UnusedVariable>
</file>
<file src="src/Generator/TypeGenerator.php">
<ImpureMethodCall occurrences="6">
<ArgumentTypeCoercion occurrences="1"/>
<ImpureMethodCall occurrences="3">
<code>allowsNull</code>
<code>getName</code>
<code>getName</code>
<code>getParentClass</code>
<code>getTypes</code>
<code>getTypes</code>
</ImpureMethodCall>
<InvalidArgument occurrences="1">
<code>$type</code>
</InvalidArgument>
<RedundantCondition occurrences="2">
<code>$atomicType !== 'null'</code>
<code>$atomicType-&gt;type !== 'mixed' &amp;&amp; $atomicType !== 'null'</code>
</RedundantCondition>
<RedundantConditionGivenDocblockType occurrences="1">
<code>$type instanceof ReflectionNamedType</code>
</RedundantConditionGivenDocblockType>
</file>
<file src="src/Generator/TypeGenerator/CompositeType.php">
<InvalidScalarArgument occurrences="1">
<code>$types</code>
</InvalidScalarArgument>
<file src="src/Generator/TypeGenerator/AtomicType.php">
<ImpureMethodCall occurrences="3">
<code>getName</code>
<code>getName</code>
<code>getParentClass</code>
</ImpureMethodCall>
</file>
<file src="src/Generator/ValueGenerator.php">
<DeprecatedMethod occurrences="1">
Expand Down Expand Up @@ -1029,9 +1048,6 @@
</UndefinedClass>
</file>
<file src="test/Generator/DocBlock/Tag/AuthorTagTest.php">
<InternalMethod occurrences="1">
<code>new TagManager()</code>
</InternalMethod>
<MissingReturnType occurrences="5">
<code>testConstructorWithOptions</code>
<code>testCreatingTagFromReflection</code>
Expand All @@ -1048,9 +1064,6 @@
</PossiblyNullPropertyAssignmentValue>
</file>
<file src="test/Generator/DocBlock/Tag/GenericTagTest.php">
<InternalMethod occurrences="1">
<code>new TagManager()</code>
</InternalMethod>
<MissingReturnType occurrences="4">
<code>testConstructorWithOptions</code>
<code>testCreatingTagFromReflection</code>
Expand All @@ -1066,9 +1079,6 @@
</PossiblyNullPropertyAssignmentValue>
</file>
<file src="test/Generator/DocBlock/Tag/LicenseTagTest.php">
<InternalMethod occurrences="1">
<code>new TagManager()</code>
</InternalMethod>
<MissingReturnType occurrences="5">
<code>testConstructorWithOptions</code>
<code>testCreatingTagFromReflection</code>
Expand All @@ -1085,9 +1095,6 @@
</PossiblyNullPropertyAssignmentValue>
</file>
<file src="test/Generator/DocBlock/Tag/MethodTagTest.php">
<InternalMethod occurrences="1">
<code>new TagManager()</code>
</InternalMethod>
<MissingReturnType occurrences="6">
<code>testConstructorWithOptions</code>
<code>testCreatingTagFromReflection</code>
Expand All @@ -1105,9 +1112,6 @@
</PossiblyNullPropertyAssignmentValue>
</file>
<file src="test/Generator/DocBlock/Tag/ParamTagTest.php">
<InternalMethod occurrences="1">
<code>new TagManager()</code>
</InternalMethod>
<MissingReturnType occurrences="6">
<code>testConstructorWithOptions</code>
<code>testCreatingTagFromReflection</code>
Expand All @@ -1125,9 +1129,6 @@
</PossiblyNullPropertyAssignmentValue>
</file>
<file src="test/Generator/DocBlock/Tag/PropertyTagTest.php">
<InternalMethod occurrences="1">
<code>new TagManager()</code>
</InternalMethod>
<MissingReturnType occurrences="6">
<code>testConstructorWithOptions</code>
<code>testCreatingTagFromReflection</code>
Expand All @@ -1145,9 +1146,6 @@
</PossiblyNullPropertyAssignmentValue>
</file>
<file src="test/Generator/DocBlock/Tag/ReturnTagTest.php">
<InternalMethod occurrences="1">
<code>new TagManager()</code>
</InternalMethod>
<MissingReturnType occurrences="3">
<code>testCreatingTagFromReflection</code>
<code>testNameIsCorrect</code>
Expand All @@ -1162,9 +1160,6 @@
</PossiblyNullPropertyAssignmentValue>
</file>
<file src="test/Generator/DocBlock/Tag/ThrowsTagTest.php">
<InternalMethod occurrences="1">
<code>new TagManager()</code>
</InternalMethod>
<MissingReturnType occurrences="3">
<code>testCreatingTagFromReflection</code>
<code>testNameIsCorrect</code>
Expand All @@ -1191,13 +1186,6 @@
<code>null</code>
</PossiblyNullPropertyAssignmentValue>
</file>
<file src="test/Generator/DocBlock/Tag/VarTagTest.php">
<InternalMethod occurrences="3">
<code>new TagManager()</code>
<code>setVariableName</code>
<code>setVariableName</code>
</InternalMethod>
</file>
<file src="test/Generator/DocBlockGeneratorTest.php">
<DeprecatedMethod occurrences="3">
<code>setDatatype</code>
Expand Down Expand Up @@ -1413,13 +1401,6 @@
</InvalidArgument>
</file>
<file src="test/Generator/TypeGenerator/CompositeTypeTest.php">
<InternalClass occurrences="1">
<code>CompositeType::fromString($typeString)</code>
</InternalClass>
<InternalMethod occurrences="2">
<code>CompositeType::fromString($typeString)</code>
<code>fullyQualifiedName</code>
</InternalMethod>
<MixedInferredReturnType occurrences="1">
<code>iterable</code>
</MixedInferredReturnType>
Expand Down Expand Up @@ -1466,9 +1447,6 @@
<DeprecatedMethod occurrences="1">
<code>setMethods</code>
</DeprecatedMethod>
<InternalMethod occurrences="1">
<code>new PrototypeClassFactory()</code>
</InternalMethod>
<MissingReturnType occurrences="3">
<code>testAddAndGetPrototype</code>
<code>testFallBackToGeneric</code>
Expand Down Expand Up @@ -1872,12 +1850,6 @@
</UndefinedInterfaceMethod>
</file>
<file src="test/Scanner/DocBlockScannerTest.php">
<InternalMethod occurrences="4">
<code>new DocBlockScanner($docComment)</code>
<code>new DocBlockScanner($docComment)</code>
<code>new DocBlockScanner($docComment)</code>
<code>new DocBlockScanner($docComment)</code>
</InternalMethod>
<MissingReturnType occurrences="3">
<code>testDocBlockScannerDescriptions</code>
<code>testDocBlockScannerParsesTagsWithNoValuesProperly</code>
Expand Down
25 changes: 4 additions & 21 deletions psalm.xml
View file
Expand Up @@ -29,34 +29,17 @@
<!-- the scanner internal component can be used by package internals -->
<referencedClass name="Laminas\Code\Generic\Prototype\PrototypeClassFactory"/>
<referencedClass name="Laminas\Code\Scanner\DocBlockScanner"/>
<!-- our tests often verify `@internal` details of our library, by design -->
<directory name="test"/>
</errorLevel>
</InternalClass>

<InternalMethod>
<errorLevel type="suppress">
<referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::method"/>
<referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::willReturn"/>
<referencedMethod name="PHPUnit\Framework\MockObject\Builder\InvocationMocker::with"/>
</errorLevel>

<!-- the scanner internal component can be used by package internals -->
<errorLevel type="suppress">
<referencedMethod name="Laminas\Code\Generic\Prototype\PrototypeClassFactory::addPrototype"/>
<referencedMethod name="Laminas\Code\Generic\Prototype\PrototypeClassFactory::getClonedPrototype"/>
<referencedMethod name="Laminas\Code\Generic\Prototype\PrototypeClassFactory::setGenericPrototype"/>
<referencedMethod name="Laminas\Code\Generic\Prototype\PrototypeInterface::getName"/>
<referencedMethod name="Laminas\Code\Scanner\DocBlockScanner::getLongDescription"/>
<referencedMethod name="Laminas\Code\Scanner\DocBlockScanner::getShortDescription"/>
<referencedMethod name="Laminas\Code\Scanner\DocBlockScanner::getTags"/>
<!-- our tests often verify `@internal` details of our library, by design -->
<directory name="test"/>
</errorLevel>
</InternalMethod>

<UndefinedAttributeClass>
<errorLevel type="suppress">
<!-- This class has been added in PHP 8.1 and Psalm does not know it yet. -->
<referencedClass name="ReturnTypeWillChange"/>
</errorLevel>
</UndefinedAttributeClass>
</issueHandlers>
<plugins>
<pluginClass class="Psalm\PhpUnitPlugin\Plugin"/>
Expand Down

0 comments on commit 83916dd

Please sign in to comment.