Implement offset/index access type #1318

rvanvelzen · 2022-05-16T11:52:11Z

ondrejmirtes

Otherwise seems pretty straightforward 👍 (but myself I see it's pretty hard to explain this feature to another person and also realize to apply it in the right cases)

ondrejmirtes · 2022-05-16T11:55:24Z

src/Type/KeyOfType.php

@@ -41,7 +46,7 @@ public function describe(VerbosityLevel $level): string

 	public function isResolvable(): bool
 	{
-		return !TypeUtils::containsTemplateType($this->type);
+		return !TypeUtils::containsTemplateType($this->type) && $this->type->isIterable()->yes();


This isn't necessary, right? For example ArrayAccess has "keys" but isn't iterable unless paired with Traversable.

ondrejmirtes · 2022-05-16T11:55:45Z

src/Type/Generic/TemplateKeyOfType.php

+use PHPStan\Type\Type;
+
+/** @api */
+final class TemplateKeyOfType extends KeyOfType implements TemplateType


It's weird to add support for key-of in generics but not for value-of at the same time :)

I did have it implemented, but I wasn't able to think of a good use-case for it.

I don't either but it looks like an ommission not to have it :)

ondrejmirtes · 2022-05-16T11:56:11Z

src/Type/ValueOfType.php

@@ -41,7 +41,7 @@ public function describe(VerbosityLevel $level): string

 	public function isResolvable(): bool
 	{
-		return !TypeUtils::containsTemplateType($this->type);
+		return !TypeUtils::containsTemplateType($this->type) && $this->type->isIterable()->yes();


ondrejmirtes · 2022-05-16T12:28:46Z

src/PhpDoc/TypeNodeResolver.php

+		$type = $this->resolve($typeNode->type, $nameScope);
+		$offset = $this->resolve($typeNode->offset, $nameScope);
+
+		return new OffsetAccessType($type, $offset);


TypeNodeResolver could detect when the type doesn't make sense. For example when $type isn't offset-accessible or when $type–>hasOffsetValueType($offset) is no(). It could return new ErrorType() in that case and the appropriate rules will pick this ErrorType and complain about an "unresolvable type" automatically.

rvanvelzen · 2022-05-16T13:23:29Z

I can't really come up with sensible tests for a value-of template type. I'd like to look at that separately

ondrejmirtes · 2022-05-16T13:26:45Z

No problem, we don't need to inhibit the current PR because of that :)

ondrejmirtes · 2022-05-16T13:27:16Z

Thank you very much!

staabm · 2022-05-16T13:37:30Z

tests/PHPStan/Analyser/data/bug-7094.php

+    /**
+     * @template K of key-of<T>
+     * @param K $key
+     * @param T[K] $val


is there a difference between T[K] and value-of<T> for this line?

T[K] will return only the types for offset K, where value-of<T> will return the types for all offset.

Seldaek · 2022-05-16T13:48:41Z

Awesome, thanks!

ondrejmirtes · 2022-05-23T15:47:13Z

Hi @rvanvelzen, it would be great if you found a moment to document this for https://phpstan.org/writing-php-code/phpdoc-types, thanks a lot :)

rvanvelzen · 2022-05-24T09:08:51Z

Will do soon :)

Documentation for phpstan/phpstan-src#1318

Seldaek · 2022-06-01T13:13:26Z

tests/PHPStan/Analyser/data/bug-7094.php

+    {
+        $attr = $this->getAttributes();
+        $attr[$key] = $val;
+        $this->setAttributes($attr);


If I can bug you with a question @rvanvelzen - I am trying to implement this pretty much got the same setup as in this test file, but I get a weird result here:

I have this in setAttribute:

$attr = $this->getAttributes() ?? []; \PHPStan\dumpType($attr); \PHPStan\dumpType($key); $attr[$key] = $val; \PHPStan\dumpType($attr); $this->setAttributes($attr);

Producing this output:

28 Dumped type: array{language?: string, timezone?: string} 29 Dumped type: string 31 Dumped type: non-empty-array<string, array<int, string>|string> 32 Parameter #1 $attributes of method User::setAttributes() expects array{language?: string, timezone?: string}, non-empty-array<string, array<int, string>|string> given.

You can see that $key got resolved to string which is too wide and then it widens the $attr type and breaks the setAttributes call.

I verified though with a dumpType of $key in unsetAttribute the direct key-of<T> works fine:

52 Dumped type: 'language'|'timezone'

So somehow the indirection with the following seems to break it?

* @template K of key-of<T> * @param K $key

Any idea what could be the cause? Why is this not breaking in the test suite? If I switch to use @param key-of<T> $key then as you described above it cannot resolve $val correctly as K is not set anymore and it ends up failing as well because it assumes every array key can be a union of any value present in the array.

I don't have time to look into the root cause right now, but I believe using K&key-of<T> as the parameter type should already improve the situation.

I didn't actually spend much time looking at the parameter types inside the methods, that definitely can use some polishing.

I tried that but could not make it work 100%, continuing this in phpstan/phpstan#7380 so it does not get lost.

ondrejmirtes · 2022-07-13T11:05:51Z

FYI @rvanvelzen I realized this still isn't documented, can you please send an update to this page? https://phpstan.org/writing-php-code/phpdoc-types Thank you :)

herndlm · 2024-07-15T19:37:02Z

Jfyi I think this was still not documented, or I just failed finding it 😊

ondrejmirtes · 2024-07-15T19:41:49Z

It's already documented. Just use the search feature on the website :)

Implement offset/index access type

d28dbb6

ondrejmirtes requested changes May 16, 2022

View reviewed changes

rvanvelzen added 2 commits May 16, 2022 14:40

Remove unnecessary checks

a0769c0

Return error type when offset type is impossible

224cafd

rvanvelzen marked this pull request as ready for review May 16, 2022 13:22

ondrejmirtes approved these changes May 16, 2022

View reviewed changes

ondrejmirtes merged commit 9e73151 into phpstan:1.7.x May 16, 2022

This was referenced May 16, 2022

Generics for key K of a generic array T phpstan/phpstan#7094

Closed

PHPStan 1.7.0 progress phpstan/phpstan#7207

Closed

staabm reviewed May 16, 2022

View reviewed changes

rvanvelzen deleted the index-access-type branch May 17, 2022 09:50

rvanvelzen added a commit to rvanvelzen/phpstan that referenced this pull request May 27, 2022

Add documentation for offset access types

b00d94b

Documentation for phpstan/phpstan-src#1318

Seldaek reviewed Jun 1, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement offset/index access type #1318

Implement offset/index access type #1318

rvanvelzen commented May 16, 2022

ondrejmirtes left a comment

ondrejmirtes May 16, 2022

ondrejmirtes May 16, 2022

rvanvelzen May 16, 2022

ondrejmirtes May 16, 2022

ondrejmirtes May 16, 2022

ondrejmirtes May 16, 2022

rvanvelzen commented May 16, 2022

ondrejmirtes commented May 16, 2022

ondrejmirtes commented May 16, 2022

staabm May 16, 2022

rvanvelzen May 16, 2022

Seldaek commented May 16, 2022

ondrejmirtes commented May 23, 2022

rvanvelzen commented May 24, 2022

Seldaek Jun 1, 2022

rvanvelzen Jun 2, 2022

Seldaek Jun 2, 2022

ondrejmirtes commented Jul 13, 2022

herndlm commented Jul 15, 2024

ondrejmirtes commented Jul 15, 2024

Implement offset/index access type #1318

Implement offset/index access type #1318

Conversation

rvanvelzen commented May 16, 2022

ondrejmirtes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rvanvelzen commented May 16, 2022

ondrejmirtes commented May 16, 2022

ondrejmirtes commented May 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Seldaek commented May 16, 2022

ondrejmirtes commented May 23, 2022

rvanvelzen commented May 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ondrejmirtes commented Jul 13, 2022

herndlm commented Jul 15, 2024

ondrejmirtes commented Jul 15, 2024