doc: Add type to properties #2652

GromNaN · 2024-06-24T07:37:51Z

Q	A
Type	doc
BC Break	no
Fixed issues	-

Summary

Using types properties is recommended to improve type safety.

I still have to check that everything works properly if the properties are not set with a default value. Because when there's no type, the value is null by default.

GromNaN · 2024-06-24T07:39:19Z

docs/en/reference/attributes-reference.rst

@@ -281,7 +281,7 @@ Optional attributes:
             ],
             defaultDiscriminatorValue: 'user',
        )]
-        private $creator;
+        private User|Author $creator;


I hope I didn't make a mistake. I find that the type helps to understand the content of the variable here.

This looks correct to me based on the discriminator map.

GromNaN · 2024-06-24T07:45:20Z

docs/en/reference/aggregation-stage-reference.rst

@@ -417,7 +417,7 @@ to be considered when looking up one-to-one relationships:
            cascade: 'all',
            storeAs: 'id',
        )]
-        private $items;
+        private Collection $items;


This is a ReferenceOne. The variable is plural so I supposed it's a Collection. But I would have expected a single Item. I'll check.

This should be:

Suggested change

private Collection $items;

private Item $item;

GromNaN · 2024-06-24T08:06:17Z

docs/en/reference/best-practices.rst

+   The properties' type hints must be ``Collection``, and cannot be
+   ``ArrayCollection``. When the ``User`` object is retrieved from the database,
+   the properties ``$addresses`` and ``$articles`` are instances of
+   ``Doctrine\ODM\MongoDB\PersistentCollection`` to track changes.


PersistentCollection is not internal, so I think we can mention this class.

GromNaN · 2024-06-24T08:07:49Z

docs/en/reference/change-tracking-policies.rst

@@ -85,11 +85,11 @@ follows:
    {
        // ...

-        private $_listeners = [];


Prefixing private properties with an underscore is so PHP 4.

GromNaN · 2024-06-24T08:09:10Z

docs/en/reference/custom-mapping-types.rst

@@ -31,7 +31,7 @@ class:
        public function convertToPHPValue($value): \DateTime
        {
            // This is called to convert a Mongo value to a PHP representation
-            return new \DateTime('@' . $value->sec);
+            return $value->toDateTime();


It's so much simpler.

Unrelated, but we should try to come up with a different example, as this only reproduces the DateTime type. I've previously used the example of storing the time zone alongside the UTCDateTime instance and applying that to the date type when reading.

Done: #2654

alcaeus · 2024-06-25T07:58:10Z

docs/en/reference/attributes-reference.rst

    }

+.. note::


Good to have this documented 👍

alcaeus · 2024-06-25T07:58:28Z

docs/en/reference/attributes-reference.rst

@@ -281,7 +281,7 @@ Optional attributes:
             ],
             defaultDiscriminatorValue: 'user',
        )]
-        private $creator;
+        private User|Author $creator;


This looks correct to me based on the discriminator map.

alcaeus · 2024-06-25T09:16:55Z

docs/en/reference/aggregation-stage-reference.rst

@@ -417,7 +417,7 @@ to be considered when looking up one-to-one relationships:
            cascade: 'all',
            storeAs: 'id',
        )]
-        private $items;
+        private Collection $items;


This should be:

Suggested change

private Collection $items;

private Item $item;

docs/en/reference/change-tracking-policies.rst

alcaeus · 2024-06-25T09:20:16Z

docs/en/reference/custom-mapping-types.rst

