Add testsand implementation for property types changing from docblock-only to native type #340
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ocramius
requested changes
Nov 27, 2021
test/unit/DetectChanges/BCBreak/PropertyBased/PropertyDocumentedTypeChangedTest.php
Outdated
Show resolved
Hide resolved
test/unit/DetectChanges/BCBreak/PropertyBased/PropertyDocumentedTypeChangedTest.php
Show resolved
Hide resolved
|
I'm not sure about the implementation, but would love some feedback on it. |
|
Thanks @tomasnorre! |
|
I don't know if this is a new PR, but I saw this, this morning, after using the 6.0.x-dev version in my CI. The diff is the leading slash. Is stripping the leading slash before compare a valid fix? The rest of the cases I had is fixes with the current version of 6.0.x |
|
Would always start from a test case, then figure it out from there. |
|
I'll create a new PR with a Test that fails. Probably not today, though. |
Ocramius
added a commit
that referenced
this pull request
Dec 5, 2021
…Reflection@1f7a13b This also handles the upcoming BC break of Roave/BetterReflection#900 Some big changes: * `PropertyDocumentedTypeChanged` no longer exists: we use only `PropertyTypeChanged` for now. This is a step backwards, but it is needed to prevent issues around docblocks being too advanced for this library to parse them, for now. * see CuyZ/Valinor#6 * see https://github.com/phpstan/phpstan-src/tree/447cd102d946a2d9883ac82793195275f54296a9/src/Type * see https://github.com/vimeo/psalm/tree/f1d47cc662500589e200a978e3af85488a5236cf/src/Psalm/Type * see https://github.com/phpDocumentor/ReflectionDocBlock/blob/622548b623e81ca6d78b721c5e029f4ce664f170/src/DocBlock/Tags/Param.php#L35 * see https://github.com/phpstan/phpdoc-parser/blob/d639142cf83e91a07b4862217a6f8aa65ee10d08/tests/PHPStan/Parser/TypeParserTest.php#L64 * /cc @tomasnorre this sadly effectively reverts #340 - it is too much effort to dive into further advanced types :-( * `PropertyTypeChanged` only expects the property to be both covariant and contravariant: if not, then a BC break was introduced (this re-uses some internal variance checks, simplifying code substantially) * usage of `ReflectionFunctionAbstract` is no longer present in the library: always using an union of `ReflectionMethod|ReflectionFunction` instead (as in `roave/better-reflection:^5`) * `FunctionName` is now `@internal`, since it's only used for internal rendering purposes
|
Important, before you consider adding more work to this: f8c25b3 I had to butcher your previous work to move forward :-( |
uzibhalepu
added a commit
to uzibhalepu/BackwardCompatibilityCheck
that referenced
this pull request
Aug 6, 2024
…Reflection@1f7a13b This also handles the upcoming BC break of Roave/BetterReflection#900 Some big changes: * `PropertyDocumentedTypeChanged` no longer exists: we use only `PropertyTypeChanged` for now. This is a step backwards, but it is needed to prevent issues around docblocks being too advanced for this library to parse them, for now. * see CuyZ/Valinor#6 * see https://github.com/phpstan/phpstan-src/tree/447cd102d946a2d9883ac82793195275f54296a9/src/Type * see https://github.com/vimeo/psalm/tree/f1d47cc662500589e200a978e3af85488a5236cf/src/Psalm/Type * see https://github.com/phpDocumentor/ReflectionDocBlock/blob/622548b623e81ca6d78b721c5e029f4ce664f170/src/DocBlock/Tags/Param.php#L35 * see https://github.com/phpstan/phpdoc-parser/blob/d639142cf83e91a07b4862217a6f8aa65ee10d08/tests/PHPStan/Parser/TypeParserTest.php#L64 * /cc @tomasnorre this sadly effectively reverts Roave/BackwardCompatibilityCheck#340 - it is too much effort to dive into further advanced types :-( * `PropertyTypeChanged` only expects the property to be both covariant and contravariant: if not, then a BC break was introduced (this re-uses some internal variance checks, simplifying code substantially) * usage of `ReflectionFunctionAbstract` is no longer present in the library: always using an union of `ReflectionMethod|ReflectionFunction` instead (as in `roave/better-reflection:^5`) * `FunctionName` is now `@internal`, since it's only used for internal rendering purposes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As see yesterday, native property types aren't supported currently.
https://twitter.com/tomasnorre/status/1464301968264994816
I have now added tests that proves this. I'm still trying to get my head around how to implement the change, I'm not really strong at AST, even though I have written some Rector rules.
Any pointers on how to approach this would be helpful, thanks.