Skip to content

bug: protected method in final class -> private is not a BC break#986

Merged
Ocramius merged 2 commits intoRoave:8.18.xfrom
acoulton:bug-remove-protected-method-from-final-class
Feb 9, 2026
Merged

bug: protected method in final class -> private is not a BC break#986
Ocramius merged 2 commits intoRoave:8.18.xfrom
acoulton:bug-remove-protected-method-from-final-class

Conversation

@acoulton
Copy link
Copy Markdown
Contributor

@acoulton acoulton commented Feb 9, 2026

A method should only be considered "accessible" outside the class if:

  • It is public
  • It is protected and the class is NOT final

Previously, protected methods in final classes were incorrectly
considered to be accessible. This meant that changing one to protected
would incorrectly be reported as a BC break.

This PR introduces two commits - one that introduces a new testcase to prove the issue, and a second with the fix. I will push separately so that CI shows the new test failing initially.

A final class cannot be extended, therefore `protected` and `private`
are functionally identical. `protected` methods may exist in final
classes for historical reasons, but making them `private` is not a BC
break.
A method should only be considered "accessible" outside the class if:

* It is `public`
* It is `protected` and the class is NOT `final`

Previously, `protected` methods in `final` classes were incorrectly
considered to be accessible. This meant that changing one to `protected`
would incorrectly be reported as a BC break.

This commit fixes that logic.
@acoulton
Copy link
Copy Markdown
Contributor Author

acoulton commented Feb 9, 2026

I realised my "push separately" strategy doesn't work as the CI needs approval for new contributors. I figured it was therefore easier to push both commits now, but let me know if you'd like to see proof of the test failing.

Comment thread test/unit/DetectChanges/BCBreak/ClassBased/MethodRemovedTest.php
Copy link
Copy Markdown
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

🚢 thanks @acoulton!

@Ocramius Ocramius self-assigned this Feb 9, 2026
@Ocramius Ocramius added the bug label Feb 9, 2026
@Ocramius Ocramius added this to the 8.18.0 milestone Feb 9, 2026
@Ocramius Ocramius merged commit 8bbe200 into Roave:8.18.x Feb 9, 2026
14 checks passed
@acoulton acoulton deleted the bug-remove-protected-method-from-final-class branch February 9, 2026 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants