Conversation
The php:method signature parser rejects modern PHP type syntax (?string, union types) with build warnings or wrong output; the page does not mention it. Document the limitation and the :returntype: / :param: workaround. Assisted-by: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
e44ddfe to
fd8e8a6
Compare
CybotTM
changed the title
|
I feel like we should maybe fix the parser to also accept modern PHP syntax and open an issue in https://github.com/TYPO3-Documentation/guides-php-domain |
The first version blamed nullable and union types "in the signature", which is not what the parser does. METHOD_SIGNATURE_REGEX in MethodNameService matches the return type as (\w+) while the parameter list is unrestricted, so retrieve(?string $id) and retrieve(string|null $id) are accepted and only the part after the colon is not. Measured against the regex: ?string, string|null and \Vendor\Thing as a return type are rejected; the same types as parameter types pass. Correct the text accordingly and drop the advice to give a nullable parameter a null default, which was never needed. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
|
Agreed on fixing the parser — and going through The restriction is not "nullable and union types in the signature".
So parameters were never the problem, and the advice to give a nullable parameter a Happy to open the issue in guides-php-domain with those cases — a fully qualified return type failing is arguably the more annoying half. Say the word and I will file it there. |
Rendered every variant through the real container instead of reasoning about the regex. A rejected signature is not just a warning: the whole text becomes the method name, so the parameters vanish, an empty () is rendered, and the anchor is built from the entire signature, which breaks cross references to that method. Also add intersection types to the rejected list and name the accepted return types, both measured rather than assumed. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
|
Closing this in favour of fixing the parser, as you suggested: TYPO3-Documentation/guides-php-domain#54. Going through One detail worth having here: the lexer is used without If the fix lands, this page has nothing left to warn about. If you would rather document the limitation in the meantime, say so and I will reopen it with the corrected text — the version on the branch already names the return type as the restriction and describes what a rejected signature actually does. |
Merging this carries the outcomes of the nine upstream pull requests from [#75](#75) back into the skill: three copies of content that is now in the manual are pruned to their delta, and the `php:method` section — whose claim was wrong — is corrected against the shipped parser. Six of the nine merged, three closed. Nothing here is new guidance; it is the skill catching up with what the manual now says and with what a closed pull request established. ## Promoted → pruned to the delta Three sections still restated upstream content with no marker. Each keeps only what the manual does not give: - **`rendering.md` `--output`** — the container path, the disappearance with `--rm`, and the "reports the files as placed anyway" tell are all in the merged note ([HowToDocument#540](TYPO3-Documentation/TYPO3CMS-Guide-HowToDocument#540)). What stays is the consequence upstream does not draw: it matters when a check *reads back* the rendered HTML, where the assertion fails with `No such file or directory` on a render that genuinely succeeded. - **`intercept-deployment.md` response codes** — `200`/`204`/`412` with their meanings are upstream ([#546](TYPO3-Documentation/TYPO3CMS-Guide-HowToDocument#546)), and that page carries the point this file never had: a branch push and a tag push are separate deliveries, so a version missing from docs.typo3.org is a question about the **tag** delivery. - **`guides-xml.md`** — the inventory trailing slash and the missing-`class` symptom are upstream ([#545](TYPO3-Documentation/TYPO3CMS-Guide-HowToDocument#545)), now marked rather than restated. Three were already pruned correctly and are untouched: `screenshots.md` ([#539](TYPO3-Documentation/TYPO3CMS-Guide-HowToDocument#539)), `rendering.md` symlinks ([#543](TYPO3-Documentation/TYPO3CMS-Guide-HowToDocument#543)), `rst-syntax.md` todo ([#542](TYPO3-Documentation/TYPO3CMS-Guide-HowToDocument#542)). ## The correction that matters The `php:method` section had the restriction **backwards**, and the pull request that found it was closed before the correction came home. Measured against the shipped parser, `MethodNameService.php:16` on `main`: ```php private const METHOD_SIGNATURE_REGEX = '/^\s*(\w+)\s*\(\s*(.*?)\s*\)\s*(?::\s*(\w+))?\s*$/'; ``` The parameters are `(.*?)` and therefore **unrestricted** — `?string $pattern` and `string|array $data` parse fine, and this file told you to rewrite them. The **return type** after the colon is `(\w+)`, which `?string` and `string|null` do not match; that is the real limit. A second defect is documented here for the first time: line 28 splits the parameter list with `preg_split('/\s*,\s*/')`, which knows nothing about brackets, so `paginate(array $range = [1, 2], int $page = 1)` yields three parameters instead of two. Both come from reading a signature with patterns rather than a lexer, which is what [guides-php-domain#54](TYPO3-Documentation/guides-php-domain#54) changes. It is open, and the section says to re-check when it merges. ## Testing done All seven signature cases were run against the real regex before the text was written — nullable and union **return** types rejected, nullable and union **parameters** accepted, plain and absent return types accepted, array default accepted-then-mis-split. Zero mismatches against the corrected claim. That run also caught an error in my own upstream comment on [#541](TYPO3-Documentation/TYPO3CMS-Guide-HowToDocument#541), which said the split yields four parameters where it yields three. The skill says three. `markdownlint-cli2` clean on all 21 reference files; `tests/check-changelog-version-coverage.sh` and `tests/checkpoint-scripts.sh` both pass. Refs #75 _Assisted by claude-code:claude-opus-5 — [Session](https://claude.ai/code/session_01NLJXhUjDbihKeu1d51gwWD)_
2 participants
The php:method signature parser rejects modern PHP type syntax (
?string, union types) with build warnings or wrong output, and the page currently does not mention it. This documents the limitation and the:returntype:/:param:workaround, plus a short note that array-based configuration (TCA, FlexForm, YAML) belongs in confval rather than the PHP domain.