feat(console): add make:controller and make:model commands

[gturpin-dev]

This PR replace the PR #568
I started from scratch because lot of work in the other PR is stale.
Here's a clean version of the real changes.

I've updated the overall "generator commands" as discussed in Discord and in the other PR reviews.

So this is a first step to generator commands MVP. It's a starting point for #472

If everything is ok, the #568 could be closed.

@brendt Could you please add @innocenzi as co-author of this one as a lot of work is also done by/with him 🙏

[brendt]

Looks really nice now! I left a review, but there's one thing I'm unsure about that I wanted to discuss with this PR.

We now also have the concept of "installers": https://tempestphp.com/docs/internals/package-development/

These installers aren't the same as stub generators, but there are some similarities. I wonder if we can make it so that these generators "feel" as close as possible to installers?

In particular, I'm thinking about the PublishesFiles trait, which is actually very similar to what StubFileGenerator does. It already has support for both files and classes, it will also take care of updating imports where necessary, and it will handle the override part automatically.

Here's an example of what that would look like:

final class MakeControllerCommand
{
    use PublishesFiles;
    use HasGeneratorCommand;

    #[ConsoleCommand(
        name       : 'make:controller',
        description: 'Create a new controller class with a basic route',
        aliases    : ['controller:make', 'controller:create', 'create:controller'],
    )]
    public function __invoke(
        #[ConsoleArgument(
            help: 'The name of the controller class to create ( "Controller" will be suffixed )',
        )]
        string $className,
        #[ConsoleArgument(
            help: 'The route path inside the controller',
        )]
        ?string $controllerPath = null,
        #[ConsoleArgument(
            help: 'The view name to return in the controller',
        )]
        ?string $controllerView = null,
    ): void {
        $suggestedPath = $this->getSuggestedPath(
            className  : $className,
            pathPrefix : 'Controllers',
            classSuffix: 'Controller',
        );

        $targetPath = $this->promptTargetPath($suggestedPath);

        $this->publish(
            source: ControllerStub::class,
            destination: $targetPath,
            callback: function (string $source, string $destination) use ($controllerView, $controllerPath): void 
            {
                $content = str(file_get_contents($destination));
                
                $replacements = arr([
                    'dummy-path' => $controllerPath,
                    'dummy-view' => $controllerView,
                ]);
                
                file_put_contents(
                    $destination,
                    $content->replace(
                        $replacements->keys()->toArray(),
                        $replacements->values()->toArray()
                    )
                );
            }
        );
    }
}

We could of course improve the PublishesFiles trait to have a shorthand for string replacements as well, something like:

        $this->publish(
            source: ControllerStub::class,
            destination: $targetPath,
            replace: [
                'dummy-path' => $controllerPath,
                'dummy-view' => $controllerView,
            ],
        );

I think it would be nice if the PublishesFiles trait could be reused. I realise it goes against my first suggestion of extracting StubFileGenerator, but I only realised the similarities after reading through the simplified implementation just now.

It's an iterative process, I hope you'll indulge me 😅

Anyway, I'm totally open to discuss this, I just want to make sure we go with the absolute easiest, most consistent approach 👍 And, of course, really great job on the PR already, we're getting there! 💪

[gturpin-dev]

I agree with all you said.

