-
Notifications
You must be signed in to change notification settings - Fork 664
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
@psalm-assert iterable<T1|T2>
isn't being picked up, and leads to failed type inference
#5052
Comments
I found these snippets: https://psalm.dev/r/e07c4aaa1c<?php
/** @psalm-assert iterable<positive-int|0> $input */
function assertAllNatural(mixed $input): void {throw new \Exception(\var_export($input, true));}
function allNatural(mixed $input): array
{
assertAllNatural($input);
return $input;
}
|
Ocramius
added a commit
to Ocramius/assert-1
that referenced
this issue
Jan 19, 2021
…ural()` doing the same thing Note: we refined the assertion on `Assert::natural()` to correctly restrict to `positive-int|0`, but we had to remove some minor test on `Assert::allNaturalNumber()` due to `@psalm-assert iterable<positive-int|0>` not yet working in upstream. See vimeo/psalm#5052
5 tasks
Ocramius
added a commit
to Ocramius/assert-1
that referenced
this issue
Jan 19, 2021
…ural()` doing the same thing Note: we refined the assertion on `Assert::natural()` to correctly restrict to `positive-int|0`, but we had to remove some minor test on `Assert::allNaturalNumber()` due to `@psalm-assert iterable<positive-int|0>` not yet working in upstream. See vimeo/psalm#5052
Ocramius
added a commit
to Ocramius/assert-1
that referenced
this issue
Jan 19, 2021
…ural()` doing the same thing Note: we refined the assertion on `Assert::natural()` to correctly restrict to `positive-int|0`, but we had to remove some minor test on `Assert::allNaturalNumber()` due to `@psalm-assert iterable<positive-int|0>` not yet working in upstream. See vimeo/psalm#5052
Ocramius
added a commit
to Ocramius/assert-1
that referenced
this issue
Jan 19, 2021
…ural()` doing the same thing Note: we refined the assertion on `Assert::natural()` to correctly restrict to `positive-int|0`, but we had to remove some minor test on `Assert::allNaturalNumber()` due to `@psalm-assert iterable<positive-int|0>` not yet working in upstream. See vimeo/psalm#5052
Ocramius
added a commit
to Ocramius/assert-1
that referenced
this issue
Mar 2, 2021
…ural()` doing the same thing Note: we refined the assertion on `Assert::natural()` to correctly restrict to `positive-int|0`, but we had to remove some minor test on `Assert::allNaturalNumber()` due to `@psalm-assert iterable<positive-int|0>` not yet working in upstream. See vimeo/psalm#5052
Nyholm
pushed a commit
to webmozarts/assert
that referenced
this issue
Mar 7, 2021
…int` assertions (#214) * Implemented #213: `Assert::positiveInteger()` to have psalm `positive-int` assertions * #213 use `static::valueToString()` instead of `static::typeToString()` for `Assert::positiveInteger()` failures Ref: #214 (comment) * Added `Assert::naturalNumber` to verify that a number is `positive-int|0` * Upgraded to latest `vimeo/psalm` * As suggested by @muglug, dropping `@mixin` and importing a trait as `Assert` type inference mechanism Ref: https://symfony-devs.slack.com/archives/C8SFXTD2M/p1611064766060600 Quoting: ``` ocramius 2:59 PM can anyone help me tackle down vimeo/psalm#4381 ? Mostly trying to figure out whether the assertions are being read differently for @mixin and concrete instances 😐 muglug 3:18 PM mixins are a massive hack, both whenever they're used but also in Psalm anything that relies on magic methods to work is going to be more flaky, so I'd encourage you to think of a non-mixin approach if at all possible ocramius 3:19 PM yeah, I'm thinking of doing that indeed, just unsure about approach yet 😐 can't they somehow be unified with traits though? they're the same thing, no? muglug 3:22 PM if they were, you'd just use a trait! ocramius 3:22 PM ack, gonna actually try that approach for webmozart/assert then 🙂 muglug 3:24 PM with mixins the method that's actually being called is either __call or __callStatic, whereas with traits it's still that exact method ocramius 3:24 PM yes, I was just wondering if it could import the method as if it was a trait (at type level) that would squash out *a lot* of code, but it's also true that a mixin does not have a trait definition I think it makes sense to use the language feature for this 🙂 ``` The `@mixin` support in `vimeo/psalm` is a massive hack, which also requires other tooling to increase complexity in the static analysis parsing capabilities. In order to reduce that complexity, we instead rely on `Assert` importing `Mixin` as a trait, which is much simpler and much more stable long-term. While this indeed leads to `Mixin` being changed from an `interface` to a `trait`, that is not really a BC break, as `Mixin` was explicitly documented to be used only as a type system aid, and not as an usable symbol. * Removed `Assert::naturalNumber()`, since we already have `Assert::natural()` doing the same thing Note: we refined the assertion on `Assert::natural()` to correctly restrict to `positive-int|0`, but we had to remove some minor test on `Assert::allNaturalNumber()` due to `@psalm-assert iterable<positive-int|0>` not yet working in upstream. See vimeo/psalm#5052 * Adjusted mixin generator to comply with current coding standards * Upgraded `vimeo/psalm` to `4.6.1` to get a green build (note: `4.6.2` broken as per vimeo/psalm#5310) While there is no guarantee that `vimeo/psalm:4.6.3` will fix the issue, we know for sure that `vimeo/psalm:4.6.2` is broken, as per vimeo/psalm#5310, and we cannot therefore rely on its static analysis there until upstream issue is fixed. Meanwhile, this allows for installation of `webmozart/assert` with `vimeo/psalm:4.6.1` and `vimeo/psalm:>4.6.2`, hoping for a brighter future.
This seems to have been fixed already: https://psalm.dev/r/f514529cce |
I found these snippets: https://psalm.dev/r/f514529cce<?php
/** @psalm-assert iterable<positive-int|0> $input */
function assertAllNatural(mixed $input): void {throw new \Exception(\var_export($input, true));}
function allNatural(mixed $input): iterable
{
assertAllNatural($input);
/** @psalm-trace $input */;
return $input;
}
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
https://psalm.dev/r/e07c4aaa1c
In this case, the entire type assertion
@psalm-assert iterable<positive-int|0>
is being suppressed, and psalm cannot infer the type of$input
Ref: https://symfony-devs.slack.com/archives/C8SFXTD2M/p1611068007070700
Quoting:
The text was updated successfully, but these errors were encountered: