You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
PHPStan runs at level 5 (per web/phpstan.neon:10) with the deprecation-rules plugin, the in-tree SmartyTemplateRule, and staabm/phpstan-dba for raw-SQL type-checking against the live schema. That's a real foundation, but the surface area where a bug becomes a static failure (red CI on the PR) instead of a runtime failure (500 in prod, or worse, silently wrong row inserted) is still narrow. A lot of patterns that Rust would refuse to compile slide through this codebase as silent coercions, untyped arrays, mixed returns, non-exhaustive match / switch, loose == comparisons, and unconstrained $_GET / $_POST reads.
#1101 covers the level climb (5 → 6 → 7 → 8, one step at a time). #1273 added the deprecation gate. #1290 covers the modernization (enums, namespaces, native types). This issue is the umbrella for the third leg of the static-analysis tripod: stricter rules, more expressive types, and project-specific custom rules. The bar is "what would Rust catch at compile time" — exhaustive matches, no silent coercions, no mixed in public APIs, sealed sum types, generics, purity annotations. PHP can't quite get there, but PHPStan + strict-rules + declare(strict_types=1) + custom in-tree rules close most of the gap.
This is opportunistic: each phase strictly increases the static-failure surface without changing wire format, schema, or behavior. The phases are independently revertable and compose with #1101 / #1290 — they don't block each other.
What's already in place (the baseline to build on)
Gate
Source
What it catches
PHPStan level 5
web/phpstan.neon:10
Standard reachability + parameter-count + obvious type bugs
Marquee page handlers fail PHPUnit if they raise E_DEPRECATED (catches what PHPStan can't see — auth/openid.php, runtime-null values that look non-null to the type system)
web/phpstan-baseline.neon is 781 lines (wc -l). The dominant identifiers are variable.undefined (global $userbank invisible to PHPStan), class.notFound (Smarty bridging), empty.variable, booleanNot.alwaysFalse / notEqual.alwaysTrue (already-known dead branches), phpDoc.parseError (old comment style). Most of these collapse naturally under one or more of the phases below.
What we're missing (Rust-comparable)
A. declare(strict_types=1) everywhere
Currently 81 files declare it (every Sbpp\…-namespaced file). The legacy core (web/includes/CUserManager.php, Database.php, Auth.php, Host.php, Log.php, Config.php, JWT.php, CSRF.php, Crypto.php, Api.php, ApiError.php, AdminTabs.php, auth/handler/*.php, system-functions.php, page-builder.php, init.php, every web/pages/*.php, every web/api/handlers/*.php) does NOT.
Without declare(strict_types=1), calling function foo(int $x) with '42' silently coerces to 42. Calling it with '42abc' coerces to 42 and emits a Notice PHP increasingly treats as Warning and PHP 9 will turn into a TypeError. With it, every coercion at a function boundary becomes a TypeError immediately — exactly what Rust does at compile time, except PHP has to do it at the call site because there's no compile step.
This is a pure-additive runtime gate. It interacts with #1290 phase A (typing the legacy method signatures) — declare(strict_types=1) only matters once parameters are typed. Order: ship it as the last commit in each #1290 phase A class-by-class PR, after the parameters are typed, so the strict gate has something to enforce.
Closing invariant: rg -L 'declare\(strict_types=1\)' web/includes web/pages web/api web/install web/updater --type php returns only the documented third-party exceptions (auth/openid.php, the SteamID library).
B. phpstan/phpstan-strict-rules
The official strict-rules plugin (phpstan/phpstan-strict-rules) adds the rule pack PHP itself doesn't enforce:
Forbid loose comparisons (== / != flagged; require === / !==). Today: 39 sites in page.banlist.php, 33 in admin.bans.php, 34 in page.commslist.php, 17 in admin.edit.admindetails.php, 41 in admin.edit.group.php, … The codebase is full of if ($_GET['mode'] == "delete") — those work, but $_GET['mode'] == 0 is true (string "delete" coerces to 0 via the (int) rule), so the same shape applied to a numeric route param ships a real bug. Strict-rules forces every == site to either be promoted to === (exact match — what the author meant) or wrapped in an explicit cast.
Forbid empty() — empty('0') is true, empty(null) is true, empty(0) is true, empty([]) is true, empty('false') is false. The function is the source of every "why is this empty / not empty?" stack-overflow question. 200+ sites across the codebase (rough count from rg -c '\bempty\('); each one becomes either === '' / === null / === 0 / === [] per the actual intent.
Require @throws — for every throw site, the calling chain has to declare or catch the exception in @throws. This is the closest PHP gets to Java's checked exceptions / Rust's Result<T, E> propagation; PHPStan tracks the throws-graph and flags un-caught un-declared exceptions.
Forbid uninitialized variable use — already largely covered by level 5, strict-rules makes it ironclad.
Forbid static calls on instance methods, forbid dynamicCallOnStaticMethod, etc. — small wins, mechanical.
The baseline will balloon — probably another 500-800 entries on top of the current 781. That's expected; the AGENTS.md "raise one step at a time" rule applies. Strategy: enable strict-rules, regenerate the baseline, and carve out follow-up PRs to drain the baseline category by category (one PR per identifier — equal.notAllowed, empty.notAllowed, switch.noDefault, etc.). #1101's level climb stays on its own track.
C. PHPStan level 9 on the new layer (carved out from excludePaths)
#1101 ramps the global level one notch at a time and "stops at whatever level produces a baseline we can't make sense of" — likely 7 or 8 for the codebase as a whole. That's the right call for the legacy core, but the modern layer (web/includes/View/*, Mail/*, Markup/*, Util/*, Theme.php, Version.php, Renderer.php, phpstan/* rules) is already strict-types + typed everywhere — it can carry level 9 today.
PHPStan supports per-path level overrides via parametersSchema.level per-include. Concretely, web/phpstan.neon would add:
parameters:level:5# … existing config …# Per-path overrides for the modern layer that already meets a higher bar.includes:- phpstan-modern.neon# phpstan-modern.neon:parameters:level:9paths:- includes/View- includes/Mail- includes/Markup- includes/Util- includes/Theme.php- includes/Version.php- phpstan
Level 9 disallows mixed everywhere. The view DTOs already type every property; the $theme / $userbank globals don't reach into includes/View, so this is tractable. The win: a regression that introduces a mixed return into the modern layer fails the build immediately, instead of waiting for the layer-wide level climb to catch it years later.
D. Generics via @template
PHPStan supports generics in docblocks (@template T, @template-implements, @phpstan-template-covariant). PHP doesn't have them natively but the analyzer treats them with full Rust-like rigor.
Concrete wins:
Database::resultset(?array $inputParams = null, $fetchType = PDO::FETCH_ASSOC) returns array<int, array<string, mixed>> today. With a @template TRow of array<string, mixed> on a hypothetical Repository<TRow> wrapper, the result type carries the row shape — BansRepository::activeBans() returns array<int, BanRow> where BanRow = array{bid: int, authid: string, name: string, ...} and PHPStan checks every $row['authid'] access against the actual schema. (SbppSyntaxErrorInQueryMethodRule + phpstan-dba already give us this for raw SQL; lifting it into a typed-result wrapper makes the call sites type-safe too.)
Api::register($action, $fn, …) accepts callable $fn today. With @template TParams of array, @template TResult on a HandlerRegistration<TParams, TResult>, the callable's input + output shapes are pinned at registration time and the dispatcher's invocation site is checked against them.
A Result<T, E> shape via @template-implements on a sealed interface (see phase F below) — Result<BanRow, ApiError> becomes an honest type.
This is purely additive — no runtime change. Rollout: pick one repository / collection / dispatcher at a time and add the generics; PHPStan immediately starts catching every shape-mismatch downstream. Reference: Sbpp\View\Perms::for already uses array<string, bool> shape, but a PermissionSnapshot value object with @template-style shape would be tighter.
E. Typed DTOs replacing array<string, mixed> for DB rows
Today, $row['authid'] reads from a mixed slot of a mixed array — PHPStan can prove the array exists, but not what's in 'authid'. phpstan-dba partially closes this for SQL strings it can introspect, but as soon as the row leaves Database::single() / ::resultset(), the shape is lost.
Pattern (matches the View DTO shape already in place):
finalreadonlyclass BanRow {
publicfunction__construct(
publicint$bid,
publicstring$authid,
publicstring$name,
public ?string$ip,
publicint$created,
publicint$length,
// …
) {}
/** @param array<string, mixed> $row */publicstaticfunctionfromRow(array$row): self {
returnnewself(
bid: (int) $row['bid'],
authid: (string) $row['authid'],
// … one cast per column, scoped to one place …
);
}
}
The (int) / (string) casts move from every consumer to the DTO factory — surgical instead of scattered. Every consumer reads $ban->authid (typed string, statically narrowed) instead of $row['authid'] (typed mixed, runtime-coerced). A column rename in struc.sql becomes one PHPStan error instead of seven Notices in production.
Scope per PR: one row shape per PR (BanRow, CommRow, AdminRow, ServerRow, LogRow, NoteRow, …). Each PR replaces the consumers in lockstep so the array form stops being read in the same commit the DTO factory lands.
F. Sealed sum types — Result<T, E> shape via interfaces
PHP doesn't have sealed classes natively (PHP 8.4 added #[\AllowDynamicProperties] but not sealed hierarchies). PHPStan supports sealed-tagged-union via the @phpstan-sealed-tagged-union annotation on interfaces:
PHPStan checks the match is exhaustive against the sealed set — adding a new BanLookup\AwaitingApproval case fails the build at every call site that hasn't handled it. This is the closest PHP gets to Rust's match on enum/Result, and it composes with #1290's WebPermission enum work.
Where this lands big: API handlers that today either return data, throw ApiError, or return ['__redirect' => '...'] — three return shapes shoved through one signature. Modeling them as a HandlerResult = Data | Error | Redirect sealed union makes the dispatcher's branching exhaustive.
G. Exhaustive match over enums (auto-gain on top of #1290 phase D)
PHPStan already flags non-exhaustive match (Enum::*) { … } when the input is typed as a backed enum and a case is missing — no plugin needed. Today this catches nothing because there's exactly one enum (Sbpp\Mail\EmailType, used in two match blocks both already exhaustive).
#1290 phase D adds LogType, BanType, BanRemoval, WebPermission. The moment those land, every Log::add(LogType::Error, …) / BanType::Steam / etc. match site is checked against the enum's full case set. Forgetting to handle LogType::Warning in a future log-rendering switch is a static failure — same shape as Rust's exhaustiveness check.
This issue's contribution: codify in AGENTS.md that enums must be matched, not switched (matching gives exhaustiveness; switching doesn't), and add a paragraph in the "Conventions" section pointing at this property as the reason enum is the preferred shape over class constants.
H. Custom in-tree PHPStan rules for project invariants
Sbpp\PHPStan\SmartyTemplateRule is the precedent: a small focused rule that knows something specific about this codebase (View DTO ↔ template binding) that no generic rule could catch. Same pattern, more rules:
SuperGlobalAccessRule — fail when $_GET[...] / $_POST[...] / $_SESSION[...] / $_COOKIE[...] is read outside an explicit allowlist (page-builder.php, init.php, the Request reader once it exists). Today these are scattered across every page handler and API handler; centralizing the read forces every input through a typed-validation seam.
GlobalKeywordRule — fail when global $userbank / global $theme appears outside a documented allowlist of files. Same "the legacy pattern is contained, not spreading" enforcement as the super-global rule.
ApiHandlerSignatureRule — fail when an api_<topic>_<action> function in web/api/handlers/*.php doesn't have @param / @return with concrete shapes (an array<string, mixed>@param is too loose).
PrefixedTableInQueryRule — extension of the existing SbppSyntaxErrorInQueryMethodRule that flags inline 'sb_' table-name usage in any string passed to Database::query (today only the syntax of the :prefix_ placeholder is checked; inline-prefix is the anti-pattern AGENTS.md calls out).
NoFilterRequiresCommentRule — Smarty {$foo nofilter} requires the {* nofilter: <why> *} comment immediately above it, per AGENTS.md. The convention is documented; a rule turns it into a build failure if anyone forgets.
Each rule is a single PHP class under web/includes/PHPStan/, a single registration block in web/phpstan.neon, and a unit test. Pattern is established (SmartyTemplateRule); incremental cost per rule is bounded.
I. Typed input boundary — Sbpp\Http\Request reader
That (int) / (string) cast + ?? default + bare comparison shape is repeated hundreds of times. Each repetition is a chance to forget the cast or the default and ship a null-into-scalar deprecation (#1273) or a loose-comparison bug.
A small Sbpp\Http\Request value object centralizes the boundary:
Page handlers consume the typed reader; the SuperGlobalAccessRule from phase H locks the boundary down so $_GET[...] accesses outside the reader are a static failure. This is the "all input crosses one type-checking gate" pattern — Rust does it via serde::Deserialize derive macros; PHP needs it explicit.
Strategy: ship the reader as an additive utility (zero call-site changes), then migrate handlers one at a time, then enable the rule. Same shape as #1290 phase B's class_alias shim.
J. @phpstan-pure / @phpstan-impure annotations
PHP isn't pure-by-default like Rust, but PHPStan supports @phpstan-pure to mark functions that have no side effects. The analyzer then checks the body — any I/O, global write, exception, or call to an impure function fails the build. The win: the obvious helpers (trunc, sizeFormat, checkExtension, BitToString, parseRconStatus, Sbpp\Util\Duration::*, Sbpp\View\PermissionCatalog::groupedDisplayFromMask, …) get the annotation and a future regression that adds a Log::add(...) call in the middle of one of them fails CI immediately.
This is purely additive (annotations only — runtime behavior unaffected). Pick a handful of obviously-pure helpers per PR; let PHPStan tell you which ones aren't actually pure.
K. Typed class constants (PHP 8.3, gated on #1289)
Once #1289 lands the 8.5 floor, every const FOO = 'bar'; in the codebase can become const string FOO = 'bar';. A future change to const FOO = 0; is a static failure. Examples:
B — phpstan/phpstan-strict-rules. Big-bang PR adds the dependency + regenerates the baseline; follow-up PRs drain the baseline category by category (equal.notAllowed, empty.notAllowed, switch.noDefault, missing.throws, …).
I — Sbpp\Http\Request value object. One PR for the class, then one per page/handler family for the migration, then one PR enabling SuperGlobalAccessRule's tight allowlist.
D — generics on Database::resultset / Api::register / new Repository<TRow> wrappers. One PR per surface.
E — typed DB-row DTOs. One PR per row shape (BanRow, CommRow, AdminRow, …).
F — sealed sum types where they replace array{ok: bool, error?: …, redirect?: …} shapes. Land after a handful of E PRs prove the row-DTO pattern works; the API handler HandlerResult is the marquee target.
Each phase composes with the level climb (#1101); none of them block it. The only ordering dependency on other issues is #1290 phase D for the enum rules and #1289 for typed class constants.
Acceptance criteria
This is an umbrella, not a single-PR ticket. Marking it complete requires:
Every file under web/includes/ (excluding auth/openid.php, the SteamID library), web/pages/, web/api/handlers/ declares declare(strict_types=1). The rg -L 'declare\(strict_types=1\)' invariant passes.
phpstan/phpstan-strict-rules is in web/composer.json (dev-dep), wired in web/phpstan.neon, and the baseline doesn't grow over the strict-rules-only entries from the initial bring-up.
Per-path level-9 override for the modern layer is in web/phpstan.neon (or a sibling include) and CI fails on any new mixed in includes/View / includes/Mail / includes/Markup / includes/Util / includes/Theme.php / includes/Version.php / phpstan/.
At least three custom in-tree rules from phase H are landed (LogTypeIsEnumRule, SuperGlobalAccessRule, GlobalKeywordRule). Each has a unit test that rejects the bad shape and accepts the good shape.
Sbpp\Http\Request exists and is the only entry point reading $_GET / $_POST / $_SESSION / $_COOKIE outside the documented allowlist. The SuperGlobalAccessRule's allowlist is the source of truth.
At least one repository / collection uses @template generics with the typed-row DTO (phase D + E proven on one shape).
At least one sealed sum type replaces an array{ok: bool, …} return shape (phase F proven on one shape).
AGENTS.md "Conventions" grew rows for: declare(strict_types=1) everywhere; "enums are matched, not switched"; the Request reader as the input boundary; the per-path level-9 override as the modern-layer guardrail.
AGENTS.md "Anti-patterns" grew rows for: loose == / != comparison; empty() (replaced by per-shape explicit checks); $_GET / $_POST access outside Request; mixed returns in the modern layer.
No regression on the wire-format snapshots or the API-contract snapshot. None of this changes behavior — if a phase ever does, that phase is mis-scoped.
Schema migrations. Phase E's typed DB-row DTOs wrap around the existing schema. Adding actual ENUM(...) columns (e.g. for :prefix_log.type to back the LogType enum at the DB level) is a paired migration with an updater script — different blast radius.
vimeo/psalm as a second analyzer. PHPStan is already the established gate; running both costs more in CI time and contributor cognitive overhead than the marginal bug-catch is worth. Pick one and push it hard. (PHPStan already dominates in this codebase via phpstan-dba + the custom rules.)
JS-side type tightening.web/scripts/*.js is // @ts-check on vanilla JS today. The TS-side equivalent (turn on noImplicitAny, strictNullChecks, etc.) is a separate ticket.
web/install/ and web/updater/ (wrapper + data scripts) are both in scope — strict-types, typed signatures, sealed-union refactors, custom rules, all of it. They're live code on the main path for self-hosters. The one practical wrinkle, same as in #1290: a strict-types pass on a shipped web/updater/data/<N>.php is fine (the script's effect doesn't change), but if the typing exposes a real behavior bug whose fix would alter the SQL the script runs, land that fix as a new <N+1>.php instead of editing in place. See AGENTS.md "Updater migrations".
phpstan/phpstan-strict-rules (phase B) — kills == / empty() / non-exhaustive switch as build-time failures.
Sealed sum types (phase F) — Result<T, E> for handlers; PHPStan checks every match arm.
Typed DB-row DTOs (phase E) — column rename → one analyzer error instead of seven runtime notices.
The baseline will grow during phases B + C and shrink during phases D + E. Net direction over the umbrella's lifetime is shrink. Each phase's PR description should call out the baseline diff so the maintainer can spot baseline-bypass attempts (the same review hygiene Raise PHPStan level from 4 toward 8 #1101 already established).
This issue does NOT change behavior, schema, wire format, or permissions semantics. If a phase ever feels like it does, that phase is mis-scoped — split it.
Problem
PHPStan runs at level 5 (per
web/phpstan.neon:10) with the deprecation-rules plugin, the in-treeSmartyTemplateRule, andstaabm/phpstan-dbafor raw-SQL type-checking against the live schema. That's a real foundation, but the surface area where a bug becomes a static failure (red CI on the PR) instead of a runtime failure (500 in prod, or worse, silently wrong row inserted) is still narrow. A lot of patterns that Rust would refuse to compile slide through this codebase as silent coercions, untyped arrays,mixedreturns, non-exhaustivematch/switch, loose==comparisons, and unconstrained$_GET/$_POSTreads.#1101 covers the level climb (5 → 6 → 7 → 8, one step at a time). #1273 added the deprecation gate. #1290 covers the modernization (enums, namespaces, native types). This issue is the umbrella for the third leg of the static-analysis tripod: stricter rules, more expressive types, and project-specific custom rules. The bar is "what would Rust catch at compile time" — exhaustive matches, no silent coercions, no
mixedin public APIs, sealed sum types, generics, purity annotations. PHP can't quite get there, but PHPStan + strict-rules +declare(strict_types=1)+ custom in-tree rules close most of the gap.This is opportunistic: each phase strictly increases the static-failure surface without changing wire format, schema, or behavior. The phases are independently revertable and compose with #1101 / #1290 — they don't block each other.
What's already in place (the baseline to build on)
web/phpstan.neon:10phpstan/phpstan-deprecation-rules(web/phpstan.neon:7)staabm/phpstan-dba(web/phpstan.neon:3)Database::query("SELECT …")validated against the live MariaDB schemaSbpp\PHPStan\SmartyTemplateRule(web/includes/PHPStan/SmartyTemplateRule.php)Sbpp\View\…Viewcross-checked against the.tplit renders — declared-but-unused properties, used-but-undeclared variablesSbpp\PhpStan\SbppSyntaxErrorInQueryMethodRule(web/phpstan.neon:36-47):prefix_placeholder rewriting inDatabase::query's first argweb/tests/integration/Php82DeprecationsTest.php(#1273)E_DEPRECATED(catches what PHPStan can't see —auth/openid.php, runtime-null values that look non-null to the type system)web/phpstan-baseline.neonis 781 lines (wc -l). The dominant identifiers arevariable.undefined(global $userbankinvisible to PHPStan),class.notFound(Smarty bridging),empty.variable,booleanNot.alwaysFalse/notEqual.alwaysTrue(already-known dead branches),phpDoc.parseError(old comment style). Most of these collapse naturally under one or more of the phases below.What we're missing (Rust-comparable)
A.
declare(strict_types=1)everywhereCurrently 81 files declare it (every
Sbpp\…-namespaced file). The legacy core (web/includes/CUserManager.php,Database.php,Auth.php,Host.php,Log.php,Config.php,JWT.php,CSRF.php,Crypto.php,Api.php,ApiError.php,AdminTabs.php,auth/handler/*.php,system-functions.php,page-builder.php,init.php, everyweb/pages/*.php, everyweb/api/handlers/*.php) does NOT.Without
declare(strict_types=1), callingfunction foo(int $x)with'42'silently coerces to42. Calling it with'42abc'coerces to42and emits aNoticePHP increasingly treats asWarningand PHP 9 will turn into aTypeError. With it, every coercion at a function boundary becomes aTypeErrorimmediately — exactly what Rust does at compile time, except PHP has to do it at the call site because there's no compile step.This is a pure-additive runtime gate. It interacts with #1290 phase A (typing the legacy method signatures) —
declare(strict_types=1)only matters once parameters are typed. Order: ship it as the last commit in each #1290 phase A class-by-class PR, after the parameters are typed, so the strict gate has something to enforce.Closing invariant:
rg -L 'declare\(strict_types=1\)' web/includes web/pages web/api web/install web/updater --type phpreturns only the documented third-party exceptions (auth/openid.php, the SteamID library).B.
phpstan/phpstan-strict-rulesThe official strict-rules plugin (
phpstan/phpstan-strict-rules) adds the rule pack PHP itself doesn't enforce:==/!=flagged; require===/!==). Today: 39 sites inpage.banlist.php, 33 inadmin.bans.php, 34 inpage.commslist.php, 17 inadmin.edit.admindetails.php, 41 inadmin.edit.group.php, … The codebase is full ofif ($_GET['mode'] == "delete")— those work, but$_GET['mode'] == 0is true (string"delete"coerces to0via the(int)rule), so the same shape applied to a numeric route param ships a real bug. Strict-rules forces every==site to either be promoted to===(exact match — what the author meant) or wrapped in an explicit cast.empty()—empty('0')istrue,empty(null)istrue,empty(0)istrue,empty([])istrue,empty('false')isfalse. The function is the source of every "why is this empty / not empty?" stack-overflow question. 200+ sites across the codebase (rough count fromrg -c '\bempty\('); each one becomes either=== ''/=== null/=== 0/=== []per the actual intent.switchwithoutdefault— pairs with Adopt modern PHP features across legacy core (enums, native types, namespacing, match, str_contains, …) #1290 phase C (switch → match), sincematchalways requires exhaustiveness anyway.@throws— for everythrowsite, the calling chain has to declare or catch the exception in@throws. This is the closest PHP gets to Java's checked exceptions / Rust'sResult<T, E>propagation; PHPStan tracks the throws-graph and flags un-caught un-declared exceptions.staticcalls on instance methods, forbiddynamicCallOnStaticMethod, etc. — small wins, mechanical.The baseline will balloon — probably another 500-800 entries on top of the current 781. That's expected; the AGENTS.md "raise one step at a time" rule applies. Strategy: enable strict-rules, regenerate the baseline, and carve out follow-up PRs to drain the baseline category by category (one PR per identifier —
equal.notAllowed,empty.notAllowed,switch.noDefault, etc.). #1101's level climb stays on its own track.C. PHPStan level 9 on the new layer (carved out from
excludePaths)#1101 ramps the global level one notch at a time and "stops at whatever level produces a baseline we can't make sense of" — likely 7 or 8 for the codebase as a whole. That's the right call for the legacy core, but the modern layer (
web/includes/View/*,Mail/*,Markup/*,Util/*,Theme.php,Version.php,Renderer.php,phpstan/*rules) is already strict-types + typed everywhere — it can carry level 9 today.PHPStan supports per-path level overrides via
parametersSchema.levelper-include. Concretely,web/phpstan.neonwould add:Level 9 disallows
mixedeverywhere. The view DTOs already type every property; the$theme/$userbankglobals don't reach intoincludes/View, so this is tractable. The win: a regression that introduces amixedreturn into the modern layer fails the build immediately, instead of waiting for the layer-wide level climb to catch it years later.D. Generics via
@templatePHPStan supports generics in docblocks (
@template T,@template-implements,@phpstan-template-covariant). PHP doesn't have them natively but the analyzer treats them with full Rust-like rigor.Concrete wins:
Database::resultset(?array $inputParams = null, $fetchType = PDO::FETCH_ASSOC)returnsarray<int, array<string, mixed>>today. With a@template TRow of array<string, mixed>on a hypotheticalRepository<TRow>wrapper, the result type carries the row shape —BansRepository::activeBans()returnsarray<int, BanRow>whereBanRow = array{bid: int, authid: string, name: string, ...}and PHPStan checks every$row['authid']access against the actual schema. (SbppSyntaxErrorInQueryMethodRule+phpstan-dbaalready give us this for raw SQL; lifting it into a typed-result wrapper makes the call sites type-safe too.)Api::register($action, $fn, …)acceptscallable $fntoday. With@template TParams of array, @template TResulton aHandlerRegistration<TParams, TResult>, the callable's input + output shapes are pinned at registration time and the dispatcher's invocation site is checked against them.Result<T, E>shape via@template-implementson a sealed interface (see phase F below) —Result<BanRow, ApiError>becomes an honest type.This is purely additive — no runtime change. Rollout: pick one repository / collection / dispatcher at a time and add the generics; PHPStan immediately starts catching every shape-mismatch downstream. Reference:
Sbpp\View\Perms::foralready usesarray<string, bool>shape, but aPermissionSnapshotvalue object with@template-style shape would be tighter.E. Typed DTOs replacing
array<string, mixed>for DB rowsToday,
$row['authid']reads from amixedslot of amixedarray — PHPStan can prove the array exists, but not what's in'authid'.phpstan-dbapartially closes this for SQL strings it can introspect, but as soon as the row leavesDatabase::single()/::resultset(), the shape is lost.Pattern (matches the View DTO shape already in place):
The
(int)/(string)casts move from every consumer to the DTO factory — surgical instead of scattered. Every consumer reads$ban->authid(typedstring, statically narrowed) instead of$row['authid'](typedmixed, runtime-coerced). A column rename instruc.sqlbecomes one PHPStan error instead of sevenNotices in production.Scope per PR: one row shape per PR (
BanRow,CommRow,AdminRow,ServerRow,LogRow,NoteRow, …). Each PR replaces the consumers in lockstep so the array form stops being read in the same commit the DTO factory lands.F. Sealed sum types —
Result<T, E>shape via interfacesPHP doesn't have sealed classes natively (PHP 8.4 added
#[\AllowDynamicProperties]but not sealed hierarchies). PHPStan supports sealed-tagged-union via the@phpstan-sealed-tagged-unionannotation on interfaces:Then the consumer:
PHPStan checks the
matchis exhaustive against the sealed set — adding a newBanLookup\AwaitingApprovalcase fails the build at every call site that hasn't handled it. This is the closest PHP gets to Rust'smatchonenum/Result, and it composes with #1290'sWebPermissionenum work.Where this lands big: API handlers that today either return data, throw
ApiError, or return['__redirect' => '...']— three return shapes shoved through one signature. Modeling them as aHandlerResult = Data | Error | Redirectsealed union makes the dispatcher's branching exhaustive.G. Exhaustive
matchover enums (auto-gain on top of #1290 phase D)PHPStan already flags non-exhaustive
match (Enum::*) { … }when the input is typed as a backed enum and a case is missing — no plugin needed. Today this catches nothing because there's exactly one enum (Sbpp\Mail\EmailType, used in twomatchblocks both already exhaustive).#1290 phase D adds
LogType,BanType,BanRemoval,WebPermission. The moment those land, everyLog::add(LogType::Error, …)/BanType::Steam/ etc. match site is checked against the enum's full case set. Forgetting to handleLogType::Warningin a future log-rendering switch is a static failure — same shape as Rust's exhaustiveness check.This issue's contribution: codify in AGENTS.md that enums must be matched, not switched (matching gives exhaustiveness; switching doesn't), and add a paragraph in the "Conventions" section pointing at this property as the reason
enumis the preferred shape over class constants.H. Custom in-tree PHPStan rules for project invariants
Sbpp\PHPStan\SmartyTemplateRuleis the precedent: a small focused rule that knows something specific about this codebase (View DTO ↔ template binding) that no generic rule could catch. Same pattern, more rules:LogTypeIsEnumRule— fail whenLog::add(...)'s first arg is anything other than aLogType::*case. Currently'm'/'e'/'w'are passed everywhere; once Adopt modern PHP features across legacy core (enums, native types, namespacing, match, str_contains, …) #1290 phase D.1 lands the enum, this rule prevents reintroducing the magic-char form.SuperGlobalAccessRule— fail when$_GET[...]/$_POST[...]/$_SESSION[...]/$_COOKIE[...]is read outside an explicit allowlist (page-builder.php,init.php, theRequestreader once it exists). Today these are scattered across every page handler and API handler; centralizing the read forces every input through a typed-validation seam.GlobalKeywordRule— fail whenglobal $userbank/global $themeappears outside a documented allowlist of files. Same "the legacy pattern is contained, not spreading" enforcement as the super-global rule.ApiHandlerSignatureRule— fail when anapi_<topic>_<action>function inweb/api/handlers/*.phpdoesn't have@param/@returnwith concrete shapes (anarray<string, mixed>@paramis too loose).PrefixedTableInQueryRule— extension of the existingSbppSyntaxErrorInQueryMethodRulethat flags inline'sb_'table-name usage in any string passed toDatabase::query(today only the syntax of the:prefix_placeholder is checked; inline-prefix is the anti-pattern AGENTS.md calls out).NoFilterRequiresCommentRule— Smarty{$foo nofilter}requires the{* nofilter: <why> *}comment immediately above it, per AGENTS.md. The convention is documented; a rule turns it into a build failure if anyone forgets.Each rule is a single PHP class under
web/includes/PHPStan/, a single registration block inweb/phpstan.neon, and a unit test. Pattern is established (SmartyTemplateRule); incremental cost per rule is bounded.I. Typed input boundary —
Sbpp\Http\RequestreaderToday every page / API handler does:
That
(int)/(string)cast +??default + bare comparison shape is repeated hundreds of times. Each repetition is a chance to forget the cast or the default and ship anull-into-scalar deprecation (#1273) or a loose-comparison bug.A small
Sbpp\Http\Requestvalue object centralizes the boundary:Page handlers consume the typed reader; the
SuperGlobalAccessRulefrom phase H locks the boundary down so$_GET[...]accesses outside the reader are a static failure. This is the "all input crosses one type-checking gate" pattern — Rust does it viaserde::Deserializederive macros; PHP needs it explicit.Strategy: ship the reader as an additive utility (zero call-site changes), then migrate handlers one at a time, then enable the rule. Same shape as #1290 phase B's
class_aliasshim.J.
@phpstan-pure/@phpstan-impureannotationsPHP isn't pure-by-default like Rust, but PHPStan supports
@phpstan-pureto mark functions that have no side effects. The analyzer then checks the body — any I/O, global write, exception, or call to an impure function fails the build. The win: the obvious helpers (trunc,sizeFormat,checkExtension,BitToString,parseRconStatus,Sbpp\Util\Duration::*,Sbpp\View\PermissionCatalog::groupedDisplayFromMask, …) get the annotation and a future regression that adds aLog::add(...)call in the middle of one of them fails CI immediately.This is purely additive (annotations only — runtime behavior unaffected). Pick a handful of obviously-pure helpers per PR; let PHPStan tell you which ones aren't actually pure.
K. Typed class constants (PHP 8.3, gated on #1289)
Once #1289 lands the 8.5 floor, every
const FOO = 'bar';in the codebase can becomeconst string FOO = 'bar';. A future change toconst FOO = 0;is a static failure. Examples:CSRF::FIELD_NAME = 'csrf_token'→const string FIELD_NAME = 'csrf_token'Mailer::DEFAULT_FROM_NAME = 'SourceBans++'→const string DEFAULT_FROM_NAME = 'SourceBans++'const TEMPLATE = 'page_*.tpl'on the View subclassesMechanical sweep, single PR after #1289.
Phasing
Order to minimize churn pile-up and so each phase lands on a stable predecessor:
phpstan-modern.neon), highest signal-to-noise; locks the modern layer at its current strictness so Adopt modern PHP features across legacy core (enums, native types, namespacing, match, str_contains, …) #1290 doesn't accidentally regress it.declare(strict_types=1)everywhere. Bundle commit-by-commit with Adopt modern PHP features across legacy core (enums, native types, namespacing, match, str_contains, …) #1290 phase A (each legacy class gets its parameters typed AND its strict-types declaration in the same PR — the two are useless apart).@phpstan-pureon the obvious helpers. Single PR.LogTypeIsEnumRule,SuperGlobalAccessRule,GlobalKeywordRule. One PR per rule. Land after Adopt modern PHP features across legacy core (enums, native types, namespacing, match, str_contains, …) #1290 phase D.1 (LogType enum) for the first rule.phpstan/phpstan-strict-rules. Big-bang PR adds the dependency + regenerates the baseline; follow-up PRs drain the baseline category by category (equal.notAllowed,empty.notAllowed,switch.noDefault,missing.throws, …).Sbpp\Http\Requestvalue object. One PR for the class, then one per page/handler family for the migration, then one PR enablingSuperGlobalAccessRule's tight allowlist.Database::resultset/Api::register/ newRepository<TRow>wrappers. One PR per surface.BanRow,CommRow,AdminRow, …).array{ok: bool, error?: …, redirect?: …}shapes. Land after a handful of E PRs prove the row-DTO pattern works; the API handlerHandlerResultis the marquee target.Each phase composes with the level climb (#1101); none of them block it. The only ordering dependency on other issues is #1290 phase D for the enum rules and #1289 for typed class constants.
Acceptance criteria
This is an umbrella, not a single-PR ticket. Marking it complete requires:
web/includes/(excludingauth/openid.php, the SteamID library),web/pages/,web/api/handlers/declaresdeclare(strict_types=1). Therg -L 'declare\(strict_types=1\)'invariant passes.phpstan/phpstan-strict-rulesis inweb/composer.json(dev-dep), wired inweb/phpstan.neon, and the baseline doesn't grow over the strict-rules-only entries from the initial bring-up.web/phpstan.neon(or a sibling include) and CI fails on any newmixedinincludes/View/includes/Mail/includes/Markup/includes/Util/includes/Theme.php/includes/Version.php/phpstan/.LogTypeIsEnumRule,SuperGlobalAccessRule,GlobalKeywordRule). Each has a unit test that rejects the bad shape and accepts the good shape.Sbpp\Http\Requestexists and is the only entry point reading$_GET/$_POST/$_SESSION/$_COOKIEoutside the documented allowlist. TheSuperGlobalAccessRule's allowlist is the source of truth.@templategenerics with the typed-row DTO (phase D + E proven on one shape).array{ok: bool, …}return shape (phase F proven on one shape).declare(strict_types=1)everywhere; "enums are matched, not switched"; theRequestreader as the input boundary; the per-path level-9 override as the modern-layer guardrail.==/!=comparison;empty()(replaced by per-shape explicit checks);$_GET/$_POSTaccess outsideRequest;mixedreturns in the modern layer.Out of scope (file separately if pursued)
ENUM(...)columns (e.g. for:prefix_log.typeto back theLogTypeenum at the DB level) is a paired migration with an updater script — different blast radius.vimeo/psalmas a second analyzer. PHPStan is already the established gate; running both costs more in CI time and contributor cognitive overhead than the marginal bug-catch is worth. Pick one and push it hard. (PHPStan already dominates in this codebase viaphpstan-dba+ the custom rules.)global $userbank;/$GLOBALS['PDO']. Phase H'sGlobalKeywordRulecontains the spread — the existing global-state pattern stays. Killing it is a real DI migration (out of scope in Adopt modern PHP features across legacy core (enums, native types, namespacing, match, str_contains, …) #1290 too; same reasoning).web/scripts/*.jsis// @ts-checkon vanilla JS today. The TS-side equivalent (turn onnoImplicitAny,strictNullChecks, etc.) is a separate ticket.web/install/andweb/updater/(wrapper + data scripts) are both in scope — strict-types, typed signatures, sealed-union refactors, custom rules, all of it. They're live code on the main path for self-hosters. The one practical wrinkle, same as in #1290: a strict-types pass on a shippedweb/updater/data/<N>.phpis fine (the script's effect doesn't change), but if the typing exposes a real behavior bug whose fix would alter the SQL the script runs, land that fix as a new<N+1>.phpinstead of editing in place. See AGENTS.md "Updater migrations".Notes
Deprecated: strlen(): Passing null to parameter #1across page handlers + auth (PHP 9 makes this a fatal) #1273 (deprecation rules + runtime trap). It composes with Adopt modern PHP features across legacy core (enums, native types, namespacing, match, str_contains, …) #1290 (modern features) — most phases there enable phases here (typed signatures unlockdeclare(strict_types=1)'s teeth; enums unlock exhaustivematch; namespaces unlock the per-path overrides). Adopt modern PHP features across legacy core (enums, native types, namespacing, match, str_contains, …) #1290 is not a hard prerequisite — most of this can land independently — but the order suggested above pairs them where the value compounds.matchon enums (phase G + Adopt modern PHP features across legacy core (enums, native types, namespacing, match, str_contains, …) #1290 phase D) — automatic once enums exist; analyzer catches every missing case.declare(strict_types=1)everywhere (phase A) — runtime gate that closes the silent-coercion trapdoor PHP 8.2+Deprecated: strlen(): Passing null to parameter #1across page handlers + auth (PHP 9 makes this a fatal) #1273 documented.phpstan/phpstan-strict-rules(phase B) — kills==/empty()/ non-exhaustiveswitchas build-time failures.Result<T, E>for handlers; PHPStan checks every match arm.