Squiz/FunctionComment: support generics in docblock

[tm1000]

Description

This supports the following:

    /**
	 * @param Collection<int, TelephoneNumber> $telephoneNumbers
	 */
	public function myFunction(
		Collection $telephoneNumbers,

Where-as before it would say that the typehint needs to be Collection<int, TelephoneNumber> now it will correctly deduce that the type hint needs to be Collection and will not return an error

I based this off of squizlabs/PHP_CodeSniffer#708 which is similar in scope.

Suggested changelog entry

Related issues/external references

Based off squizlabs/PHP_CodeSniffer#3544

Fixes squizlabs/PHP_CodeSniffer#3589
Fixes #746

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
  • This change is only breaking for integrators, not for external standards or end-users.
• Documentation improvement

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.
• [Required for new sniffs] I have added XML documentation for the sniff.

[tm1000]

Give me a moment to fix the conflicts

[tm1000]

Ref: #746 (comment)

[tm1000]

This may fail and if so I'll just have to work on this locally instead of blind through github.

[tm1000]

@jrfnl can you initiate the actions?

nevermind there are so many it didnt looking like they were working

[jrfnl]

@tm1000 Thanks for this PR. I'm starting to review it now and noticed the PR template had been removed. I've taken the liberty to restore it and would like to ask you to fill in the missing bits & pieces (and maybe verify the bits I already filled in).

[tm1000]

@jrfnl sorry about that I did a quick copy and paste.

Also I'll need to sit down and fix the pr issues

At least it's here now. Finally :)

[jrfnl]

@jrfnl sorry about that I did a quick copy and paste.

No worries and I appreciate you porting the PR over.

Also I'll need to sit down and fix the pr issues

I think the primary issue (which is failing the build) is a missing new line at the /src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc file.

Expanding the tests a little more would also be good, but I'll go through the PR and will leave you a review.

At least it's here now. Finally :)

Indeed ;-)

[jrfnl]

@tm1000 Thanks for this PR and for your willingness to contribute!

I've gone through the PR now and left a few comments inline.

Aside from that, as I mentioned in the issue, the tests will need to be expanded a bit. I'm thinking:

• Include the other examples from the code sample in the issue (Squiz.Commenting.FunctionComment.IncorrectTypeHint recognizes generics a part of type hints #746).
• And then have a second test with that same complete set, but then for a function which doesn't have type declarations yet. Even though the test setup as-is does not verify the message, having those tests there, still allows to verify (manually) that the message is now correct and also safeguards that the message will be thrown in those situations.

Other than that, I'd be happy to see this go into the codebase.

[jrfnl]

Please remove this change. It doesn't belong with this PR and may have been there for a reason (as the sniff is about checking docblock formatting). Same for the fixed file.

[jrfnl]

Please end the file on a new line char.

[jrfnl]

@tm1000 Just checking in to see whether you have a chance to update this PR based on the review. Would be great to get this merged.