Skip to content
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

[4.x] Add template annotations #40

Merged

Conversation

WyriHaximus
Copy link
Member

@WyriHaximus WyriHaximus commented Mar 24, 2022

These annotations will aid static analyses like PHPStan and Psalm to enhance type-safety for this project and projects depending on it

These changes make the following example understandable by PHPStan:

final readonly class User
{
    public function __construct(
        public string $name,
    )
}

/**
 * \React\Promise\PromiseInterface<User>
 */
function getCurrentUserFromDatabase(): \React\Promise\PromiseInterface
{
    // The following line would do the database query and fetch the result from it
    // but keeping it simple for the sake of the example.
    return \React\Promise\resolve(new User('WyriHaximus'));
}

// For the sake of this example we're going to assume the following code runs
// in \React\Async\async call
echo await(getCurrentUserFromDatabase())->name; // This echos: WyriHaximus

This PR builds on the discussion at vimeo/psalm#7559 and the following PR's
reactphp/promise#247, reactphp/promise#246, and others down the line.

@WyriHaximus WyriHaximus added this to the v4.0.0 milestone Mar 24, 2022
WyriHaximus added a commit to WyriHaximus-secret-labs/promise that referenced this pull request Jun 21, 2022
This adds basic type safety annotations for static analyzers like PHPStan and Psalm. This will cover around 80% of the use cases and a follow-up PR for all supported versions will be proposed later to get it to a 100% of close to a 100%.

This PR is a requirement for reactphp/async#40
WyriHaximus added a commit to WyriHaximus-secret-labs/promise that referenced this pull request Jun 21, 2022
This adds basic type safety annotations for static analyzers like PHPStan and Psalm. This will cover around 80% of the use cases and a follow-up PR for all supported versions will be proposed later to get it to a 100% of close to a 100%.

This PR is a requirement for reactphp/async#40
WyriHaximus added a commit to WyriHaximus-secret-labs/promise that referenced this pull request Jun 22, 2022
This adds basic type safety annotations for static analyzers like PHPStan and Psalm. This will cover around 80% of the use cases and a follow-up PR for all supported versions will be proposed later to get it to a 100% of close to a 100%.

By adding these annotations methods returning a promise can hint their resolving type by adding `@return PromiseInterface<bool>` when they for example resolve to a boolean. By doing that Psalm and PHPStan will understand that the following bit of code will not become an issue because the method's contract promised a boolean through the promise:

```php
$promise->then(static function (bool $isEnabled) {});
```

However, the following will yield errors:

```php
$promise->then(static function (string $isEnabled) {});
```

This PR is a requirement for reactphp/async#40
WyriHaximus added a commit to WyriHaximus-secret-labs/promise that referenced this pull request Jun 22, 2022
This adds basic type safety annotations for static analyzers like PHPStan and Psalm. This will cover around 80% of the use cases and a follow-up PR for all supported versions will be proposed later to get it to a 100% of close to a 100%.

By adding these annotations methods returning a promise can hint their resolving type by adding `@return PromiseInterface<bool>` when they for example resolve to a boolean. By doing that Psalm and PHPStan will understand that the following bit of code will not become an issue because the method's contract promised a boolean through the promise:

```php
$promise->then(static function (bool $isEnabled) {});
```

However, the following will yield errors:

```php
$promise->then(static function (string $isEnabled) {});
```

This PR is a requirement for reactphp/async#40
WyriHaximus added a commit to WyriHaximus-secret-labs/promise that referenced this pull request Jun 23, 2022
This adds basic type safety annotations for static analyzers like PHPStan and Psalm. This will cover around 80% of the use cases and a follow-up PR for all supported versions will be proposed later to get it to a 100% of close to a 100%.

By adding these annotations methods returning a promise can hint their resolving type by adding `@return PromiseInterface<bool>` when they for example resolve to a boolean. By doing that Psalm and PHPStan will understand that the following bit of code will not become an issue because the method's contract promised a boolean through the promise:

```php
$promise->then(static function (bool $isEnabled) {});
```

However, the following will yield errors:

```php
$promise->then(static function (string $isEnabled) {});
```

