Repository navigation
chore(php): native types + final class on legacy core (#1290) - #1296
Merged
Merged
Conversation
Phases A and J from #1290: - A: native parameter / return types replacing @PARAM / @return docblocks across CUserManager, Database, Log, Auth, JWT, CSRF, Crypto, Api, ApiError, AdminTabs, Theme, Mailer, the auth handlers, Config, Host, system-functions.php. Coordinated with PR1's phase I — three CUserManager methods (is_logged_in, is_admin, HasAccess simplified branch) take their `: bool` from PR1 in the same commit they're simplified. - J: `final class` on every legacy leaf that nothing extends. View stays abstract. Plus AGENTS.md anti-pattern + conventions rows. No functional change. PHPStan should infer better; baseline shrinks (380 -> 371): 9 entries removed (Smarty class.notFound x3, is_numeric.alreadyNarrowedType, mixed. parseError, deadCode.unreachable, parameter.notFound on Log::getCount @PARAM $search, JWT private method static call), 2 added (booleanOr.alwaysTrue + rightAlwaysTrue on pages/admin.edit.admindetails.php — legitimate findings the new types unmask). Coordinated with parallel PR1 (mechanical sweep) on CUserManager / Database / Api / auth handlers — bodies are PR1's, signatures are this PR's.
- M1: type SmartyCustomFunctions.php (5 procedural plugin functions
+ remove legacy @PARAM shape docblocks).
- M2: type page-builder.php's route() and build() signatures.
- m1: Database::lastInsertId() narrows to string|false (matches the
PHP manual; ?string $name parameter added).
- m2: Database::iterate() $fetchType parameter typed int (matches
sibling resultset() / single()).
- m3: AGENTS.md "Native types over docblocks" prose tightened to
add SmartyCustomFunctions.php / page-builder.php to the list
and acknowledge CUserManager's three deferred methods.
- n1/n2/n3: tighten loose return types (SecondsToString : string;
GetCommunityName : string; Log::getAll : array).
PHPStan + PHPUnit + api-contract still clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes part of #1290. Phases A + J from the umbrella issue:
@param/@returndocblocks across every legacy class inweb/includes/—CUserManager,Database,Log,Auth,JWT,CSRF,Crypto,Api,ApiError,AdminTabs,Theme,Mailer,Mail, the auth handlers,Config,Host,system-functions.php,SmartyCustomFunctions.php,page-builder.php. Coordinates with chore(php): mechanical syntax modernization sweep + switch→match (#1290) #1295 (mechanical sweep) — the three CUserManager methods chore(php): mechanical syntax modernization sweep + switch→match (#1290) #1295 simplified (is_logged_in,is_admin,HasAccessnumeric-flag branch) take their: boolreturn type from chore(php): mechanical syntax modernization sweep + switch→match (#1290) #1295; this PR types every other signature.final classon every legacy leaf that nothing extends.Viewstaysabstract(every concrete view DTO extends it). SteamID classes left as third-party.Plus AGENTS.md anti-pattern + conventions rows, and a new "Native types over docblocks" subsection.
Notable decisions
Database::execute()narrows from@return mixedto: bool— PHP's manual specifiesPDOStatement::execute(): bool. WithATTR_ERRMODE = ERRMODE_EXCEPTION(set in our PDO bootstrap), the only return paths aretrueor thrown. Verified by reading every caller.Database::resultset()narrows to: array—fetchAll()returnsarrayon PHP 8+ (thefalselegacy return only happens withERRMODE_SILENT).AdminTabs::__constructSmarty type fix:Smarty $theme→\Smarty\Smarty $theme(matchesRenderer.php's namespaced resolution). Removes 3 PHPStan baseline entries.admin.edit.admindetails.php:294— legitimate code smells the better type inference unmasked (HasAccess(ADMIN_OWNER) || HasAccess(ADMIN_EDIT_ADMINS) || $_GET['id'] == GetAid()is always true given the early-return guard at lines 51-52). Out-of-scope for this PR — file as a follow-up against the page handler.Closing invariants verified
Test plan
./sbpp.sh phpstanclean./sbpp.sh test388 tests pass (PR1's RoutingTest cases included)./sbpp.sh ts-checkclean./sbpp.sh composer api-contractbyte-identical