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 Fixer #1293

Merged
merged 15 commits into from
Mar 30, 2023
Merged

Laravel Fixer #1293

merged 15 commits into from
Mar 30, 2023

Conversation

DariusIII
Copy link
Member

This pull request includes changes and recommendations for crafting your Laravel application. These are based on the Laravel documentation and widely adopted practices within the community.

Before merging, you need to:

  • Checkout the shift-86725 branch
  • Review all comments for additional changes
  • Thoroughly test your application (no tests?, no CI?)

Don't hesitate to send your feedback to [email protected] or share on Twitter.

PHP 5.5.9 adds the new static `class` property which provides the fully qualified class name. This is preferred over using strings for class names since the `class` property references are checked by PHP.
Laravel offers many built-in Blade directives, including: `@auth`, `@guest` ,`@csrf`, `@method`, `@json`, and more.

Review the [Blade][1] documentation for more details.

[1]: https://laravel.com/docs/blade
Laravel recommends using the `Str` and `Arr` class methods directly instead of the respective helper functions. These helper functions are deprecated in Laravel 5.8 and removed in Laravel 6.
Laravel automatically injects the current Http [request object][1] to all Controller actions and Middleware. Leveraging this object improves consistency and testability.

[1]: https://laravel.com/docs/requests#accessing-the-request
Laravel [recommends][1] only using `env()` within configuration files and using `config()` everywhere else. Doing so allows you to improve performance by running `php artisan config:cache`.

[1]: https://laravel.com/docs/5.7/configuration#configuration-caching
By convention, Laravel uses the "snake case", plural name of the class as the table name.
@DariusIII
Copy link
Member Author

ℹ️ Shift noticed you have additional namespaces in your application. You may use the Consolidate Namespaces Shift to simplify your namespaces into the default Laravel App namespace.

@DariusIII
Copy link
Member Author

⚠️ 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 may rework these into resource controllers.

  • app/Http/Controllers/Admin/AdminAjaxController.php
  • app/Http/Controllers/Admin/AdminCollectionRegexesController.php
  • app/Http/Controllers/Admin/AdminGroupController.php
  • app/Http/Controllers/Admin/AdminNzbController.php
  • app/Http/Controllers/Admin/AdminReleaseNamingRegexesController.php
  • app/Http/Controllers/Admin/AdminSiteController.php
  • app/Http/Controllers/Admin/AdminUserController.php
  • app/Http/Controllers/AjaxController.php
  • app/Http/Controllers/AnimeController.php
  • app/Http/Controllers/Api/ApiController.php
  • app/Http/Controllers/Api/ApiInformController.php
  • app/Http/Controllers/Api/ApiV2Controller.php
  • app/Http/Controllers/Api/RSS.php
  • app/Http/Controllers/Api/XML_Response.php
  • app/Http/Controllers/ApiHelpController.php
  • app/Http/Controllers/BasePageController.php
  • app/Http/Controllers/BrowseController.php
  • app/Http/Controllers/BtcPaymentController.php
  • app/Http/Controllers/ContactUsController.php
  • app/Http/Controllers/FailedReleasesController.php
  • app/Http/Controllers/ForumController.php
  • app/Http/Controllers/GetNzbController.php
  • app/Http/Controllers/MovieController.php
  • app/Http/Controllers/MyShowsController.php
  • app/Http/Controllers/NfoController.php
  • app/Http/Controllers/PasswordSecurityController.php
  • app/Http/Controllers/RssController.php
  • app/Http/Controllers/SearchController.php
  • app/Http/Controllers/TermsController.php

@DariusIII
Copy link
Member Author

⚠️ Shift detected instances of controller validation still remaining. Shift may have been unable to automate their conversion due to the structure of the code.

You should review these instances for dynamic validation rules or custom exception handling . If you may inline these, feel free to request a rerun of this Shift.

  • app/Http/Controllers/Auth/LoginController.php
  • app/Http/Controllers/ContactUsController.php
  • app/Http/Controllers/ProfileController.php

@DariusIII
Copy link
Member Author

ℹ️ Laravel recommends you only use env() within configuration files and use config() everywhere else.

Shift converted env uses within your application to use config and added any custom options to the settings.php configuration file. You should review these options for any overlap with your existing configuration options.

@DariusIII
Copy link
Member Author

DariusIII commented Mar 30, 2023

❌ Shift automated the conversion to fluent routes. However, some of your route groups contained unknown options. You should review the following files for any group() calls still passing an options array and manually convert them to their fluent method.

  • routes/rss.php

@DariusIII
Copy link
Member Author

ℹ️ Now with type hints in your code, defining types within PHP DocBlocks is redundant. Laravel has removed all of the @param and @return tags from its DocBlocks where types are defined with PHP. Similarly, Shift removed these tags from any DocBlock where the code now has equivalent type hints.

@DariusIII
Copy link
Member Author

ℹ️ While the Laravel Fixer performs automation to adopt the latest Laravel conventions, Shift understands developers have different preferences. All of Shift's automation is done in nice, atomic commits. This makes it easier to undo any of the changes Shift makes using `git revert.

@DariusIII
Copy link
Member Author

❌ PHP syntax errors were detected after running your Shift. Often these are simply differences between the PHP version on the Shift server (8.1) and your project. Occasionally they are misplaced lines or duplicate import statements.

You may quickly check the PHP syntax locally by running php -l on the following files:

  • app/Http/Controllers/FailedReleasesController.php

@DariusIII DariusIII merged commit 8861df6 into master Mar 30, 2023
@DariusIII DariusIII deleted the shift-86725 branch September 4, 2023 06:58
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