@@ -31,7 +31,7 @@ class:
        public function convertToPHPValue($value): \DateTime
        {
            // This is called to convert a Mongo value to a PHP representation
-            return new \DateTime('@' . $value->sec);
+            return $value->toDateTime();


Unrelated, but we should try to come up with a different example, as this only reproduces the DateTime type. I've previously used the example of storing the time zone alongside the UTCDateTime instance and applying that to the date type when reading.

alcaeus · 2024-06-25T09:24:47Z

docs/en/reference/query-builder-api.rst


            #[Field(notSaved: true)]
-            public $score;
+            public int $score;


The type of the field defaults to string, but I agree that the property name indicates an integer. With that in mind, we should add type: 'int' to the field attribute.

alcaeus · 2024-06-25T09:25:54Z

docs/en/reference/reference-mapping.rst

            #[ReferenceMany(targetDocument: Account::class)]
-            private $accounts = [];
+            private Collection $accounts;


This may apply to multiple classes, but we should default to initialising this with an empty ArrayCollection instance so it doesn't have to happen in the constructor:

Suggested change

private Collection $accounts;

private Collection $accounts = new ArrayCollection();

That's not allowed unless CPP is used. Adding the constructor.

Yep, this is still PHP 😁

OMFG - I could've sworn that you can initialise properties like that in general now. Boo :(

alcaeus

One minor change to namespaces, but LGTM aside from that

docs/en/reference/introduction.rst

alcaeus · 2024-06-26T09:04:47Z

docs/en/reference/reference-mapping.rst

            #[ReferenceMany(targetDocument: Account::class)]
-            private $accounts = [];
+            private Collection $accounts;


OMFG - I could've sworn that you can initialise properties like that in general now. Boo :(

Co-authored-by: Andreas Braun <git@alcaeus.org>

* 2.9.x: (24 commits) Fix typo in code example (#2670) Label PRs about GH actions with "CI" (#2632) Review basic mapping (#2668) Fix wording (#2667) Add native type to private properties and final classes (#2666) Review and add tests on `ResolveTargetDocumentListener` (#2660) Remove soft-delete-cookbook (#2657) doc: Remove wakeup and clone cookbook (#2663) Modernize generated code for Hydrators (#2665) Add tests for introduction (#2664) doc: Review mapping ORM and ODM cookbook (#2658) doc: Review cookbook on blending ORM and ODM (#2656) doc: Review and test validation cookbook (#2662) Update custom mapping example (#2654) doc: Review Simple Search Engine Cookbook (#2659) doc: Add cookbook about embedding referenced documents using $lookup (#2655) doc: Add type to properties (#2652) doc: Review custom collections and repository docs (#2653) doc: Review Getting Started (#2650) Move annotations-reference to attributes-reference (#2651) ...

GromNaN force-pushed the doc-prop-type branch from 13e2e36 to 77b525e Compare June 24, 2024 07:42

GromNaN commented Jun 24, 2024

View reviewed changes

GromNaN marked this pull request as ready for review June 24, 2024 13:54

GromNaN force-pushed the doc-prop-type branch from 688a9d1 to 6298d0a Compare June 25, 2024 07:54

IonBazan approved these changes Jun 25, 2024

View reviewed changes

alcaeus reviewed Jun 25, 2024

View reviewed changes

GromNaN added 3 commits June 25, 2024 12:36

Add type to properties

9229c7c

Add phpdoc for Collection generics

e18cde4

Fix errors

4554ac1

GromNaN force-pushed the doc-prop-type branch from 6298d0a to 4554ac1 Compare June 25, 2024 10:46

Andreas' review

33b87a6

GromNaN requested a review from alcaeus June 25, 2024 10:51

SenseException approved these changes Jun 25, 2024

View reviewed changes

alcaeus approved these changes Jun 26, 2024

View reviewed changes

Update docs/en/reference/introduction.rst

e8b7d97

Co-authored-by: Andreas Braun <git@alcaeus.org>

GromNaN merged commit 1408466 into doctrine:2.9.x Jun 26, 2024
17 of 18 checks passed

GromNaN deleted the doc-prop-type branch June 26, 2024 09:40

GromNaN added the Documentation label Jun 26, 2024

GromNaN added this to the 2.9.0 milestone Jun 27, 2024

github-actions bot mentioned this pull request Sep 20, 2024

Merge release 2.9.0 into 3.0.x #2677

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: Add type to properties #2652

doc: Add type to properties #2652

GromNaN commented Jun 24, 2024

GromNaN Jun 24, 2024

alcaeus Jun 25, 2024

GromNaN Jun 24, 2024

alcaeus Jun 25, 2024

GromNaN Jun 24, 2024

GromNaN Jun 24, 2024

GromNaN Jun 24, 2024

alcaeus Jun 25, 2024

GromNaN Jun 25, 2024 •

edited

Loading

alcaeus Jun 25, 2024

alcaeus Jun 25, 2024

alcaeus Jun 25, 2024

alcaeus Jun 25, 2024

alcaeus Jun 25, 2024

alcaeus Jun 25, 2024

GromNaN Jun 25, 2024

SenseException Jun 25, 2024

alcaeus Jun 26, 2024

alcaeus left a comment

alcaeus Jun 26, 2024

	private Collection $accounts;
	private Collection $accounts = new ArrayCollection();

doc: Add type to properties #2652

doc: Add type to properties #2652

Conversation

GromNaN commented Jun 24, 2024

Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GromNaN Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alcaeus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GromNaN Jun 25, 2024 •

edited

Loading