Skip to content

Commit

Permalink
misc: refactor node builders stack
Browse files Browse the repository at this point in the history
The following changes aim to decrease the complexity of the node
builders architecture, increasing overall performance and decreasing
the stack depth:

- Remove `StrictNodeBuilder` and put the logic in more adapted places.
- Remove `CasterProxyNodeBuilder` which was called for every node and
  would check if the node's type would accept the current value which
  could result in huge performance loss when working with iterables.
- Rename `CasterNodeBuilder` to `TypeNodeBuilder` and change its way
  to find the proper node builder to use.
  • Loading branch information
romm committed Jan 16, 2025
1 parent a233499 commit 4a6bbaf
Show file tree
Hide file tree
Showing 31 changed files with 300 additions and 286 deletions.
2 changes: 1 addition & 1 deletion qa/PHPStan/Extension/ApiAndInternalAnnotationCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public function processNode(Node $node, Scope $scope): array
return [
RuleErrorBuilder::message(
'Missing annotation `@api` or `@internal`.'
)->identifier('valinor.api_or_internal_annotation')->build(),
)->identifier('valinor.apiOrInternalAnnotation')->build(),
];
}

Expand Down
45 changes: 15 additions & 30 deletions src/Library/Container.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@
use CuyZ\Valinor\Mapper\Object\Factory\StrictTypesObjectBuilderFactory;
use CuyZ\Valinor\Mapper\Object\ObjectBuilder;
use CuyZ\Valinor\Mapper\Tree\Builder\ArrayNodeBuilder;
use CuyZ\Valinor\Mapper\Tree\Builder\CasterNodeBuilder;
use CuyZ\Valinor\Mapper\Tree\Builder\CasterProxyNodeBuilder;
use CuyZ\Valinor\Mapper\Tree\Builder\TypeNodeBuilder;
use CuyZ\Valinor\Mapper\Tree\Builder\InterfaceNodeBuilder;
use CuyZ\Valinor\Mapper\Tree\Builder\IterableNodeBuilder;
use CuyZ\Valinor\Mapper\Tree\Builder\ListNodeBuilder;
Expand All @@ -39,7 +38,8 @@
use CuyZ\Valinor\Mapper\Tree\Builder\RootNodeBuilder;
use CuyZ\Valinor\Mapper\Tree\Builder\ScalarNodeBuilder;
use CuyZ\Valinor\Mapper\Tree\Builder\ShapedArrayNodeBuilder;
use CuyZ\Valinor\Mapper\Tree\Builder\StrictNodeBuilder;
use CuyZ\Valinor\Mapper\Tree\Builder\MixedNodeBuilder;
use CuyZ\Valinor\Mapper\Tree\Builder\UndefinedObjectNodeBuilder;
use CuyZ\Valinor\Mapper\Tree\Builder\UnionNodeBuilder;
use CuyZ\Valinor\Mapper\Tree\Builder\ValueAlteringNodeBuilder;
use CuyZ\Valinor\Mapper\TreeMapper;
Expand All @@ -52,18 +52,9 @@
use CuyZ\Valinor\Normalizer\Transformer\KeyTransformersHandler;
use CuyZ\Valinor\Normalizer\Transformer\RecursiveTransformer;
use CuyZ\Valinor\Normalizer\Transformer\ValueTransformersHandler;
use CuyZ\Valinor\Type\ObjectType;
use CuyZ\Valinor\Type\Parser\Factory\LexingTypeParserFactory;
use CuyZ\Valinor\Type\Parser\Factory\TypeParserFactory;
use CuyZ\Valinor\Type\Parser\TypeParser;
use CuyZ\Valinor\Type\ScalarType;
use CuyZ\Valinor\Type\Types\ArrayType;
use CuyZ\Valinor\Type\Types\IterableType;
use CuyZ\Valinor\Type\Types\ListType;
use CuyZ\Valinor\Type\Types\NonEmptyArrayType;
use CuyZ\Valinor\Type\Types\NonEmptyListType;
use CuyZ\Valinor\Type\Types\NullType;
use CuyZ\Valinor\Type\Types\ShapedArrayType;
use Psr\SimpleCache\CacheInterface;

use function array_keys;
Expand Down Expand Up @@ -99,26 +90,21 @@ public function __construct(Settings $settings)
),

NodeBuilder::class => function () use ($settings) {
$listNodeBuilder = new ListNodeBuilder();
$arrayNodeBuilder = new ArrayNodeBuilder();

$builder = new CasterNodeBuilder([
ListType::class => $listNodeBuilder,
NonEmptyListType::class => $listNodeBuilder,
ArrayType::class => $arrayNodeBuilder,
NonEmptyArrayType::class => $arrayNodeBuilder,
IterableType::class => $arrayNodeBuilder,
ShapedArrayType::class => new ShapedArrayNodeBuilder(),
ScalarType::class => new ScalarNodeBuilder(),
NullType::class => new NullNodeBuilder(),
ObjectType::class => new ObjectNodeBuilder(
$builder = new TypeNodeBuilder(
new ArrayNodeBuilder(),
new ListNodeBuilder(),
new ShapedArrayNodeBuilder(),
new ScalarNodeBuilder(),
new UnionNodeBuilder(),
new NullNodeBuilder(),
new MixedNodeBuilder(),
new UndefinedObjectNodeBuilder(),
new ObjectNodeBuilder(
$this->get(ClassDefinitionRepository::class),
$this->get(ObjectBuilderFactory::class),
$settings->exceptionFilter,
),
]);

$builder = new UnionNodeBuilder($builder);
);

$builder = new InterfaceNodeBuilder(
$builder,
Expand All @@ -131,7 +117,6 @@ public function __construct(Settings $settings)
$settings->exceptionFilter,
);

$builder = new CasterProxyNodeBuilder($builder);
$builder = new IterableNodeBuilder($builder);

if (count($settings->valueModifier) > 0) {
Expand All @@ -144,7 +129,7 @@ public function __construct(Settings $settings)
);
}

return new StrictNodeBuilder($builder);
return $builder;
},

ObjectImplementations::class => fn () => new ObjectImplementations(
Expand Down
10 changes: 4 additions & 6 deletions src/Mapper/Object/Exception/PermissiveTypeNotAllowed.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,18 @@

namespace CuyZ\Valinor\Mapper\Object\Exception;

use CuyZ\Valinor\Mapper\Object\Argument;
use CuyZ\Valinor\Mapper\Object\ObjectBuilder;
use CuyZ\Valinor\Utility\PermissiveTypeFound;
use CuyZ\Valinor\Type\Type;
use LogicException;

/** @internal */
final class PermissiveTypeNotAllowed extends LogicException
{
public function __construct(ObjectBuilder $builder, Argument $argument, PermissiveTypeFound $original)
public function __construct(string $argumentSignature, Type $permissiveType)
{
parent::__construct(
"Error for `{$argument->name()}` in `{$builder->signature()}`: {$original->getMessage()}",
"The type of `$argumentSignature` contains `{$permissiveType->toString()}`, which is not " .
"allowed in strict mode. If really needed, the `allowPermissiveTypes` setting can be used.",
1655389255,
$original
);
}
}
27 changes: 20 additions & 7 deletions src/Mapper/Object/Factory/StrictTypesObjectBuilderFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@

use CuyZ\Valinor\Definition\ClassDefinition;
use CuyZ\Valinor\Mapper\Object\Exception\PermissiveTypeNotAllowed;
use CuyZ\Valinor\Utility\PermissiveTypeFound;
use CuyZ\Valinor\Utility\TypeHelper;
use CuyZ\Valinor\Type\CompositeType;
use CuyZ\Valinor\Type\Type;
use CuyZ\Valinor\Type\Types\MixedType;
use CuyZ\Valinor\Type\Types\UndefinedObjectType;

/** @internal */
final class StrictTypesObjectBuilderFactory implements ObjectBuilderFactory
Expand All @@ -22,14 +24,25 @@ public function for(ClassDefinition $class): array
$arguments = $builder->describeArguments();

foreach ($arguments as $argument) {
try {
TypeHelper::checkPermissiveType($argument->type());
} catch (PermissiveTypeFound $exception) {
throw new PermissiveTypeNotAllowed($builder, $argument, $exception);
}
$argumentSignature = $builder->signatureForArgument($argument->name());

$this->checkPresenceOfPermissiveType($argumentSignature, $argument->type());
}
}

return $builders;
}

private function checkPresenceOfPermissiveType(string $signature, Type $type): void
{
if ($type instanceof CompositeType) {
foreach ($type->traverse() as $subType) {
self::checkPresenceOfPermissiveType($signature, $subType);
}
}

if ($type instanceof MixedType || $type instanceof UndefinedObjectType) {
throw new PermissiveTypeNotAllowed($signature, $type);
}
}
}
5 changes: 5 additions & 0 deletions src/Mapper/Object/FunctionObjectBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,9 @@ public function signature(): string
{
return $this->function->definition->signature;
}

public function signatureForArgument(string $argumentName): string
{
return $this->function->definition->parameters->get($argumentName)->signature;
}
}
5 changes: 5 additions & 0 deletions src/Mapper/Object/MethodObjectBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,9 @@ public function signature(): string
{
return "$this->className::$this->methodName()";
}

public function signatureForArgument(string $argumentName): string
{
return $this->parameters->get($argumentName)->signature;
}
}
5 changes: 5 additions & 0 deletions src/Mapper/Object/NativeConstructorObjectBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,9 @@ public function signature(): string
{
return $this->class->methods->constructor()->signature;
}

public function signatureForArgument(string $argumentName): string
{
return $this->class->methods->constructor()->parameters->get($argumentName)->signature;
}
}
5 changes: 5 additions & 0 deletions src/Mapper/Object/NativeEnumObjectBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,9 @@ public function signature(): string
{
return $this->enum->readableSignature();
}

public function signatureForArgument(string $argumentName): string
{
return $this->enum->className() . '::$value';
}
}
8 changes: 8 additions & 0 deletions src/Mapper/Object/ObjectBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,13 @@ public function describeArguments(): Arguments;
*/
public function build(array $arguments): object;

/**
* @return non-empty-string
*/
public function signature(): string;

/**
* @return non-empty-string
*/
public function signatureForArgument(string $argumentName): string;
}
5 changes: 5 additions & 0 deletions src/Mapper/Object/ReflectionObjectBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,9 @@ public function signature(): string
{
return $this->class->name . ' (properties)';
}

public function signatureForArgument(string $argumentName): string
{
return $this->class->properties->get($argumentName)->signature;
}
}
30 changes: 0 additions & 30 deletions src/Mapper/Tree/Builder/CasterNodeBuilder.php

This file was deleted.

55 changes: 0 additions & 55 deletions src/Mapper/Tree/Builder/CasterProxyNodeBuilder.php

This file was deleted.

24 changes: 24 additions & 0 deletions src/Mapper/Tree/Builder/MixedNodeBuilder.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

declare(strict_types=1);

namespace CuyZ\Valinor\Mapper\Tree\Builder;

use CuyZ\Valinor\Mapper\Tree\Exception\CannotMapToPermissiveType;
use CuyZ\Valinor\Mapper\Tree\Shell;
use CuyZ\Valinor\Type\Types\MixedType;

/** @internal */
final class MixedNodeBuilder implements NodeBuilder
{
public function build(Shell $shell, RootNodeBuilder $rootBuilder): TreeNode
{
assert($shell->type() instanceof MixedType);

if (! $shell->allowPermissiveTypes()) {
throw new CannotMapToPermissiveType($shell);
}

return TreeNode::leaf($shell, $shell->value());
}
}
4 changes: 4 additions & 0 deletions src/Mapper/Tree/Builder/ObjectNodeBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ public function build(Shell $shell, RootNodeBuilder $rootBuilder): TreeNode
// @infection-ignore-all
assert($type instanceof ObjectType);

if ($type->accepts($shell->value())) {
return TreeNode::leaf($shell, $shell->value());
}

if ($shell->enableFlexibleCasting() && $shell->value() === null) {
$shell = $shell->withValue([]);
}
Expand Down
9 changes: 9 additions & 0 deletions src/Mapper/Tree/Builder/RootNodeBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace CuyZ\Valinor\Mapper\Tree\Builder;

use CuyZ\Valinor\Mapper\Tree\Exception\MissingNodeValue;
use CuyZ\Valinor\Mapper\Tree\Shell;

/** @internal */
Expand All @@ -13,6 +14,14 @@ public function __construct(private NodeBuilder $root) {}

public function build(Shell $shell): TreeNode
{
if (! $shell->hasValue()) {
if (! $shell->enableFlexibleCasting()) {
return TreeNode::error($shell, new MissingNodeValue($shell->type()));
}

$shell = $shell->withValue(null);
}

return $this->root->build($shell, $this);
}
}
4 changes: 4 additions & 0 deletions src/Mapper/Tree/Builder/ScalarNodeBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ public function build(Shell $shell, RootNodeBuilder $rootBuilder): TreeNode

assert($type instanceof ScalarType);

if ($type->accepts($value)) {
return TreeNode::leaf($shell, $value);
}

if (! $shell->enableFlexibleCasting() || ! $type->canCast($value)) {
return TreeNode::error($shell, $type->errorMessage());
}
Expand Down
Loading

0 comments on commit 4a6bbaf

Please sign in to comment.