-
-
Notifications
You must be signed in to change notification settings - Fork 65
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
Describe some types more accurately #183
Conversation
e662316
to
05c99b3
Compare
@@ -136,7 +136,9 @@ public function getAllClassNames($globalBasename) | |||
} | |||
|
|||
// NOTE: All files found here means classes are not transient! | |||
$classes[] = str_replace('.', '\\', $fileName); | |||
/** @psalm-var class-string */ |
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.
Using assert(class_exists($class))
on this particular one makes the ORM test suite fail https://github.com/doctrine/orm/blob/2.8.x/tests/Doctrine/Tests/ORM/Tools/ConvertDoctrine1SchemaTest.php because there is heavy mocking involved.
@orklah @franmomu please review. More things could be done if it were not for vimeo/psalm#5788 |
3bb0d6a
to
580729e
Compare
I think a good amount of the arrays could be described as |
c8fd4c9
to
87daf16
Compare
@@ -13,6 +13,7 @@ interface MappingDriver | |||
* Loads the metadata for the specified class into the provided container. | |||
* | |||
* @param string $className | |||
* @psalm-param class-string $className |
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.
I guess here @template T of object
could be used to specify class-string<T>
and ClassMetadata<T>
.
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.
Oh right, fixed, thanks!
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.
I don't really understand how I can fix the resulting error, maybe I should ignore it…
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.
hmmm I guess so, what I don't know is why it didn't complain calling object::loadMetadata
🤔
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.
87daf16
to
c655f60
Compare
Downstream builds are broken because the last release of persistence describes its inputs more accurately, but not its outputs.
c655f60
to
c62177e
Compare
Thanks @greg0ire! |
Downstream builds are broken partly because the last release of persistence
describes its inputs more accurately, but not its outputs.