Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Laravel Linter #168

Open
wants to merge 1 commit into
base: desarrollo_v8
Choose a base branch
from
Open

Laravel Linter #168

wants to merge 1 commit into from

Conversation

wcadena
Copy link
Owner

@wcadena wcadena commented Nov 24, 2020

This pull request includes changes and recommendations for crafting your application "The Laravel Way". Feel free to commit any additional changes to the shift-36868 branch.

Before merging, you need to:

  • Checkout the shift-36868 branch
  • Review all pull request comments for additional changes
  • Thoroughly test your application

If you need help with your upgrade, check out the Human Shifts. You may also join the Shifty Coders Slack workspace to level-up your Laravel skills.

@wcadena
Copy link
Owner Author

wcadena commented Nov 24, 2020

⚠️ Shift found uses of the old array() syntax. Laravel adopted the short array syntax [] since it became available in PHP 5.4.

2 similar comments
@wcadena
Copy link
Owner Author

wcadena commented Nov 24, 2020

⚠️ Shift found uses of the old array() syntax. Laravel adopted the short array syntax [] since it became available in PHP 5.4.

@wcadena
Copy link
Owner Author

wcadena commented Nov 24, 2020

⚠️ Shift found uses of the old array() syntax. Laravel adopted the short array syntax [] since it became available in PHP 5.4.

@wcadena
Copy link
Owner Author

wcadena commented Nov 24, 2020

⚠️ Shift found instances of string based class references. Laravel adopted the ::class static property since it became available in PHP 5.5. You should update your code to use references like App\SomeModel::class instead of strings like 'App\SomeModel'.

@wcadena
Copy link
Owner Author

wcadena commented Nov 24, 2020

ℹ️ Laravel 7 defaults the middlewarePriority internally. You only need to overwrite this property if you wish to customize the order. If you do not, you may remove this property from your Http Kernel.

@wcadena
Copy link
Owner Author

wcadena commented Nov 24, 2020

⚠️ The following files reference Laravel facades through the global namespace. For example, you're referencing \DB or importing use DB. Instead import Illuminate\Support\Facades\DB and reference DB.

While global references are allowed through aliases, you should import the facade explicitly. This can improve clarity not only for developers, but static analysis used by your IDE.

  • app/Http/Controllers/ArbolController.php
  • app/Http/Controllers/AreasController.php
  • app/Http/Controllers/Auth/RegisterController.php
  • app/Http/Controllers/BitacoraController.php
  • app/Http/Controllers/CheckListController.php
  • app/Http/Controllers/CheckList_OpcionesCheckListController.php
  • app/Http/Controllers/ConfiguracionController.php
  • app/Http/Controllers/CustodiosController.php
  • app/Http/Controllers/DenunciasController.php
  • app/Http/Controllers/EquiposController.php
  • app/Http/Controllers/EstacionesController.php
  • app/Http/Controllers/InformeMantenimientoPreventivoController.php
  • app/Http/Controllers/InformeMantenimientoPreventivoTecnicoController.php
  • app/Http/Controllers/ModeloEquipoController.php
  • app/Http/Controllers/ModuloController.php
  • app/Http/Controllers/MotivosDenunciasController.php
  • app/Http/Controllers/OpcionesCheckListController.php
  • app/Http/Controllers/OrdenDeCompraController.php
  • app/Http/Controllers/PdfController.php
  • app/Http/Controllers/PuestosController.php
  • app/Http/Controllers/RepoNovedadesController.php
  • app/Http/Controllers/ReporteController.php
  • app/Http/Controllers/RolController.php
  • app/Http/Controllers/UbicacionController.php
  • app/Http/Controllers/UserApiController.php
  • app/Http/Controllers/UsuarioLogController.php
  • app/Http/Middleware/AuthEmpleado.php

@wcadena
Copy link
Owner Author

wcadena commented Nov 24, 2020

⚠️ The following classes do not extend the standard Laravel Model and Controller class. This may add complexity which makes your application harder to upgrade. Often a trait can be used instead of inheritance. You should review the following classes:

  • app/Permission.php
  • app/Role.php
  • app/Team.php

@wcadena
Copy link
Owner Author

wcadena commented Nov 24, 2020

⚠️ Shift detected Eloquent calls in the following views. You should keep views from interacting directly with your models by passing the necessary data from the controller.

  • resources/views/directory/informes/create.blade.php
  • resources/views/directory/informes/edit.blade.php
  • resources/views/directory/puesto/create.blade.php
  • resources/views/directory/puesto/edit.blade.php
  • resources/views/directory/ubicacion/create.blade.php
  • resources/views/directory/ubicacion/edit.blade.php
  • resources/views/directory/usuario/edit.blade.php

@wcadena
Copy link
Owner Author

wcadena commented Nov 24, 2020

⚠️ The following controllers contain actions outside of the 7 resource actions (index, create, store, show, edit, update, destroy). For more details, review the docs or watch Cruddy by Design to see if you can rework these into resource controllers.

  • app/Http/Controllers/Auth/Oauth2Controller.php
  • app/Http/Controllers/AuthenticateController.php
  • app/Http/Controllers/BusquedaController.php
  • app/Http/Controllers/CheckListController.php
  • app/Http/Controllers/CheckList_OpcionesCheckListController.php
  • app/Http/Controllers/CustodiosController.php
  • app/Http/Controllers/EquiposController.php
  • app/Http/Controllers/PdfController.php
  • app/Http/Controllers/ReporteController.php
  • app/Http/Controllers/UbicacionController.php
  • app/Http/Controllers/UserApiController.php
  • app/Http/Controllers/api/CheckList_OpcionesCheckListController.php
  • app/Http/Controllers/api/CustodioController.php
  • app/Http/Controllers/api/CustodioPuestoController.php
  • app/Http/Controllers/api/EquipoCheckListController.php
  • app/Http/Controllers/api/EquiposController.php
  • app/Http/Controllers/api/PuestoController.php
  • app/Http/Controllers/api/PuestoCustodioController.php
  • app/Http/Controllers/api/RoleController.php
  • app/Http/Controllers/api/UserController.php

