Auto-fix PEAR.Commenting.FunctionComment.SpacingAfter

squizlabs / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.

BSD 3-Clause "New" or "Revised" License

10.63k stars 1.48k forks source link

Auto-fix PEAR.Commenting.FunctionComment.SpacingAfter #3908

Closed fredden closed 8 months ago

fredden commented 8 months ago

Description

I have witnessed some complaints coming from Squiz.Commenting.FunctionComment.SpacingAfter which look like they could be trivially auto-fixed. This pull request adds auto-fixing for this particular complaint. I intend to investigate feasibility of auto-fixing other cases in this sniff another time.

Suggested changelog entry

Allow auto-fixing ...FunctionComment.SpacingAfter

Types of changes

New feature (non-breaking change which adds functionality)

PR checklist

[x] I have checked there is no other PR open for the same change.
[x] I have read the Contribution Guidelines.
[x] 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).
[x] I have added tests to cover my changes.
[x] I have verified that the code complies with the projects coding standards.

fredden commented 8 months ago

So, instead of the currently proposed change, the implementation should remove all blank lines between the comment closer and the line containing the function keyword in one go.

Yes, this makes sense. I'll look at making this change in the coming days.

fredden commented 8 months ago

Thanks for the feedback @jrfnl.

With the patch as is, if any of the blank lines between the comment closer and the first code has spaces/tabs on them (still a blank line), the fixer will now go into conflict state.

I did consider the case where there are lines with only spaces/tabs. I thought that another sniff (like Squiz.WhiteSpace.SuperfluousWhitespace) should handle removing trailing white-space from such lines. I did not however check that this sniff would not produce a "failed to fix" situation in that case.

I have updated the code to treat lines containing only spaces/tabs as blank too. I'm now using exactly the same condition/logic to determine if a line is "blank" in the fixer as was used to identify the issue and add the error.

I have added tests to cover these scenarios.

jrfnl commented 7 months ago

FYI: this fix is included in today's PHP_CodeSniffer 3.8.0 release.

As per #3932, development on PHP_CodeSniffer will continue in the PHPCSStandards/PHP_CodeSniffer repository. If you want to stay informed, you may want to start "watching" that repo (or watching releases from that repo).