Implement colocated mapping driver #9587
Conversation
greg0ire
force-pushed
the
implement-colocated-driver
branch
from
f25b066
to
eeeee82
Compare
| * Initializes a new AnnotationDriver that uses the given AnnotationReader for reading | ||
| * docblock annotations. | ||
| * | ||
| * @param Reader $reader The AnnotationReader to use, duck-typed. |
There was a problem hiding this comment.
We should move away from the duck-typed part, imho. It has caused us trouble already.
| * | ||
| * @var Reader | ||
| */ | ||
| protected $reader; |
There was a problem hiding this comment.
Shall we flag the property as @internal and make it private on 3.0?
There was a problem hiding this comment.
Shouldn't we deprecate AnnotationDriver and remove it entirely in 3.0?
Also, I'm wondering why AttributeDriver has a getReader() method, and is not final 🤔
| @@ -25,8 +26,10 @@ | |||
|
|
|||
| use const PHP_VERSION_ID; | |||
|
|
|||
| class AttributeDriver extends AnnotationDriver | |||
| class AttributeDriver implements MappingDriver | |||
There was a problem hiding this comment.
Ah, I should implement getReader as well, removing that method is a BC-break I guess 😅 … but since it's not breaking any tests, I think I should instantly deprecate it.
This allows us to decouple further from doctrine/annotations, and to fix some static analysis issues. The assumption being made here is that the abstract class we are no longer extending is not used in type declarations and instanceof checks.
greg0ire
force-pushed
the
implement-colocated-driver
branch
from
eeeee82
to
cd57768
Compare
| * Initializes a new AnnotationDriver that uses the given AnnotationReader for reading | ||
| * docblock annotations. | ||
| * | ||
| * @param Reader $reader The AnnotationReader to use |
There was a problem hiding this comment.
why not using an actual parameter type ?
There was a problem hiding this comment.
That would be a BC break. The old constructor explicitly mentioned the possibility of duck-typed annotation readers.
| @@ -29,8 +32,19 @@ | |||
| /** | |||
| * The AnnotationDriver reads the mapping metadata from docblock annotations. | |||
| */ | |||
| class AnnotationDriver extends AbstractAnnotationDriver | |||
| class AnnotationDriver implements MappingDriver | |||
There was a problem hiding this comment.
This BC break needs to be reverted, and moved to the next major.
Ref: #9668
This allows us to decouple further from
doctrine/annotations, and to fixsome static analysis issues.
The assumption being made here is that the abstract class we are no
longer extending is not used in type declarations and
instanceofchecks.