@wcadena
Copy link
Owner Author

wcadena commented Nov 24, 2020

⚠️ Shift found inline validation in the following controllers. Unless you require this level of control, you should use a Form Request to encapsulate this validation logic and keep your controller clean. You may automate this conversion with the Laravel Fixer.

  • app/Http/Controllers/AreasController.php
  • app/Http/Controllers/Auth/Oauth2Controller.php
  • app/Http/Controllers/BusquedaController.php
  • app/Http/Controllers/CustodiosController.php
  • app/Http/Controllers/EmpresaController.php
  • app/Http/Controllers/EquiposController.php
  • app/Http/Controllers/EstacionesController.php
  • app/Http/Controllers/InformeMantenimientoPreventivoController.php
  • app/Http/Controllers/InformeMantenimientoPreventivoTecnicoController.php
  • app/Http/Controllers/ModeloEquipoController.php
  • app/Http/Controllers/OpcionesCheckListController.php
  • app/Http/Controllers/OrdenDeCompraController.php
  • app/Http/Controllers/RepoNovedadesController.php
  • app/Http/Controllers/RolController.php
  • app/Http/Controllers/UbicacionController.php
  • app/Http/Controllers/UsuarioLogController.php
  • app/Http/Controllers/api/CheckList_OpcionesCheckListController.php
  • app/Http/Controllers/api/CustodioController.php
  • app/Http/Controllers/api/CustodioPuestoController.php
  • app/Http/Controllers/api/EquipoCheckListController.php
  • app/Http/Controllers/api/EquiposController.php
  • app/Http/Controllers/api/PuestoController.php
  • app/Http/Controllers/api/PuestoCustodioController.php
  • app/Http/Controllers/api/UserController.php

@wcadena
Copy link
Owner Author

wcadena commented Nov 24, 2020

⚠️ Shift detected the following HTTP components accessing the authenticated user through Auth::user(). Within these components, you can access the authenticated user through the request object as well. Doing so limits your dependencies by leveraging objects already available.

  • app/Http/Controllers/EquiposController.php
  • app/Http/Controllers/FileEntriesController.php
  • app/Http/Controllers/HomeController.php

@wcadena
Copy link
Owner Author

wcadena commented Nov 24, 2020

⚠️ Shift found opportunities to use the built-in Blade directives like @csrf, @json in the following views:

  • resources/views/layouts/backend.blade.php
  • resources/views/vendor/installer/environment-classic.blade.php
  • resources/views/vendor/installer/layouts/master-update.blade.php
  • resources/views/vendor/installer/layouts/master.blade.php
  • resources/views/vendor/passport/authorize.blade.php

@wcadena
Copy link
Owner Author

wcadena commented Nov 24, 2020

⚠️ Laravel contains many built-in helpers functions which allow you to write more expressive code or leverage core functionality from anywhere in your app.

Shift found the following code which could use a helper instead:

  • Response::json()
  • Response::make()
  • View::make()

@wcadena
Copy link
Owner Author

wcadena commented Nov 24, 2020

⚠️ To improve performance by caching your configuration, Laravel recommends you only use the env() helper within configuration files and use config() everywhere else. Shift found potential uses of env() in:

  • app/Http/Controllers/Auth/Oauth2Controller.php
  • app/Http/Controllers/Auth/RegisterController.php
  • app/Http/Controllers/HomeController.php
  • app/Transformers/CustodiosTransformer.php
  • app/Transformers/UbicacionTransformer.php
  • resources/lang/es/home.php
  • resources/views/auth/login.blade.php
  • resources/views/layouts/app.blade.php
  • resources/views/layouts/backend.blade.php
  • resources/views/layouts/simple.blade.php

@wcadena
Copy link
Owner Author

wcadena commented Nov 24, 2020

ℹ️ Laravel 8 reintroduced the app/Models folder by default. This is an optional change. Laravel and the artisan commands will automatically detect if you are using the app/Models folder or not.

If you wish to modernize your application to use the app/Models folder, you may run the Namespace Models Shift for free.

@wcadena
Copy link
Owner Author

wcadena commented Nov 24, 2020

ℹ️ Shift detected controller namespaces being set in your RouteServiceProvider. Laravel 8 began registering controllers using static class references instead of namespace prefixes and controller names.

You may automate this conversion using the Laravel Fixer or tasks within the Shift Workbench.

@wcadena
Copy link
Owner Author

wcadena commented Nov 24, 2020

⚠️ Shift found the following config files differ from the defaults. While you are welcome to customize your configuration, you should leverage ENV variables rather than hardcode values. If you find you're adding a lot of configuration options, consider creating a domain specific config file, such as core.php. Both will make app upgrades and deployments easier.

  • config/app.php
  • config/auth.php
  • config/database.php
  • config/filesystems.php
  • config/logging.php
  • config/services.php
  • config/session.php

@wcadena
Copy link
Owner Author

wcadena commented Nov 24, 2020

ℹ️ As noted, much of the lint detected above can be automatically fixed using the Laravel Fixer. Save yourself time and clean up your codebase quickly with this new Shift.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants