Skip to content

Commit

Permalink
feat: option to generate code with strict types (#180)
Browse files Browse the repository at this point in the history
PHP, by default, will coerce values of the wrong type into the expected
scalar type declaration if possible. This leads to unexpected behavior.
For example, losing decimal digits when casting float to int
```PHP
readonly class IntDto
{
    public function __construct(
        public int $var,
    ) {}
}

$autoMapper = \AutoMapper\AutoMapper::create();
$result = $autoMapper->map(['var' => 1.1111], IntDto::class);
$result->var; // var is 1
```
This PR adds configuration to add `declare(strict_types=1)` to generated
code when mapping, so instead of unexpected behavior, you get TypeError.

If everything is ok, I will update the docs, but I am not very fluent
with english, so maybe someone else could write a more understandable
description.
  • Loading branch information
Korbeil authored Sep 3, 2024
2 parents 11a9c5c + 89c33df commit 8fa3035
Show file tree
Hide file tree
Showing 14 changed files with 95 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
### Added
- [GH#180](https://github.com/jolicode/automapper/pull/180) Add configuration to generate code with strict types

## [9.1.2] - 2024-09-03
### Fixed
Expand Down
1 change: 1 addition & 0 deletions src/Attribute/Mapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public function __construct(
public ?bool $checkAttributes = null,
public ?ConstructorStrategy $constructorStrategy = null,
public ?bool $allowReadOnlyTargetToPopulate = null,
public ?bool $strictTypes = null,
public int $priority = 0,
public ?string $dateTimeFormat = null,
) {
Expand Down
4 changes: 4 additions & 0 deletions src/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ public function __construct(
* Does the mapper should throw an exception if the target is read-only.
*/
public bool $allowReadOnlyTargetToPopulate = false,
/**
* Add declare(strict_types=1) to generated code.
*/
public bool $strictTypes = false,
) {
}
}
1 change: 1 addition & 0 deletions src/Event/GenerateMapperEvent.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public function __construct(
public ?bool $checkAttributes = null,
public ?ConstructorStrategy $constructorStrategy = null,
public ?bool $allowReadOnlyTargetToPopulate = null,
public ?bool $strictTypes = null,
) {
}
}
1 change: 1 addition & 0 deletions src/EventListener/MapperListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public function __invoke(GenerateMapperEvent $event): void
$event->checkAttributes ??= $mapper->checkAttributes;
$event->constructorStrategy ??= $mapper->constructorStrategy;
$event->allowReadOnlyTargetToPopulate ??= $mapper->allowReadOnlyTargetToPopulate;
$event->strictTypes ??= $mapper->strictTypes;
$event->mapperMetadata->dateTimeFormat = $mapper->dateTimeFormat;
}
}
16 changes: 14 additions & 2 deletions src/Generator/MapperGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
use PhpParser\Node\Stmt;
use Symfony\Component\ExpressionLanguage\ExpressionLanguage;

use function AutoMapper\PhpParser\create_declare_item;
use function AutoMapper\PhpParser\create_scalar_int;

/**
* Generates code for a mapping class.
*
Expand Down Expand Up @@ -58,22 +61,31 @@ public function __construct(
/**
* Generate Class AST given metadata for a mapper.
*
* @return Stmt[]
*
* @throws CompileException
* @throws InvalidMappingException
*/
public function generate(GeneratorMetadata $metadata): Stmt\Class_
public function generate(GeneratorMetadata $metadata): array
{
if ($this->disableGeneratedMapper) {
throw new InvalidMappingException('No mapper found for source ' . $metadata->mapperMetadata->source . ' and target ' . $metadata->mapperMetadata->target);
}

return (new Builder\Class_($metadata->mapperMetadata->className))
$statements = [];
if ($metadata->strictTypes) {
// @phpstan-ignore argument.type
$statements[] = new Stmt\Declare_([create_declare_item('strict_types', create_scalar_int(1))]);
}
$statements[] = (new Builder\Class_($metadata->mapperMetadata->className))
->makeFinal()
->extend(GeneratedMapper::class)
->addStmt($this->constructorMethod($metadata))
->addStmt($this->mapMethod($metadata))
->addStmt($this->registerMappersMethod($metadata))
->getNode();

return $statements;
}

/**
Expand Down
6 changes: 3 additions & 3 deletions src/Loader/EvalLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ public function __construct(

public function loadClass(MapperMetadata $mapperMetadata): void
{
$class = $this->generator->generate($this->metadataFactory->getGeneratorMetadata($mapperMetadata->source, $mapperMetadata->target));

eval($this->printer->prettyPrint([$class]));
eval($this->printer->prettyPrint($this->generator->generate(
$this->metadataFactory->getGeneratorMetadata($mapperMetadata->source, $mapperMetadata->target)
)));
}

public function buildMappers(MetadataRegistry $registry): bool
Expand Down
5 changes: 3 additions & 2 deletions src/Loader/FileLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,9 @@ public function createGeneratedMapper(MapperMetadata $mapperMetadata): string
$className = $mapperMetadata->className;
$classPath = $this->directory . \DIRECTORY_SEPARATOR . $className . '.php';

$generatorMetadata = $this->metadataFactory->getGeneratorMetadata($mapperMetadata->source, $mapperMetadata->target);
$classCode = $this->printer->prettyPrint([$this->generator->generate($generatorMetadata)]);
$classCode = $this->printer->prettyPrint($this->generator->generate(
$this->metadataFactory->getGeneratorMetadata($mapperMetadata->source, $mapperMetadata->target)
));

$this->write($classPath, "<?php\n\n" . $classCode . "\n");

Expand Down
3 changes: 2 additions & 1 deletion src/Metadata/GeneratorMetadata.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ public function __construct(
public readonly array $propertiesMetadata,
public readonly bool $checkAttributes = true,
public readonly ConstructorStrategy $constructorStrategy = ConstructorStrategy::AUTO,
public bool $allowReadOnlyTargetToPopulate = false,
public readonly bool $allowReadOnlyTargetToPopulate = false,
public readonly bool $strictTypes = false,
public readonly ?string $provider = null,
) {
$this->variableRegistry = new VariableRegistry();
Expand Down
1 change: 1 addition & 0 deletions src/Metadata/MetadataFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ private function createGeneratorMetadata(MapperMetadata $mapperMetadata): Genera
$mapperEvent->checkAttributes ?? $this->configuration->attributeChecking,
$mapperEvent->constructorStrategy ?? $this->configuration->constructorStrategy,
$mapperEvent->allowReadOnlyTargetToPopulate ?? $this->configuration->allowReadOnlyTargetToPopulate,
$mapperEvent->strictTypes ?? $this->configuration->strictTypes,
$mapperEvent->provider,
);
}
Expand Down
16 changes: 16 additions & 0 deletions src/php-parser.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,19 @@ function create_expr_array_item(Expr $value, Expr $key = null, bool $byRef = fal

return new $class($value, $key, $byRef, $attributes, $unpack);
}

/**
* Constructs a declare key=>value pair node.
*
* @param string|Node\Identifier $key Key
* @param Expr $value Value
* @param array<string, mixed> $attributes Additional attributes
*
* @internal
*/
function create_declare_item($key, Expr $value, array $attributes = []): Node\DeclareItem|Node\Stmt\DeclareDeclare
{
$class = class_exists(Node\DeclareItem::class) ? Node\DeclareItem::class : Node\Stmt\DeclareDeclare::class;

return new $class($key, $value, $attributes);
}
18 changes: 18 additions & 0 deletions tests/AutoMapperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,24 @@ public function testNoAutoRegister(): void
$automapper->getMapper(Fixtures\User::class, Fixtures\UserDTO::class);
}

public function testStrictTypes(): void
{
$this->expectException(\TypeError::class);

$automapper = AutoMapper::create(new Configuration(strictTypes: true, classPrefix: 'StrictTypes_'));
$data = ['foo' => 1.1];
$automapper->map($data, Fixtures\IntDTO::class);
}

public function testStrictTypesFromMapper(): void
{
$this->expectException(\TypeError::class);

$automapper = AutoMapper::create(new Configuration(strictTypes: false, classPrefix: 'StrictTypesFromMapper_'));
$data = ['foo' => 1.1];
$automapper->map($data, Fixtures\IntDTOWithMapper::class);
}

public function testWithMixedArray(): void
{
$user = new Fixtures\User(1, 'yolo', '13');
Expand Down
13 changes: 13 additions & 0 deletions tests/Fixtures/IntDTO.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php

declare(strict_types=1);

namespace AutoMapper\Tests\Fixtures;

readonly class IntDTO
{
public function __construct(
public int $foo,
) {
}
}
16 changes: 16 additions & 0 deletions tests/Fixtures/IntDTOWithMapper.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

declare(strict_types=1);

namespace AutoMapper\Tests\Fixtures;

use AutoMapper\Attribute\Mapper;

#[Mapper(strictTypes: true)]
readonly class IntDTOWithMapper
{
public function __construct(
public int $foo,
) {
}
}

0 comments on commit 8fa3035

Please sign in to comment.