RFC – 6.0 – Routing: reduce string-magic by instead using class/method references · Issue #19509 · cakephp/cakephp · GitHub
Skip to content

RFC – 6.0 – Routing: reduce string-magic by instead using class/method references #19509

Description

@web-dev-passion

Routing: reduce string-magic by instead using class/method references - open for discussion and opinions

Introduction & Background thoughts

Generating URLs / Routes in CakePHP relies since early project times on string magic or string-typed arrays as you probably all know:

One example:

$this->redirect(['prefix' => 'Admin', 'controller' => 'Users', 'action' => 'profile']);

This would relate by CakePHP naming conventions to src/Controller/Admin/UsersController.php and there searches for a function profile()

Everything else in a modern app is statically analyzable, but these aren't. Typos Usres instead of Users or misusages of singular / plural forms, only blow up at runtime, renaming / refactoring a controller (or class name in general) or action (function inside a controller class) silently breaks every call site, "find usages" + PhpStorm's great refactoring + usage tools show nothing (you could tweak around with the 'string occurences'). This makes maintainability and refactoring a hassle.

Also what personally breaks my workflow is i am not able to use CMD+CLICK (CTRL+CLICK Linux/Win) for easy "go-to" using PhpStorm (and probably all other IDEs as well).

Another great issue I encountered throughout my various CakePHP projects is that omitting plugin/prefix quietly inherits the current request's context causing unexpected issues. I will explain this later throughout this RFC.

How we workaround this in personal projects (not the cleanest way)

In our personal apps we've worked around this with a convention: never write the controller as a string literal, always pass the FQCN through a small helper ControllerNameExtractor.php always spell out plugin/prefix (to avoid possible unwanted inheritance by current request), and add a @see PHPDoc to each route array specifiying the method so the action/function is at least click-navigable (IDE go-to CMD+CLICK feature & usage find). This @see seems like a hack for me as well, but it is based on the fact we can not write in PHP 8.5 things like UsersController::class::profile() but would need to use reflection API or other ways. With that approach I have:

  1. made the controller class:- usage-findable
  • hence easily refactorable
  • get IDE autocompletion support (I know about the great cakephp-ide-helper)
  1. the method (action) name:
  • via @see PHPDoc usage-findable
  • a bit better refactorable
  1. in cost of adding more specific route arrays, gained the benefit of clearly & explicitly stating which prefix or plugin context I am referring to.

