Skip to content

Fix parseVarLine() splitting complex @var types at whitespace#2000

Closed
krissss wants to merge 1 commit intozircote:masterfrom
krissss:fix/1999-var-generic-description
Closed

Fix parseVarLine() splitting complex @var types at whitespace#2000
krissss wants to merge 1 commit intozircote:masterfrom
krissss:fix/1999-var-generic-description

Conversation

@krissss
Copy link
Copy Markdown
Contributor

@krissss krissss commented Apr 22, 2026

Summary

  • Fix DocblockTrait::parseVarLine() incorrectly splitting generic types (e.g. array<string, string>) at internal whitespace, treating the trailing fragment as description
  • Replace regex-based type extraction with bracket-depth tracking to correctly handle generic, union, nullable, and array-shorthand types

Closes #1999

…rcote#1999)

The regex in `parseVarLine()` split types like `array<string, string>`
at internal whitespace, treating the trailing fragment as description.
Replace regex-based type extraction with bracket-depth tracking to
correctly handle generic, union, nullable, and array-shorthand types.
@DerManoMann
Copy link
Copy Markdown
Collaborator

Funny that this overlaps with my current PR! I tried to incorporate your changes into #1998 but that seems tricky - however Claude managed to improve(!) the regexp to cover generics too 🍤

Not ideal either as it's really not readable any more. However, I think the long term solution is to use phpstan/phpdoc-parser anyway...

@DerManoMann
Copy link
Copy Markdown
Collaborator

Actually did just that (refactor to using the parser). I also took the liberty of adding the new test from this branch. Let me know if that is ok - its a very useful test (that I was skirting around)....

This means we can close this PR in favour of #1998 unless I missed anything.

@krissss
Copy link
Copy Markdown
Contributor Author

krissss commented Apr 23, 2026

Thanks for the heads-up! I did look at #1998 before opening this PR, but at the time it seemed tricky to integrate the bracket-depth-tracking approach alongside your changes, so I went with a separate PR.

Great to hear the test cases are useful — glad they could be folded into #1998.

the long term solution is to use phpstan/phpdoc-parser

I actually tried going down that road before (in another project, also for docblock parsing) but ended up backing out — the overhead of pulling it in for what amounts to a few regex matches felt like overkill. Regex gets a bad rap, but for targeted extractions like @var lines it's hard to beat for simplicity and speed.

Closing this in favour of #1998.

@krissss
Copy link
Copy Markdown
Contributor Author

krissss commented Apr 23, 2026

Closing in favour of #1998.

@krissss krissss closed this Apr 23, 2026
@DerManoMann
Copy link
Copy Markdown
Collaborator

Great to hear the test cases are useful — glad they could be folded into #1998.

the long term solution is to use phpstan/phpdoc-parser

I actually tried going down that road before (in another project, also for docblock parsing) but ended up backing out — the overhead of pulling it in for what amounts to a few regex matches felt like overkill. Regex gets a bad rap, but for targeted extractions like @var lines it's hard to beat for simplicity and speed.

Yeah, fair enough.

Its a bit different here as we have the dependency already anyway and tweaking regexp's that parse (mostly) various docblock fragments is not really what this project is about ;)

@DerManoMann
Copy link
Copy Markdown
Collaborator

Thanks @krissss

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@var generic type fragment incorrectly overrides docblock description

2 participants