I have another idea, what about keep the StubFileGenerator as a separate service from the tempest/generation package.
Then the PublishesFiles trait stay at the tempest/core and could use the StubFileGenerator under the hood to be DRY ( with some refactoring as the publish method works with files that are not classes ( this could solve this review #647 (comment) )

I think of this because the PublishesFiles currently use the HasConsole but what if we want to create a stub file from an HTTP event or something else.
I think we must decouplate this ( there's currently a little bit of console in the StubFileGenerator to show success/error but it can be moved in the command itself ).

Then the StubFileGenerator could live as a Service from the tempest/generator package that could be used standalone by end-users or by a user that want to generate files from a web UI or something else.

WDYT ?

Another question, in general and for this PR, do you think the generators commands (e.g. make:controller ) should live in the tempest/console or as commands in tempest/generation ?

[brendt]

I have another idea, what about keep the StubFileGenerator as a separate service from the tempest/generation package.
Then the PublishesFiles trait stay at the tempest/core and could use the StubFileGenerator under the hood to be DRY ( with some refactoring as the publish method works with files that are not classes ( this could solve this review #647 (comment) )

The downside here is that now core has a dependency on generation. But that's not a problem: the trait can move to the generation package. We just need to make sure all components that depend on it, also require it.

Apart from that, I think it's a good idea to have the trait simple wrap an underlying service, just like we do with HasConsole.

Another question, in general and for this PR, do you think the generators commands (e.g. make:controller ) should live in the tempest/console or as commands in tempest/generation ?

The third option, actually, it should live in http 😂

So, I realise it won't be available if people install tempest/http, but don't install tempest/console. But that's actually fine: it won't break anything, and it will be available as soon as people decide they want to pull in console as well 👍

[innocenzi]

Here are a few style nitpicks, as well as a concern regarding conventions suggested by the implementation.

Also, the pull request title should be:

feat(console): add `make:controller` and `make:model` commands

[gturpin-dev]

The third option, actually, it should live in http 😂

So, I realise it won't be available if people install tempest/http, but don't install tempest/console. But that's actually fine: it won't break anything, and it will be available as soon as people decide they want to pull in console as well 👍

@brendt So, we should provide the make:controller command in tempest/http package, the make:model in tempest/database ( or another one because a model isn't tied to database until we add the IsDatabaseModel trait ? )
and so on.. So in one word, the command should live in its related package right ?

Then how I must handle the fact that ConsoleCommand will be available only if the user require the tempest/console if the make:controller is in tempest/http I will use the command from the console package, aren't we getting a fatal or something like that ?

[gturpin-dev]

@innocenzi Huge thanks for your review and requested changes 🙏

Also, the pull request title should be:

Thanks for that aswell 😄

I will try to fix everything you pointed out

[coveralls]

Pull Request Test Coverage Report for Build 11800557345

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 175 of 213 (82.16%) changed or added relevant lines in 12 files are covered.
• 19 unchanged lines in 2 files lost coverage.
• Overall coverage decreased (-0.02%) to 82.436%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/Tempest/Generation/src/ClassManipulator.php	2	3	66.67%
src/Tempest/Generation/src/DataObjects/StubFile.php	14	15	93.33%
src/Tempest/Database/src/Commands/MakeModelCommand.php	10	12	83.33%
src/Tempest/Database/src/Stubs/DatabaseModelStub.php	0	2	0.0%
src/Tempest/Http/src/Commands/MakeControllerCommand.php	14	16	87.5%
src/Tempest/Http/src/Stubs/ControllerStub.php	0	2	0.0%
src/Tempest/Reflection/src/ClassReflector.php	0	2	0.0%
src/Tempest/Core/src/PublishesFiles.php	50	62	80.65%
src/Tempest/Generation/src/StubFileGenerator.php	36	50	72.0%

Files with Coverage Reduction	New Missed Lines	%
src/Tempest/Console/src/ConsoleApplication.php	7	10.0%
src/Tempest/Http/src/HttpApplication.php	12	0.0%

Totals
Change from base Build 11788680368:	-0.02%
Covered Lines:	7425
Relevant Lines:	9007

---

💛 - Coveralls

[innocenzi]

@gturpin-dev I saw you changed the PR title, but it's still wrong (missing the colon after the commit type, missing backticks around commands and wrong scope) 😅

You can copy paste this one:

feat(console): add `make:controller` and `make:model` commands

[gturpin-dev]

@innocenzi Nevermind this is not my day 😂
Thanks 🙏

[brendt]

So in one word, the command should live in its related package right ?

Correct

Then how I must handle the fact that ConsoleCommand will be available only if the user require the tempest/console if the make:controller is in tempest/http I will use the command from the console package, aren't we getting a fatal or something like that ?

That's the beauty of discovery :) If you install one of the standalone components in a project that doesn't have tempest/console installed, then nothing will happen because there's no discovery class dedicated for console commands :) But as soon as you install tempest/console, it'll detect the package-specific commands and just work :)

[gturpin-dev]

That's the beauty of discovery :) If you install one of the standalone components in a project that doesn't have tempest/console installed, then nothing will happen because there's no discovery class dedicated for console commands :) But as soon as you install tempest/console, it'll detect the package-specific commands and just work :)

This is what I believed but I was wondering about the fact theoretically anyone can call itself and break things, but nobody have to do that, so I think it's ok 😅

[brendt]

This is what I believed but I was wondering about the fact theoretically anyone can call itself and break things, but nobody have to do that, so I think it's ok

Yeah, no reason to worry about it :)

Let me know when I need to review again?

[gturpin-dev]

@brendt Sure !
There is this review still waiting your thoughts #647 (comment)

I'm still working on things we've discussed.
I hope I'll finish them soon.

Do you think some tests on those commands ( and the StubFileGenerator ) could come in another PR ?
I think I can get some inspiration from the current Installers tests but I'll make a long time to understand them ( it's new to me )
And there is some refactor in this PR, I'd like it to not become stale again because too much time has passed.

[brendt]

Do you think some tests on those commands ( and the StubFileGenerator ) could come in another PR ? I think I can get some inspiration from the current Installers tests but I'll make a long time to understand them ( it's new to me )

Hmm no tests are pretty crucial, but I'm happy to help you with them :) Here's what we can do: you tell me which parts you think should definitely be tested, and I'll write those tests. I think it would be a good opportunity for you to learn to identify testable parts, and then you'll get an example of how those tests can be written, which you can learn from again.

[gturpin-dev]

Hmm no tests are pretty crucial, but I'm happy to help you with them :) Here's what we can do: you tell me which parts you think should definitely be tested, and I'll write those tests. I think it would be a good opportunity for you to learn to identify testable parts, and then you'll get an example of how those tests can be written, which you can learn from again.

Absolutely, would love this 🙏
I'll let you know when I'm ready

[brendt]

Let's go!!

[innocenzi]

Well done!