PHP 8 named arguments support

[fadrian06]

PHP named arguments helps in some situations for cleaned code, it locks library maintainers to not change the argument names so easily without breaking changes, but php 8 must have support too...

public function test_static_route(): void {
  Flight::request()->url = '/test';

  $route = Flight::route(
    pass_route: true,
    alias: 'testRoute',
    callback: function () {
      echo 'test';
    },
    pattern: '/test'
  );

  self::assertInstanceOf(Route::class, $route);
  self::expectOutputString('test');
  Flight::start();
}

[n0nag0n]

So I see what you're doing here and I think it's great for the future version of Flight, but it breaks the 7.4 compatibility even for just unit testing. Because most of my machines are configured for 7.4 (because of the LTS support), I couldn't actually pass unit tests anymore if this change was merged in. I'd have to always manually skip that class, which is kind of defeating the point of having that unit test.

The other thing with the Flight::path() change. I know you had mentioned in the past to get rid of the comments and actually build static methods for each of them, and I guess we can do that, but all the methods would just be return self::$app->whatever() or like you put with static::__callStatic('whatever') and seem like a lot of duplicated code, to just add some comments.

[fadrian06]

Ok to start the tests for php 8 are excluded from the main suite, I run the tests of that specific class by hand

[fadrian06]

Calling the app directly instead of __callStatic does make sense because it would help the static analyzer

[fadrian06]

[fadrian06]

I have several ideas to solve it but I don't know how difficult it is to implement it and keep everything working as is.

• Extract shared functionalities into traits (it violates SOLID everywhere, although the Flight class does it but in an elegant way delegating everything to the Dispatcher, the Loader and the Engine)

• It could be by extracting everything into a single trait that is automatically generated with an automated script, so with each docblock that is added, the static method will be created

[fadrian06]

I feel that the PR has covered something else related to the container, but I felt that I should fix it before dealing with other things.

[fadrian06]

Changes about the container

Before

If you install a psr11 compatible container, and pass it to Flight::registerContainerHandler($container), works only if a class you have previously registered in the container.

For example previously

$container = new Container;
$container->set(DatetimeImmutable::class, fn() => new DatetimeImmutable);

Flight::registerContainerHandler($container);

// later in some controller
function __construct(DatetimeImmutable $dt) {}

Works. Only because you previously registered that class

Modern containers like illuminate/container or flightphp/container resolves that classes automagically because it has optional parameters, others classes doesn't have constructor parameters, container resolves them too...

Note

But previously fails, the solution to use that automagically class resolution was Flight::registerContainerHandler([$container, 'get']); or Flight::registerContainerHandler($container->get(...)); in php 8

But now:

After (using flightphp/container or another psr11 container with autowiring)

$container = new Container;
-$container->set(DatetimeImmutable::class, fn() => new DatetimeImmutable);

-Flight::registerContainerHandler([$container, 'get']);
+Flight::registerContainerHandler($container);

// later in some controller
function __construct(DatetimeImmutable $dt) {}

And works too, because, flightphp/container resolves a DatetimeImmutable object because, all its constructor parameters are optional.