New syntax by rustamwin · Pull Request #225 · yiisoft/router

rustamwin · 2023-11-09T07:22:02Z

Q	A
Is bugfix?	❌
New feature?	✔️
Breaks BC?	✔️
Fixed issues	#193, #171

Current syntax

- use Yiisoft\Router\Route;
- use Yiisoft\Router\Group;
+ use Yiisoft\Router\Builder\RouteBuilder as Route;
+ use Yiisoft\Router\Builder\GroupBuilder as Group;


Group::create()->routes(
    Route::get('')->name('index'),
-   Route::get('/posts')->name('posts'),
+   Route::get('/posts')->name('posts'), // No getters, only setters
);

codecov · 2023-11-09T07:22:14Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (1e9bfd5) to head (51fca55).

Additional details and impacted files

@@             Coverage Diff              @@
##              master      #225    +/-   ##
============================================
  Coverage     100.00%   100.00%            
- Complexity       129       192    +63     
============================================
  Files             13        15     +2     
  Lines            400       528   +128     
============================================
+ Hits             400       528   +128

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

what-the-diff · 2023-11-09T07:23:46Z

PR Summary

This pull request introduces numerous updates aimed at better organizing and streamlining how routes are handled in the system:

New Classes
- Two new classes, GroupBuilder and RouteBuilder, have been added. These classes represent a group of routes and a single route respectively, and they include methods for defining diverse route characteristics, such as HTTP methods, route pattern, middlewares, hosts, and more.
Method and Property Modifications
- Several existing methods (getName(), getHost(), getPattern(), getMethods()) in the CurrentRoute.php file and Debug/RouterCollector.php were adjusted to use new direct methods instead of a generic getData() method.
- In the Group.php file, some properties were removed and replaced with new ones, and a variety of new methods designed for handling these properties were added.
“Route”
- The Route.php file underwent a major update where new properties were added, methods were adjusted to utilize these new properties, and unnecessary methods were removed. The changes aim to streamline the usage of the class and make it more efficient.
Route Collection Update
- The RouteCollection.php was refactored to better use new methods in the Route class and to remove outdated code.
Route Collector Update
- The RouteCollector.php underwent several property and method name changes and had type assertions and argument validations added to improve code robustness and reliability.
Inclusion of Test Cases
- A new test file for RouteBuilder class was added and several existing test files were updated to comply with the changes made to the core classes.
Miscellaneous
- Adjustments were made in RouteCollectionTest.php and RouteCollectorTest.php to reflect changes made in the main classes.

These changes all contribute to a more robust and efficient route handling system within the software, aligning the codebase with modern best practices in PHP coding.

vjik

#191 suggest use named arguments, and this must improve performance. In this PR keep routes/groups object and additional add new routes/groups objects with named arguments. Seems, this decrease performance...
This not fix #171.

rustamwin · 2023-11-12T18:07:18Z

In this PR keep routes/groups object and additional add new routes/groups objects with named arguments. Seems, this decrease performance...

We can mention it in docs or drop the current syntax.

This not fix #171.

Yes, doesn't fix directly. Since there is no exception, there is no need for it.

vjik · 2023-11-22T17:23:02Z

We can mention it in docs or drop the current syntax.

But what goal of syntax change?

# Conflicts: # src/Debug/RouterCollector.php # src/Route.php # tests/RouteTest.php

vjik

Can we make Route and Group constructors public and keep BC?

I think, GroupBuilder and RouteBuilder are not needed.

rustamwin · 2025-11-07T06:59:12Z

Can we make Route and Group constructors public

Did you mean builders? Route and Group constructors are public in this PR.

I think, GroupBuilder and RouteBuilder are not needed.

Will we abandon the current syntax?

vjik · 2025-11-07T08:14:02Z

Can we make Route and Group constructors public

Did you mean builders? Route and Group constructors are public in this PR.

No, I mean Yiisoft\Router\Router and Yiisoft\Router\Group

I think, GroupBuilder and RouteBuilder are not needed.

Will we abandon the current syntax?

Mark as deprecated, then remove in major version.

rustamwin · 2025-11-13T06:02:20Z

Mark as deprecated, then remove in major version.

Let's abandon right away since the PR has major changes.

…ray_is_list, and apply readonly properties

vjik · 2025-11-13T07:59:59Z

Let's abandon right away since the PR has major changes.

What major changes are needed?

# Conflicts: # composer.json # src/Debug/DebugRoutesCommand.php # src/Route.php

# Conflicts: # src/Group.php # src/Route.php # src/RouteCollection.php # src/RouteCollector.php # tests/ConfigTest.php # tests/Debug/DebugRoutesCommandTest.php # tests/Debug/RouterCollectorTest.php # tests/Debug/UrlMatcherInterfaceProxyTest.php # tests/GroupTest.php # tests/RouteTest.php

…ove method consistency

Copilot

Pull request overview

This PR introduces a new routing syntax by adding immutable builder classes (RouteBuilder, GroupBuilder) while refactoring core Route/Group to mutable objects with constructors + getters/setters. It also updates routing collection/dispatch/debug code and rewrites tests to match the new APIs (breaking BC as noted).

