The future of Common 3.0

[Majkl578]

The proposal is pretty simple: have no Common 3.0, instead split Common into smaller packages. We've already had discussion about this and Common 3.0 seems pretty much like relic and multiple components have already been split to their own packages.

Current proposal:

Component	Suggested action
Persistence	Move to new package doctrine/persistence (rename namespace to Doctrine\Persistence)
Proxy	To be dropped without replacement.
Reflection	Not really sure about this one, either move to doctrine/reflection or drop it.
Util/ClassUtils	Proxy-related methods are obsolete making the rest also pretty much obsolete - drop it
Util/Debug	Drop it or move to doctrine/debug.
Util/Dumper	Unsure about its usefulness (there already exist more advanced tools like Tracy or Symfony Dumper).
Util/Inflector	Already moved out, only BC.
ClassLoader	Already deprecated, drop it.
CommonException	~
Comparable	I know no place where Comparable is used across Doctrine (is there any?) but it seems useful i.e. for ORM 3.0 - move it to doctrine/comparable.
EventArgs + EventManager + EventSubscriber	We've already discussed replacing it by i.e. Symfony's EventDispatcher - drop it if suitable replacement is selected, doctrine/event-manager.
Lexer	Already moved out, only BC.
NotifyPropertyChanged + PropertyChangedListener	These sound useful and are used in ORM so move it to doctrine/property-change-notifier (what a name!) doctrine/persistence.
Version	~

Each new package would use PHP 7.2 (or even 7.1 where 7.2 has no benefit), follow CS and would be ported to typed code.

WDYT?

[Ocramius]

EventArgs + EventManager + EventSubscriber We've already discussed replacing it by i.e. Symfony's EventDispatcher - drop it if suitable replacement is selected, doctrine/event-manager.

Only if proven to be faster at runtime.

Lexer Already moved out, only BC.

Deprecate and use Hoa components

NotifyPropertyChanged + PropertyChangedListener These sound useful and are used in ORM so move it to doctrine/property-change-notifier (what a name!).

Move to ORM, rework as isolated layer if possible

Version ~

Drop it - relying on package versions should only be done via composer.json

[jmikola]

NotifyPropertyChanged + PropertyChangedListener These sound useful and are used in ORM so move it to doctrine/property-change-notifier

Move to ORM, rework as isolated layer if possible

Would ODM need these as well?

[malarzm]

NotifyPropertyChanged + PropertyChangedListener These sound useful and are used in ORM so move it to doctrine/property-change-notifier

Move to ORM, rework as isolated layer if possible

Would ODM need these as well?

We'd need to port these as well

[alcaeus]

I know no place where Comparable is used across Doctrine (is there any?) but it seems useful i.e. for ORM 3.0 - move it to doctrine/comparable.

We were thinking of using it to compare value objects instead of comparing them by reference in ODM 2.0. Extracting it to a separate package would make sense.

[alcaeus]

The proposal is pretty simple: have no Common 3.0, instead split Common into smaller packages.

Yes, please. doctrine/common should die. I've given the whole BC problem and the fact that upgrading one to 3.0 will conflict with a bunch of other stuff, so here's an idea on how to migrate to 3.0 versions of everything:

1. Split code in individual packages, remove from doctrine/common
2. update composer.json to require all these new packages (e.g. doctrine/comparable)
3. Add autoload file to doctrine/common that triggers a deprecation notice for doctrine/common itself
4. Release the whole contraption as 2.9.0: this essentially makes doctrine/common a metapackage that throws a deprecation notice, but keeps everything else intact
5. Start updating our own dependents of doctrine/common to no longer require doctrine/common but individual packages
6. Start releasing 3.0 versions with BC breaks of those individual packages

Reflection: Not really sure about this one, either move to doctrine/reflection or drop it.

What do we need it for? IIRC there are far better reflection libraries out there, so we might want to use those

Each new package would use PHP 7.2 (or even 7.1 where 7.2 has no benefit), follow CS and would be ported to typed code.

Ack on PHP versions, Ack on CS, not entirely sure about typed code. Many instances probably are fine, but there are some cases where we might not want to add type hints just yet. Take the following interface for example:

interface Collection
{
    // ...
    public function get(int $index): object;
}

Makes sense, right? Except that without full co- and contravariance in PHP, I can't do this:

class SpecialCollection implements Collection
{
    // ...
    public function get(int $index): MyEntityClass {}
}

This is perfectly fine when Collection::get does not specify a return type, but fails when it does. I believe there are a few cases where we might want to hold back on strictly typing our interfaces to allow for stricter implementations.

[Majkl578]

We were thinking of using it to compare value objects instead of comparing them by reference in ODM 2.0. Extracting it to a separate package would make sense.

Same idea is there for ORM 3.0 to compare DBAL values and Embeddables by value.

Add autoload file to doctrine/common that triggers a deprecation notice for doctrine/common itself

Also class aliases will be needed since we change namespace.

Start releasing 3.0 versions with BC breaks of those individual packages

For consistency with other packages that were already split earlier, we should start versioning at 1.0.

Reflection

What do we need it for?

I have found no reference for it. It was added as part of #154/#152 probably as a more memory effective variant based on PSR-0.

not entirely sure about typed code

Collection as it is designed now is all about mixed members, not object, so no object typehints there unless we decide it will be object-only.
Your example is otherwise valid, there is no variance along with object due to controversy and possible future scope. But honestly I know noone (or no project) that'd use custom collections to change return types (and you couldn't change parameter for i.e. Collection::contains(object $element) types anyway).
In general for such case (same applies more or less to EntityManager or ORM repositories), object is the only correct way and if you miss the opportunity to add it now, you can miss it for another X years while possibly having PHP variance for object natively (or not) in future 7.x release, who knows.

[alcaeus]

Also class aliases will be needed since we change namespace.

We haven't done so for inflector, lexer, collections, or other packages, which is why I didn't think of it.

and you couldn't change parameter [...] types anyway

You're right - this one is probably best solved using documentation (or generics in PHP)

[Majkl578]

We haven't done so for ...

Hmm true, they are still in Doctrine\Common namespace... That should probably also change in major release of those packages + providing forward compat by adding class aliases in 1.x (something like PHPUnit did).

[alcaeus]

That should probably also change in major release of those packages + providing forward compat by adding class aliases in 1.x

FWIW, this is something that has been discussed for annotations 2.0, where it has triggered quite the discussion: doctrine/annotations#75 (comment). I'm split on this issue: on one hand, cleaning up namespaces would be nice, on the other hand it would make things way more complicated. I will however concede that we could ease the migration by providing a forward compat layer. That would allow people to better prepare for the new package versions.

[Majkl578]

When it comes to changing package's namespace, I think most people are concerned about compatibility between both major versions - x.y & x+1.0. This shouldn't be hard to achieve with mentioned forward compatibility layer where we introduce new namespace and class aliases in x.y version and migrate classes in x+1.0. Something like PHPUnit did around version 4.
Then doing such migration is as easy as one regexp replace (sed/IDE) over whole code base.

[stof]

Util/Debug Drop it or move to doctrine/debug.

I suggest dropping it. this Debug utility is shitty anyway, as dumping an uninitialized PersistentCollection will have the side effect of initializing it (and so dumping some variable to debug something will change the behavior of the code, which could then not have the bug you were debugging).
And the output format is not nice either. We should just recommend symfony/var-dumper instead.

[Majkl578]

Update:

• Event Manager was separated into doctrine/event-manager: Separate Event Manager to doctrine/event-manager #842
• Persistence was separated into doctrine/persistence: Separate persistence and reflection #844
• Reflection was separated into doctrine/reflection: Separate persistence and reflection #844

Currently with undecided fate (so still left without deprecation):

• NotifyPropertyChanged + PropertyChangedListener
• Comparable

Deprecations for the rest of remaining things via #845, targeting 2.9.0 which should be released in upcoming days.

[Majkl578]

https://www.doctrine-project.org/2018/07/12/common-2-9-and-dbal-2-8-and-orm-2-6-2.html

[pamil]

Great job, happy to see so much progress towards 3.0! 🎉

[sterichards]

Is there a checklist available to describe the steps to be taken?

Obviously remove doctrine/common....

Which Doctrine packages should be installed to cover the removal of doctrine/common?

Am I correct in presuming:

• doctrine/persistence
• doctrine/event-manager
• doctrine/reflection

?

[Ocramius]

@sterichards doctrine/common would provide the list inside composer.json (it already does for some moved out packages)

[sterichards]

@Ocramius In that case, it would be:

• "doctrine/inflector"
• "doctrine/cache"
• "doctrine/collections"
• "doctrine/lexer"
• "doctrine/annotations"
  - "doctrine/event-manager"
  - "doctrine/reflection"
  - "doctrine/persistence"

[Majkl578]

Except the last three, the other were moved out years ago. Please see the release notes for 2.9.0 where the recent changes are described.

[sterichards]

Removing doctrine/common is looking more and more difficult as nearly every latest stable doctrine package requires doctrine/common

As an example - https://packagist.org/packages/doctrine/data-fixtures#v1.3.1:

Requires:

• doctrine/common: ~2.2

[Ocramius]

@sterichards we'd just migrate each of our own downstream package: we're not breaking BC anyway, so relying on doctrine/common is fine for now

[Majkl578]

Removing doctrine/common is looking more and more difficult

The ultimate aim is not to drop doctrine/common in all consumers now, but to phase it out gradually. First major step was separating components that will outlive Common. Right now DBAL already benefits from this change (see doctrine/dbal#3181). It's more tricky in other pacakges - i.e. ORM uses ProxyGenerator and NotifyPropertyChanged so it can't drop doctrine/common in 2.x, only ORM 3.0.

TL;DR: In long-term it's preferable if can make your application depend on specific packages instead of doctrine/common (your dependencies may still draw doctrine/common in, but that wouldn't be your problem anymore - new packages are co-installable with doctrine/common 2.9+). Otherwise doctrine/common is still here and not abandoned, and it won't (can't) be at least until ORM 3.0.

[sterichards]

Makes sense

I was looking for an approach that would eliminate deprecated warnings, which is what lead me to this issue ticket initially

[fmonts]

Makes sense

I was looking for an approach that would eliminate deprecated warnings, which is what lead me to this issue ticket initially

Agree, please remove the deprecation warning since it's quite annoying and it's a false positive, it's not something the developer of a Symfony app is supposed to fix

[Ocramius]

Send a patch: https://www.doctrine-project.org/policies/deprecation.html

[Majkl578]

I was looking for an approach that would eliminate deprecated warnings

please remove the deprecation warning since it's quite annoying and it's a false positive

Which warning(s) exactly? Most people notice the issue coming from ORM (doctrine/orm#7306 that will be fixed in ORM 2.6.3 once released.

[sterichards]

I see the deprecated warning when running PHPUnit

It's no hassle to me if it's going to be resolved in a future version and isn't causing any lack of functionality at the moment

[sterichards]

If anybody else comes across this thread, there is a method for suppressing deprecation notices in PHPUnit - https://stackoverflow.com/questions/35897550/remove-remaining-deprecation-notices-232-in-symfony-2-8

[fmonts]

I was looking for an approach that would eliminate deprecated warnings

please remove the deprecation warning since it's quite annoying and it's a false positive

Which warning(s) exactly? Most people notice the issue coming from ORM (doctrine/doctrine2#7306 that will be fixed in ORM 2.6.3 once released.

Why it hasn't been released yet? How long will it take to have a release? It's been many months now...

[fmonts]

lol, it was released yesterday :)

good job, the deprecation warning is finally gone

[TerjeBr]

What about Doctrine\Common\Util\ClassUtils::getRealClass(string $class) ?
Where is that function going to live? Or what is the replacement?

[Ocramius]

@TerjeBr it would likely be replaced by https://github.com/Ocramius/ProxyManager/blob/addb7d39c990d3d2299315f0200e78ebe658cd7a/src/ProxyManager/Inflector/ClassNameInflector.php#L41

[TerjeBr]

@Ocramius you should put that in the deprecation warning then. It is not an obvious place (at least for me) to look for a replacement.

Why do you call it an inflector anyway? I only assicated inflection with grammar and verbs/substantives being inflected.

[Ocramius]

@TerjeBr feel free to send suggestions to the upstream repository - I'm in no way good at naming, specifically not in a language that isn't my native one.

[TerjeBr]

@Ocramius The naming of the class is one thing. But my main point was that the new class should be mentioned in the deprecation warning. It is not nice to get deprecations, and then have no idea where to find the new implementation.

[Majkl578]

@TerjeBr There are two issues here:

• First, Doctrine\Common\Proxy is being deprecated, and while ProxyManager is suggested replacement for consumers, you can't just swap the ClassUtils with ClassNameInflector without changing the actual proxy layer.
• Second, your issue is rather with subsequent deprecation in ORM/ODM where the same as above applies, you will be able to use ClasNameInflector in ORM 3.0 but not in 2.x.

[TerjeBr]

The reason you have deprecation warnings, is so that applications can change to not depend on the deprecated code that is scheduled to be removed later. But what is the purpose of having deprecation warnings if there is nothing to change to? If I understand you right there is no alternative to Doctrine\Common\Util\ClassUtils::getRealClass(string $class) in doctrine 2.x, right?

This means that as long as my application remain on the latest 2.x version, we are stuck with those deprecation warnings? That is just insane. It will also do a huge disservice to the cummunity, as it just motivates people to ignore deprecation warnings all together.

[Ocramius]

Well, yeah, this is an endpoint where we don't have a clear migration path at the moment. If you want, you can make ClassUtils::getRealClass() proxy through to the new API in 3.x, and then we can drop it in 4.x.

Can you please fork off the discussion to a new issue though?

[TerjeBr]

Ok, moving this discussion to #867