Skip to content

Generic/FunctionCallArgumentSpacing: improve XML documentation #923

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

Conversation

rodrigoprimo
Copy link
Contributor

Description

Generic/FunctionCallArgumentSpacing: update XML doc to remove info about spacing around equal sign

The FunctionCallArgumentSpacing sniff used to check for spacing around the equal sign in the arguments of a function declaration. But this was removed in squizlabs/PHP_CodeSniffer#2399
(commit feeda47).

This commit updates the sniff XML doc to reflect this change.

Generic/FunctionCallArgumentSpacing: fix code examples in the XML doc

The code examples in the XML documentation of this sniff were incorrect, as the sniff does not check spacing in function argument definitions. It only checks spacing in function and method calls.

As far as I could check, this has always been the case, or at least it has been the case for a long time. Since tests were added for this sniff when it was part of the PEAR standard and called MethodCallArgumentSpacing, it ignored function definitions and checked only function calls:

squizlabs/PHP_CodeSniffer@7b84951#diff-56e2f69852c3071fbf86d20fbb49f38dae17cff8731154d8b1b7495c4ad8eeb0R24-R27

Here is the sniff code where it ignores function definitions:

// Skip tokens that are the names of functions or classes
// within their definitions. For example:
// function myFunction...
// "myFunction" is T_STRING but we should skip because it is not a
// function or method *call*.
$functionName = $stackPtr;
$ignoreTokens = Tokens::$emptyTokens;
$ignoreTokens[] = T_BITWISE_AND;
$functionKeyword = $phpcsFile->findPrevious($ignoreTokens, ($stackPtr - 1), null, true);
if ($tokens[$functionKeyword]['code'] === T_FUNCTION || $tokens[$functionKeyword]['code'] === T_CLASS) {
return;
}

Generic/FunctionCallArgumentSpacing: improve XML documentation

  • <documentation> title now matches the sniff name
  • Mention in the description that the sniff is about function and method calls.
  • <standard> and <code> descriptions now mention that the sniff also checks for spaces before the comma.

Suggested changelog entry

Generic.Functions.FunctionCallArgumentSpacing: minor fixes and improvements to the XML documentation

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.

Sorry, something went wrong.

Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice finds! Looking good. I just have one small suggestion for the description to improve the accuracy and readability.

@rodrigoprimo
Copy link
Contributor Author

Thanks for your review, @jrfnl. I just committed the change you suggested, and this PR is ready for another review.

…out spacing around equal sign

The FunctionCallArgumentSpacing sniff used to check for spacing around
the equal sign in the arguments of a function declaration. But this was
removed in squizlabs/PHP_CodeSniffer#2399
(commit feeda47).

This commit updates the sniff XML doc to reflect this change.
The code examples in the XML documentation of this sniff were incorrect
as the sniff does not check spacing in function argument definitions. It
only checks spacing in function and method calls.

As far as I could check this has always been the case, or at least it
has been the case for a long time. Since tests were added for this
sniff when it was part of the PEAR standard and called
MethodCallArgumentSpacing, it ignored function definitions and checked
only function calls:

squizlabs/PHP_CodeSniffer@7b84951#diff-56e2f69852c3071fbf86d20fbb49f38dae17cff8731154d8b1b7495c4ad8eeb0R24-R27

Here is the sniff code where it ignores function definitions:

https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/57a9ed734e2ab06c062fe0e463745eab483cf722/src/Standards/Generic/Sniffs/Functions/FunctionCallArgumentSpacingSniff.php#L55-L66
- `<documentation>` title now matches the sniff name
- Mention in the description that the sniff is about function and method
calls.
- `<standard>` and `<code>` descriptions now mention that the sniff also
checks for spaces before the comma.
Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rodrigoprimo !

I will rebase the PR to squash the fix-up commit into the correct atomic commit. Merging once the build has passed.

@jrfnl jrfnl force-pushed the documentation-function-call-argument-spacing-fix branch from b1b2dcf to 02ff468 Compare April 3, 2025 22:17
@jrfnl jrfnl merged commit a51ef18 into PHPCSStandards:master Apr 3, 2025
46 checks passed
@jrfnl jrfnl deleted the documentation-function-call-argument-spacing-fix branch April 3, 2025 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants