Skip to content

suppress some psalm reported errors #142

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

Merged
merged 1 commit into from
Mar 2, 2021

Conversation

drealecs
Copy link
Contributor

suppress some psalm errors that are actually bugs in current psalm implementation or are related to deserialization workaround

…plementation or are related to deserialization workaround
@drealecs drealecs force-pushed the fix_psalm_reported_issues branch from af073a6 to c55aa7f Compare February 27, 2021 06:59
Copy link
Member

@mnapoli mnapoli left a comment

Choose a reason for hiding this comment

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

Thanks for dealing with these issues!

Looks good to me, I added 2 questions inline.

@@ -215,6 +225,7 @@ public static function isValid($value)
*
* @psalm-pure
* @psalm-assert T $value
* @param mixed $value
Copy link
Member

Choose a reason for hiding this comment

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

Why add this parameter docblock?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a MissingParamType psalm error.
https://psalm.dev/docs/running_psalm/issues/MissingParamType/

@@ -226,6 +237,8 @@ public static function assertValidValue($value): void
*
* @psalm-pure
* @psalm-assert T $value
* @param mixed $value
* @return string
Copy link
Member

Choose a reason for hiding this comment

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

Same here, why add this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here. The @return I guess it's just the PhpStorm complaining about.

@drealecs
Copy link
Contributor Author

drealecs commented Mar 2, 2021

Just a mention here. This is not very important to be released after merge. The changes are mostly for fixing pipeline on master.

@mnapoli
Copy link
Member

mnapoli commented Mar 2, 2021

Thanks!

@mnapoli mnapoli merged commit af4b562 into myclabs:master Mar 2, 2021
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.

2 participants