Add return statements in Commands/GeneratorCommand.php

[rcbgalido]

These return statements are useful when a subclass of the GeneratorCommand class wants to execute the handle() of its parent class but wants to know if the file creation was successful or not.

[nWidart]

Hello & thank you for the contribution.

Could you also add return type hints in this case then? Thanks

[rcbgalido]

Hi @nWidart , added return type hint as requested.

[nWidart]

Sorry I meant the pho 7 type hint : bool (and remove the comment version)

[rcbgalido]

@nWidart It seems adding a return type hint of bool in the handle() of GenerateCommand also forces its subclasses to return a bool as well on its own corresponding implementations. I don't know if it's ideal, please advise. Thank you.

[nWidart]

Hm yea that's the cost of this change.

I'm not sure if this is worth it, as this is the first time this request has been made.
What's your use-case for this?
I understand you want to check for a successful generation, but why for example?

[rcbgalido]

I was trying to make a custom command (which extends the GeneratorCommand class of laravel-modules) which is similar to Laravel's php artisan make:model -a but for modules. The -a option will create additional files related to the created model. In Laravel, the GeneratorCommand class returns a boolean indicating if the creation of the model was successful. This is beneficial since a checking could be implemented in order for the additional files not to be created if there is a failure in the creation of the model (ex. model already exists).

[nWidart]

Hm, I see that's indeed useful.
In that case, I'd say let's add the return type hint on all handle methods so that every command can be checked for its success.

[rcbgalido]

If I may suggest, let's use error codes instead where 0 indicates success and E_ERROR or 1 indicates error. This is in relation to this test case in ModelMakeCommandTest:

public function it_generates_module()
{
    $code = $this->artisan('module:make', ['name' => ['Blog']]);

    $this->assertDirectoryExists($this->modulePath);
    $this->assertSame(0, $code);
}

[nWidart]

I like that idea about return codes a lot @rcbgalido ! Indeed we will be able to test for it.

[rcbgalido]

@nWidart Great! Will try to implement it then.

[rcbgalido]

Here are the new updates:

1. Set all handle functions to return a code (0 for success, E_ERROR or 1 for error)
2. Set generate function of Generators/ModuleGenerator to return a code (0 for success, E_ERROR or 1 for error)
3. Updated test cases to include checking of code
4. Added logic where the RouteServiceProvider file is deleted before the execution of module:route-provider for some test cases (execution of module:route-provider is unnecessary if the RouteServiceProvider file already exists).
5. Updated snapshot for the it_can_change_the_default_namespace_specific test case since it contains the wrong namespace.

[nWidart]

Thanks! This is looking really good, very helpful! 💯

Interesting that #5 did not fail before :o

[rcbgalido]

No problem! For #5, I believe it was testing the already created RouteServiceProvider file during the execution of module:make command. That's why the deletion of the file is necessary.