Rector — enable instanceOf, earlyReturn and if. Each was measured at zero changes
across the analysed tree, so they cost nothing now and only hold the line on new
code. The counts for every set that stays off are recorded next to them, with the
reason, so the decision does not have to be re-derived: typeDeclarations(319) is
the native-type footgun TYPE_AUDIT.md tracks, typeDeclarationDocblocks(86) feeds
that same footgun one step removed because `make cs-fix` derives native types from
docblocks, codingStyle(255) rewrites the `use` imports check-procedural-use depends
on, naming(132) fights the $r-prefix convention and render()-by-name views, and
privatization reads as 0 only because there are almost no final classes yet.
Makefile — `make rector` now passes --clear-cache. Rector's cache key does not
track the rule set, so after editing the config a cached run prints "Rector is
done!" and a newly enabled set looks like a no-op. The measurements above first
came back all-zero for exactly this reason.
phpcs — silence the four sniffs that were still reporting. Every one of them was
checked for a fixer first (none calls addFixable), so `make cs-fix` could never
have cleared them; what was left was 769 warnings no tool can act on:
NoSilencedErrors(473) each @ is either deliberate or masking a real error, so
they need judging one at a time, with a test each
CyclomaticComplexity (201 err + 190 warn) and NestingLevel (2 + 84) — already
ratcheted per-function by the CRAP gate, which blocks NEW
complexity; the sniffs only restated the old backlog
PSR1.Files.SideEffects(22) needs files split, which moves loading and autoload
LineLength(3480) went the same way earlier and is marked as not part of the
ratchet: it has no fixer at all, so "re-enable and fix" would mean splitting those
lines by hand.
Own rules are commented out so they read as toggles; the PSR12-inherited ones
(SideEffects, LineLength) can only be <exclude>d, which the ratchet summary now
says out loud. That summary is updated with every new toggle and its count.
`make cs` is now clean, with 85 sniffs still active — checked, because an empty
report looks the same whether nothing is wrong or everything got muted.
The prelude shims (Paths/AppConfig/Binaries/ErrorCodes.php) were deleted and the
$rErrorCodes global is gone. Point the developer guides at the new homes:
constants → ConstantsInitializer (paths()/appConfig()/binaries() maps), error
catalogue → ErrorResponder::codes(). Also refresh the now-outdated "refactored
later" note in build/rector.php's skip list.
Only docs/en is edited (docs/ru is regenerated from it before a release);
make docs-build passes.
Rector's FollowRequireByDirRector (applied in the stage-2 pass, 9665a5a2)
rewrote the admin/reseller table bootstrap include:
include "functions.php"; -> include __DIR__ . "/functions.php";
The bare include resolved the legacy admin bootstrap via include_path /
CWD at runtime; that bootstrap lives under Public/Views/admin, NOT next
to the controller. Prepending __DIR__ pinned the path to
Public/Controllers/Admin/functions.php, which does not exist, so every
session-authenticated table request (the `isset($_SESSION['hash'])` /
`isset($_SESSION['reseller'])` branch — i.e. the normal browser-panel
path) hit "include(...functions.php): Failed to open stream" and lost the
$db / $rUserInfo / $rPermissions / $rServers globals the rest of index()
needs.
Restore the exact pre-Rector includes and document the rule as unsafe in
build/rector.php so it stays disabled if a future Rector version
reintroduces it. 721 tests green.
The stage-2 pass ran SimplifyDeMorganBinaryRector, which — negating a condition
whose operand is an assignment — dropped the grouping parens:
// was (correct):
if (!(($rLine = UserRepository::getLineById($rID)) && Authorization::check('line', $rID)))
// Rector produced (broken):
if (!$rLine = UserRepository::getLineById($rID) || !Authorization::check('line', $rID))
By PHP precedence the second form does NOT mean `!((r=...) && check())`: it
returns "not failed" when the entity is missing OR the caller is unauthorised,
and leaves $rLine holding a bool. That is a broken-access-control bypass — a
background security review flagged it in ResellerAPIWrapper.
Scope: 49 conditions were mangled — 5 in ResellerAPIWrapper, 44 in
AdminAPIWrapper — most guarding Authorization::check / isset on get* lookups.
All were correct (parenthesised) on main; only the unpushed refactor/rector
branch was affected. Restored each to `!($x = f()) || !cond` (De Morgan of the
original). The `strtotime`/StreamView `!$x = f()` sites (no trailing `||`) parse
correctly and were left as-is.
Also disable SimplifyDeMorganBinaryRector in build/rector.php (its parens-drop
on assignment-in-condition outweighs the cosmetic wins) and document both
parens-bug variants + review greps in the config header.
Verified: both bug-pattern greps return nothing; PHPStan level 5 clean; suite
721 tests / 0 errors; Rector at fixpoint.
Widen the Rector config to the remaining class-based trees (Streaming,
Public\Controllers, Ministra), keeping the templates, procedural
front-controllers (Public/stream, Public/admin, Ministra/portal.php) and the
streaming hot-path bootstraps out. Also document the recurring dropped-parens
bug + the review grep in the config header.
The resulting 107-file pass (94 Public\Controllers, 11 Streaming, 2 Ministra)
was reviewed against the suite + PHPStan + the usual scans:
- No dropped-parens bug this time (the assignment-in-condition pattern didn't
occur in these files).
- 6 locally-called private static helpers became instance methods
(behaviour-preserving; all called via $this->, no static call sites); one
unused private param dropped (FanoutConfig::desired $rSnapshot, call site
updated).
- De Morgan / ternary / dead-code simplifications; two `$x = getById()`
truthy-assignments folded into the condition (no comparison, safe).
PHPStan level 5 clean; suite 721 tests / 0 errors.
Adopt Rector as a require-dev tool alongside PHPStan/phpcs for safe, mechanical
refactoring:
- src/composer.json: add rector/rector ^2.0 (require-dev) + refactor/refactor:dry
composer scripts.
- build/rector.php: narrowly-scoped config over the PSR-4 class trees
(Core/Domain/Cli/Infrastructure). Skips the \TMDB lib, the streaming hot-path,
vendor, tmp/backups. Import-adding stays OFF (the check-procedural-use gate
relies on positional use imports). Behaviour-changing rules are disabled
(SafeDeclareStrictTypes, UseIdenticalOverEqualWithSameType); only the safe
deadCode + codeQuality prepared sets run (incl. the empty-if/else collapse).
- Makefile: `make rector` (dry-run, non-zero on pending changes) and
`make rector-fix` (apply). Both require dev-tools.
- docs/en/guides/refactoring.md + dev-workflow.md + mkdocs.yml nav: the
detect -> diff -> verify -> apply workflow.
No source code is changed by this commit — scaffolding only. `make rector-fix`
output will be reviewed separately.
Divarion_D