Changes:

Add RoutableInterface plus new Builder\RouteBuilder and Builder\GroupBuilder that convert to core Route/Group via toRoute().
Refactor Route, Group, RouteCollector, and RouteCollection APIs (getters/setters; middleware collection rename; host→hosts changes).
Update tests and debug tooling to use builders/getters, and bump dev dependencies.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/RouteTest.php	Reworked tests for new `Route` constructor + setters/getters and validation.
tests/RouteCollectorTest.php	Updated to builder aliases and `getMiddlewares()` API.
tests/RouteCollectionTest.php	Updated for new route/group APIs and enabled middleware retrieval via getters.
tests/Middleware/RouterTest.php	Adjusted matcher to return built routes via `toRoute()`.
tests/MatchingResultTest.php	Updated to instantiate `Route` directly with methods/pattern.
tests/HydratorAttribute/RouteArgumentTest.php	Updated to convert builder route to core route with `toRoute()`.
tests/GroupTest.php	Reworked tests for new `Group` constructor + setters/getters and validation.
tests/Debug/UrlMatcherInterfaceProxyTest.php	Updated to instantiate `Route` with methods/pattern.
tests/Debug/RouterCollectorTest.php	Updated expectations for collected keys (`hosts`) and new route creation patterns.
tests/Debug/DebugRoutesCommandTest.php	Updated to use `RouteBuilder` in fixtures.
tests/CurrentRouteTest.php	Updated `CurrentRoute` API host→hosts and new `Route` constructor usage.
tests/ConfigTest.php	Updated config test to use `RouteBuilder` and `toRoute()`.
tests/Builder/RouteBuilderTest.php	New test coverage for builder-based fluent syntax and immutability expectations.
tests/Builder/GroupBuilderTest.php	New test coverage for group builder syntax, CORS behavior, and immutability expectations.
src/RouteCollectorInterface.php	Accept `RoutableInterface` and rename middleware getter to `getMiddlewares()`.
src/RouteCollector.php	Implement `RoutableInterface` support and rename internal middleware storage/getter.
src/RouteCollection.php	Convert `RoutableInterface` via `toRoute()`, switch to setter-based middleware/host/pattern/name updates.
src/Route.php	Make `Route` mutable with constructor + getters/setters; action separated from middleware list; add validations/caching.
src/RoutableInterface.php	New interface to unify builder→route/group conversion.
src/Middleware/Router.php	Dispatch using `Route::getEnabledMiddlewares()` instead of `getData()`.
src/Group.php	Make `Group` mutable with constructor + getters/setters; add validation and enabled middleware caching.
src/Debug/RouterCollector.php	Switch debug output to new getters; host→hosts key changes; middleware/action extraction updated.
src/Debug/DebugRoutesCommand.php	Replace custom array list detection with `array_is_list()`.
src/CurrentRoute.php	Update to use route getters and host→hosts API.
src/Builder/RouteBuilder.php	New immutable fluent builder for routes, converting to core `Route`.
src/Builder/GroupBuilder.php	New immutable fluent builder for groups, converting to core `Group`.
composer.json	Bump dev dependencies (PHPUnit, Rector, Psalm, yiisoft packages).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…on, and apply `readonly` for constructor injection

…dlewares and adjust defaults assignment logic

Copilot

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-20T06:42:18Z

+            /** @var Route $item */
+            $item->setPattern($prefix . $item->getPattern());

-            if (!str_contains($modifiedItem->getData('name'), implode(', ', $modifiedItem->getData('methods')))) {
-                $modifiedItem = $modifiedItem->name($namePrefix . $modifiedItem->getData('name'));
+            if (!str_contains($item->getName(), implode(', ', $item->getMethods()))) {
+                $item->setName($namePrefix . $item->getName());
            }


injectGroup() mutates the original Route instance in-place (pattern/name/hosts/middlewares). If the same Route object is reused across multiple groups or added both inside and outside a group, later injections will see already-prefixed state (double prefix/name, duplicated middleware layering), producing incorrect route definitions. Consider cloning the Route/Group before applying group/collector transformations (prefix/namePrefix/hosts/middlewares/CORS), or otherwise ensure each injection works on a distinct instance.

Copilot · 2026-04-20T06:42:18Z


-        $middlewares = $route->getData('enabledMiddlewares');
+        $middlewares = $route->getEnabledMiddlewares();
        $action = array_pop($middlewares);


getMiddlewaresAndAction() always treats the last enabled middleware as the action via array_pop(). Routes can legitimately have middlewares but no action (e.g. a route defined without action() but inheriting group middlewares), so this will misreport the last middleware as the action. Use $route->getAction() directly (and $route->getMiddlewares() / $route->getEnabledMiddlewares() accordingly), or only pop when $route->getAction() !== null.

Suggested change

$action = array_pop($middlewares);

$action = $route->getAction();

Copilot · 2026-04-20T06:42:18Z

+        foreach ($defaults as $key => $value) {
+            if (!is_scalar($value) && !($value instanceof Stringable) && null !== $value) {
+                throw new InvalidArgumentException(
+                    'Invalid $defaults provided, indexed array of scalar or `Stringable` or null expected.',


The exception message in setDefaults() says "indexed array", but the implementation allows associative keys (and defaults are described as being indexed by parameter names). Consider rewording the message to avoid implying numeric indexing (e.g. "array of scalar/Stringable/null values expected").

Suggested change

'Invalid $defaults provided, indexed array of scalar or `Stringable` or null expected.',

'Invalid $defaults provided, array of scalar, Stringable, or null values expected.',

Copilot · 2026-04-20T06:42:19Z

+     * Returns the current route hosts.
     *
-     * @return string|null The current route host.
+     * @return array|null The current route hosts.
     */
-    public function getHost(): ?string
+    public function getHosts(): ?array
    {
-        return $this->route?->getData('host');
+        return $this->route?->getHosts();


CurrentRoute::getHosts() returns ?array, but the underlying value is a list of host strings. Tightening the return type/phpdoc to ?string[] (or ?array<int,string>) would improve static analysis and make the BC break clearer for consumers.

rustamwin and others added 4 commits November 6, 2023 09:27

Allow to create route instance directly

02971c6

Adjust Group

7848f14

Simplify route classes & add route builders

39f05ff

Apply fixes from StyleCI

626151c

rustamwin added the status:code review The pull request needs review. label Nov 9, 2023

rustamwin requested a review from a team November 9, 2023 07:22

rustamwin and others added 6 commits November 9, 2023 16:26

Minor improvements

3390ea9

Merge remote-tracking branch 'origin/new-syntax' into new-syntax

0b0806d

Improvements

a6b7acd

Merge branch 'master' into new-syntax

bdaec2c

Apply fixes from StyleCI

33f3003

Fix

782bf91

vjik requested changes Nov 12, 2023

View reviewed changes

rustamwin added 3 commits October 21, 2025 21:05

Merge remote-tracking branch 'origin/master' into new-syntax

4530d76

# Conflicts: # src/Debug/RouterCollector.php # src/Route.php # tests/RouteTest.php

Fix tests

9c2fa2a

Fix cs

ef3eb15

vjik requested changes Nov 2, 2025

View reviewed changes

vjik mentioned this pull request Nov 2, 2025

Add attributes support #196

Open

rustamwin and others added 3 commits November 13, 2025 11:14

Update dependencies, replace redundant isArrayList polyfill with ar…

a9af466

…ray_is_list, and apply readonly properties

Apply Rector changes (CI)

2bc1672

Update dev dependencies in composer.json

f025e9d

Merge remote-tracking branch 'origin/master' into new-syntax

98c0890

# Conflicts: # composer.json # src/Debug/DebugRoutesCommand.php # src/Route.php

rustamwin and others added 3 commits April 15, 2026 22:11

Refactor tests and route logic: align types, fix assertions, and impr…

f26efb4

…ove method consistency

Apply PHP CS Fixer and Rector changes (CI)

c9bacd1

samdark requested a review from Copilot April 16, 2026 11:23

Copilot started reviewing on behalf of samdark April 16, 2026 11:24 View session

Copilot AI reviewed Apr 16, 2026

View reviewed changes

Comment thread src/Route.php

Comment thread src/Group.php

Comment thread src/Debug/RouterCollector.php Outdated

Comment thread src/Route.php Outdated

Comment thread src/Route.php

Comment thread src/Route.php

rustamwin added 2 commits April 19, 2026 02:04

Refactor: optimize middleware handling, improve property initializati…

2e737fa

…on, and apply `readonly` for constructor injection

Refactor middleware handling: ensure action is included in enabledMid…

51fca55

…dlewares and adjust defaults assignment logic

samdark requested a review from Copilot April 20, 2026 06:36

Copilot started reviewing on behalf of samdark April 20, 2026 06:37 View session

Copilot AI reviewed Apr 20, 2026

View reviewed changes

	$action = array_pop($middlewares);
	$action = $route->getAction();

	'Invalid $defaults provided, indexed array of scalar or `Stringable` or null expected.',
	'Invalid $defaults provided, array of scalar, Stringable, or null values expected.',

Uh oh!

Conversation

rustamwin commented Nov 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Nov 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

what-the-diff Bot commented Nov 9, 2023

PR Summary

Uh oh!

vjik left a comment

Choose a reason for hiding this comment

Uh oh!

rustamwin commented Nov 12, 2023

Uh oh!

vjik commented Nov 22, 2023

Uh oh!

vjik left a comment

Choose a reason for hiding this comment

Uh oh!

rustamwin commented Nov 7, 2025

Uh oh!

vjik commented Nov 7, 2025

Uh oh!

rustamwin commented Nov 13, 2025

Uh oh!

vjik commented Nov 13, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rustamwin commented Nov 9, 2023 •

edited

Loading

codecov Bot commented Nov 9, 2023 •

edited

Loading