Skip to content

Fix DocBlockTagGrouping reporting a fix it never applied - #74

Merged
dereuromark merged 1 commit into
masterfrom
docblock-taggrouping-unfixed-error
Aug 6, 2026
Merged

Fix DocBlockTagGrouping reporting a fix it never applied#74
dereuromark merged 1 commit into
masterfrom
docblock-taggrouping-unfixed-error

Conversation

@dereuromark

Copy link
Copy Markdown
Contributor

checkBeginningOfDocBlock() guarded its fixer body with the condition inverted:

$fix = $phpcsFile->addFixableError(..., 'NoExtraNewlineBeforeTags');
if ($fix) {
    return;          // <- returns exactly when it should fix
}

$phpcsFile->fixer->beginChangeset();

addFixableError() returns true only while the fixer is running. So the changeset ran only when the fixer was disabled - a no-op - and phpcbf reported NoExtraNewlineBeforeTags as fixable and then changed nothing. Every other fixable error in the file uses if (!$fix) { return; }.

Found while writing the smoke fixtures in #73, which recorded the broken output:

    /**
     *
     * @var string
     */
    public const TAGS_START_TOO_LATE = 'start';

The fixture now records the blank line actually being removed.

Copilot AI lite review requested due to automatic review settings August 6, 2026 11:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Fixes a logic inversion in DocBlockTagGroupingSniff::checkBeginningOfDocBlock() that caused PHPCBF to report NoExtraNewlineBeforeTags as fixable but skip applying the actual fix, and updates the corresponding smoke fixture output to reflect the now-applied change.

Changes:

  • Correct the fixable-error guard in checkBeginningOfDocBlock() by switching from if ($fix) { return; } to if (!$fix) { return; }.
  • Update the DocBlockTagGrouping “after” fixture to remove the extra blank * line before tags, matching the fixed behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
PhpCollective/Sniffs/Commenting/DocBlockTagGroupingSniff.php Fixes the inverted fixer guard so the changeset runs only when fixing is enabled.
tests/_data/DocBlockTagGrouping/after.php Updates expected fixed output to reflect removal of the extra blank line before the first tag.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

checkBeginningOfDocBlock() returned as soon as addFixableError() came back
true, so the fixer body ran only when the fixer was switched off. phpcbf
listed NoExtraNewlineBeforeTags as fixable and then left the blank line in
place, every time.

Every other fixable error in the file guards with `if (!$fix)`. This one had
the condition inverted.

The fixture added with the smoke tests recorded the broken output; it now
records the blank line actually being removed.
@dereuromark
dereuromark force-pushed the docblock-taggrouping-unfixed-error branch from 143209e to b671e1e Compare August 6, 2026 11:31
Copilot AI review requested due to automatic review settings August 6, 2026 11:31
@dereuromark
dereuromark merged commit fa45af1 into master Aug 6, 2026
6 checks passed
@dereuromark
dereuromark deleted the docblock-taggrouping-unfixed-error branch August 6, 2026 11:31

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants