GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — TypeParser: read the "|" that follows a "?" by valzargaming · Pull Request #321 · phpstan/phpdoc-parser

Conversation

@valzargaming

@valzargaming valzargaming commented Sep 14, 2026

Copy link
Copy Markdown

The problem

A ? written before a type ends the type: the | that may follow it is left unread, for the caller to fail on.

$typeParser->parse(new TokenIterator($lexer->tokenize('?int|null')));
// ?int, with "|null" left over

So every tag written that way is turned into an error:

@property ?int|null $handler   Unexpected token "|", expected variable at offset 18

It is not specific to null — ?int|string fails the same way — and it happens at every depth a type may be written at: array<?int|null>, list<?int|null>, array{a: ?int|null} and (?int|string) are all turned down.

Why the | can be read

?A|B is the same set of values whichever of the two the ? is read to belong to:

  • (?A)|B → A|null|B
  • ?(A|B) → A|B|null

There is nothing to choose between, and nothing for a reader to get wrong. The parser already reads both of those spellings when they are written with parentheses; only the plain one is turned down, so what is being rejected is the spelling rather than the type.

An & is a different matter, and this PR deliberately leaves it as it is:

  • (?A)&B and ?(A&B) are not the same set

so ?A&B stays an error rather than being quietly read as one of them. PHP writes that type as (A&B)|null, and a test is added pinning the behaviour.

Where it is written

PHPDoc written from an API specification that carries its own nullable marker ends up with both markers on the same type. Discord's API reference marks a nullable field by prefixing its type with a question mark, and marks an optional field by suffixing its name with one (Nullable and Optional Resource Fields) — a ?string column there is a field that may be null.

A property transcribed from such a table carries the specification's marker and PHPDoc's own, and is written ?int|null. The two agree: the type is int|null on either reading.

Reported downstream as phpDocumentor/phpDocumentor#3987, where it surfaces as an ERROR line per tag.

On #86

#86 asked for this and was closed on the grounds that ?A|B is not valid PHP syntax, which is true and is not in dispute here — PHPDoc already reads a good deal that PHP will not (array{a: int}, list<int>, int-mask, conditional types). What that PR did not put forward, and what this one rests on, is that the union case is unambiguous and the intersection case is not, so the two can be told apart rather than refused together. It also carried no tests and no grammar. If the answer is still no, the tests here at least write the current behaviour down.

The change

  • TypeParser::parse() and TypeParser::subParse() carry a nullable type into a following union, and only into a union.
  • parseNullable() now enriches the node it builds with its own attributes, as parseAtomic() already does — without it the NullableTypeNode carries no line or index once it is a union member.
  • doc/grammars/types.pp3 says the same, so the fuzzer generates these types and GrammarSyncTest keeps the two in step.

?A|B is read as a union whose first member is ?A, which prints back exactly as written.

Checks

  • make tests — 1766 pass (16 added; no existing test changed or removed)
  • FuzzyTest — 6000 generated inputs pass
  • make phpstan — no errors
  • make cs, make lint — clean

GrammarSyncTest::testPhpDocGrammarReadsWhatPhpDocParserReads fails for me on Windows on an unrelated @return Foo <strong> input, because of \r\n in the known-disagreement list; it fails identically on a clean checkout, so it is not from this change.

🤖 Generated with Claude Code

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@valzargaming @claude
TypeParser: read the "|" that follows a "?"
a4be44e
"?A|B" was read as the nullable type "?A", leaving the "|B" for the caller
to fail on, so every tag written that way became an error:

    @Property ?int|null $handler   Unexpected token "|", expected variable

The "?" is now carried over the union, because "?A|B" is the same set of
values whichever of the two the "?" is read to belong to: "(?A)|B" and
"?(A|B)" are both "A|B|null". It is read as a union whose first member is
"?A", which prints back as it was written.

An "&" is deliberately left as it was: "(?A)&B" and "?(A&B)" are not the
same set, so "?A&B" stays an error rather than being read as either one.
PHP writes that type as "(A&B)|null".

The grammars in doc/grammars are updated to say the same, and the union is
read at every depth a type may be written at, so "array<?int|null>",
"array{a: ?int|null}" and "(?int|string)" are read too.

Co-Authored-By: Claude Opus 5 <[email protected]>
@staabm

staabm commented Sep 14, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

?int|null is not a valid type, see https://wiki.php.net/rfc/union_types#nullable_types

its a parse-error: https://3v4l.org/kNPeM#v

allowing such type in phpdoc while its invalid in native types would be very confusing

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@valzargaming

valzargaming commented Sep 14, 2026

Copy link
Copy Markdown
Author

Thank you for providing me with the RFC! I was unaware of it before now, and so I've missed that the RFC gives a readability reason. My only outstanding issue currently is that Intellephense does correctly drop the ?. Rather than erroring, would it be more appropriate to drop the symbol entirely rather than throwing a hard error? I can update my PR to reflect this behavior to keep things consistent across the ecosystem.
image
image

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@staabm

staabm commented Sep 14, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

you should fix your phpdoc to read int|null or ?int - which both are valid.

it might make sense to open a issue on intelliphense about the bug in phpdoc parsing.
its not correct that they treat this phpdoc as a valid one. its invalid.

I think it works correct at the PHPStan end

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@staabm staabm closed this Sep 14, 2026
@valzargaming

valzargaming commented Sep 14, 2026

Copy link
Copy Markdown
Author

I don't think it's a bug either as Intellephense correctly drops the ? without throwing an error whereas phpstan actually throws an error.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@staabm

staabm commented Sep 14, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

yes, intelliphense should also throw an error. ?int|null is invalid.

valzargaming reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@valzargaming

valzargaming commented Sep 14, 2026

Copy link
Copy Markdown
Author

For anyone who comes across this PR in the future I patched this with https://github.com/discord-php/phpdoc-tool

I understand it diverts from the RFC, however our library needs to be as faithful to Discord's official documentation as other languages follow this schema, and their docs utilize the ? specifically to denote optional fields, not just optional values.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@valzargaming
valzargaming deleted the nullable-union branch September 14, 2026 18:46
@valzargaming valzargaming mentioned this pull request Sep 28, 2026
Types: Member::$roles discord-php/DiscordPHP#1431
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@valzargaming @staabm
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.