Skip to content

Skip rector visibility rule on PolicyCommand (fix cs-stan)#333

Merged
dereuromark merged 3 commits into
3.xfrom
fix-policy-command-visibility
May 12, 2026
Merged

Skip rector visibility rule on PolicyCommand (fix cs-stan)#333
dereuromark merged 3 commits into
3.xfrom
fix-policy-command-visibility

Conversation

@dereuromark
Copy link
Copy Markdown
Member

@dereuromark dereuromark commented May 12, 2026

Cake\Console\Command::buildOptionParser() is protected but the override on Authorization\Command\PolicyCommand is public, which Rector's MakeInheritedMethodVisibilitySameAsParentRector flags. As of the recent rector setup work on 3.x, the cs-stan workflow is red on master and every open PR inherits the failure.

Why not just narrow the override?

The straightforward fix — making the override protected — looks safe locally but breaks the (8.2, lowest) matrix and, more importantly, breaks real-world installs on older bake. PHP forbids narrowing a parent's visibility; Bake\Command\SimpleBakeCommand::buildOptionParser() was public in cakephp/bake < 3.6.4 (which only tagged on 2026-05-08) and protected from 3.6.4 onward:

authorization cakephp/bake PolicyCommand status
current 3.2.0 – 3.6.3 ✅ works (public child, public parent)
current 3.6.4+ ✅ works (public child, protected parent — widening)
narrowed override (rejected) 3.2.0 – 3.6.3 PHP Fatal error: Access level … must be public
narrowed override (rejected) 3.6.4+ ✅ works

Narrowing the override would have meant either a coordinated cakephp/bake: ^3.6.4 bump (BC-breaking for the narrow audience of users on bake 3.2–3.6.3 who run bin/cake bake policy) or moving bake from require-dev to require (larger change, deserves its own discussion).

Refs https://github.com/cakephp/bake/pull/1077/changes#r3227429622

What this PR does instead

Adds a per-file skip in rector.php for MakeInheritedMethodVisibilitySameAsParentRector on src/Command/PolicyCommand.php. The override stays public, which is forward- and backward-compatible against any supported bake version — widening is always allowed.

Net diff vs 3.x is a single addition to rector.php.

Cake\Console\Command declares buildOptionParser() as protected; the
override on PolicyCommand was public. Rector's
MakeInheritedMethodVisibilitySameAsParentRector flags this and the
cs-stan workflow is currently red on master.

Drop the override visibility to protected to match the parent. Behavior
is unchanged for callers, since the Bake/Command runner invokes the
parent's protected getOptionParser() pipeline.
@dereuromark dereuromark added this to the 3.x milestone May 12, 2026
The previous protected-visibility change on PolicyCommand::buildOptionParser
fails the (8.2, lowest) matrix: cakephp/bake < 3.6.4 still declares
SimpleBakeCommand::buildOptionParser as public, and PHP forbids narrowing
visibility in a subclass. Bake 3.6.4 (the same release that introduced its
own rector setup) switched the parent to protected; align our lowest bound
accordingly so both ends of the matrix are consistent.
@dereuromark dereuromark requested a review from ADmad May 12, 2026 14:55
…ng visibility

The earlier two commits on this branch narrowed PolicyCommand::buildOptionParser
to protected and bumped cakephp/bake's minimum to ^3.6.4 to keep CI green.
That introduces a real (if narrow) BC break: users on cakephp/bake 3.2 - 3.6.3
who upgrade authorization to this revision hit a PHP fatal when loading
PolicyCommand, because PHP forbids narrowing a parent's visibility.

Revert both code changes and instead add a per-file skip to rector.php for
MakeInheritedMethodVisibilitySameAsParentRector on src/Command/PolicyCommand.php.
The override stays public, which is forward- and backward-compatible against
any bake version - widening is always allowed.

Net diff vs 3.x is a single addition to rector.php.
@dereuromark dereuromark changed the title Match PolicyCommand::buildOptionParser visibility to parent (fix cs-stan) Skip rector visibility rule on PolicyCommand (fix cs-stan) May 12, 2026
@dereuromark dereuromark merged commit 21614bb into 3.x May 12, 2026
6 checks passed
@dereuromark dereuromark deleted the fix-policy-command-visibility branch May 12, 2026 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant