Skip to content
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

Squiz/FunctionSpacing: fixer is broken with doc comment on closing brace line #784

Merged
merged 2 commits into from
Jan 12, 2025

Conversation

klausi
Copy link
Contributor

@klausi klausi commented Jan 11, 2025

The fixer in Squiz.WhiteSpace.FunctionSpacing gets confused when there is a doc comment on the closing brace line:

Description

Solves this fixer infinite loop

/**
 * foo.
 */
function a() {
}/**
 * foo.
 */
function b() {
}

Suggested changelog entry

Squiz.WhiteSpace.FunctionSpacing: Fix the fixer when there is a doc comment on the function closing line.

Related issues/external references

https://www.drupal.org/project/coder/issues/3461139

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.

Sorry, something went wrong.

@jrfnl jrfnl changed the title fix(FunctionSpacing): Fixer is broken with doc comment on closing brace line Squiz/FunctionSpacing: fixer is broken with doc comment on closing brace line Jan 12, 2025
@jrfnl jrfnl added this to the 3.11.3 milestone Jan 12, 2025
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.

I can confirm the sniff getting into a fixer conflict with itself with the provided code sample and that the patch fixes this.

Generally speaking this sniff doesn't care about other content on the same line as the function declaration. Code like the below will not be flagged by the sniff:



function c() {} function d() {}


But in the case of a "trailing" docblock, this meant that the sniff was adding the new lines within the doc block, which, in turn, meant it would still not see the "blank lines after" in the next loop, as the added lines would be seen as doc comment lines, not blank lines. Hence, the fixer conflict.

Thanks for fixing this @klausi !

@jrfnl jrfnl merged commit aebd84b into PHPCSStandards:master Jan 12, 2025
45 checks passed
jrfnl pushed a commit that referenced this pull request Jan 12, 2025

Verified

This commit was signed with the committer’s verified signature.
jrfnl Juliette
…ace line (#784)

The fixer in Squiz.WhiteSpace.FunctionSpacing gets confused when there is a doc comment on the closing brace line.

Generally speaking this sniff doesn't care about other content on the same line as the function declaration.

But in the case of a "trailing" docblock, this meant that the sniff was adding the new lines _within_ the doc block, which, in turn, meant it would still not see the "blank lines after" in the next loop, as the added lines would be seen as doc comment lines, not blank lines. Hence, the fixer conflict.

Fixed now.
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