Skip to content
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

BUGFIX: prevent invalid return type error #169

Merged
merged 2 commits into from
Jun 10, 2022
Merged

Conversation

MCStreetguy
Copy link
Contributor

Removes most of the fixed return types inside the ContentCacheSegmentAspect class, as proposed in the linked issue.

Fixes #168

@MCStreetguy
Copy link
Contributor Author

MCStreetguy commented Jun 8, 2022

Unfortunately, I still had to touch up my changes several times due to errors in CircleCI, so I just squashed my commits to keep the history as clean as possible. I hope everything is okay now and can be merged, but I'd be happy to receive further input.

*/
public function wrapUncachedSegment(JoinPointInterface $joinPoint): string
public function wrapUncachedSegment(JoinPointInterface $joinPoint)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here the original function actually always returns a string.

*/
public function wrapCachedSegment(JoinPointInterface $joinPoint): string
public function wrapCachedSegment(JoinPointInterface $joinPoint)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original function returns a string

*/
public function wrapDynamicSegment(JoinPointInterface $joinPoint): string
public function wrapDynamicSegment(JoinPointInterface $joinPoint)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the others. The original function only returns a string.

*/
public function wrapEvaluateUncached(JoinPointInterface $joinPoint): string
public function wrapEvaluateUncached(JoinPointInterface $joinPoint)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense, thx 👍

@@ -170,8 +178,10 @@ public function interceptFusionObject(JoinPointInterface $joinPoint): void
/**
* @param mixed $segment This is mixed as the RuntimeContentCache might also return none string values
* @param mixed[] $info
*
* @return mixed
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @return mixed
* @return mixed the cached data might not be of type string, so we cannot define the return type

Copy link
Collaborator

@Sebobo Sebobo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think only two of the methods need to be adjusted. Wdyt?

@MCStreetguy
Copy link
Contributor Author

You are absolutely right! I have changed the return types directly in all uses of the function to avoid a follow up error, but the ContentCache can indeed return nothing else than strings. I'll adjust the code right away and also include your proposed return type description. Thank you a lot for your review.

restored string return types on segment wrapper advices where applicable
Copy link
Collaborator

@Sebobo Sebobo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good thx!

@Sebobo Sebobo merged commit b427bcf into t3n:master Jun 10, 2022
@MCStreetguy
Copy link
Contributor Author

With pleasure! I am glad that I could contribute a part.

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.

Invalid return type in ContentCacheSegmentAspect
2 participants