Laravel & PHP Style Guide

About Laravel

First and foremost, Laravel provides the most value when you write things the way Laravel intended you to write. If there's a documented way to achieve something, follow it. Whenever you do something differently, make sure you have a justification for why you didn't follow the defaults.

General PHP Rules

Code style must follow PSR-1 and PSR-2. Generally speaking, everything string-like that's not public-facing should use camelCase. Detailed examples on these are spread throughout the guide in their relevant sections.

Docblocks

Only add a description when it provides more context than the method signature itself. Use full sentences for descriptions, including a period at the end.

// Good
class Url
{
    public static function fromString(string $url)
    {
        // ...
    }
}

// Bad: The description is redundant, and the method describes itself.
class Url
{
    /**
     * Create a url from a string.
     * 
     * @param string $url
     * 
     * @return \App\Url\Url
     */
    public static function fromString(string $url)
    {
        // ...
    }
}

Always use fully qualified class names in docblocks.

// Good

/**
 * @param string $url
 *
 * @return \App\Url\Url
 */

// Bad

/**
 * @param string $foo
 *
 * @return Url
 */

Ternary operators

Every portion of a ternary expression should be on its own line unless it's a really short expression.

// Good
$result = $object instanceof Model
    ? $object->name
    : 'A default value';

$name = $isFoo ? 'foo' : 'bar';

// Bad
$result = $object instanceof Model ?
    $object->name : 
   'A default value';

Comments

Comments should be avoided as much as possible by writing expressive code. If you do need to use a comment format it like this:

// There should be space before a single line comment.

/*
 * If you need to explain a lot you can use a comment block. Notice the
 * single * on the first line. Comment blocks don't need to be three
 * lines long or three characters shorter than the previous line.
 */

Whitespace

Statements should have to breathe. In general always add blank lines between statements, unless they're a sequence of single-line equivalent operations. This isn't something enforceable, it's a matter of what looks best in its context.

// Good
public function getPage($url)
{
    $page = $this->pages()->where('slug', $url)->first();

    if (! $page) {
        return null;
    }

    if ($page['private'] && ! Auth::check()) {
        return null;
    }

    return $page;
}

// Bad: Everything's cramped together.
public function getPage($url)
{
    $page = $this->pages()->where('slug', $url)->first();
    if (! $page) {
        return null;
    }
    if ($page['private'] && ! Auth::check()) {
        return null;
    }
    return $page;
}

// Good: A sequence of single-line equivalent operations.
public function up()
{
    Schema::create('users', function (Blueprint $table) {
        $table->increments('id');
        $table->string('name');
        $table->string('email')->unique();
        $table->string('password');
        $table->rememberToken();
        $table->timestamps();
    });
}

Don't add any extra empty lines between {} brackets.

// Good
if ($foo) {
    $this->foo = $foo;
}

// Bad
if ($foo) {

    $this->foo = $foo;

}

Configuration

Configuration files must use kebab-case.

config/
  pdf-generator.php

Configuration keys must use snake_case.

// config/pdf-generator.php
return [
    'chrome_path' => env('CHROME_PATH'),
];

Avoid using the env helper outside of configuration files. Create a configuration value from the env variable like above.

Artisan commands

The names given to artisan commands should all be kebab-cased. Ideally please also provide a namespace to the command.

# Good
php artisan delete-old-records

# Bad
php artisan deleteOldRecords

A command should always give some feedback on what the result is. Minimally you should let the handle method spit out a comment at the end indicating that all went well.

// in a Command
public function handle()
{
    // do some work

    $this->comment('All ok!');
}

If possible use a descriptive success message eg. Old records deleted.

Routing

Public-facing urls must use kebab-case.

https://osmaviation.com/open-source
https://osmaviation.com/jobs/front-end-developer

Route names must dot notated strings, with kebab-case.

Route::get('open-source', 'OpenSourceController@index')->name('open-source.index');

<a href="{{ route('openSource') }}">
    Open Source
</a>

All routes have an http verb, that's why we like to put the verb first when defining a route. It makes a group of routes very readable. Any other route options should come after it.

// good: all http verbs come first
Route::get('/', 'HomeController@index')->name('home');
Route::get('open-source', 'OpenSourceController@index')->middleware('open-source.index');

// bad: http verbs not easily scannable
Route::name('home')->get('/', 'HomeController@index');
Route::middleware('openSource')->get('OpenSourceController@index');

Controllers

Controllers that control a resource must use the plural resource name.

class PostsController
{
    // ...
}

Try to keep controllers simple and stick to the default CRUD keywords (index, create, store, show, edit, update, destroy). Extract a new controller if you need other actions.

In the following example, we could have PostsController@favorite, and PostsController@unfavorite, or we could extract it to a separate FavoritePostsController.

class PostsController
{
    public function create()
    {
        // ...
    }
    
    // ...
    
    public function favorite(Post $post)
    {
        request()->user()->favorites()->attach($post);
        
        return response(null, 200);
    }

