Skip to content

Conversation

@krzysztofrewak
Copy link
Contributor

This is a little change, but I think it could be very helpful for many developers.

After adding a template to phpdocs of Container contract we'd be able to get all hints about resolved class:

obraz

@driesvints
Copy link
Member

Please see #41504

@driesvints driesvints closed this Mar 17, 2022
@krzysztofrewak
Copy link
Contributor Author

We are not supporting generics outside of Laravel's collections for now.

Could you or @nunomaduro explain why Laravel is closed for changes like this one? It would really improve my and my team's (and a lot of other people's too) development process, it's compatible with everything else in current codebase and it's not hurting anyone. Would you like to conduct some bigger analysis "What should we improve in this file in one bigger task"?

@pionl
Copy link
Contributor

pionl commented Mar 18, 2022

@krzysztofrewak I've hold these changes for a week and made PR today (did not notice other did it to, my bad), not sure why its not supported, closed to. Not sure why. Probably there is some reason that we are unaware off, we will see.

I've tried the changes it with PHPstan and no warnings.

@pionl
Copy link
Contributor

pionl commented Sep 12, 2023

@driesvints Is it possible to revisit this?

I understand that get and make can use the type hint. I think there could be a solution by providing new method, something like makeClass.

/**
 * Get the available container instance.
 *
 * @template T of object
 *
 * @param class-string<T>   $class
 * @param array<int, mixed> $parameters
 *
 * @return T
 */
function makeClass(string $class, array $parameters = []): object
{
    $instance = $this->make($class, $parameters);

    assert(assertion: $instance instanceof $class, description: 'Instance is not of type ' . $class);

    return $instance;
}

This would save quite a of code I have to do (to make PHPStan and IDE happy - without any plugins):

$dateFormats = $this->app->get(DateFormatterService::class);
assert($dateFormats instanceof DateFormatterService);

I can make the PR.

@func0der
Copy link

func0der commented Mar 28, 2024

@driesvints Why? Is there some public reasoning/statement for rejecting changes like this?

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.

4 participants