TASK: Add type hints and minimal cleanup in configuration builder #2956
Conversation
|
Mmm, how to deal with psalm here, it IS obviously correct that the calls could run into a null value, that hasn't changed though, I just clarified that with a type hint. Do I fake the type hint to be not null, seems like a bad idea as well... Neither do I want to null check the respective calls. |
|
Added some psalm change, the rest seems unrelated... |
|
I haven't really compared but I will just close this in lieu of @mhsdesign stan changes which proably cover a bunch of these. |
|
Hu well nice nice, i didnt know about this one ;) Will put some love into it to rebase it onto 9.0 ;) |
mhsdesign
force-pushed
the
task/type-hinting-objectmanagement
branch
from
9fb734c to
eabb3ff
Compare
|
Okay i had a little fun rebasing this ^^ Actually the most conflicts arose due to roberts proxy building refactoring 😅 like were your adjusted files have been removed: #3064 But in all those cases i accepted the 9.0 part. It still looks like that there are lots of valuable cleanups in the object manager in here which i was able to preserve ;) |
| @@ -166,7 +171,7 @@ public function buildObjectConfigurations(array $availableClassAndInterfaceNames | |||
| } | |||
|
|
|||
| $objectConfigurations[$objectName] = $newObjectConfiguration; | |||
| if ($objectConfigurations[$objectName]->getPackageKey() === null) { | |||
| if ($objectConfigurations[$objectName]->getPackageKey() === '') { | |||
There was a problem hiding this comment.
the check was initially introduced via 14a3e92
you adjusted the getPackageKey to always return strings and an empty string instead of null. Im not sure if thats a good thing or not, as we may have more code in the codebase using the nullable pattern ... also null feels more anticipated than "".
Neos.Flow/Classes/ObjectManagement/Configuration/ConfigurationBuilder.php
Show resolved
Hide resolved
| if (!is_array($classMethodNames)) { | ||
| if (!class_exists($className)) { | ||
| throw new UnknownClassException(sprintf('The class "%s" defined in the object configuration for object "%s", defined in package: %s, does not exist.', $className, $objectConfiguration->getObjectName(), $objectConfiguration->getPackageKey()), 1352371371); | ||
| } else { | ||
| throw new UnknownClassException(sprintf('Could not autowire properties of class "%s" because names of methods contained in that class could not be retrieved using get_class_methods().', $className), 1352386418); | ||
| } | ||
| } |
mhsdesign
force-pushed
the
task/type-hinting-objectmanagement
branch
from
eabb3ff to
ee873c8
Compare
mhsdesign
force-pushed
the
task/type-hinting-objectmanagement
branch
from
ee873c8 to
4b5bd42
Compare
| default: | ||
| throw new InvalidObjectConfigurationException('Invalid autowiring declaration', 1283866757); | ||
| } | ||
| return match ($value) { |
There was a problem hiding this comment.
you might have uncovered something that is better hidden: #3307
will revert this method so its a cleanup and not a bugfix :O
|
For some reason i cannot get flow to boot. The changes in the configuration building seem to have killed it. I stopped now after an hour but i would still suggest to take the usefull parts #3308 Feel free to continue here or not :D |
mhsdesign
changed the title
This is a minimal cleanup to the object management classes. Adding type hints, using more modern language constructs, and introducing constants for the cryptic array keys in the
ObjectManagerconfiguration.Checklist
FEATURE|TASK|BUGFIX!!!and have upgrade-instructions