    public function unfavorite(Post $post)
    {
        request()->user()->favorites()->detach($post);
        
        return response(null, 200);
    }
}

Here we fall back to default CRUD words, create and destroy.

class FavoritePostsController
{
    public function create(Post $post)
    {
        request()->user()->favorites()->attach($post);
        
        return response(null, 200);
    }

    public function destroy(Post $post)
    {
        request()->user()->favorites()->detach($post);
        
        return response(null, 200);
    }
}

This is a loose guideline that doesn't need to be enforced.

Always use route model binding in your controllers. This removes un-necessary visual noise, and keeps your controllers clean.

This is bad

public function show($post_id)
{
    $post = Post::find($post_id);
    ...
}

This is good

public function show(Post $post)
{
    ...
}

Views

View files must use kebab-case.

resources/
  views/
    open-source.blade.php

class OpenSourceController
{
    public function index() {
        return view('open-source');
    }
}

Validation

When validating input from the user you can use controller validation helpers up to a certain amount of rules. It is, however, encouraged to use form requests to handle validation. This makes your controllers much thinner. When you have multiple rules for an attribute it should be added using the array syntax rather than using pipes.

This is bad (too many rules)

request()->validate([
    'foo' => 'required',
    'bar' => [
        'required',
        'in:baz,boo',
        function($attribute, $value, $fail) {
            if($value === 'baz') {
                $fail('Bing bong!');
            }
        }
    ],
    'baz' => new SomeValidationRule(),
    'boo' => 'required|in:bee,baa,byy|confirmed',
    'naa' => request()->has('naa') ?
        'required|min:5|max:10' :
        'sometimes'
]);

This is OK (not that many rules)

request()->validate([
    'foo' => 'required',
    'bar' => 'min:5'
]);

This is best (either way)

// FooController
class FooController extends Controller 
{
    public function store(FooRequest $request) 
    {
        return Foo::create($request->only(['foo', 'bar']));
    }
}

// FooRequest
class FooRequest extends FormRequest 
{
    public function rules() 
    {
        return [
            'foo' => 'required',
            'bar' => [
                'required',
                'in:baz,boo',
                function($attribute, $value, $fail) {
                    if($value === 'baz') {
                        $fail('Bing bong!');
                    }
                }
            ],
            'baz' => new SomeValidationRule(),
            'boo' => 'required|in:bee,baa,byy|confirmed',
            'naa' => $this->has('naa') ?
                'required|min:5|max:10' :
                'sometimes'
        ];
    }
}

Blade Templates

Indent using four spaces.

<a href="/open-source">
    Open Source
</a>

Don't add spaces after control structures.

@if($condition)
    Something
@endif

Authorization

Policies must use camelCase.

Gate::define('editPost', function ($user, $post) {
    return $user->id == $post->user_id;
});

@can('editPost', $post)
    <a href="{{ route('posts.edit', $post) }}">
        Edit
    </a>
@endcan

Try to name abilities using default CRUD words. One exception: replace show with view. A server shows a resource, a user views it.

Translations

Translations must be rendered with the __ function. We prefer using this over @lang in Blade views because __ can be used in both Blade views and regular PHP code. Here's an example:

<h2>{{ __('newsletter.form.title') }}</h2>

{!! __('newsletter.form.description') !!}

Try/Catch

Wrapping the code in try/catch blocks is fine when you're catching an exception that you are transforming for the end user. However, in pieces of code where the end user will never see the exception (like in Jobs or Console commands) you should never wrap it in a "try/catch" as this is redundant. When a job or console command fails we want the correct exception for debugging.

If you need to catch an exception and transform it, always remember to report the exception using Laravel's report() helper. There will most likely be cases where the exception would be needed for debugging, and it should always be passed to Laravel's exception handler which passes it on to our error tracking services.

Rather than practicing defensive programming it is preferred to deal with the problem upfront. Throwing exceptions in your code is great, but catching your own exceptions should be considered an anti-pattern for OSM Aviation projects.

// Accepted
class MyController 
{
    public function index()
    {
        try {
            $posts = $thirdPartyLibrary->fetchSomething();

            return $posts;
        } catch (\Exception $e) {
            report($e); // Report the exception
            return [
                'error' => true, 
                'message' => 'I know what went wrong, but the end user should not be borhtered'
            ];
        }
    }
}

// Not accepted. The try/catch is redundant, and the exception never reaches 
// Laravel's error handler.
class BackgroundJob
{
    public function handle()
    {
        try {
            $posts = $thirdPartyLibrary->fetchSomething();

            if (!$posts) {
                throw new \Exception('It blew up');
            }
        } catch (\Exception $e) {
            \Log::error($e->getMessage()); // This never reaches Laravel's exception handler.
        }
    }
}

# Laravel & PHP Style Guide

# About Laravel

# General PHP Rules

# Docblocks

# Ternary operators

# Comments

# Whitespace

# Configuration

# Artisan commands

# Routing

# Controllers

# Views

# Validation

# Blade Templates

# Authorization

# Translations

# Try/Catch