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

[PHP 8.4] Fixes for implicit nullability deprecation #20128

Closed
wants to merge 2 commits into from

Conversation

Ayesh
Copy link
Contributor

@Ayesh Ayesh commented Mar 15, 2024

Fixes all issues that emit deprecation notices on PHP 8.4 for implicit nullable parameter type declarations.

See:

Q A
Is bugfix? ✔️
New feature?
Breaks BC?
Fixed issues N/A

Copy link

what-the-diff bot commented Mar 15, 2024

PR Summary

  • More adaptable trigger method in components
    The trigger method in the component base framework can now accept either a specific defined event or a null event as its second parameter. This adds flexibility in case the event isn't defined yet.

  • Flexible table interaction in ActiveQuery
    The method viaTable in our database active query now accepts either a specific callable function or a null callable as its third parameter. This makes it more adaptable to different code scenarios.

  • Enhanced 'via' method functionality
    The via method in our active query interface and active relation trait can now handle either specific callable functions or null callable as its second parameter. This enhances functionality and improves versatility in relation to database queries.

  • Improved email sending capabilities
    The send method in our base message and message interface can now utilize either a defined MailerInterface or a null MailerInterface as its first parameter. This provides better flexibility in case the mailer interface is not defined.

  • Adaptable action injection in test environment
    The actionInjection method for both our test controllers (FakePhp71Controller and FakePhp71Controller) can now receive either a defined Post or a null Post as its fifth parameter. This contributes to a more adaptable testing environment.

  • Optional Dependencies testing adjustments
    Our testOptionalDependencies method now accepts either a QuxInterface or a null QuxInterface as its first parameter. This provides improved capabilities for testing optional dependencies within the software.

  • Increased flexibility in constructors and methods in testing stubs
    Several constructors and methods (actionAksi1 and actionStringy) within our testing environment can now receive either a specific type or a null type. These changes mean our tests can simulate a more diverse range of scenarios, contributing to robust software development.

@samdark
Copy link
Member

samdark commented Mar 16, 2024

Thanks for contributing. Is that a backwards compatibility break for the case of inheritance?

@Ayesh
Copy link
Contributor Author

Ayesh commented Mar 16, 2024

Hi @samdark , thanks for the response.

Both string $v = null and ?string $v = null are considered "nullable" for inheritance, so yes, this is not a BC break.

However I just realized Yii also has to support PHP 5.4 through 7.0 as well, which have no nullable type support, so this change is breaking on those versions.

@Ayesh
Copy link
Contributor Author

Ayesh commented Mar 16, 2024

I will close this PR because we can not merge this as-is unless we either bump the minimum version to 7.1 (or drop these type declaration).

@Ayesh Ayesh closed this Mar 16, 2024
@samdark
Copy link
Member

samdark commented Mar 16, 2024

You can target the pull request for 2.2 branch which is PHP8+

@Ayesh Ayesh reopened this Mar 16, 2024
Copy link

codecov bot commented Mar 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.62%. Comparing base (9578380) to head (b732b53).

Additional details and impacted files
@@            Coverage Diff            @@
##                2.2   #20128   +/-   ##
=========================================
  Coverage     65.62%   65.62%           
  Complexity    11213    11213           
=========================================
  Files           424      424           
  Lines         36681    36681           
=========================================
  Hits          24073    24073           
  Misses        12608    12608           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Ayesh Ayesh changed the base branch from master to 2.2 March 16, 2024 08:51
Fixes all issues that emit deprecation notices on PHP 8.4 for implicit nullable parameter type declarations.

See:
 - [RFC](https://wiki.php.net/rfc/deprecate-implicitly-nullable-types)
 - [PHP 8.4: Implicitly nullable parameter declarations deprecated](https://php.watch/versions/8.4/implicitly-marking-parameter-type-nullable-deprecated)
@Ayesh Ayesh force-pushed the php84/nullability branch from d4e4d08 to cbd41f0 Compare March 16, 2024 08:54
@Ayesh
Copy link
Contributor Author

Ayesh commented Mar 16, 2024

Thank you! I rebased for 2.2.

@rob006
Copy link
Contributor

rob006 commented Mar 16, 2024

@samdark AFAIK we had plans to increase min PHP version in v2.0.50 to 7.3. Something changed in that matter? If not, we could still merge this PR to master.

@samdark
Copy link
Member

samdark commented Mar 17, 2024

Won't this be a BC break anyway because of signature mismatch?

@Ayesh
Copy link
Contributor Author

Ayesh commented Mar 17, 2024

Quite fortunately it's not a BC break. If there any sub classes extending our classes, they will need the nullability fix to avoid the deprecation, but they will still be compatible.

@rob006
Copy link
Contributor

rob006 commented Mar 17, 2024

https://3v4l.org/i7dQn#v7.3.33

@samdark samdark changed the base branch from 2.2 to master March 20, 2024 09:56
@samdark samdark changed the base branch from master to 2.2 March 20, 2024 09:56
@samdark
Copy link
Member

samdark commented Mar 20, 2024

Could go into master then. @Ayesh sorry to ask but could you rebase it back? :)

Ayesh added a commit to Ayesh/yii2 that referenced this pull request Mar 20, 2024
Fixes all issues that emit deprecation notices on PHP 8.4 for implicit nullable parameter type declarations.
Related to yiisoft#20128. This is against the `master` branch while the other PR targets the `2.2` branch.

See:
 - [RFC](https://wiki.php.net/rfc/deprecate-implicitly-nullable-types)
 - [PHP 8.4: Implicitly nullable parameter declarations deprecated](https://php.watch/versions/8.4/implicitly-marking-parameter-type-nullable-deprecated)
@Ayesh
Copy link
Contributor Author

Ayesh commented Mar 20, 2024

No problemo, I opened #20133 separately targeting master branch. It will fail because it still builds on PHP <= 7.0, but contains all places I found to trigger the deprecation notice.

samdark added a commit that referenced this pull request Mar 26, 2024
Fixes all issues that emit deprecation notices on PHP 8.4 for implicit nullable parameter type declarations.
Related to #20128.

See:
 - [RFC](https://wiki.php.net/rfc/deprecate-implicitly-nullable-types)
 - [PHP 8.4: Implicitly nullable parameter declarations deprecated](https://php.watch/versions/8.4/implicitly-marking-parameter-type-nullable-deprecated)

Co-authored-by: Wilmer Arambula <[email protected]>
Co-authored-by: Alexander Makarov <[email protected]>
@samdark samdark closed this Mar 26, 2024
@samdark
Copy link
Member

samdark commented Mar 26, 2024

Closing since it will be merged from master into 2.2.

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.

3 participants