Gert-dev / php-ide-serenata

Atom IDE package that integrates the Serenata server to provide PHP code assistance
https://serenata.gitlab.io/
Other
275 stars 19 forks source link

Method parameters passed as reference cause PHPDoc issues #453

Closed mallardduck closed 5 years ago

mallardduck commented 5 years ago

As per the title, when you have a method that uses a referenced parameter the PHPDoc hints become flawed. The reason being is that it seems the & used for reference is seen as part of the variable name. I believe this is the case since I was able to fix the warning/issue, but adjusting the PHPDoc.

For example, this method would cause the issue:

/**
 * Checks the thing.
 *
 * @param  Thing|null $thing   The input thing to check.
 */
protected function checkThing(?Thing &$thing)
{
//Do things
}

Simply adjusting the param to: * @param Thing|null &$thing The input thing to check.

Allows for the issue to be resolved and not hinted as an issue in Atom.

However doing this leads to issues with tools like PHPStan, as they are not expecting to find a & within the PHPDoc. For example, the issue this causes with stan looks like:

PHPDoc tag @param has invalid value (Thing|null &$thing The methods input place to check against.): Unexpected token "&", expected TOKEN_VARIABLE at offset (some line)

Gert-dev commented 5 years ago

I see what you mean. I originally did this on purpose, since I believed the reference symbol should be in the name or type in the docblock as to not lose any information provided there (as docblock information takes precedence over information of the actual code).

I've also noticed recently that PHPStan seems to desire the other behavior and there is no mention of adding reference types as Serenata does on the phpDocumentor documentation either.

This problem should resolve itself in v5.0 as (semantic) linting will be removed there in favor of tools such as PHPStan. IIRC, Serenata doesn't do anything else with reference types except lint them, so this should effectively also resolve your issue.

If this currently really bothers you, I recommend disabling Serenata's linting entirely as you already seem to be using PHPStan (I do, too) as you won't really lose anything dire; if you still want some of the other linting features such as unused import linting, you can take a look at adding the sniffs from Slevomat to PHPCS, which do pretty much the same and can be added to your CI as well.

Thanks for taking the time to report this, though!