This PR is a requirement for reactphp/async#40
WyriHaximus added a commit to WyriHaximus-secret-labs/promise that referenced this pull request Jun 23, 2022
This adds basic type safety annotations for static analyzers like PHPStan and Psalm. This will cover around 80% of the use cases and a follow-up PR for all supported versions will be proposed later to get it to a 100% of close to a 100%.

By adding these annotations methods returning a promise can hint their resolving type by adding `@return PromiseInterface<bool>` when they for example resolve to a boolean. By doing that Psalm and PHPStan will understand that the following bit of code will not become an issue because the method's contract promised a boolean through the promise:

```php
$promise->then(static function (bool $isEnabled) {});
```

However, the following will yield errors:

```php
$promise->then(static function (string $isEnabled) {});
```

This PR is a requirement for reactphp/async#40
WyriHaximus added a commit to WyriHaximus-secret-labs/promise that referenced this pull request Jun 23, 2022
This adds basic type safety annotations for static analyzers like PHPStan and Psalm. This will cover around 80% of the use cases and a follow-up PR for all supported versions will be proposed later to get it to a 100% of close to a 100%.

By adding these annotations methods returning a promise can hint their resolving type by adding `@return PromiseInterface<bool>` when they for example resolve to a boolean. By doing that Psalm and PHPStan will understand that the following bit of code will not become an issue because the method's contract promised a boolean through the promise:

```php
$promise->then(static function (bool $isEnabled) {});
```

However, the following will yield errors:

```php
$promise->then(static function (string $isEnabled) {});
```

This PR is a requirement for reactphp/async#40
WyriHaximus added a commit to WyriHaximus-secret-labs/promise that referenced this pull request Jun 23, 2022
This adds basic type safety annotations for static analyzers like PHPStan and Psalm. This will cover around 80% of the use cases and a follow-up PR for all supported versions will be proposed later to get it to a 100% of close to a 100%.

By adding these annotations methods returning a promise can hint their resolving type by adding `@return PromiseInterface<bool>` when they for example resolve to a boolean. By doing that Psalm and PHPStan will understand that the following bit of code will not become an issue because the method's contract promised a boolean through the promise:

```php
$promise->then(static function (bool $isEnabled) {});
```

However, the following will yield errors:

```php
$promise->then(static function (string $isEnabled) {});
```

This PR is a requirement for reactphp/async#40
WyriHaximus added a commit to WyriHaximus-secret-labs/promise that referenced this pull request Jun 29, 2022
This adds basic type safety annotations for static analyzers like PHPStan and Psalm. This will cover around 80% of the use cases and a follow-up PR for all supported versions will be proposed later to get it to a 100% of close to a 100%.

By adding these annotations methods returning a promise can hint their resolving type by adding `@return PromiseInterface<bool>` when they for example resolve to a boolean. By doing that Psalm and PHPStan will understand that the following bit of code will not become an issue because the method's contract promised a boolean through the promise:

```php
$promise->then(static function (bool $isEnabled) {});
```

However, the following will yield errors:

```php
$promise->then(static function (string $isEnabled) {});
```

This PR is a requirement for reactphp/async#40
WyriHaximus added a commit to WyriHaximus-secret-labs/promise that referenced this pull request Jun 29, 2022
This adds basic type safety annotations for static analyzers like PHPStan and Psalm. This will cover around 80% of the use cases and a follow-up PR for all supported versions will be proposed later to get it to a 100% of close to a 100%.

By adding these annotations methods returning a promise can hint their resolving type by adding `@return PromiseInterface<bool>` when they for example resolve to a boolean. By doing that Psalm and PHPStan will understand that the following bit of code will not become an issue because the method's contract promised a boolean through the promise:

```php
$promise->then(static function (bool $isEnabled) {});
```

However, the following will yield errors:

```php
$promise->then(static function (string $isEnabled) {});
```

This PR is a requirement for reactphp/async#40
@WyriHaximus WyriHaximus changed the title [WIP] [4.x] Add template annotations [4.x] Add template annotations to async and await Jul 8, 2022
@WyriHaximus WyriHaximus requested a review from clue July 8, 2022 19:55
@WyriHaximus WyriHaximus marked this pull request as ready for review July 8, 2022 19:55
@WyriHaximus WyriHaximus force-pushed the 4.x-add-template-annotations branch 5 times, most recently from 243d3e0 to e9c6e86 Compare July 9, 2022 13:37
phpstan.types.neon.dist Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
src/functions.php Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
types/await.php Outdated Show resolved Hide resolved
src/functions.php Outdated Show resolved Hide resolved
@clue clue added the new feature New feature or request label Jul 9, 2022
@WyriHaximus WyriHaximus force-pushed the 4.x-add-template-annotations branch from e9c6e86 to 6e7887f Compare July 9, 2022 20:35
@WyriHaximus
Copy link
Member Author

@clue I assume most of the location related comments also go for the promise counter parts?

@WyriHaximus WyriHaximus force-pushed the 4.x-add-template-annotations branch from 6e7887f to 2999337 Compare July 9, 2022 21:19
WyriHaximus added a commit to WyriHaximus-secret-labs/promise that referenced this pull request Jul 9, 2022
This adds basic type safety annotations for static analyzers like PHPStan and Psalm. This will cover around 80% of the use cases and a follow-up PR for all supported versions will be proposed later to get it to a 100% of close to a 100%.

By adding these annotations methods returning a promise can hint their resolving type by adding `@return PromiseInterface<bool>` when they for example resolve to a boolean. By doing that Psalm and PHPStan will understand that the following bit of code will not become an issue because the method's contract promised a boolean through the promise:

```php
$promise->then(static function (bool $isEnabled) {});
```

However, the following will yield errors:

```php
$promise->then(static function (string $isEnabled) {});
```

This PR is a requirement for reactphp/async#40
WyriHaximus added a commit to WyriHaximus-secret-labs/promise that referenced this pull request Jul 9, 2022
This adds basic type safety annotations for static analyzers like PHPStan and Psalm. This will cover around 80% of the use cases and a follow-up PR for all supported versions will be proposed later to get it to a 100% of close to a 100%.

By adding these annotations methods returning a promise can hint their resolving type by adding `@return PromiseInterface<bool>` when they for example resolve to a boolean. By doing that Psalm and PHPStan will understand that the following bit of code will not become an issue because the method's contract promised a boolean through the promise:

```php
$promise->then(static function (bool $isEnabled) {});
```

However, the following will yield errors:

```php
$promise->then(static function (string $isEnabled) {});
```

This PR is a requirement for reactphp/async#40
@WyriHaximus WyriHaximus force-pushed the 4.x-add-template-annotations branch from 2999337 to f097d2e Compare July 9, 2022 22:09
.github/workflows/ci.yml Outdated Show resolved Hide resolved
WyriHaximus pushed a commit to WyriHaximus-secret-labs/promise that referenced this pull request Jul 11, 2023
Adds template annotations turning the `PromiseInterface` into a generic.

Variables `$p1` and `$p2` in the following code example both are `PromiseInterface<int|string>`.

```php
$f = function (): int|string {
    return time() % 2 ? 'string' : time();
};

/**
 * @return PromiseInterface<int|string>
 */
$fp = function (): PromiseInterface {
    return resolve(time() % 2 ? 'string' : time());
};

$p1 = resolve($f());
$p2 = $fp();
```

When calling `then` on `$p1` or `$p2`, PHPStan understand that function `$f1` is type hinting its parameter fine, but `$f2` will throw during runtime:

```php
$p2->then(static function (int|string $a) {});
$p2->then(static function (bool $a) {});
```

Builds on top of reactphp#246 and reactphp#188 and is a requirement for reactphp/async#40
clue pushed a commit to WyriHaximus-secret-labs/promise that referenced this pull request Jul 11, 2023
Adds template annotations turning the `PromiseInterface` into a generic.

Variables `$p1` and `$p2` in the following code example both are
`PromiseInterface<int|string>`.

```php
$f = function (): int|string {
    return time() % 2 ? 'string' : time();
};

/**
 * @return PromiseInterface<int|string>
 */
$fp = function (): PromiseInterface {
    return resolve(time() % 2 ? 'string' : time());
};

$p1 = resolve($f());
$p2 = $fp();
```

When calling `then` on `$p1` or `$p2`, PHPStan understand that function
`$f1` is type hinting its parameter fine, but `$f2` will throw during
runtime:

```php
$p2->then(static function (int|string $a) {});
$p2->then(static function (bool $a) {});
```

Builds on top of reactphp#246 and
reactphp#188 and is a requirement for
reactphp/async#40
clue pushed a commit to WyriHaximus-secret-labs/promise that referenced this pull request Jul 11, 2023
Adds template annotations turning the `PromiseInterface` into a generic.

Variables `$p1` and `$p2` in the following code example both are
`PromiseInterface<int|string>`.

```php
$f = function (): int|string {
    return time() % 2 ? 'string' : time();
};

/**
 * @return PromiseInterface<int|string>
 */
$fp = function (): PromiseInterface {
    return resolve(time() % 2 ? 'string' : time());
};

$p1 = resolve($f());
$p2 = $fp();
```

When calling `then` on `$p1` or `$p2`, PHPStan understand that function
`$f1` is type hinting its parameter fine, but `$f2` will throw during
runtime:

```php
$p2->then(static function (int|string $a) {});
$p2->then(static function (bool $a) {});
```

Builds on top of reactphp#246 and
reactphp#188 and is a requirement for
reactphp/async#40
@WyriHaximus WyriHaximus force-pushed the 4.x-add-template-annotations branch 2 times, most recently from bbd5379 to 45dae63 Compare July 11, 2023 20:26
@WyriHaximus WyriHaximus changed the title [4.x] Add template annotations to async and await [4.x] Add template annotations Jul 12, 2023
@WyriHaximus WyriHaximus added this to the v4.2.0 milestone Jul 22, 2023
@WyriHaximus WyriHaximus force-pushed the 4.x-add-template-annotations branch 6 times, most recently from a8481f9 to 2930a8e Compare July 22, 2023 09:23
@WyriHaximus WyriHaximus force-pushed the 4.x-add-template-annotations branch 2 times, most recently from e50d673 to fe528f8 Compare August 8, 2023 06:20
@WyriHaximus WyriHaximus force-pushed the 4.x-add-template-annotations branch 4 times, most recently from 5e42fbd to 54b62fb Compare August 29, 2023 07:16
These annotations will aid static analyses like PHPStan and Psalm to
enhance type-safety for this project and projects depending on it

These changes make the following example understandable by PHPStan:

```php
final readonly class User
{
    public function __construct(
        public string $name,
    )
}

/**
 * \React\Promise\PromiseInterface<User>
 */
function getCurrentUserFromDatabase(): \React\Promise\PromiseInterface
{
    // The following line would do the database query and fetch the
result from it
    // but keeping it simple for the sake of the example.
    return \React\Promise\resolve(new User('WyriHaximus'));
}

// For the sake of this example we're going to assume the following code
runs
// in \React\Async\async call
echo await(getCurrentUserFromDatabase())->name; // This echos:
WyriHaximus
```
@clue clue force-pushed the 4.x-add-template-annotations branch from 54b62fb to 643316a Compare October 27, 2023 15:27
Copy link
Member

@clue clue left a comment

Choose a reason for hiding this comment

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

@WyriHaximus Went over your changes together with @SimonFrings and addressed all outstanding issues, great work, now let's get this shipped! :shipit: :shipit: :shipit:

* @param callable $function
* @return callable(mixed ...): PromiseInterface<mixed>
* @template T
* @template A1 (any number of function arguments, see https://github.com/phpstan/phpstan/issues/8214)
Copy link
Member

Choose a reason for hiding this comment

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

"640K ought to be enough for anybody."

See phpstan/phpstan#8214 for longer-term solution once supported in PHPStan.

Copy link
Member

@SimonFrings SimonFrings left a comment

Choose a reason for hiding this comment

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

Nice one 👍

@WyriHaximus WyriHaximus merged commit 8cc37cc into reactphp:4.x Oct 27, 2023
4 checks passed
@WyriHaximus WyriHaximus deleted the 4.x-add-template-annotations branch October 27, 2023 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants