[Rule idea]: analyze usages of UrlGeneratorInterface to report missing mandatory route parameters

Open
#428 2 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
35/100
Issue type
Feature
Clarity
Mostly clear
Activity status
Stale
Tech stack
php, symfony
Domain
backend, devtools

Research direction

Start with UrlGeneratorInterface::generate and the compiled var/cache/dev/url_generating_routes.php format described in the issue. Investigate how route variables, defaults, RequestContext values, and _locale determine required parameters, then identify the existing analysis services and functional-test conventions. Done means validating known literal routes without over-refining array inputs, preserving query parameters, and exposing reusable logic for the listed Symfony and Twig entry points.

Written by the indexing model from the issue text.

Description

When the route name is a string literal, it should be possible to analyze calls to UrlGeneratorInterface::generate to report missing parameters in the array shape of the second argument.

Routes available for url generation can be read from the compiled file in var/cache/dev/url_generating_routes.php. The format of this file is semi-internal (officially, the guarantee we make is that the constructor of CompiledUrlGenerator takes an array in the format dumped by CompiledUrlGeneratorDumper, but the format does not change often so it should be safe to use it with a functional test ensuring the format does not break for new Symfony versions).

This url_generating_routes.php is a PHP file returning an array with this format (top-level keys are route names):

array<string, array{
  0: string[], // names of route variables.
  1: array<string, mixed>, // route defaults
  2: array<string, string>, // variable requirements
  3: list<array{'text', string}|array{
        0: 'variable',
        1: string, // static prefix
        2: string, // regex pattern for the requirement
        3: string, // variable name
        4?: bool, // whether it needs unicode matching
        5?: bool, // whether the token is marked as important
      }>, // tokens for url generation for the path
  4. list, // tokens for url generation for the host (same type than for the path tokens)
  5. string[], // allowed schemes (empty when no scheme restriction)
  6. list<string>, // deprecation messages
}>

the parameters passed in the second argument of UrlGeneratorInterface::generate will be handled this way (I'm talking about the keys in the array here):

  1. _fragment will be used to generate the fragment part of the URL
  2. if the name is one of the route variables, it is used to fill that variable (this case requires values to be castable to strings, so it supports scalars and Stringable)
  3. if the name corresponds to a route default (with the default value), it is skipped
  4. otherwise, the name-value pair is added to the query string (this case supports having array values for parameters)

Variables can be optional in 3 ways:

  • when they have a default in the route definition
  • when they have a value provided by the routing RequestContext. By default in core, only _locale is managed there, based on the current locale, but projects might add extra ones (maybe this can be managed through a configuration setting for the extension)
  • _locale defaults to the default locale configured (if it does not have a default set previously) so it is always optional

I suggest that the parameters are validated this way:

  • if the route name (first argument of the method) is not a known string literal, do nothing
  • if the inferred type is not an array shape, maybe do nothing (I don't think we want to report errors for cases reading $request->attributes->get('_route_params') for instance, as that would be too annoying to force refinement in such cases)
  • build an unsealed array shape based on route variables, with a key for each route variable (see key 0 of the shape of url_generating_routes.php) and a type of scalar|Stringable (or whatever phpstan uses to describe the type supported by a (string) cast), with the key being optional if the variable is optional
  • check whether the provided parameters are assignable to that expected shape

Bonus point: the logic running those checks should be provided through a reusable service, so that it can be reused in twigstan for the analysis of the path() and url() Twig functions (which are wrappers around the UrlGeneratorInterface). It should also be reused for AbstractController::generateUrl and AbstractController::redirectToRoute.

Dominant language
PHP
Stars
800
Forks
104
Avg merge
4m
Merged PRs (30d)
1

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from phpstan/phpstan-symfony

All issues in phpstan/phpstan-symfony

Similar issues

More PHP issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.