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

Add types to routing and DI configuration traits. #33261

Merged

Conversation

derrabus
Copy link
Member

Q A
Branch? 4.4
Bug fix? no
New feature? no
BC breaks? I don't think so.
Deprecations? no
Tests pass? yes
Fixed tickets #32179, #33228
License MIT
Doc PR N/A

This PR backports the type declarations added to the configurator traits of the Routing and DI components. These traits expose only final methods, so it should be pretty safe to add return types to them.

The only scenario I could make up where this change will break something is if a trait is used to override a method: https://3v4l.org/EAsk8 But I doubt that those traits are used that way.

On master, we've used the object return type for the fluent methods. That type is not available on 4.4 where we have to support php 7.1. I'm using self instead. Since the methods are final and thus cannot be overridden, I believe that we shouldn't run into covariance issues here, so self should be safe.

@nicolas-grekas nicolas-grekas added this to the next milestone Aug 20, 2019
@nicolas-grekas nicolas-grekas changed the title Add types to roting and DI configuration traits. Add types to routing and DI configuration traits. Aug 20, 2019
@nicolas-grekas
Copy link
Member

Thank you @derrabus.

nicolas-grekas added a commit that referenced this pull request Aug 20, 2019
…bus)

This PR was merged into the 4.4 branch.

Discussion
----------

Add types to routing and DI configuration traits.

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | I don't think so.
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32179, #33228
| License       | MIT
| Doc PR        | N/A

This PR backports the type declarations added to the configurator traits of the Routing and DI components. These traits expose only final methods, so it should be pretty safe to add return types to them.

The only scenario I could make up where this change will break something is if a trait is used to override a method: https://3v4l.org/EAsk8 But I doubt that those traits are used that way.

On master, we've used the `object` return type for the fluent methods. That type is not available on 4.4 where we have to support php 7.1. I'm using `self` instead. Since the methods are final and thus cannot be overridden, I believe that we shouldn't run into covariance issues here, so `self` should be safe.

Commits
-------

1ca30c9 Add types to roting and DI configuration traits.
@nicolas-grekas nicolas-grekas merged commit 1ca30c9 into symfony:4.4 Aug 20, 2019
@derrabus derrabus deleted the improvement/routing-config-traits branch August 20, 2019 21:43
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
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.

None yet

3 participants