New error code for Squiz.Commenting.FunctionComment: MissingParamType

[fredden]

Description

When a docblock comment is provided for a function parameter, but no valid type information is supplied*, the error message (and code) from PHP_CodeSniffer is misleading. This pull request aims to fix this by introducing a new error code for these cases. This could be considered a breaking change as rulesets may need to be adjusted to accommodate this new error code.

* Some editors will create boiler-plate docblocks like this, expecting the developer to write the correct information.

Additionally, while working on the above change, I noticed that in some cases an invalid suggestion of adding a type hint for this specific case was being provided. I have included a fix for this in this pull request.

Before this change, the following code would be annotated as

2 | ERROR | Doc comment for parameter "$bar" missing (Squiz.Commenting.FunctionComment.MissingParamTag) 3 | ERROR | Missing parameter name (Squiz.Commenting.FunctionComment.MissingParamName) 6 | ERROR | Type hint "bar" missing for (Squiz.Commenting.FunctionComment.TypeHintMissing) 

after this change, the same code would be annotated as

2 | ERROR | Doc comment for parameter "$bar" missing (Squiz.Commenting.FunctionComment.MissingParamTag) 3 | ERROR | Missing parameter type (Squiz.Commenting.FunctionComment.MissingParamType) 

(Note the difference for line 3 ("Missing parameter name" versus "Missing parameter type"), and the lack of error for line 6.)

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

Here's an example from the existing test file for this sniff:

PHP_CodeSniffer/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc

Lines 789 to 802 in d84144d

	/**
	* Function comment.
	*
	* @param $bar
	* Comment here.
	* @param ...
	* Additional arguments here.
	*
	* @return
	* Return value
	*
	*/
	function foo($bar) {
	}

Suggested changelog entry

[Squiz.Commenting.FunctionComment] New error code MissingParamType will be used instead of MissingParamName when a parameter name is provided, but not its type. Additionally, invalid type hint suggestion will no longer be provided in these cases.

Types of changes

• Bug fix (non-breaking change which fixes an issue)

PR checklist

• I have checked there is no other PR open for the same change.
• I have read the Contribution Guidelines.
• I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
• I have added tests to cover my changes.
• I have verified that the code complies with the projects coding standards.

[jrfnl]

@fredden Thank you for this PR.

I have to admit, I'd been hesistant to review this PR as you marked it as containing a new error code and changed behaviour, making this a breaking change.

As things are, however, you are not introducing a new error code as the MissingParamType error code was already in use by the sniff (only a few lines below where you added the new code block), just used under different conditions.

I think this change - without a new error code, but re-using the existing error code -actually makes good sense. It improves the actionability of the error messages.

While this could be considered a bug fix, I'm going to earmark the PR for the 3.9.0 release just to be on the safe side, as it does constitute a behavioural change for the sniff.

[jrfnl]

Rebased without changes to get a passing build before merging.