Enhanced contracts PHPDoc #463

adriendupuis · 2024-12-12T10:25:37Z

🎫 Issue	IBX-XXXXX

Related PRs:

ibexa/payment#161
Enhance PHPDoc rest#142

Description:

Add missing summaries (discovered during Enhance PHP API REF metadata documentation-developer#2575). A summary is the first line of a docblock. It can't have advanced formatting as it won't be rendered. Fix several not-rendered inline {@see tags
Type array keys to avoid generic array<string|int, rendering.
Type array values
- It fixes several "no value type specified in iterable type array" so PHPStan file is updated.
Use triple-backticks to have code blocks instead of <code> rendered as a single line.
Fix some @see syntax to have a working link.
Use class name instead of self to have a link (to the current page, debatable, it could also be $this).
Reword some descriptions.
ValueObject: Replace @ignore This method is for internal use with the dedicated @internal, fix some @uses tags.

Preview

Preview is build on top of ibexa/documentation-developer#2584.
But the following comparisons focus on the improvements brought by the packages' PRs.

Ibexa\Contracts\Core\FieldType\FieldType
- Before has code examples on one line, and with old school array() syntax.
- After has code examples in blocks with preserved new lines, and with a more modern [] syntax.
Ibexa\Contracts\Core\FieldType\Generic\Type
- Before has code examples on one line, checkValueType() makes a not-clickable reference to acceptValue().
- After has code example blocks with preserved new lines, and checkValueType() a working link to acceptValue().
Ibexa\Contracts\Core\FieldType\Generic\ValidationError\ConstraintViolationAdapter
- Before has not rendered @see tags in its introduction and <meta name="description" because the summary line can't have advanced format.
- After has a simple summary then a short description with @see rendered as links to references.
Ibexa\Contracts\Core\Limitation\Target\Builder\VersionBuilder
- Before has not rendered @see tags in its introduction, and some string|int untyped array keys.
- After has more links, and typed array keys.
Ibexa\Contracts\Core\Limitation\Type
- Before has a description for ACCESS_GRANTED which, in fact, is a description for the three ACCESS_ constants (grouped docblock doesn't exist and because of alphabetic order, the description ends after what it introduces), about the same for VALUE_SCHEMA_ constants, a not-rendered @see, and a not typed array key.
- After has a description for each constants, links, and typed array keys.
Ibexa\Contracts\Core\Repository\LocationService
- Before has a not-rendered @see, and untyped array keys.
- After has a new summary for count(), a working link in its description, typed keys on several arrays.
Ibexa\Contracts\Core\Repository\SearchService
- Before has several not-rendered @see.
- After has a more readable description CAPABILITY_CUSTOM_FIELDS, new summaries, and more links.
Ibexa\Contracts\Core\Repository\Values\Content\Location
- Before has some @deprecated not rendered because wrongly nested.
  - Nota bene: This happens in other classes. It's on my todo list for another PR.
- After has the read-only properties' summaries rewritten. Nota bene: The way a property is declared being both protected and magically publicly read-only (@property-read) creates duplicates: it's declared twice in the PHP code, and documented twice in the reference. This is an issue I'll come back to later.
Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion
- Before, has a not-rendered @see, misses links to properties and constants, and has an example rendered as a single line.
- After has links and a proper code block.
Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\DateMetadata
- Before has an example rendered as a single line.
- After has well rendered example, more readable descriptions, a link from DateMetadata::TRASHED to TrashService::findTrashItems(), and more links in general.
Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\UserMetadata
- Before has two one-line examples, a typo in a quoted constant, a lack of links.
- After has code blocks, enhanced descriptions, more links.
Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\Operator\Specifications
- Before's description quotes Criterion::getSpecifications() without a link.
- After: description is reworded, a link to Criterion::getSpecifications() is added with an inline tag instead of a separated tag below.
Ibexa\Contracts\Core\Repository\Values\ValueObject
- Before quotes the internal ValueObject::attributes(), has not-typed array keys,
- After has typed array keys (except for ValueObject::valueSchema()).
Ibexa\Contracts\Core\Search\Capable
- Before has a not rendered @see in its summary.
- After has a new summary, the link is in its description, and a link to SearchService helps to see the CAPABILITY_ constants.

For QA:

Documentation:

adriendupuis · 2024-12-12T14:55:23Z

src/contracts/Repository/Values/Content/Query/Criterion/DateMetadata.php

@@ -53,12 +55,12 @@ class DateMetadata extends Criterion implements TrashCriterion, FilteringCriteri
    ];

    /**
-     * Creates a new DateMetadata criterion on $metadata.
+     * Creates a new DateMetadata criterion.


I didn't find such property in the contracts namespace. (I may haven't look well enough in the namespace.) It might not be in this contracts namespace, and hidden by abstraction. If no link can be made to it, it probably don't worth being mentioned.

adamwojs · 2025-01-30T16:00:08Z

@adriendupuis Rebase is needed here

Don't use advanced format in summary, it won't be rendered.

Don't use advanced format in summary, it won't be rendered. Define array key type, or it will be rendered as array<string|int, …> phpDocumentor don't seem to understand self.

One docblock per constant.

@ignore

- Describes a bit more `getProperties` - Details `getProperties` param type and returned type - Hides link to internal function - Uses `@internal` as a short for "@ignore This method is for internal use" - Fixes internal links

…internal" This reverts commit 44aede1.

Revert 1799d56 changes on src/lib/Limitation/ as this is out of contracts scope.

Too difficult to solve in this PR

"return type has no value type specified in iterable type iterable"

sonarqubecloud · 2025-02-06T00:45:50Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

konradoboza · 2025-02-11T07:23:53Z

src/contracts/Repository/SearchService.php

-     * allow you on some search criteria to specify this custom field to rather query on that instead of the default
-     * field generated by the system.
+     * Custom fields is the capability for search engines to:
+     * - Allow you to extend the search index via plugins to


Suggested change

* - Allow you to extend the search index via plugins to

* - allow you to extend the search index via plugins to

Suggested change

* - Allow you to extend the search index via plugins to

* - allow you to extend the search index via plugins to

Doesn't that depend on how we finish the sentence in bullet point? Meaning, if it ends with:

Dot = capital letters for bullet points.

Comma = small letters.

But don't quote me on that, maybe English grammar rules are different?

konradoboza · 2025-02-11T07:23:59Z

src/contracts/Repository/SearchService.php

+     * Custom fields is the capability for search engines to:
+     * - Allow you to extend the search index via plugins to
+     *   generate custom fields, like a different representation (format, ...) of an existing field or similar.
+     * - Allow you on some search criteria to specify this custom field to rather query on that instead of the default


Suggested change

* - Allow you on some search criteria to specify this custom field to rather query on that instead of the default

* - allow you on some search criteria to specify this custom field to rather query on that instead of the default

konradoboza · 2025-02-11T07:25:11Z

src/contracts/Repository/Values/Content/Location.php

- * @property-read int $depth @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::getDepth()} instead.
+ * @property-read string $remoteId A global unique ID of the content object
+ * @property-read int $parentLocationId The ID of the parent location
+ * @property-read string $pathString Since 4.6.7, accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::getPathString()} instead.


Shouldn't @deprecated tag be left untouched?

Shouldn't @deprecated tag be left untouched?

This is actually quite significant issue. We need the ability to mark those as deprecated @adriendupuis

konradoboza · 2025-02-11T07:25:24Z

src/contracts/Repository/Values/Content/Location.php

     */
    public function getPathString(): string
    {
        return $this->pathString;
    }

    /**
-     * Same as {@see Location::getPathString()} but as array, e.g.: <code>[ '1', '2', '4', '23' ]</code>.
+     * The list of ancestor Locations' IDs, ordered in increasing depths,


Suggested change

* The list of ancestor Locations' IDs, ordered in increasing depths,

* The list of ancestor locations' IDs, ordered by increasing depth,

konradoboza · 2025-02-11T07:25:39Z

src/contracts/Repository/Values/Content/Location.php

     *
     * @var string
     */
    protected $pathString;

    /**
-     * Same as {@see Location::$pathString} but as array, e.g.: <code>[ '1', '2', '4', '23' ]</code>.
+     * The list of ancestor Locations' IDs, ordered in increasing depths,


Suggested change

* The list of ancestor Locations' IDs, ordered in increasing depths,

* The list of ancestor locations' IDs, ordered by increasing depth,

alongosz · 2025-02-11T09:12:34Z

src/contracts/FieldType/FieldType.php

+     * ```
+     * [
+     *     'stringLength' => [
+     *         'minStringLength' => [
+     *             'type'    => 'int',
+     *             'default' => 0,
+     *         ],
+     *         'maxStringLength' => [
+     *             'type'    => 'int'
+     *             'default' => null,
+     *         ],
+     *     ],
+     * ];
+     * ```


I don't mind it much, maybe even simpler and it renders well in PHPStorm, but don't you think we need a bit of consistency? There's 249 occurrences of <code> in this repository. Wouldn't it be better to handle it separately in another PR? Meaning: just replace <code> with ```?

alongosz · 2025-02-11T09:14:30Z

src/contracts/Limitation/Type.php

+     *
+     * Note: In future version constant values might change to 1, 0 and -1 as used in Symfony.
+     *
+     * @since 5.3.2


We probably need to drop these as it references eZ Publish and there's version number mismatch.

alongosz · 2025-02-11T09:18:10Z

src/contracts/Repository/SearchService.php

-     * allow you on some search criteria to specify this custom field to rather query on that instead of the default
-     * field generated by the system.
+     * Custom fields is the capability for search engines to:
+     * - Allow you to extend the search index via plugins to


Suggested change

* - Allow you to extend the search index via plugins to

* - allow you to extend the search index via plugins to

Doesn't that depend on how we finish the sentence in bullet point? Meaning, if it ends with:

Dot = capital letters for bullet points.

Comma = small letters.

But don't quote me on that, maybe English grammar rules are different?

alongosz · 2025-02-11T09:21:06Z

src/contracts/Repository/SearchService.php

+     *        - `['languages' => [<language_code_string>,…], 'useAlwaysAvailable' => <bool>]`
+     *        - `[<language_code_string>,…]` where `useAlwaysAvailable` defaults to `true` to avoid exceptions on missing translations.


This doesn't render well in PHPStorm.

alongosz · 2025-02-11T09:23:35Z

src/contracts/Repository/Values/Content/Location.php

- * @property-read \Ibexa\Contracts\Core\Repository\Values\Content\ContentInfo $contentInfo calls getContentInfo()
- * @property-read int $contentId @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::getContentId()} instead.
- * @property-read int $id @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::getId()} instead.
+ * @property-read \Ibexa\Contracts\Core\Repository\Values\Content\ContentInfo $contentInfo Calls {@see Location::getContentInfo()}
+ * @property-read int $contentId Since 4.6.7, accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::getContentId()} instead.
+ * @property-read int $id Since 4.6.7, accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::getId()} instead.
 * @property-read int $priority Position of the Location among its siblings when sorted using priority
- * @property-read bool $hidden @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::isHidden()} instead.
- * @property-read bool $invisible @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::isInvisible()} instead.
+ * @property-read bool $hidden Since 4.6.7, accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::isHidden()} instead.
+ * @property-read bool $invisible Since 4.6.7, accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::isInvisible()} instead.
 * @property-read bool $explicitlyHidden Indicates that the Location entity has been explicitly marked as hidden.
- * @property-read string $remoteId a global unique id of the content object
- * @property-read int $parentLocationId the id of the parent location
- * @property-read string $pathString @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::getPathString()} instead.
- * @property-read array $path @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::getPath()} instead.
- * @property-read int $depth @deprecated 4.6.7 accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::getDepth()} instead.
+ * @property-read string $remoteId A global unique ID of the content object
+ * @property-read int $parentLocationId The ID of the parent location
+ * @property-read string $pathString Since 4.6.7, accessing magic getter is deprecated and will be removed in 5.0.0. Use {@see Location::getPathString()} instead.


Getting more detailed overview now, this is entirely incorrect, sorry.
This is deprecated since 4.6.7, using "since" alone indicates that it's available since then.

alongosz · 2025-02-11T09:25:18Z

src/contracts/Repository/Values/Content/Location.php

    /**
     * Location ID.
     *
-     * @var int Location ID.
+     * @var int
     */


Given this is Location object, it could be even as simple as:

/** @var int */

as this code block doesn't carry any extra information that can't be inferred (I mean Location::$id is a Location ID).

alongosz · 2025-02-11T09:25:41Z

src/contracts/Repository/Values/Content/Location.php

-     * @var string[]
+     * Same as {@see Location::$pathString} but as array, e.g.: `['1', '2', '4', '23']`.
+     *
+     * @var array <int, string>


CS

Suggested change

* @var array <int, string>

* @var array<int, string>

github-actions bot assigned mikadamczyk, alongosz and tischsoic Dec 12, 2024

adriendupuis commented Dec 12, 2024

View reviewed changes

This was referenced Dec 17, 2024

Enhance PHPDoc ibexa/rest#142

Merged

[TMP] Enhance PHPDoc ibexa/documentation-developer#2583

Draft

adriendupuis marked this pull request as ready for review December 22, 2024 20:00

adriendupuis force-pushed the enh-phpdoc branch from 931b98e to 6482ce2 Compare January 6, 2025 14:15

adriendupuis added 21 commits February 3, 2025 09:43

Enhance DateMetadata::TRASHED phpdoc

37d0a18

Enhance Criterion::getSpecifications() phpdoc

cd00840

Enhance Specifications phpdoc

7e87d09

Capable.php: Fix phpdoc summary

8cc9116

Don't use advanced format in summary, it won't be rendered.

VersionBuilder.php: Fix phpdoc summary, arrays and self usages

67284d4

Don't use advanced format in summary, it won't be rendered. Define array key type, or it will be rendered as array<string|int, …> phpDocumentor don't seem to understand self.

ConstraintViolationAdapter's PHPDoc: Add a summary, reword description

bc939e0

Enhance Criterion::getSpecifications() phpdoc

8c3cacf

Enhance DateMetadata::TRASHED phpdoc

57d427f

Enhance DateMetadata phpdoc

81d53dd

Enhance UserMetadata phpdoc

d6fb1bd

Enhance DateMetadata phpdoc

88cba98

Enhance Capable phpdoc

89887f3

Enhance Criterion phpdoc

e235282

Enhance UserMetadata phpdoc (fix typo)

6b315d1

Enhance UserMetadata phpdoc

d61337e

Enhance FieldType\Generic\Type phpdoc

159b622

Enhance Limitation\Type phpdoc

429bf86

One docblock per constant.

LocationService: Fix count() phpdoc

9ce4618

Enhance LocationService phpdoc

586acec

Enhance Location phpdoc

5ffa696

Enhance Location phpdoc: @deprecated can't be nested nor used inline.

1927fcb

adriendupuis added 16 commits February 5, 2025 18:17

Enhance Location phpdoc: Add an internal link

44cfae0

Enhance ValueObject phpdoc

a315a36

- Describes a bit more `getProperties` - Details `getProperties` param type and returned type - Hides link to internal function - Uses `@internal` as a short for "@ignore This method is for internal use" - Fixes internal links

Enhance FieldType\Generic\Type phpdoc: Fix internal links

7d64601

Enhance FieldType\FieldType phpdoc: Fix code blocks

bf0ce81

Values\Content\Location: Set protected/read-only property as internal

da00d1f

Revert "Values\Content\Location: Set protected/read-only property as …

c1befcf

…internal" This reverts commit 44aede1.

Revert src/lib/Limitation/

ae1eaff

Revert 1799d56 changes on src/lib/Limitation/ as this is out of contracts scope.

Limitation\Type: type array keys

d609a8d

ValueObject: Format desc

f39e25b

ValueObject: Format desc

dd05db9

ValueObject: Format desc

2c09e95

ValueObject.php: fix phpdoc_inline_tag_normalizer

44981a0

Reset \Ibexa\Contracts\Core\Limitation\Type::valueSchema()

026cdcd

Too difficult to solve in this PR

LocationService.php: type iterable's values

212c88d

"return type has no value type specified in iterable type iterable"

SearchService.php: Fix $languageFilter usage

6515cb2

SearchService.php: Enh. $languageFilter usage's doc

5f681f9

adriendupuis force-pushed the enh-phpdoc branch from 4aaeeea to 5f681f9 Compare February 5, 2025 17:17

adriendupuis added 3 commits February 5, 2025 18:19

Merge branch 'enh-phpdoc' into enh-phpdoc-fix-phpstan

66bb70d

phpstan-baseline.neon: 🤞 Fix last PHP 8.1 errors

e3968cf

Fix/Update PHPStan after PHPDoc enhancement (#473)

91f989f

adriendupuis marked this pull request as ready for review February 6, 2025 08:20

adriendupuis requested a review from a team February 6, 2025 08:29

adamwojs approved these changes Feb 11, 2025

View reviewed changes

konradoboza reviewed Feb 11, 2025

View reviewed changes

ciastektk approved these changes Feb 11, 2025

View reviewed changes

wiewiurdp approved these changes Feb 11, 2025

View reviewed changes

tbialcz approved these changes Feb 11, 2025

View reviewed changes

alongosz reviewed Feb 11, 2025

View reviewed changes

alongosz changed the title ~~Enhance PHPDoc~~ Enhanced contracts PHPDoc Feb 11, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhanced contracts PHPDoc #463

Enhanced contracts PHPDoc #463

adriendupuis commented Dec 12, 2024 •

edited

Loading

adriendupuis Dec 12, 2024

adamwojs commented Jan 30, 2025

sonarqubecloud bot commented Feb 6, 2025

konradoboza Feb 11, 2025

alongosz Feb 11, 2025

konradoboza Feb 11, 2025

konradoboza Feb 11, 2025

alongosz Feb 11, 2025

konradoboza Feb 11, 2025

konradoboza Feb 11, 2025

alongosz Feb 11, 2025

alongosz Feb 11, 2025

alongosz Feb 11, 2025

alongosz Feb 11, 2025

alongosz Feb 11, 2025

alongosz Feb 11, 2025

alongosz Feb 11, 2025

	* - Allow you to extend the search index via plugins to
	* - allow you to extend the search index via plugins to

	* - Allow you on some search criteria to specify this custom field to rather query on that instead of the default
	* - allow you on some search criteria to specify this custom field to rather query on that instead of the default

	* The list of ancestor Locations' IDs, ordered in increasing depths,
	* The list of ancestor locations' IDs, ordered by increasing depth,

		* - `['languages' => [<language_code_string>,…], 'useAlwaysAvailable' => <bool>]`
		* - `[<language_code_string>,…]` where `useAlwaysAvailable` defaults to `true` to avoid exceptions on missing translations.

Enhanced contracts PHPDoc #463

Are you sure you want to change the base?

Enhanced contracts PHPDoc #463

Conversation

adriendupuis commented Dec 12, 2024 • edited Loading

Related PRs:

Description:

Preview

For QA:

Documentation:

Choose a reason for hiding this comment

adamwojs commented Jan 30, 2025

sonarqubecloud bot commented Feb 6, 2025

Quality Gate passed

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

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

adriendupuis commented Dec 12, 2024 •

edited

Loading