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

Mark return values in throw arguments as used #5989

Merged
merged 1 commit into from
Jun 25, 2021

Conversation

weirdan
Copy link
Collaborator

@weirdan weirdan commented Jun 23, 2021

Fixes #5975

@@ -940,6 +940,7 @@ public static function verifyType(
|| $context->inside_use
|| $context->inside_assignment
|| $context->inside_conditional
|| $context->inside_throw
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure whether I really needed to add $context->inside_throw to all of these places, but no tests broke 🤷‍♂️

@weirdan weirdan changed the title Mark return values in as throw argument as used Mark return values in throw arguments as used Jun 23, 2021
@weirdan weirdan requested a review from muglug June 23, 2021 22:36
@muglug muglug merged commit 6d4262e into vimeo:master Jun 25, 2021
@muglug
Copy link
Collaborator

muglug commented Jun 25, 2021

Thanks!

@frederikbosch
Copy link

This method still errors. Why?

    /** @psalm-pure */
    public static function divide(string $amount, string $divisor): string
    {
        if (bccomp($divisor, '0', self::SCALE) === 0) {
            throw InvalidArgumentException::divisionByZero();
        }

        return bcdiv($amount, $divisor, self::SCALE);
    }

ERROR: PossiblyUnusedReturnValue - src/Calculator/BcMathCalculator.php:50:69 - The return value for this method is never used (see https://psalm.dev/273) public static function divide(string $amount, string $divisor): string

frederikbosch added a commit to moneyphp/money that referenced this pull request Jun 29, 2021
@weirdan
Copy link
Collaborator Author

weirdan commented Jun 29, 2021

@frederikbosch is it used, and if so, where?

@weirdan
Copy link
Collaborator Author

weirdan commented Jun 29, 2021

Also, I don't think this has anything to do with throw, as it's quite unlikely someone throws the result of divide() method.

@orklah
Copy link
Collaborator

orklah commented Jun 29, 2021

It seems used at least here: https://github.com/moneyphp/money/blob/2f882263c73d75ddbea49244da5425a9c18289fb/src/Money.php#L265

Also in tests: https://github.com/moneyphp/money/blob/4f9c4fa10f7615a5d7f0c25de6658e0479957be9/tests/Calculator/GmpCalculatorTest.php#L55

and I checked, the tests directory is included in Psalm's analysis

@frederikbosch can you create a new issue for this?

@frederikbosch
Copy link

@orklah See #6021

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.

PossiblyUnusedReturnValue false-positive with throw
4 participants