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

fix: Don't create dynamic property in Dispatcher #56

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

TysonAndre
Copy link
Contributor

@TysonAndre TysonAndre commented Nov 27, 2021

https://wiki.php.net/rfc/deprecate_dynamic_properties
causes this to emit an E_DEPRECATION notice and this will throw in PHP 9.0 unless subclasses happen to declare that property as protected/public

A private property was proposed because:

  • This was undocumented and I assume unintentional.
  • This will not conflict with any properties of the same name and
    different types (or readonly properties) in subclasses.

@codecov
Copy link

codecov bot commented Nov 27, 2021

Codecov Report

Merging #56 (6a2482d) into master (b5f37db) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master      #56   +/-   ##
=========================================
  Coverage     89.07%   89.07%           
  Complexity       58       58           
=========================================
  Files             8        8           
  Lines           119      119           
=========================================
  Hits            106      106           
  Misses           13       13           
Impacted Files Coverage Δ
lib/Dispatcher.php 88.88% <ø> (ø)

https://wiki.php.net/rfc/deprecate_dynamic_properties
causes this to emit an E_DEPRECATION notice in php 8.2+
and this will throw an Error in php 9.0

A private property was chosen because:
- This was undocumented and I assume unintentional.
- This will not conflict with any properties of the same name and
  different types (or readonly properties) in subclasses.
@TysonAndre TysonAndre force-pushed the fix-deprecated-dynamic-property branch from e3ad6e3 to 6a2482d Compare November 27, 2021 20:26
@TysonAndre TysonAndre changed the title Fix creating an undocumented dynamic property fix: Don't create dynamic property in Dispatcher Nov 27, 2021
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.

1 participant