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 #951

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Laravel Linter #951

wants to merge 2 commits into from

Conversation

duxet
Copy link
Member

@duxet duxet commented Mar 13, 2024

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

Before merging, you need to:

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

Much of the lint detected may be automatically fixed by running the Laravel Fixer or tasks within the Shift Workbench.

@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

⚠️ 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
@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

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

@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

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

@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

⚠️ 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'.

@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

❌ The following configuration files differ from the Laravel defaults. If you are modifying configuration values directly, consider using an environment variable instead to keep these files maintainable. You should compare your configuration files against the latest version and merge any updates.

  • config/app.php
  • config/auth.php
  • config/broadcasting.php
  • config/cache.php
  • config/database.php
  • config/filesystems.php
  • config/hashing.php
  • config/logging.php
  • config/mail.php
  • config/queue.php
  • config/services.php
  • config/session.php
  • config/view.php

@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

⚠️ The dates property was deprecated in Laravel 8.0 and removed in Laravel 10. Instead, you should use the casts property with a datetime type. Shift found the dates property in the following files:

  • app/Models/Content.php
  • app/Models/User.php

@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

⚠️ 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 may be used instead of inheritance. You should review the following classes:

  • app/Models/Comment.php
  • app/Models/CommentReply.php
  • app/Models/Content.php
  • app/Models/ContentRelated.php
  • app/Models/Conversation.php
  • app/Models/ConversationMessage.php
  • app/Models/DailyAction.php
  • app/Models/Entry.php
  • app/Models/EntryReply.php
  • app/Models/Folder.php
  • app/Models/Folders/All.php
  • app/Models/Folders/Banned.php
  • app/Models/Folders/Blocked.php
  • app/Models/Folders/Downvoted.php
  • app/Models/Folders/Moderated.php
  • app/Models/Folders/Notvoted.php
  • app/Models/Folders/Observed.php
  • app/Models/Folders/Popular.php
  • app/Models/Folders/Saved.php
  • app/Models/Folders/Subscribed.php
  • app/Models/Folders/Upvoted.php
  • app/Models/Group.php
  • app/Models/GroupBan.php
  • app/Models/GroupModerator.php
  • app/Models/ModeratorAction.php
  • app/Models/Notification.php
  • app/Models/NotificationTarget.php
  • app/Models/OAuth/Client.php
  • app/Models/Save.php
  • app/Models/User.php
  • app/Models/UserAction.php
  • app/Models/UserSetting.php
  • app/Models/Vote.php

@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

⚠️ The following classes do not contain any public methods. These may be better served as a helper or service rather than a controller:

  • app/Http/Controllers/BaseController.php

@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

⚠️ 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/Auth/RegistrationController.php
  • app/Http/Controllers/AuthController.php
  • app/Http/Controllers/CommentController.php
  • app/Http/Controllers/Content/EmbedController.php
  • app/Http/Controllers/Content/ThumbnailController.php
  • app/Http/Controllers/ContentController.php
  • app/Http/Controllers/ConversationController.php
  • app/Http/Controllers/DuckController.php
  • app/Http/Controllers/EntryController.php
  • app/Http/Controllers/FolderController.php
  • app/Http/Controllers/Group/BanController.php
  • app/Http/Controllers/Group/ModeratorController.php
  • app/Http/Controllers/GroupController.php
  • app/Http/Controllers/HealthController.php
  • app/Http/Controllers/ImageController.php
  • app/Http/Controllers/NotificationController.php
  • app/Http/Controllers/PollController.php
  • app/Http/Controllers/RankingController.php
  • app/Http/Controllers/RelatedController.php
  • app/Http/Controllers/SaveController.php
  • app/Http/Controllers/SearchController.php
  • app/Http/Controllers/SettingsController.php
  • app/Http/Controllers/StaticFileController.php
  • app/Http/Controllers/UserController.php
  • app/Http/Controllers/UtilsController.php
  • app/Http/Controllers/VoteController.php

@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

⚠️ 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/Auth/RegistrationController.php
  • app/Http/Controllers/CommentController.php
  • app/Http/Controllers/ContentController.php
  • app/Http/Controllers/ConversationController.php
  • app/Http/Controllers/EntryController.php
  • app/Http/Controllers/FolderController.php
  • app/Http/Controllers/Group/BanController.php
  • app/Http/Controllers/GroupController.php
  • app/Http/Controllers/RelatedController.php
  • app/Http/Controllers/SettingsController.php
  • app/Http/Controllers/UserController.php

@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

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

  • resources/views/global/parts/navbar.blade.php
  • resources/views/group/bans.blade.php
  • resources/views/group/list.blade.php
  • resources/views/group/moderators.blade.php
  • resources/views/group/settings/moderators.blade.php
  • resources/views/group/sidebar/moderator.blade.php
  • resources/views/group/widget.blade.php
  • resources/views/ranking/ranking.blade.php
  • resources/views/user/profile.blade.php

@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

⚠️ 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:

  • App::abort()

@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

❌ Laravel 9.0 moved the resources/lang folder to a top-level lang folder. While Laravel allows the old folder, future versions may not. You should migrate this folder and add any new files.

@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

❌ Laravel 5.3 added a top-level routes folder for storing your app routes. You should create this folder and move your routes into one of the default api.php or web.php route files.

@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

❌ Shift found files using the Route methods within your app folder. You should consolidate your routes into one of the default route/api.php or route/web.php route files.

  • app/Api/routes.php
  • app/Http/routes.php
  • app/Providers/RouteServiceProvider.php

@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

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

@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

❌ Laravel 10 added PHP type hints to all user-land code included in a new Laravel application. Shift detected these files are missing type hints. Shift recommends adding them to align with the latest Laravel conventions. You may automate this process by running the Laravel Type Hints task within Shift Workbench.

@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

⚠️ Laravel uses PSR-4 autoloading for the app/ folder. While you may configure your application to load these files within composer.json, all files under the app folder should be classes with a PSR-4 compliant namespace.

The following PHP files are missing a namespace:

  • app/Api/routes.php
  • app/Http/routes.php
  • app/utils.php

@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

ℹ️ Shift detected a custom App namespace. Leaving the default namespace makes your Laravel application easier to maintain. Even Taylor Otwell recommends this.

@duxet
Copy link
Member Author

duxet commented Mar 13, 2024

ℹ️ 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.

None yet

2 participants