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

Fixing MissingThrowsDocblock adds relative class names #8467

Closed
d-claassen opened this issue Sep 7, 2022 · 1 comment · Fixed by #8480
Closed

Fixing MissingThrowsDocblock adds relative class names #8467

d-claassen opened this issue Sep 7, 2022 · 1 comment · Fixed by #8480

Comments

@d-claassen
Copy link
Contributor

d-claassen commented Sep 7, 2022

The fix for MissingThrowsDocblock does not prepend the full class names of the exception(s) with a leading slash. The result of this is that within a namespace, the class name in an added @throws annotation will be relative to the namespace, probably resulting in an UndefinedDocblockClass. See https://psalm.dev/r/32c7b62a8d, which shows the result of fixing after initially no @throws annotation was available.

I haven’t seen a way to add an example of the fixer via psalm.dev psalm.dev doesn't seem able to fix the MissingThrowsDocblock so I can't show an example of that, but this can be tested by adding an additional dataset to the data provider of ThrowsBlockAdditionTest. The dataset should look like:

             'addThrowsAnnotationToFunctionInNamespace' => [
                '<?php
                    namespace Foo;
                    function foo(string $s): string {
                        if("" === $s) {
                            throw new \InvalidArgumentException();
                        }
                        return $s;
                    }',
                '<?php
                    namespace Foo;
                    /**
                     * @throws \InvalidArgumentException
                     */
                    function foo(string $s): string {
                        if("" === $s) {
                            throw new \InvalidArgumentException();
                        }
                        return $s;
                    }',
                '7.4',
                ['MissingThrowsDocblock'],
                true,
            ],

I fear there’s maybe an underlying challenge in regards to adding or detecting use statements. But I think it can be sufficient to simply add the fully qualified class name. Other tools like PHP_CodeSniffer can be used to further transform that into nice formatting, the goal here should be to ensure the @throws annotation is added with the right type.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/32c7b62a8d
<?php

namespace Foo;

/**
 * @throws RuntimeException
 */
function f(): void {
    throw new \RuntimeException();
}
Psalm output (using commit afe85fa):

ERROR: UndefinedDocblockClass - 6:12 - Docblock-defined class, interface or enum named Foo\RuntimeException does not exist

INFO: MissingThrowsDocblock - 9:5 - RuntimeException is thrown but not caught - please either catch or add a @throws annotation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants