Framework: make:event & make:listener allows file overwrite

Created on 7 Oct 2017  路  10Comments  路  Source: laravel/framework

  • Laravel Version: 5.5.14
  • PHP Version: 7.2
  • Database Driver & Version:

Description:

make:event and make:listener allows file overwrite which kind of dangerous to do especially on listeners where some logic can be added. Base on the code, it checks existence based on class_exists instead of checking if the file exists (as the default behavior of the Illuminate\Console\GeneratorCommand).
https://github.com/laravel/framework/blob/5.5/src/Illuminate/Foundation/Console/EventMakeCommand.php#L36

Is there a reason why it was designed that way?

Steps To Reproduce:

Run php artisan make:event SampleEvent twice (or more). No error/warning is shown

Most helpful comment

@edsamonte feel free to open a PR, your concern is valid imo.

All 10 comments

Use git, svn, mercurial, or whatever VCS(SCM) :).

@Kyslik whether you're using a VCS or not, this feature should run a confirmation check if the file exists.

I actually see that with other commands it errors out -- ($ art make:model Abc), which is interesting since whoever implemented event make command did not notice.

Nice catch :)


Despite all above, I think on dev env it should overwrite, and git would solve everything else (same like migrations have different behaviour dependent on env).

Why would you run make:model or anything of make on an env other than dev?

@Dylan-DPC Well, exactly NEVER.

I don't see what git has to do with this issue.

My first comment on was meant like this: I wouldn't worry about it, disaster is prevented by using git.

I think that issue here is that, whether you're on dev env or not, any file overwrite should at least warn you and not just plainly succeed. Besides, the default behavior of the GeneratorCommand (the parent class of ListenerMakeCommand and EventMakeCommand) is to check if the file already exists and abort if that is the case. And the use of class_exists prevents the alreadyExists method to function correctly.

@edsamonte feel free to open a PR, your concern is valid imo.

Made a PR here. It seems that make:exception also overrides the alreadyExists method. It works the same way as the parent one so I deleted it still. GeneratorCommand::alreadyExists looks much safer.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

kerbylav picture kerbylav  路  3Comments

PhiloNL picture PhiloNL  路  3Comments

SachinAgarwal1337 picture SachinAgarwal1337  路  3Comments

lzp819739483 picture lzp819739483  路  3Comments

progmars picture progmars  路  3Comments