Support multiple lines for array shape #33
Conversation
|
Only @JanTvrdik or @arnaud-lb are able to review this 😊 |
| @@ -26,7 +26,7 @@ public function __construct(string $name, ?TypeNode $bound, string $description) | |||
|
|
|||
| public function __toString(): string | |||
| { | |||
| $bound = $this->bound ? " of {$this->bound}" : ''; | |||
| $bound = isset($this->bound) ? " of {$this->bound}" : ''; | |||
There was a problem hiding this comment.
should be $this->bound !== null
| b: array{ | ||
| c: callable(): int | ||
| } | ||
| }', |
There was a problem hiding this comment.
You should add new test instead of modifying an existing one
There was a problem hiding this comment.
Also the test is incorrect, as it's missing the *. What you are actually parsing is sth like
array{
* a: int,
* b: array{
* c: callable(): int
* }
* }
There was a problem hiding this comment.
Is it all right if I just add a single test showing that the leading *s don't disrupt anything? I'd otherwise like to omit them for readability. 414daf3
| $items[] = $this->parseArrayShapeItem($tokens); | ||
| } | ||
|
|
||
| $tokens->tryConsumeTokenType(Lexer::TOKEN_PHPDOC_EOL); | ||
| $tokens->consumeTokenType(Lexer::TOKEN_CLOSE_CURLY_BRACKET); |
There was a problem hiding this comment.
I think that we should also allow line breaks before the comma, so that this works:
array{
int
, int
, int
}
We might want to allow trailing commas as well:
array{
int,
int,
int,
}
There was a problem hiding this comment.
I agree. Trailing commas are awesome :)
There was a problem hiding this comment.
Each of these examples should have a separate test case.
There was a problem hiding this comment.
Trailing commas 571feed
There was a problem hiding this comment.
Line breaks before commas 1f26308
|
If we're able to finish this and merge it, I'm gonna release next phpdoc-parser in the current 0.3.x series and PHPStan 0.11.19 with support for multiline array shapes! 👍 |
|
Did my best to add the requested features. I feel like the while (!$tokens->tryConsumeTokenType(Lexer::TOKEN_CLOSE_CURLY_BRACKET)) {
$tokens->consumeTokenType(Lexer::TOKEN_COMMA);
$tokens->tryConsumeTokenType(Lexer::TOKEN_PHPDOC_EOL);
$items[] = $this->parseArrayShapeItem($tokens);
$tokens->tryConsumeTokenType(Lexer::TOKEN_PHPDOC_EOL);
}But then it just sort of runs wild and doesn't throw when an expected closing curly isn't encountered. Any suggestions? Also, what's the best way to run lint and stan on this project? I didn't see a |
|
You can always check what |
That's a bit more comprehensive, thank you. The |
|
I'm happy with this. If someone has any more comments, we can fix them on master :) Thank you very much! |
|
Yee-haw, thanks much. Looking forward to the next release... |
|
@ondrejmirtes By the way, can you shed any light on what is going on with slevomat/coding-standard? Array-shape support was added in June, but there hasn't been a release since March ... |
|
Nothing bad is going on with Slevomat CS, it’s an actively maintained project. @kukulich is planning to do a release. |
|
Very well, then, thank you. |
Possible fix for phpstan/phpstan#2497
Hey there, great library, fantastic work to all involved. So, like a lot of folks I'm excited about using array shapes, but without multiline syntax it doesn't really seem practical.
I haven't done much with parsers since my school days of long ago, and I was told that you have previously dismissed this idea as not feasible, so please tell me what I'm doing wrong.