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

Mark classes as final with an annotation #596

Open
wants to merge 1 commit into
base: 3.5.x
Choose a base branch
from

Conversation

greg0ire
Copy link
Member

@greg0ire greg0ire commented Feb 3, 2025

This can serve as a deprecation warning for people extending those classes.

This can serve as a deprecation warning for people extending those
classes.
@greg0ire greg0ire changed the base branch from 3.4.x to 3.5.x February 3, 2025 17:12
@@ -16,6 +16,7 @@
use function count;
use function get_class;

/** @final */
Copy link
Member

Choose a reason for hiding this comment

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

the convention used in Symfony when introducing those annotations is to write them as @final since 3.5.0. The DebugClassLoader of the ErrorHandler will not treat them differently, however our BC policy is different:

  • @final since ... requires contributors to consider those classes as not final regarding the kind of changes allowed in them, as BC still needs to be supported until the next major version
  • @final allows contributors to apply the same rules than for final classes

Our usages of @final are then mostly for cases where we still want to allow mocking that class (which is not possible for classes using the final keyword in PHP) and we convert the @final since ... annotations into either an actual final keyword or a @final annotation when preparing the next major version.

Note that this distinction between @final and @final since ... also exists for @internal and @internal since ... (even though @internal is then a lot more common, as there is no alternative using a PHP keyword)

Copy link
Member Author

Choose a reason for hiding this comment

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

OK… right now I don't think we have any @final since or @internal since inside Doctrine, so I'd stick with this for the sake of consistency. Whether we need to switch to something more complicated would need to be discussed separately. Anyway, in Doctrine, I don't think we consider @final reason enough to allow BC-breaking changes, otherwise adding @final should be considered a breaking change.

Copy link
Member

Choose a reason for hiding this comment

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

In Symfony, we indeed don't allow adding @final or @internal in minor versions (on existing code from previous releases, of course), as that would remove them from the BC guarantees. they are always added first with the since <version> variant first

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.

2 participants