New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FunctionStorage
was not initialized with FunctionParamsProviderInterface
#7450
Comments
Hey @klimick, can you reproduce the issue on https://psalm.dev ? |
I don't understand what FunctionStorage you're refering to, can you give more details? |
psalm/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php Lines 171 to 183 in 5c4be6b
When Inside of the map([1, 2, 3], fn($i) => [$i]) // $i is 1|2|3, because it was infer by previous arg. For functions processed via psalm/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php Lines 526 to 543 in 5c4be6b
If function storage will never created then |
Can you check if veewee's fix solves your issue? |
It doesn't solve my issue. /** @psalm-trace $_ */
$_ = pipe(
getList(), // list<string>
map(function ($a) { // $a should be string but mixed
return new Box(stringToInt($a));
}),
map(function ($box) { // $box should be Box<string> but mixed
/** @psalm-trace $box */;
return ['Val' => $box->a];
}),
);
// ERROR: MixedArgument
// at /home/klimick/PhpstormProjects/other/psalm/pipe-example.php:100:36
// Argument 1 of stringToInt cannot be mixed, expecting string (see https://psalm.dev/030)
// return new Box(stringToInt($a));
//
//
// ERROR: Trace
// at /home/klimick/PhpstormProjects/other/psalm/pipe-example.php:103:33
// $box: mixed (see https://psalm.dev/224)
// /** @psalm-trace $box */;
//
//
// ERROR: MixedPropertyFetch
// at /home/klimick/PhpstormProjects/other/psalm/pipe-example.php:104:26
// Cannot fetch property on mixed var $box (see https://psalm.dev/034)
// return ['Val' => $box->a]; Even if I specify type hints this affects inference of templates: /** @psalm-trace $_ */
$_ = pipe(
getList(),
map(function (string $a) {
return new Box(stringToInt($a));
}),
map(function (Box $box) { // $box should be Box<string> but just Box
/** @psalm-trace $box */;
return ['Val' => $box->a];
}),
);
// ERROR: Trace
// at /home/klimick/PhpstormProjects/psalm/pipe-example.php:103:33
// $box: Box (see https://psalm.dev/224)
// /** @psalm-trace $box */; |
But if I comment re-running of the psalm/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php Lines 565 to 572 in d1a946c
@veewee Have you checked how your pipe infers template types? For example: /**
* @template A
* @template B
*
* @param callable(A): B $_ab
* @return callable(list<A>): list<B>
*/
function map(callable $_ab): callable
{
throw new RuntimeException('???');
}
/**
* @return list<int>
*/
function getList(): array
{
return [];
}
/**
* @template T
* @psalm-immutable
*/
final class Box
{
/** @var T */
public $prop;
/**
* @param T $a
*/
public function __construct($a)
{
$this->prop = $a;
}
}
// Inferred type is list<Box<mixed>>
// Without double run ArgumentsAnalyzer is list<Box<int>>
$result = pipe(
getList(),
map(function (int $a) {
return new Box($a + 1);
}),
map(function (Box $a) {
return new Box($a->prop + 1);
})
); |
It seems first running of the First running also may raise issues that second doesn't raise. |
I've tested the example above with my plugin (with closures instead of callables though) Test: Result:
The templated value of the item inside Box gets lost and becomes Can you share me your function + plugin maybe, so that I can take a look in there? I do notice that I did not add any template map to the arguments analyzer here: Adding it with This additional analyzer call is only ran for the pipe function, since the other functions dont have any plugin. So I don't think this is a big issue. It only analuzes twice for the pipe function itself: The first time with the guessed types, the second time with the improved types. Not sure if it is related, but callables don't play well with generic types either. |
Pipe function: https://github.com/klimick/psalm/blob/pipe-example/pipe-example.php P.S. I've commented re-running |
P.S.S Before #7394 my plugin doesn't work because |
So after #7394, the storage you require is available now? The big difference in our versions is that I tried parsing the argument types and you try to link a predefined set of template parameters instead. So the issue I have, is that those argument types were not available - and now are. But if you comment out the analyzer, it won't be available again? I blindly expected the types to have the correctly resolved templates after argument analysis. That way you don't need those nested template resolutions and you could use the argument types to determine the different stages. My best guess it that the reason for this, is that closures and templates don't work well together (#7244) If you skip the analysis, as you did, the argument type information is not available in front of triggering the plugin. I am not sure what the best direction would be here for both our approaches? |
After #7394.
Pipe arguments should be analyzed sequentially:
The true As I understand my way called Contextual type inference. I'm not sure that we can do it somehow else. |
The way I see it, there are 2 issues:
Problem 1: Since the context is known within the pipe due to the order of functions, it could be possible to resolve the templates correctly by only reading the arguments it has: pipe(
map(function (int $a) {
return new Box($a + 1);
}),
map(function (Box $a) {
return new Box($a->prop + 1);
})
) In this case:
So if the closures would resolve the correctly resolved template types, that fixes the contextual issue. (That's basically how my version of the plugin deals with this issue: https://github.com/veewee/pipe-plugin/blob/6b47861e85060056b0c829f49cabbbb95673ebc9/src/Psalm/PipeArgumentsProvider.php#L43-L69) Problem 2: If we could fix problem 1, we might not need to fix problem 2. Because that would mean types are resolved correctly as well. Adding 12 templates feels a bit clumsey - similar like the typescript overloading approaches. About reverting the code: Not sure ... The additional analysis has a different purpose. |
Or if type pre-analysis for args in the final class SomeFunctionParamsProvider implements FunctionParamsProviderInterface
{
// new method that determine pre-analysis running
public static function preAnalyseArgTypes(): bool
{
return false;
}
/**
* @return array<lowercase-string>
*/
public static function getFunctionIds(): array
{
// ...
}
/**
* @return ?array<int, FunctionLikeParameter>
*/
public static function getFunctionParams(FunctionParamsProviderEvent $event): ?array
{
// ...
}
} |
You can add 24 templates. 48 templates. 94 templates. In real life, hardly anyone will make a pipeline of 12 calls. P.S. typescript can handle curry, compose, pipe and other variadic arg functions. For example curry: https://github.com/type-challenges/type-challenges/issues?q=label%3A462+label%3Aanswer
You can use |
Adding a method to toggle how it works doesn't seem like a good idea to me. Since problem 1 will still be there.
True ... stiil clumsy :)
True, but it is only for determing the return type. Whereas you should use curry(array_map(...), 'wrong');
^^^^^^ I'm logging off for the day. Maybe @orklah can moderate from his point of view? |
Honestly, you're both way above me on those concepts... I can't give advices here unfortunately |
Ok, I'm gonna look into problem 1 next week: the templated callable does not get resolved issue. Since it might end up being a workable fix for @klimick as well. |
keep it in mind @veewee
/**
* @template A
* @template B
*
* @param callable(A): B $_ab
* @return callable(list<A>): list<B>
*/
function map(callable $_ab): callable
{
throw new RuntimeException('???');
} We know that
P.S "work correctly" meant:
|
@klimick If I understand corectly, in Following example
The higher order logic won't be able to resolve T1 and T2 (and possibly R) correctly - but can resolve T3? I am on board with your pipe approach at this moment. However, how would je be able to detect the type of the first argument from a plugin POV in this case? Maybe if we can find answers to the curry issue, we can find an agreement that has every case covered? |
Just chiming in, if the plugin hook is not positioned at the right time, the inference behind curry and pipe feature could be included in Core I think. You'll probably have your hands less tied like that |
Ho would you see that @orklah ? |
Not sure, but doesn't the principles applied independant from any third-party tool? (I may be completely wrong here, feel free to ignore me 😄 ) |
Simplified example showing a flaw of argument pre-analysing: No idea how args pre-analysis can help. But I'll think about how you can make both pipe and curry work. |
I found these snippets: https://psalm.dev/r/0b43a7300b<?php
/**
* @template A
* @template B
*
* @param list<A> $list
* @param callable(A): B $ab
* @return list<B>
*/
function map(array $list, callable $ab): array
{
throw new RuntimeException('???');
}
/**
* @return list<int>
*/
function getList(): array
{
return [];
}
$result = map(getList(), fn($i) => [$i]);
/** @psalm-trace $result */;
https://psalm.dev/r/bca7840b9d<?php
/**
* @template A
* @template B
*
* @param list<A> $list
* @param callable(A): B $ab
* @return list<B>
*/
function map(array $list, callable $ab): array
{
throw new RuntimeException('???');
}
/**
* @return list<int>
*/
function getList(): array
{
return [];
}
$ab = fn($i) => [$i];
$result = map(getList(), $ab);
/** @psalm-trace $result */;
https://psalm.dev/r/83994466a4<?php
/**
* @template A
* @template B
*
* @param list<A> $list
* @param callable(A): B $ab
* @return list<B>
*/
function map(array $list, callable $ab): array
{
throw new RuntimeException('???');
}
/**
* @return list<int>
*/
function getList(): array
{
return [];
}
$ab =
/**
* @param int $i
* @return array{int}
*/
fn($i) => [$i];
$result = map(getList(), $ab);
/** @psalm-trace $result */;
https://psalm.dev/r/f4f9502598<?php
/**
* @template A
* @template B
*
* @param list<A> $list
* @param callable(A): B $ab
* @return list<B>
*/
function map(array $list, callable $ab): array
{
throw new RuntimeException('???');
}
/**
* @template T
*/
final class Box
{
/**
* @param T $value
*/
public function __construct(public mixed $value) { }
}
/**
* @return list<Box<int>>
*/
function getList(): array
{
return [];
}
$ab =
/**
* @param Box<int> $i
* @return array{Box<int>}
*/
fn(Box $i) => [$i];
$result = map(getList(), $ab);
/** @psalm-trace $result */;
|
Indeed. Figured it out this morning as well, hence my previous comment: it is not a good solution.
Don't get me wrong here: I love the template based solution. |
klimick@1d988ad
After #7417 was merged I tried to make a plugin for variadic pipe function (This may look tricky).
It would even work but
FunctionStorage
is not created for functions that are processed byFunctionParamsProviderInterface
.Without
FunctionStorage
will not createdTemplateResult
that matters for argument types inference.After a "quick fix" I caught failing tests.
Can you answer the question why is
FunctionStorage
is not created?The text was updated successfully, but these errors were encountered: