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

Whip_RequirementsChecker: explicitly declare all properties #117

Merged
merged 1 commit into from
Jan 6, 2022

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Dec 23, 2021

Dynamic (non-explicitly declared) property usage is expected to be deprecated as of PHP 8.2 and will become a fatal error in PHP 9.0.

There are a number of ways to mitigate this:

  • If it's an accidental typo for a declared property: fix the typo.
  • For known properties: declare them on the class.
  • For unknown properties: add the magic __get(), __set() et al methods to the class or let the class extend stdClass which has highly optimized versions of these magic methods build in.
  • For unknown use of dynamic properties, the #[AllowDynamicProperties] attribute can be added to the class. The attribute will automatically be inherited by child classes.

In this case, both the $configuration property, as well as the $messageManager property (both set in the class constructor) fall in the "known property" category and should be declared.

Note: while this could be considered a BC-break, I've elected to make these properties private (was previously automatically public due to these being dynamic properties).

Refs:

@jrfnl jrfnl added this to the 1.x Next Release milestone Dec 23, 2021
Dynamic (non-explicitly declared) property usage is expected to be deprecated as of PHP 8.2 and will become a fatal error in PHP 9.0.

There are a number of ways to mitigate this:
* If it's an accidental typo for a declared property: fix the typo.
* For known properties: declare them on the class.
* For unknown properties: add the magic `__get()`, `__set()` et al methods to the class or let the class extend `stdClass` which has highly optimized versions of these magic methods build in.
* For unknown _use of_ dynamic properties, the `#[AllowDynamicProperties]` attribute can be added to the class. The attribute will automatically be inherited by child classes.

In this case, both the `$configuration` property, as well as the `$messageManager` property fall in the "known property" category and should be declared.

Note: while this _could_ be considered a BC-break, I've elected to make these properties `private` (was previously automatically `public` due to these being dynamic properties).

Refs:
* https://wiki.php.net/rfc/deprecate_dynamic_properties
@jrfnl jrfnl force-pushed the JRF/php-8.2/handle-dynamic-property branch from ce5ba97 to df5ab81 Compare December 23, 2021 07:00
@jrfnl
Copy link
Contributor Author

jrfnl commented Dec 23, 2021

@diedexx As you are working on something to do with WHIP, want to take a look at this ?

Copy link
Member

@diedexx diedexx left a comment

Choose a reason for hiding this comment

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

Look good 👍

@diedexx diedexx merged commit 022695c into master Jan 6, 2022
@diedexx diedexx deleted the JRF/php-8.2/handle-dynamic-property branch January 6, 2022 08:17
@enricobattocchi enricobattocchi modified the milestones: 1.x Next Release, 2.0 Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants