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.

Sign up for GitHub

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
Open

Mark classes as final with an annotation #596

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions UPGRADE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Upgrade

## Upgrade to 3.5

## Final classes

Some classes have been marked as `@final` because they are not supposed to be
extended. They will be `final`, and most of them will be marked with
`@internal` in 4.0.0.

## From 2.x to 3.0.0

- The configuration for the migration namespace and directory changed as follows:
Expand Down Expand Up @@ -79,8 +87,8 @@ doctrine_migrations:
### Underlying doctrine/migrations library

Upgrading this bundle to `3.0` will also update the `doctrine/migrations` library to the version `3.0`.
Backward incompatible changes in `doctrine/migrations` 3.0
are documented in the dedicated [UPGRADE](https://github.com/doctrine/migrations/blob/3.0.x/UPGRADE.md) document.
Backward incompatible changes in `doctrine/migrations` 3.0
are documented in the dedicated [UPGRADE](https://github.com/doctrine/migrations/blob/3.0.x/UPGRADE.md) document.

- The container is not automatically injected anymore when a migration implements `ContainerAwareInterface`. Custom
migration factories should be used to inject additional dependencies into migrations.
1 change: 1 addition & 0 deletions src/Collector/MigrationsCollector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

class MigrationsCollector extends DataCollector
{
/** @var DependencyFactory */
Expand Down
1 change: 1 addition & 0 deletions src/Collector/MigrationsFlattener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use function array_map;

/** @final */
class MigrationsFlattener
{
/**
Expand Down
1 change: 1 addition & 0 deletions src/DependencyInjection/CompilerPass/ConfigureDependencyFactoryPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use function is_string;
use function sprintf;

/** @final */
class ConfigureDependencyFactoryPass implements CompilerPassInterface
{
public function process(ContainerBuilder $container): void
Expand Down
1 change: 1 addition & 0 deletions src/DependencyInjection/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use function strtoupper;
use function substr;

/** @final */
class Configuration implements ConfigurationInterface
{
public function getConfigTreeBuilder(): TreeBuilder
Expand Down
1 change: 1 addition & 0 deletions src/DependencyInjection/DoctrineMigrationsExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
use function strlen;
use function substr;

/** @final */
class DoctrineMigrationsExtension extends Extension
{
/**
Expand Down
1 change: 1 addition & 0 deletions src/DoctrineMigrationsBundle.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

use function dirname;

/** @final */
class DoctrineMigrationsBundle extends Bundle
{
/** @return void */
Expand Down