Skip to content

Conversation

@samsonasik
Copy link
Member

@samsonasik samsonasik commented Dec 6, 2022

@TomasVotruba per discussion on #3161 (comment)

This PR remove PhpDocTypeChanger->changeVarType() without changing Property Type on TypedPropertyFromAssignsRector

The rule is also using:

$this->propertyTypeDecorator->decoratePropertyUnionType($inferredType, $typeNode, $node, $phpDocInfo);

which chaning both type and docblock, but as the service is used in multiple rules in vendor/, so I added $changeVarTypeFallback for it, so it is safer.

so the usage can be:

$this->propertyTypeDecorator->decoratePropertyUnionType($inferredType, $typeNode, $node, $phpDocInfo, false);

@samsonasik
Copy link
Member Author

All checks have passed 🎉 @TomasVotruba it is ready for review.

@samsonasik
Copy link
Member Author

@TomasVotruba I added $changeVarTypeFallback flag param on PropertyTypeDecorator->decoratePropertyUnionType() as method re-use in vendor so it only apply typed strict on this rule only, as the method is used in vendor/ as well to avoid broken test in vendor.

@samsonasik samsonasik force-pushed the remove-change-var-type branch from 73699b4 to 898e956 Compare December 6, 2022 15:05
@samsonasik
Copy link
Member Author

I rolled back add $changeVarTypeFallback flag param on PropertyTypeDecorator->decoratePropertyUnionType() as it cause error in rules-tests

@samsonasik
Copy link
Member Author

Not sure why suddently the rules-tests error:

There were 2 errors:

1) Rector\Tests\PSR4\Rector\FileWithoutNamespace\NormalizeNamespaceByPSR4ComposerAutoloadRector\NormalizeNamespaceByPSR4ComposerAutoloadRectorTest::test with data set #10 ('/home/runner/work/rector-src/...hp.inc')
Error: Object of class PhpParser\Node\Stmt\Namespace_ could not be converted to string

@samsonasik
Copy link
Member Author

CI finally green again 🎉 , let's add back $changeVarTypeFallback flag param on PropertyTypeDecorator->decoratePropertyUnionType()

@samsonasik
Copy link
Member Author

I added back $changeVarTypeFallback flag param on PropertyTypeDecorator->decoratePropertyUnionType() 46c7d73

@samsonasik samsonasik closed this Dec 6, 2022
@samsonasik samsonasik reopened this Dec 6, 2022
@samsonasik
Copy link
Member Author

Close-ReOpen to restart build.

@samsonasik
Copy link
Member Author

Finally 🎉 All checks have passed 🎉 @TomasVotruba I think it is ready.

@TomasVotruba
Copy link
Member

Thank you 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants