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

Psalm doesn't throw an InvalidDocblock error if a parameter is missing a type #20

Closed
nickyr opened this issue Dec 15, 2016 · 6 comments
Closed

Comments

@nickyr
Copy link
Contributor

nickyr commented Dec 15, 2016

Paste

<?php
/**
 * @param $bar
 * @return void
 */
function fooBarBaz() {
}

into getpsalm.org.

Observed: No errors
Expected: InvalidDocblock because @param $bar is missing a type

@muglug
Copy link
Collaborator

muglug commented Dec 23, 2016

I don't particularly want to go down the path of Psalm alerting the user to issues that can be better found with phpcs or similar – InvalidDocblock is not used where the syntax is invalid, but where it affects type inference. To that end I've removed the one instance where it duplicated phpcs's functionality.

@muglug muglug closed this as completed Dec 23, 2016
@nickyr
Copy link
Contributor Author

nickyr commented Dec 25, 2016

Hmm I'm not sure that's the right call @muglug. If I create a docblock with params, I expect Psalm to validate against that docblock. If I accidentally leave the type out, then psalm just wouldn't validate against that docblock. But it would be completely silent about it. So I would think it was validating against the docblock, but it actually wouldn't be.

Given how heavily Psalm relies on docblocks for validation, I think it's completely reasonable for it to check that the docblocks are accurate and it seems like phpcs shouldn't be required to make sure Psalm is analyzing things fully. The intent of phpcs is to check style, whereas the intent of Psalm is to check correctness. You shouldn't have to check your style first in order to fully check your correctness.

@erunion
Copy link
Contributor

erunion commented Dec 25, 2016

It's also really edge-casey, but Psalm throwing exceptions for a missing variable in the method where a documented @param is present might be weird for instances where the dev is using func_get_args for processing params.

@nickyr
Copy link
Contributor Author

nickyr commented Dec 25, 2016 via email

@muglug muglug reopened this Dec 25, 2016
@muglug muglug closed this as completed in b6eea4e Dec 25, 2016
@nickyr
Copy link
Contributor Author

nickyr commented Dec 25, 2016 via email

@muglug
Copy link
Collaborator

muglug commented Dec 25, 2016

Merry merry!

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

No branches or pull requests

3 participants