-
Notifications
You must be signed in to change notification settings - Fork 15
Draft: Add missing property types #24
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
base: master
Are you sure you want to change the base?
Conversation
…use we have a lot errors
…se of mixed types
…e test + and src/WireMock/SerDe and src/WireMock/SerdeGen) Now unit tests are broken, because I added a lot of "mixed" property types, that are not supported by the Deserialization library.
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.
my next Project is then to type Hamcrest ;)
| use phpDocumentor\Reflection\Types\Object_; | ||
| use phpDocumentor\Reflection\Types\String_; | ||
| use phpDocumentor\Reflection\Types\True_; | ||
| use phpDocumentor\Reflection\PseudoTypes\True_; |
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.
that way the real class is used (not the alias)
| try { | ||
| $propType = $this->serdeTypeParser->resolveTypeToSerdeType($varTag->getType()); | ||
| } catch (SerializationException $e) { | ||
| throw new SerializationException('Cannot resolve type from '.$parentClass.'::$'.$refProp->getName().' with @var of '.$varTag->getType(), 0, $e); | ||
| } |
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.
added this to debug the failing tests. This creates a chain of exceptions that allow you to see whats actually failing:
1) WireMock\SerdeGen\WireMockSerdeGenTest::testGeneratingLookup
WireMock\Serde\SerializationException: Cannot resolve type from WireMock\Client\GetServeEventsResult::$requests with @var of \WireMock\Client\ServeEvent[]
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeClassDefinitionFactory.php:140
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeClassDefinitionFactory.php:61
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:256
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:80
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:53
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeLookupFactory.php:25
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/WireMockSerdeGen.php:57
/home/psc/projects/wiremock-php/test/WireMock/SerdeGen/WireMockSerdeGenTest.php:18
/home/psc/projects/wiremock-php/test/WireMock/HamcrestTestCase.php:17
phpvfscomposer:///home/psc/projects/wiremock-php/vendor/phpunit/phpunit/phpunit:97
Caused by
WireMock\Serde\SerializationException: Cannot resolve type from WireMock\Client\ServeEvent::$response with @var of \WireMock\Client\LoggedResponse
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeClassDefinitionFactory.php:140
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeClassDefinitionFactory.php:61
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:256
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:80
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:141
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:122
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:73
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeClassDefinitionFactory.php:138
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeClassDefinitionFactory.php:61
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:256
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:80
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:53
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeLookupFactory.php:25
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/WireMockSerdeGen.php:57
/home/psc/projects/wiremock-php/test/WireMock/SerdeGen/WireMockSerdeGenTest.php:18
/home/psc/projects/wiremock-php/test/WireMock/HamcrestTestCase.php:17
phpvfscomposer:///home/psc/projects/wiremock-php/vendor/phpunit/phpunit/phpunit:97
Caused by
WireMock\Serde\SerializationException: Cannot resolve type from WireMock\Client\LoggedResponse::$headers with @var of array<int|string,mixed>
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeClassDefinitionFactory.php:140
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeClassDefinitionFactory.php:61
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:256
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:80
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeClassDefinitionFactory.php:138
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeClassDefinitionFactory.php:61
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:256
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:80
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:141
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:122
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:73
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeClassDefinitionFactory.php:138
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeClassDefinitionFactory.php:61
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:256
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:80
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:53
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeLookupFactory.php:25
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/WireMockSerdeGen.php:57
/home/psc/projects/wiremock-php/test/WireMock/SerdeGen/WireMockSerdeGenTest.php:18
/home/psc/projects/wiremock-php/test/WireMock/HamcrestTestCase.php:17
phpvfscomposer:///home/psc/projects/wiremock-php/vendor/phpunit/phpunit/phpunit:97
Caused by
WireMock\Serde\SerializationException: Unsupported type phpDocumentor\Reflection\Types\Mixed_: mixed
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:105
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:158
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:124
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:73
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeClassDefinitionFactory.php:138
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeClassDefinitionFactory.php:61
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:256
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:80
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeClassDefinitionFactory.php:138
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeClassDefinitionFactory.php:61
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:256
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:80
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:141
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:122
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:73
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeClassDefinitionFactory.php:138
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeClassDefinitionFactory.php:61
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:256
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:80
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeParser.php:53
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/SerdeTypeLookupFactory.php:25
/home/psc/projects/wiremock-php/src/WireMock/SerdeGen/WireMockSerdeGen.php:57
/home/psc/projects/wiremock-php/test/WireMock/SerdeGen/WireMockSerdeGenTest.php:18
/home/psc/projects/wiremock-php/test/WireMock/HamcrestTestCase.php:17
phpvfscomposer:///home/psc/projects/wiremock-php/vendor/phpunit/phpunit/phpunit:97
| /** | ||
| * @var string|AdvancedPathPattern | ||
| * @serde-named-by matchingType | ||
| * @serde-possible-names matchingValueNames | ||
| */ | ||
| protected $matchingValue; |
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.
the property is defined in the ValueMatchingStrategy, can I delete them without breaking Serde?
| } | ||
|
|
||
| $requestPattern = $patternBuilder->build(); | ||
| $requestPattern = $patternBuilder->build(); // @phpstan-ignore method.nonObject (@TODO this looks like an actual bug to me?) |
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.
so this looks like a bug to me.
if its an int then $pattnerBuilder is $requestPatternBuilder;
but this can be null and is not checked here? before its called.
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.
this class did not had an constructor, does Serde handle this?
|
Hello! Sorry for the super long delay. This turned into something much larger than I was expecting, and to be honest I don't have the time to dig into this to give the kind of response it needs. Do you think there's a route to a much, much smaller change (e.g. just typing a few key public methods)? If not, I'm afraid I think we should abandon this approach... |
|
Yeah, its always like this, if you start without phpstan its really hard to get started with it. But wanted to test the waters first, before diving into it entirely. Of course we can
but given the time it took to write a review comment and you saying:
I wouldn't pursue this either. |
have to think about how to continue and if this makes sense and how
maybe you can look at my comments questions.
Of course I will split this PR in several smaller PRs, one by one.