Skip to content

[ErrorHandler] trigger deprecations for @final properties #45360

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

Merged
merged 2 commits into from
Feb 10, 2022

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Feb 8, 2022

Q A
Branch? 6.1
Bug fix? no
New feature? yes
Deprecations? yes
Tickets Fix #43600
License MIT
Doc PR -

This PR makes DebugClassLoader trigger a deprecation when a property in a child class is redefined while the same property is @final on the parent class (the deprecation is silenced when both classes live in the exact same namespace or when the child property is deprecated.)

It also makes all properties in the Symfony\ namespace implicitly @final, unless they are typed. The goal here is to be able to add types to all properties in 7.0, thus fixing #43600.

Copy link
Member

@derrabus derrabus 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. Maybe we should document a way forward.

Copy link
Contributor

@fancyweb fancyweb left a comment

Choose a reason for hiding this comment

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

I'm approving the code but some tests are missing for me:

  • test with two classes in the same namespace
  • test with a class outside the Symfony\ namespace
  • test that the behavior is different for a static property
  • test that the behavior is different when the property is typed

I can help you if you want.

@nicolas-grekas
Copy link
Member Author

test with two classes in the same namespace
test with a class outside the Symfony\ namespace
test that the behavior is different when the property is typed

PR welcome on my fork :)

test that the behavior is different for a static property

I removed that check

@nicolas-grekas
Copy link
Member Author

This PR will be green after #45371 is merged. Review welcome.

@nicolas-grekas nicolas-grekas added this to the 6.1 milestone Feb 9, 2022
@carsonbot carsonbot changed the title [ErrorHandler] trigger deprecations for @final properties trigger deprecations for @final properties Feb 9, 2022
@carsonbot carsonbot changed the title trigger deprecations for @final properties [ErrorHandler] trigger deprecations for @final properties Feb 9, 2022
@nicolas-grekas
Copy link
Member Author

Thank you @fancyweb.

@nicolas-grekas nicolas-grekas merged commit 6e3d5c1 into symfony:6.1 Feb 10, 2022
@nicolas-grekas nicolas-grekas deleted the eh-final-props branch February 10, 2022 10:29
@fabpot fabpot mentioned this pull request Apr 15, 2022
nicolas-grekas added a commit that referenced this pull request Jul 28, 2023
…grekas)

This PR was merged into the 7.0 branch.

Discussion
----------

Add types to public and protected properties

| Q             | A
| ------------- | ---
| Branch?       | 7.0
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

:hot_face:

Allowed by #45360

Follows #51068 and #51067

Commits
-------

7ea2461 Add types to public and protected properties
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.

Using typed properties on public/protected properties
5 participants