Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add missing return types and enforce return types on all methods#50821

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@wouterj
Copy link
Member

QA
Branch?6.4
Bug fix?no
New feature?no
Deprecations?yes
Tickets-
LicenseMIT
Doc PR-

In 6.3, we enforced return types on all interface methods. We're missing just a handful return types on non-interface methods (all fixed in this PR). As you can see, the added return types are either data collectors or methods introduced by 6.3 features.

In my opinion, we should start to enforce return types on all methods from 6.4 (either as PHPdoc - forresource and to avoid BC breaks - or PHP native declaration).

@carsonbotcarsonbot added this to the6.4 milestoneJun 29, 2023
@wouterjwouterj changed the titleAdd missing return types and enforce return types on all methods[WIP] Add missing return types and enforce return types on all methodsJun 29, 2023
@wouterjwouterjforce-pushed theenforce-all-return-types branch fromd17bde8 to7529f32CompareJune 29, 2023 12:17
@wouterj
Copy link
MemberAuthor

@nicolas-grekas I'm afraid I need your help with the data collector return types. It sometimes seem to return aData object and other times a plain value. I can't really reason when a method can returnData. I've fixed the types that caused the tests to fail, but I'm not sure about the untested methods. Can you have a close look at them? 🙏

@wouterjwouterjforce-pushed theenforce-all-return-types branch from7529f32 to29a8eecCompareJune 29, 2023 12:28
@nicolas-grekas
Copy link
Member

What about going with mixed? We changed some of them to return Data instead of the original data because from the pov of a twig template, we can make a Data object behave as the original one. mixed and@return Data|mixed if that makes sense?

@stof
Copy link
Member

Data|mixed is still mixed though.

derrabus and ro0NL reacted with thumbs up emoji

@wouterj
Copy link
MemberAuthor

Alright, I did some further testing and it seems like scalar types are kept as-is, while other types becomeData when callingcloneVar(). For the data collectors that do$this->cloneVar($this->data), I've updated the non-scalar return types to beData|array. Seems like that does the trick (at least, to make all tests green).

@wouterjwouterjforce-pushed theenforce-all-return-types branch from7f5270c to25376c6CompareJune 29, 2023 14:26
@wouterjwouterj changed the title[WIP] Add missing return types and enforce return types on all methodsAdd missing return types and enforce return types on all methodsJun 29, 2023
@nicolas-grekas
Copy link
Member

Thank you@wouterj.

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

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@stofstofstof left review comments

@chalasrchalasrAwaiting requested review from chalasrchalasr is a code owner

@xabbuhxabbuhAwaiting requested review from xabbuhxabbuh is a code owner

@ycerutoycerutoAwaiting requested review from ycerutoyceruto is a code owner

@lyrixxlyrixxAwaiting requested review from lyrixx

@dunglasdunglasAwaiting requested review from dunglas

@OskarStarkOskarStarkAwaiting requested review from OskarStark

@jderussejderusseAwaiting requested review from jderusse

Assignees

No one assigned

Projects

None yet

Milestone

6.4

Development

Successfully merging this pull request may close these issues.

4 participants

@wouterj@nicolas-grekas@stof@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp