Skip to content

Fix alter ignoring MissingThrowsDocblock suppression#11586

Open
d-mitrofanov-v wants to merge 15 commits intovimeo:6.xfrom
d-mitrofanov-v:fix/alter-ignores-suppression-of-MissingThrowsDocblock
Open

Fix alter ignoring MissingThrowsDocblock suppression#11586
d-mitrofanov-v wants to merge 15 commits intovimeo:6.xfrom
d-mitrofanov-v:fix/alter-ignores-suppression-of-MissingThrowsDocblock

Conversation

@d-mitrofanov-v
Copy link
Copy Markdown

@d-mitrofanov-v d-mitrofanov-v commented Nov 6, 2025

Psalter ignored MissingThrowsDocblock suppression in config and changed suppressed files anyway.
This happened because the check for isSuppressed was missing in if statement for $codebase->alter_code and because check for alter_code was not inside if (!$is_expected) block


Note

Medium Risk
Changes the conditions under which Psalm’s code-alteration path inserts @throws annotations, which can affect automated file rewrites during psalter runs. Risk is moderate because it alters auto-fix behavior but is scoped to the MissingThrowsDocblock fix flow.

Overview
Fixes psalter’s MissingThrowsDocblock auto-fix so it does not modify code when the issue is suppressed. The analyzer now creates a reusable MissingThrowsDocblock issue instance, passes suppression info into IssueBuffer::maybeAdd, and only calls FunctionDocblockManipulator::addThrowsDocblock() when alter_code is enabled, the issue is configured to be fixed, and the specific instance is not suppressed.

Also includes a minor whitespace-only adjustment around the MissingOverrideAttribute auto-fix block.

Reviewed by Cursor Bugbot for commit 95034c7. Bugbot is set up for automated code reviews on this repo. Configure here.

@danog danog added the release:fix The PR will be included in 'Fixes' section of the release notes label Dec 23, 2025
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit c102fb1. Configure here.

$this->source->getFilePath(),
$this->function,
);
$manipulator->addThrowsDocblock($missingThrowsDocblockErrors);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suppressed exceptions included in altered throws docblock

Medium Severity

The $missingThrowsDocblockErrors array accumulates ALL non-expected exception names unconditionally (before the suppression check), but addThrowsDocblock passes this full array when any non-suppressed exception triggers the alter path. If exception A is suppressed and exception B is not, processing B will call addThrowsDocblock([A, B]), adding suppressed exception A to the @throws docblock. The suppression check gates whether addThrowsDocblock is called, but doesn't filter the array contents.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c102fb1. Configure here.

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

Labels

release:fix The PR will be included in 'Fixes' section of the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants