refactor: remove redundant $toClass from MicroMapper, allowing improvement of generic templating #6
Conversation
bendavies
force-pushed
the
generics-further-fixing
branch
3 times, most recently
from
6ee4ee8
to
083ed66
Compare
…ement of generic templating
bendavies
force-pushed
the
generics-further-fixing
branch
from
083ed66
to
6d6be32
Compare
There was a problem hiding this comment.
As is, the analysis of the project won't be valid.
The only reason it passes is because the CI runs phpstan at level 4, which disables many rules. Try analysing this code at level 6 at least (or even at level 8 if possible). This will show you that it will likely require more changes (in the MapperConfig class for instance) to properly support those generics.
| * @param TTo $to | ||
| * @param TFrom $from | ||
| * @param TTo $to | ||
| * @param array<mixed> $context |
There was a problem hiding this comment.
should this be array<string, mixed> ? I think the context is expected to be an associative array.
…rics-further-fixing
|
@bendavies are you going to continue working on this PR? I updated this branch with latest changes from |
|
@bocharsky-bw thanks for the nudge. |
|
ok, bumping phpstan up from level 4 to at least 6 looks important. |
There was a problem hiding this comment.
Changes look good to me here.
But it might be so that tests pass because I temporarily skipped out the related test, see #12 (comment)
If you could take a look at that failed test and make it working - would be awesome. I'm not sure how to get that done yet
Follows #4
This PR removes the
$toClassparamters fromMicroMapper::load(), allowing for better generics templating. no stable release yet so I hope this is acceptable!Here is a full example with no
\assert(...)or@varrequired on the params ofMicroMapper::load()andMicroMapper::populate():https://phpstan.org/r/bae2f18f-8eb5-4377-827a-0814b86bc383
thanks to @ondrejmirtes for the help!