Skip to content

fix(admin-edit-ban): resolve PHPStan false positive - #1541

Merged
Rushaway merged 1 commit into
sbpp:mainfrom
Rushaway:1540
Aug 3, 2026
Merged

fix(admin-edit-ban): resolve PHPStan false positive#1541
Rushaway merged 1 commit into
sbpp:mainfrom
Rushaway:1540

Conversation

@Rushaway

@Rushaway Rushaway commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Description

Gate the IP-length/validity check on $postBanType === BanType::Ip alone, then nest the $_POST['ip']-dependent conditions inside, instead of re-checking the enum inside a compound && alongside a mixed-typed operand.

Before:

if ($error === 0 && empty($_POST['ip']) && $postBanType === BanType::Ip) {
    // Didn't type an IP
    $error++;
    $validationErrors['ip'] = 'You must type an IP';
} elseif ($error === 0 && $postBanType === BanType::Ip && !filter_var($_POST['ip'], FILTER_VALIDATE_IP)) {
    $error++;
    $validationErrors['ip'] = 'You must type a valid IP';
}

After:

if ($postBanType === BanType::Ip) {
    if ($error === 0 && empty($_POST['ip'])) {
        // Didn't type an IP
        $error++;
        $validationErrors['ip'] = 'You must type an IP';
    } elseif ($error === 0 && !filter_var($_POST['ip'], FILTER_VALIDATE_IP)) {
        $error++;
        $validationErrors['ip'] = 'You must type a valid IP';
    }
}

Also regenerates the PHPStan baseline to realign the two pre-existing entries (ternary.alwaysTrue, booleanNot.alwaysFalse) whose line numbers shifted after this change.

No behavior change — pure equivalence transform ($postBanType === BanType::Ip was already a common factor to both branches, just factored out as an outer gate).

Motivation and Context

#1540
PHPStan was flagging the previous compound condition as always-false dead code:

Result of && is always false.
Result of && is always false.
Strict comparison using === between BanType::Steam and BanType::Ip will always evaluate to false.

blocking CI on pages/admin.edit.ban.php.

Confirmed false positive via isolated repro: combining a re-check of $postBanType with a mixed-typed $_POST value in the same && expression, right after an earlier if/elseif/elseif/else chain that discriminates on $postBanType only in its first branch, causes PHPStan to mis-merge the type. Renaming the variable or precomputing a boolean flag did not resolve it; removing the compound && combination does.

Fixes #

How Has This Been Tested?

  • Ran includes/vendor/bin/phpstan analyse --configuration=phpstan.neon --no-progress --error-format=github --memory-limit=1G locally — clean, no errors on pages/admin.edit.ban.php.
  • Manual smoke test on a local instance: edited an IP-type ban with an empty IP field → "You must type an IP"; with a malformed IP → "You must type a valid IP"; with a valid IP → saves normally.
  • Manual smoke test: edited a Steam-type ban → unaffected, IP block correctly skipped entirely.

Screenshots (if appropriate):

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)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the [CONTRIBUTING](CONTRIBUTING.md) document.

Note: If your PR touches web/** and this is your first
contribution to the web panel, the CLA bot will comment within
~30 seconds with one-line sign instructions. You only need to sign
once per repo. See [CLA.md](https://claude.ai/chat/CLA.md) for the full text and
[CONTRIBUTING.md](https://claude.ai/chat/CONTRIBUTING.md) for the rationale.

Gate the IP-length/validity check on `$postBanType === BanType::Ip` alone,
then nest the `$_POST['ip']`-dependent conditions inside, instead of
re-checking the enum inside a compound `&&` alongside a mixed-typed
operand.

PHPStan was flagging the previous compound condition as always-false dead
code. Confirmed false positive via isolated repro: combining a re-check of
$postBanType with a mixed-typed $_POST value in the same && expression,
right after an earlier if/elseif/elseif/else chain that discriminates on
$postBanType only in its first branch, causes PHPStan to mis-merge the
type. Renaming the variable or precomputing a boolean flag did not resolve
it; removing the compound && combination does.

Also regenerates the PHPStan baseline to realign the two pre-existing
entries (ternary.alwaysTrue, booleanNot.alwaysFalse) whose line numbers
shifted after this change.

No behavior change — pure equivalence transform.
Copilot AI review requested due to automatic review settings August 3, 2026 12:32
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@Rushaway

Rushaway commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@Rushaway
Rushaway added this pull request to the merge queue Aug 3, 2026
Merged via the queue into sbpp:main with commit 43b42f2 Aug 3, 2026
5 of 6 checks passed
@Rushaway
Rushaway deleted the 1540 branch August 3, 2026 12:49
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants