From e974721d8c8d6008ae811c107bd8e7db942fe2fa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Barto=C5=A1?= <bartos.developer152@gmail.com>
Date: Sun, 9 Jul 2023 22:42:47 +0200
Subject: [PATCH] ReflectionMetaSource: improve error messages, add tests

---
 src/Meta/Source/ReflectorMetaSource.php       |  75 ++++++----
 .../Definition/TargetLessRuleDefinition.php   |  28 ++++
 .../Definition/UnsupportedDefinition.php      |  26 ++++
 .../Doubles/Meta/FieldWithMultipleRulesVO.php |  18 +++
 tests/Doubles/Meta/FieldWithNoRuleVO.php      |  14 ++
 tests/Doubles/Meta/RuleAboveClassVO.php       |  14 ++
 .../Meta/UnsupportedClassDefinitionVO.php     |  14 ++
 .../Meta/UnsupportedPropertyDefinitionVO.php  |  14 ++
 tests/Doubles/Meta/VariantFieldParentVO.php   |  14 ++
 tests/Doubles/Meta/VariantFieldVO.php         |  13 ++
 tests/Unit/Meta/MetaLoaderTest.php            |   6 +
 .../Meta/Source/ReflectorMetaSourceTest.php   | 128 ++++++++++++++++++
 12 files changed, 334 insertions(+), 30 deletions(-)
 create mode 100644 tests/Doubles/Definition/TargetLessRuleDefinition.php
 create mode 100644 tests/Doubles/Definition/UnsupportedDefinition.php
 create mode 100644 tests/Doubles/Meta/FieldWithMultipleRulesVO.php
 create mode 100644 tests/Doubles/Meta/FieldWithNoRuleVO.php
 create mode 100644 tests/Doubles/Meta/RuleAboveClassVO.php
 create mode 100644 tests/Doubles/Meta/UnsupportedClassDefinitionVO.php
 create mode 100644 tests/Doubles/Meta/UnsupportedPropertyDefinitionVO.php
 create mode 100644 tests/Doubles/Meta/VariantFieldParentVO.php
 create mode 100644 tests/Doubles/Meta/VariantFieldVO.php
 create mode 100644 tests/Unit/Meta/Source/ReflectorMetaSourceTest.php

diff --git a/src/Meta/Source/ReflectorMetaSource.php b/src/Meta/Source/ReflectorMetaSource.php
index 784e6a6..6ac64fd 100644
--- a/src/Meta/Source/ReflectorMetaSource.php
+++ b/src/Meta/Source/ReflectorMetaSource.php
@@ -3,6 +3,7 @@
 namespace Orisai\ObjectMapper\Meta\Source;
 
 use Orisai\Exceptions\Logic\InvalidArgument;
+use Orisai\Exceptions\Message;
 use Orisai\ObjectMapper\Callbacks\CallbackDefinition;
 use Orisai\ObjectMapper\Docs\DocDefinition;
 use Orisai\ObjectMapper\MappedObject;
@@ -48,8 +49,8 @@ public function load(ReflectionClass $class): CompileMeta
 		}
 
 		return new CompileMeta(
-			$this->loadClassMeta($structures),
-			$this->loadPropertiesMeta($structures),
+			$this->loadClassMeta($class, $structures),
+			$this->loadPropertiesMeta($class, $structures),
 			$sources,
 		);
 	}
@@ -67,9 +68,10 @@ private function getStructureGroup(ReflectionClass $class): StructureGroup
 	}
 
 	/**
+	 * @param ReflectionClass<MappedObject> $rootClass
 	 * @return list<ClassCompileMeta>
 	 */
-	private function loadClassMeta(StructureGroup $group): array
+	private function loadClassMeta(ReflectionClass $rootClass, StructureGroup $group): array
 	{
 		$resolved = [];
 		foreach ($group->getClasses() as $class) {
@@ -84,12 +86,16 @@ private function loadClassMeta(StructureGroup $group): array
 				$definition = $this->checkDefinitionType($definition);
 
 				if ($definition instanceof RuleDefinition) {
-					throw InvalidArgument::create()
-						->withMessage(sprintf(
-							'Rule definition %s (subtype of %s) cannot be used on class, only properties are allowed',
+					$message = Message::create()
+						->withContext("Resolving metadata of mapped object '{$rootClass->getName()}'.")
+						->withProblem(sprintf(
+							"Rule definition '%s' (subtype of '%s') cannot be used on class, only properties are allowed.",
 							get_class($definition),
 							RuleDefinition::class,
 						));
+
+					throw InvalidArgument::create()
+						->withMessage($message);
 				}
 
 				if ($definition instanceof CallbackDefinition) {
@@ -121,9 +127,10 @@ private function loadClassMeta(StructureGroup $group): array
 	}
 
 	/**
+	 * @param ReflectionClass<MappedObject> $rootClass
 	 * @return list<FieldCompileMeta>
 	 */
-	private function loadPropertiesMeta(StructureGroup $group): array
+	private function loadPropertiesMeta(ReflectionClass $rootClass, StructureGroup $group): array
 	{
 		$resolved = [];
 		foreach ($group->getGroupedProperties() as $groupedProperty) {
@@ -132,9 +139,6 @@ private function loadPropertiesMeta(StructureGroup $group): array
 				$reflector = $propertyStructure->getSource()->getReflector();
 				$definitions = $this->reader->readProperty($reflector, MetaDefinition::class);
 
-				$property = $propertyStructure->getContextReflector();
-				$class = $property->getDeclaringClass();
-
 				$callbacks = [];
 				$docs = [];
 				$modifiers = [];
@@ -145,15 +149,18 @@ private function loadPropertiesMeta(StructureGroup $group): array
 
 					if ($definition instanceof RuleDefinition) {
 						if ($rule !== null) {
+							$message = Message::create()
+								->withContext("Resolving metadata of mapped object '{$rootClass->getName()}'.")
+								->withProblem(
+									"Property '{$propertyStructure->getSource()->toString()}' has"
+									. ' multiple rule definitions, but only one is allowed.',
+								)
+								->withSolution(
+									sprintf("Combine multiple with '%s' or '%s'.", AnyOf::class, AllOf::class),
+								);
+
 							throw InvalidArgument::create()
-								->withMessage(sprintf(
-									'Mapped property %s::$%s has multiple expectation definitions, while only one is allowed. ' .
-									'Combine multiple with %s or %s',
-									$class->getName(),
-									$property->getName(),
-									AnyOf::class,
-									AllOf::class,
-								));
+								->withMessage($message);
 						}
 
 						$rule = new RuleCompileMeta(
@@ -183,11 +190,15 @@ private function loadPropertiesMeta(StructureGroup $group): array
 				}
 
 				if ($rule === null) {
-					throw InvalidArgument::create()
-						->withMessage(
-							"Property {$class->getName()}::\${$property->getName()} has mapped object definition, " .
-							'but no rule definition.',
+					$message = Message::create()
+						->withContext("Resolving metadata of mapped object '{$rootClass->getName()}'.")
+						->withProblem(
+							"Property '{$propertyStructure->getSource()->toString()}' has"
+							. ' mapped object definition, but no rule definition.',
 						);
+
+					throw InvalidArgument::create()
+						->withMessage($message);
 				}
 
 				$resolvedGroup[] = new FieldCompileMeta(
@@ -203,7 +214,7 @@ private function loadPropertiesMeta(StructureGroup $group): array
 				continue;
 			}
 
-			$this->checkFieldInvariance($resolvedGroup);
+			$this->checkFieldInvariance($rootClass, $resolvedGroup);
 			$resolved[] = $resolvedGroup[array_key_first($resolvedGroup)];
 		}
 
@@ -223,7 +234,7 @@ private function checkDefinitionType(MetaDefinition $definition): MetaDefinition
 		) {
 			throw InvalidArgument::create()
 				->withMessage(sprintf(
-					'Definition %s (subtype of %s) should implement %s, %s %s or %s',
+					"Definition '%s' (subtype of '%s') should implement '%s', '%s', '%s' or '%s'.",
 					get_class($definition),
 					MetaDefinition::class,
 					CallbackDefinition::class,
@@ -237,19 +248,23 @@ private function checkDefinitionType(MetaDefinition $definition): MetaDefinition
 	}
 
 	/**
+	 * @param ReflectionClass<MappedObject> $rootClass
 	 * @param list<FieldCompileMeta> $resolvedGroup
 	 */
-	private function checkFieldInvariance(array $resolvedGroup): void
+	private function checkFieldInvariance(ReflectionClass $rootClass, array $resolvedGroup): void
 	{
 		$previousFieldMeta = null;
 		foreach ($resolvedGroup as $fieldMeta) {
 			if ($previousFieldMeta !== null && !$fieldMeta->hasEqualMeta($previousFieldMeta)) {
-				throw InvalidArgument::create()
-					->withMessage(
-						"Definition of property '{$fieldMeta->getClass()->getContextReflector()->getName()}"
-						. "::\${$fieldMeta->getProperty()->getContextReflector()->getName()}'"
-						. " can't be changed but it differs from definition in '{$previousFieldMeta->getClass()->getContextReflector()->getName()}'.",
+				$message = Message::create()
+					->withContext("Resolving metadata of mapped object '{$rootClass->getName()}'.")
+					->withProblem(
+						"Definition of property '{$fieldMeta->getProperty()->getSource()->toString()}'"
+						. " can't be changed but it differs from definition '{$previousFieldMeta->getProperty()->getSource()->toString()}'.",
 					);
+
+				throw InvalidArgument::create()
+					->withMessage($message);
 			}
 
 			$previousFieldMeta = $fieldMeta;
diff --git a/tests/Doubles/Definition/TargetLessRuleDefinition.php b/tests/Doubles/Definition/TargetLessRuleDefinition.php
new file mode 100644
index 0000000..c05d15f
--- /dev/null
+++ b/tests/Doubles/Definition/TargetLessRuleDefinition.php
@@ -0,0 +1,28 @@
+<?php declare(strict_types = 1);
+
+namespace Tests\Orisai\ObjectMapper\Doubles\Definition;
+
+use Attribute;
+use Doctrine\Common\Annotations\Annotation\NamedArgumentConstructor;
+use Orisai\ObjectMapper\Rules\MixedRule;
+use Orisai\ObjectMapper\Rules\RuleDefinition;
+
+/**
+ * @Annotation
+ * @NamedArgumentConstructor()
+ */
+#[Attribute()]
+final class TargetLessRuleDefinition implements RuleDefinition
+{
+
+	public function getType(): string
+	{
+		return MixedRule::class;
+	}
+
+	public function getArgs(): array
+	{
+		return [];
+	}
+
+}
diff --git a/tests/Doubles/Definition/UnsupportedDefinition.php b/tests/Doubles/Definition/UnsupportedDefinition.php
new file mode 100644
index 0000000..62dca87
--- /dev/null
+++ b/tests/Doubles/Definition/UnsupportedDefinition.php
@@ -0,0 +1,26 @@
+<?php declare(strict_types = 1);
+
+namespace Tests\Orisai\ObjectMapper\Doubles\Definition;
+
+use Attribute;
+use Orisai\ObjectMapper\Meta\MetaDefinition;
+use stdClass;
+
+/**
+ * @Annotation
+ */
+#[Attribute]
+final class UnsupportedDefinition implements MetaDefinition
+{
+
+	public function getType(): string
+	{
+		return stdClass::class;
+	}
+
+	public function getArgs(): array
+	{
+		return [];
+	}
+
+}
diff --git a/tests/Doubles/Meta/FieldWithMultipleRulesVO.php b/tests/Doubles/Meta/FieldWithMultipleRulesVO.php
new file mode 100644
index 0000000..1738b57
--- /dev/null
+++ b/tests/Doubles/Meta/FieldWithMultipleRulesVO.php
@@ -0,0 +1,18 @@
+<?php declare(strict_types = 1);
+
+namespace Tests\Orisai\ObjectMapper\Doubles\Meta;
+
+use Orisai\ObjectMapper\MappedObject;
+use Orisai\ObjectMapper\Rules\IntValue;
+use Orisai\ObjectMapper\Rules\StringValue;
+
+final class FieldWithMultipleRulesVO implements MappedObject
+{
+
+	/**
+	 * @StringValue()
+	 * @IntValue()
+	 */
+	public string $field;
+
+}
diff --git a/tests/Doubles/Meta/FieldWithNoRuleVO.php b/tests/Doubles/Meta/FieldWithNoRuleVO.php
new file mode 100644
index 0000000..a78fcab
--- /dev/null
+++ b/tests/Doubles/Meta/FieldWithNoRuleVO.php
@@ -0,0 +1,14 @@
+<?php declare(strict_types = 1);
+
+namespace Tests\Orisai\ObjectMapper\Doubles\Meta;
+
+use Orisai\ObjectMapper\MappedObject;
+use Orisai\ObjectMapper\Modifiers\FieldName;
+
+final class FieldWithNoRuleVO implements MappedObject
+{
+
+	/** @FieldName("foo") */
+	public string $field;
+
+}
diff --git a/tests/Doubles/Meta/RuleAboveClassVO.php b/tests/Doubles/Meta/RuleAboveClassVO.php
new file mode 100644
index 0000000..d7f471a
--- /dev/null
+++ b/tests/Doubles/Meta/RuleAboveClassVO.php
@@ -0,0 +1,14 @@
+<?php declare(strict_types = 1);
+
+namespace Tests\Orisai\ObjectMapper\Doubles\Meta;
+
+use Orisai\ObjectMapper\MappedObject;
+use Tests\Orisai\ObjectMapper\Doubles\Definition\TargetLessRuleDefinition;
+
+/**
+ * @TargetLessRuleDefinition()
+ */
+final class RuleAboveClassVO implements MappedObject
+{
+
+}
diff --git a/tests/Doubles/Meta/UnsupportedClassDefinitionVO.php b/tests/Doubles/Meta/UnsupportedClassDefinitionVO.php
new file mode 100644
index 0000000..1570a5e
--- /dev/null
+++ b/tests/Doubles/Meta/UnsupportedClassDefinitionVO.php
@@ -0,0 +1,14 @@
+<?php declare(strict_types = 1);
+
+namespace Tests\Orisai\ObjectMapper\Doubles\Meta;
+
+use Orisai\ObjectMapper\MappedObject;
+use Tests\Orisai\ObjectMapper\Doubles\Definition\UnsupportedDefinition;
+
+/**
+ * @UnsupportedDefinition()
+ */
+final class UnsupportedClassDefinitionVO implements MappedObject
+{
+
+}
diff --git a/tests/Doubles/Meta/UnsupportedPropertyDefinitionVO.php b/tests/Doubles/Meta/UnsupportedPropertyDefinitionVO.php
new file mode 100644
index 0000000..8ce11ed
--- /dev/null
+++ b/tests/Doubles/Meta/UnsupportedPropertyDefinitionVO.php
@@ -0,0 +1,14 @@
+<?php declare(strict_types = 1);
+
+namespace Tests\Orisai\ObjectMapper\Doubles\Meta;
+
+use Orisai\ObjectMapper\MappedObject;
+use Tests\Orisai\ObjectMapper\Doubles\Definition\UnsupportedDefinition;
+
+final class UnsupportedPropertyDefinitionVO implements MappedObject
+{
+
+	/** @UnsupportedDefinition() */
+	public string $field;
+
+}
diff --git a/tests/Doubles/Meta/VariantFieldParentVO.php b/tests/Doubles/Meta/VariantFieldParentVO.php
new file mode 100644
index 0000000..1326e86
--- /dev/null
+++ b/tests/Doubles/Meta/VariantFieldParentVO.php
@@ -0,0 +1,14 @@
+<?php declare(strict_types = 1);
+
+namespace Tests\Orisai\ObjectMapper\Doubles\Meta;
+
+use Orisai\ObjectMapper\MappedObject;
+use Orisai\ObjectMapper\Rules\StringValue;
+
+abstract class VariantFieldParentVO implements MappedObject
+{
+
+	/** @StringValue(notEmpty=true) */
+	public string $field;
+
+}
diff --git a/tests/Doubles/Meta/VariantFieldVO.php b/tests/Doubles/Meta/VariantFieldVO.php
new file mode 100644
index 0000000..f566b5f
--- /dev/null
+++ b/tests/Doubles/Meta/VariantFieldVO.php
@@ -0,0 +1,13 @@
+<?php declare(strict_types = 1);
+
+namespace Tests\Orisai\ObjectMapper\Doubles\Meta;
+
+use Orisai\ObjectMapper\Rules\StringValue;
+
+final class VariantFieldVO extends VariantFieldParentVO
+{
+
+	/** @StringValue() */
+	public string $field;
+
+}
diff --git a/tests/Unit/Meta/MetaLoaderTest.php b/tests/Unit/Meta/MetaLoaderTest.php
index 688ba5b..dd96fec 100644
--- a/tests/Unit/Meta/MetaLoaderTest.php
+++ b/tests/Unit/Meta/MetaLoaderTest.php
@@ -112,7 +112,13 @@ public function testPreload(): void
 		$excludes[] = __DIR__ . '/../../Doubles/Meta/EnumVO.php';
 		$excludes[] = __DIR__ . '/../../Doubles/Meta/FieldMetaInvalidScopeRootVO.php';
 		$excludes[] = __DIR__ . '/../../Doubles/Meta/FieldTraitMetaInvalidScopeRootVO.php';
+		$excludes[] = __DIR__ . '/../../Doubles/Meta/FieldWithMultipleRulesVO.php';
+		$excludes[] = __DIR__ . '/../../Doubles/Meta/FieldWithNoRuleVO.php';
+		$excludes[] = __DIR__ . '/../../Doubles/Meta/RuleAboveClassVO.php';
 		$excludes[] = __DIR__ . '/../../Doubles/Meta/StaticMappedPropertyVO.php';
+		$excludes[] = __DIR__ . '/../../Doubles/Meta/UnsupportedClassDefinitionVO.php';
+		$excludes[] = __DIR__ . '/../../Doubles/Meta/UnsupportedPropertyDefinitionVO.php';
+		$excludes[] = __DIR__ . '/../../Doubles/Meta/VariantFieldVO.php';
 		$excludes[] = __DIR__ . '/../../Doubles/Meta/WrongArgsTypeVO.php';
 
 		if (PHP_VERSION_ID < 8_00_00) {
diff --git a/tests/Unit/Meta/Source/ReflectorMetaSourceTest.php b/tests/Unit/Meta/Source/ReflectorMetaSourceTest.php
new file mode 100644
index 0000000..f7e0a75
--- /dev/null
+++ b/tests/Unit/Meta/Source/ReflectorMetaSourceTest.php
@@ -0,0 +1,128 @@
+<?php declare(strict_types = 1);
+
+namespace Tests\Orisai\ObjectMapper\Unit\Meta\Source;
+
+use Generator;
+use Orisai\Exceptions\Logic\InvalidArgument;
+use Orisai\ObjectMapper\MappedObject;
+use Orisai\ObjectMapper\Meta\Source\AnnotationsMetaSource;
+use Orisai\ObjectMapper\Meta\Source\ReflectorMetaSource;
+use PHPUnit\Framework\TestCase;
+use ReflectionClass;
+use Tests\Orisai\ObjectMapper\Doubles\Meta\FieldWithMultipleRulesVO;
+use Tests\Orisai\ObjectMapper\Doubles\Meta\FieldWithNoRuleVO;
+use Tests\Orisai\ObjectMapper\Doubles\Meta\RuleAboveClassVO;
+use Tests\Orisai\ObjectMapper\Doubles\Meta\UnsupportedClassDefinitionVO;
+use Tests\Orisai\ObjectMapper\Doubles\Meta\UnsupportedPropertyDefinitionVO;
+use Tests\Orisai\ObjectMapper\Doubles\Meta\VariantFieldVO;
+
+final class ReflectorMetaSourceTest extends TestCase
+{
+
+	private ReflectorMetaSource $source;
+
+	protected function setUp(): void
+	{
+		$this->source = new AnnotationsMetaSource();
+	}
+
+	/**
+	 * @param class-string<MappedObject> $class
+	 *
+	 * @dataProvider provideUnsupportedDefinitionType
+	 */
+	public function testUnsupportedDefinitionType(string $class): void
+	{
+		$this->expectException(InvalidArgument::class);
+		$this->expectExceptionMessage(
+			"Definition 'Tests\Orisai\ObjectMapper\Doubles\Definition\UnsupportedDefinition' "
+			. "(subtype of 'Orisai\ObjectMapper\Meta\MetaDefinition') should implement "
+			. "'Orisai\ObjectMapper\Callbacks\CallbackDefinition', "
+			. "'Orisai\ObjectMapper\Docs\DocDefinition', "
+			. "'Orisai\ObjectMapper\Modifiers\ModifierDefinition' or "
+			. "'Orisai\ObjectMapper\Rules\RuleDefinition'.",
+		);
+
+		$this->source->load(new ReflectionClass($class));
+	}
+
+	public function provideUnsupportedDefinitionType(): Generator
+	{
+		yield [
+			UnsupportedClassDefinitionVO::class,
+		];
+
+		yield [
+			UnsupportedPropertyDefinitionVO::class,
+		];
+	}
+
+	public function testFieldInvariance(): void
+	{
+		$this->expectException(InvalidArgument::class);
+		$this->expectExceptionMessage(
+			<<<'MSG'
+Context: Resolving metadata of mapped object
+         'Tests\Orisai\ObjectMapper\Doubles\Meta\VariantFieldVO'.
+Problem: Definition of property
+         'Tests\Orisai\ObjectMapper\Doubles\Meta\VariantFieldVO->$field' can't
+         be changed but it differs from definition
+         'Tests\Orisai\ObjectMapper\Doubles\Meta\VariantFieldParentVO->$field'.
+MSG,
+		);
+
+		$this->source->load(new ReflectionClass(VariantFieldVO::class));
+	}
+
+	public function testRuleAboveClass(): void
+	{
+		$this->expectException(InvalidArgument::class);
+		$this->expectExceptionMessage(
+			<<<'MSG'
+Context: Resolving metadata of mapped object
+         'Tests\Orisai\ObjectMapper\Doubles\Meta\RuleAboveClassVO'.
+Problem: Rule definition
+         'Tests\Orisai\ObjectMapper\Doubles\Definition\TargetLessRuleDefinition'
+         (subtype of 'Orisai\ObjectMapper\Rules\RuleDefinition') cannot be used
+         on class, only properties are allowed.
+MSG,
+		);
+
+		$this->source->load(new ReflectionClass(RuleAboveClassVO::class));
+	}
+
+	public function testFieldWithMultipleRules(): void
+	{
+		$this->expectException(InvalidArgument::class);
+		$this->expectExceptionMessage(
+			<<<'MSG'
+Context: Resolving metadata of mapped object
+         'Tests\Orisai\ObjectMapper\Doubles\Meta\FieldWithMultipleRulesVO'.
+Problem: Property
+         'Tests\Orisai\ObjectMapper\Doubles\Meta\FieldWithMultipleRulesVO->$field'
+         has multiple rule definitions, but only one is allowed.
+Solution: Combine multiple with 'Orisai\ObjectMapper\Rules\AnyOf' or
+          'Orisai\ObjectMapper\Rules\AllOf'.
+MSG,
+		);
+
+		$this->source->load(new ReflectionClass(FieldWithMultipleRulesVO::class));
+	}
+
+	public function testFieldWithNoRule(): void
+	{
+		$this->expectException(InvalidArgument::class);
+		$this->expectExceptionMessage(
+			<<<'MSG'
+Context: Resolving metadata of mapped object
+         'Tests\Orisai\ObjectMapper\Doubles\Meta\FieldWithNoRuleVO'.
+Problem: Property
+         'Tests\Orisai\ObjectMapper\Doubles\Meta\FieldWithNoRuleVO->$field' has
+         mapped object definition, but no rule definition.
+MSG,
+		);
+
+		$this->source->load(new ReflectionClass(FieldWithNoRuleVO::class));
+	}
+
+}