Skip to content

Implement offset access type syntax#119

Merged
ondrejmirtes merged 1 commit into
phpstan:1.4.xfrom
rvanvelzen:offset-access-type
May 5, 2022
Merged

Implement offset access type syntax#119
ondrejmirtes merged 1 commit into
phpstan:1.4.xfrom
rvanvelzen:offset-access-type

Conversation

@rvanvelzen

Copy link
Copy Markdown
Contributor

This implements offset access types keeping as much BC as possible. Notably, whitespace between the type and [ is not allowed (so A[B] is valid but A [B] parses as just A, as before)

@ondrejmirtes ondrejmirtes left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A test for the A [B] scenario wouldn't hurt :)

]),
];

yield [

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests for A [B] already exist - just a few lines above

@rvanvelzen

Copy link
Copy Markdown
Contributor Author

@JanTvrdik what's the reason for the "confused" reaction? Do you disagree with the idea in general, with the implementation, or something else?

@ondrejmirtes

Copy link
Copy Markdown
Member

The context: phpstan/phpstan#7094

/** @var TypeNode */
public $offset;

public function __construct(TypeNode $type, TypeNode $offset)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to allow any TypeNode here? Shouldn't it be narrowed down to just IdentifierTypeNode?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it might make no sense in practice, array{foo: string}['foo'] could resolve to string.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, make sure to add a test case for this in phpstan-src when you work on it :) Thanks :)

@ondrejmirtes ondrejmirtes merged commit 28cb38b into phpstan:1.4.x May 5, 2022
@rvanvelzen rvanvelzen deleted the offset-access-type branch May 17, 2022 09:51
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.

2 participants