A few real examples of the workaround from our codebase (it's in active use a lot of files — controllers, view templates, traits, even bake templates 😁):

The normal case — a redirect to another controller, importing the target class:

use App\Controller\Admin\UsersController;

$this->redirect([
    'plugin' => null,
    'prefix' => 'Admin',
    'controller' => ControllerNameExtractor::extract(UsersController::class),
    /** @see UsersController::index() */
    'action' => 'index',
]);

Redirect destination in a controller (note self::class for same-controller links, which avoids the import):

$destination = [
    'plugin' => null,
    'prefix' => 'Participant',
    'controller' => ControllerNameExtractor::extract(self::class),
    /** @see ProfileController::index() */
    'action' => 'index',
];

An Html->link() with a query param:

$this->Html->link(__d('our-project-domain', 'Categories'), [
    'plugin' => null,
    'prefix' => 'Admin',
    'controller' => ControllerNameExtractor::extract(CategoriesController::class),
    /** @see CategoriesController::index() */
    'action' => 'index',
    '?' => ['type' => 'news'],
], ['class' => 'btn btn-light', 'escape' => false]);

And the same thing inline with some view helper:

echo $this->Html->link('<i class="ti ti-pencil"></i>' . __d('our-project-domain', 'Edit'), ['plugin' => null, 'prefix' => 'Admin', 'controller' => ControllerNameExtractor::extract(NewsController::class), /** @see NewsController::edit() */ 'action' => 'edit', $newsEntity->id], ['class' => 'dropdown-item']);

In our code bases a custom PHPStan rule enforces it. It works, but it's ~6 lines of ceremony per link and the action is still just a string the @see describes rather than checks. We'd much rather this be solved in core.

Glimpse into the custom ControllerNameExtractor & PHPStan rule:

  • src/Utility/ControllerNameExtractor.php
<?php

declare(strict_types=1);

namespace App\Utility;

/**
 * Controller Name Extractor
 *
 * Utility class for extracting controller base names from FQDN class strings.
 */
final readonly class ControllerNameExtractor
{
    /**
     * Extracts the base name of a controller class by removing the "Controller" suffix.
     *
     * @param class-string $controllerClass The FQDN of the controller (e.g., `"App\Controller\UsersController"`)
     *
     * @return string The base name of the controller without the `"Controller"` suffix (e.g., `"Users"`)
     */
    public static function extract(string $controllerClass): string
    {
        // Get short class name from FQDN via string manipulation (no autoloading)
        $pos = strrpos($controllerClass, '\\');
        $shortName = $pos !== false ? substr($controllerClass, $pos + 1) : $controllerClass;

        // Remove "Controller" suffix if present
        if (str_ends_with($shortName, 'Controller')) {
            return substr($shortName, 0, -10);
        }

        return $shortName;
    }
}
  • src/PHPStan/RouteArrayConventionRule.php
<?php

declare(strict_types=1);

namespace App\PHPStan;

use App\Utility\ControllerNameExtractor;
use PhpParser\Node;
use PhpParser\Node\ArrayItem;
use PhpParser\Node\Expr\Array_;
use PhpParser\Node\Expr\StaticCall;
use PhpParser\Node\Identifier;
use PhpParser\Node\Name;
use PhpParser\Node\Scalar\String_;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\ShouldNotHappenException;

use function in_array;
use function sprintf;
use function str_contains;
use function ucfirst;

/**
 * @implements Rule<Array_>
 */
class RouteArrayConventionRule implements Rule
{
    private const array REQUIRED_KEYS = ['plugin', 'prefix', 'action'];

    public function getNodeType(): string
    {
        return Array_::class;
    }

    /** @return list<\PHPStan\Rules\IdentifierRuleError> */
    public function processNode(Node $node, Scope $scope): array
    {
        if (!$node instanceof Array_) {
            return [];
        }

        $itemsByKey = $this->indexByStringKey($node->items);
        if (!isset($itemsByKey['controller'])) {
            return [];
        }

        $errors = [];

        foreach (self::REQUIRED_KEYS as $required) {
            if (!isset($itemsByKey[$required])) {
                $errors[] = $this->error(
                    sprintf(
                        'CakePHP route array is missing the explicit "%s" key. '
                        . 'Set plugin / prefix / controller / action all explicitly so '
                        . 'CakePHP does not inherit a slot from the current request context.',
                        $required,
                    ),
                    'missing' . ucfirst($required),
                );
            }
        }

        $controllerValue = $itemsByKey['controller']->value;
        if (!$this->isControllerNameExtractorCall($controllerValue)) {
            $errors[] = $this->error(
                'CakePHP route array "controller" must use '
                . 'ControllerNameExtractor::extract(SomeController::class). '
                . 'String literals break IDE rename / find-usages and let '
                . 'typos hide until runtime.',
                'controllerNotExtractor',
            );
        }

        if (isset($itemsByKey['action']) && !$this->hasSeeAnnotation($itemsByKey['action'])) {
            $errors[] = $this->error(
                'CakePHP route array "action" key must be preceded by a '
                . '/** @see ControllerClass::method() */ docblock so the IDE '
                . 'can navigate from the action string back to the source. '
                . 'Place the docblock on the line directly above the '
                . '`\'action\' => \'\',` entry.',
                'actionMissingSeeDoc',
            );
        }

        return $errors;
    }

    /**
     * @param array<int, ArrayItem|null> $items
     *
     * @return array<string, ArrayItem>
     */
    private function indexByStringKey(array $items): array
    {
        $result = [];
        foreach ($items as $item) {
            if ($item === null) {
                continue;
            }
            if (!$item->key instanceof String_) {
                continue;
            }
            $result[$item->key->value] = $item;
        }

        return $result;
    }

    private function isControllerNameExtractorCall(Node $value): bool
    {
        if (!$value instanceof StaticCall) {
            return false;
        }
        if (!$value->class instanceof Name) {
            return false;
        }
        if (!$value->name instanceof Identifier) {
            return false;
        }

        $className = $value->class->toString();
        $allowed = [
            'ControllerNameExtractor',
            'App\Utility\ControllerNameExtractor',
            ControllerNameExtractor::class,
        ];

        return in_array($className, $allowed, true) && $value->name->name === 'extract';
    }

    private function hasSeeAnnotation(ArrayItem $item): bool
    {
        foreach ($item->getComments() as $comment) {
            if (str_contains($comment->getText(), '@see')) {
                return true;
            }
        }

        return false;
    }

    /**
     * Native return stays the broader `RuleError` so PHPStorm — which can't
     * follow `RuleErrorBuilder`'s fluent state to see that `build()` narrows
     * once `identifier()` is set — doesn't warn. The `return` narrows to
     * `IdentifierRuleError` for PHPStan, as `Rule::processNode()`'s
     * `list<IdentifierRuleError>` contract demands at level 8.
     *
     * @return \PHPStan\Rules\IdentifierRuleError
     *
     * @noinspection PhpUnhandledExceptionInspection (build() declares ShouldNotHappenException; safe here)
     *
     * @throws ShouldNotHappenException
     */
    private function error(string $message, string $identifierSuffix): RuleError
    {
        return RuleErrorBuilder::message($message)
            ->identifier('ourProjectName.routeArray.' . $identifierSuffix)
            ->build();
    }
}

The Prefix / Plugin inheritance trap

I promised in the intro I'd come back to this one. If you build a URL array and leave out prefix or plugin, CakePHP doesn't read the missing key as "none" — it fills it in from the current request (Router::url() does exactly this when there's no _name). So the same array can resolve differently depending on which page renders it.

Example: you're on /admin/articles (the request carries prefix => 'Admin') and you link to the public profile:

$this->Html->link('Profile', ['controller' => 'Users', 'action' => 'view', $id]);

You meant src/Controller/UsersController.php, but because you didn't pass prefix, it inherits Admin and resolves to src/Controller/Admin/UsersController.php. If that controller happens to exist, the link silently points at the wrong place and you only notice when someone clicks it. The same code in a front-end template would have worked. Plugins behave identically: omit plugin inside a plugin controller and a link you meant for the main app stays inside the plugin.

So the only safe habit today is spelling out plugin and prefix on every array, even as null — easy to forget, and nothing warns you.

A controller's namespace already encodes all of this (App\Controller\Admin\UsersController vs App\Controller\UsersController vs SomePlugin\Controller\UsersController). Reference the class instead of a bare string and the framework could derive plugin/prefix itself, with nothing left to inherit.

Why I do not provide a concrete code change proposal at point of writing this RFC

I deliberately stopped short of putting up a PR and that's on purpose. This touches something that's been part of CakePHP since basically forever (like I shared with you in the intro sentence and below in the Target CakePHP version paragraph) and I'd rather not show up with a near-done implementation that makes everyone feel like they're voting y/n on my design instead of shaping the direction together.

So before any code, I'd genuinely like to hear where you core devs stand on a few things:

  1. Is there desire to evolve reverse routing / URL generation at all, or is the string-array API considered "fine, we did things like that all the time and everywhere across CakePHP core so leave it alone"? I mean we have string magic all over CakePHP, in Cake ORM and many more (even deeper in the several components / repositories all deeply connected with CakePHP string-magic + conventions). I think part of that is why the cakephp-ide-helper is mostly even needed in first-hand).

  2. If we do this, what's the BC stance you'd be comfortable with? A graceful deprecation policy over a major or two, or keeping both ways side by side indefinitely? (I've written down below I'd personally prefer not to carry two forever)

  3. Is a class + method reference the direction that resonates, or would you rather lean into named routes (with maybe generated constants to make the names refactor-safe) as the "modern" path? I have opinions, but I'd like to hear yours first.

Whatever the answers, I'm happy to do the actual work. Be it value objects, Router::url() (or possible view helpers like Link or Html) integration, tests, benchmarks, a PHPStan extension, once we've agreed on the shape. I just didn't want to anchor the conversation to an implementation before we've had the conversation..


Target CakePHP Version: It probably makes sense to add this as a non breaking, build on-top feature for 5.next, and mainly proposed for 6.0. The to-be-discussed deprecation of "classic" CakePHP Routing is a probably very emotional topic for some of you core maintainers, as it is that way for ages in CakePHP now. Without digressing too much, recent discussions in the PHP community (just read the current threads in PHP subreddit) criticize that not moving on with features just for sake of backwards compatibility can lead to developer frustration and feeling stagnant. I understand this is a bold and opiniated statement by me, hence I am open to discuss why I think we should bring CakePHP Routing to the next milestone. (we could be graceful with our deprecation policy, or even agree on keeping two ways simultaneously - although that is not what I would wish for CakePHP to develop into).

CakePHP Version

6.0

Metadata

Metadata

Assignees

No one assigned
    No fields configured for Enhancement.

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions