Skip to content

Latest commit

 

History

History
927 lines (691 loc) · 18.4 KB

Styles.md

File metadata and controls

927 lines (691 loc) · 18.4 KB

Go Style Guide

A mostly reasonable approach to Go

Wisdom Quotes

KISS: Keep it simple, stupid

DRY: Don't repeat yourself

Stay idiomatic

Embrace 12factor as much as possible

Package names

Avoid too much level for your packages (3 levels is mostly enough).

Prefer package names based on feature instead of which software they are interacting to.

For example:

  • redis will become caching
  • rabbitmq will become brokers or messaging

For these kind of packages, you will include a dummy implementation to facilitate unittests.

An example of rabbitmq dummy implementation can be an in-memory storage to store events.

For unittests, you will need to rename your package if unittests are in the same level of your logic.

Example: models package will have models_test a package name at the same level.

models/
	users.go
	users_test.go

models/users.go

package models

models/users_test.go

package models_test

Required development tools

Integrate those tools with your own editor.

golangci-lint is recommended -as your default linter- with the following configuration:

.golangci.yml

run:
  concurrency: 4
  deadline: 1m
  issues-exit-code: 1
  tests: true


output:
  format: colored-line-number
  print-issued-lines: true
  print-linter-name: true


linters-settings:
  errcheck:
    check-type-assertions: false
    check-blank: false
  govet:
    check-shadowing: false
    use-installed-packages: false
  golint:
    min-confidence: 0.8
  gofmt:
    simplify: true
  gocyclo:
    min-complexity: 10
  maligned:
    suggest-new: true
  dupl:
    threshold: 80
  goconst:
    min-len: 3
    min-occurrences: 3
  misspell:
    locale: US
  lll:
    line-length: 120
  unused:
    check-exported: false
  unparam:
    algo: cha
    check-exported: false
  nakedret:
    max-func-lines: 30

linters:
  enable:
    - megacheck
    - govet
    - errcheck
    - gas
    - structcheck
    - varcheck
    - ineffassign
    - deadcode
    - typecheck
    - golint
    - interfacer
    - unconvert
    - gocyclo
    - gofmt
    - misspell
    - lll
    - nakedret
  enable-all: false
  disable:
    - depguard
    - prealloc
    - dupl
    - maligned
  disable-all: false


issues:
  exclude-use-default: false
  max-per-linter: 1024
  max-same: 1024
  exclude:
    - "G304"
    - "G101"
    - "G104"

You can copy this script in scripts/lint to obtain a straightforward command:

#!/bin/bash

set -eo pipefail

golinter_path="${GOPATH}/bin/golangci-lint"

if [[ ! -x "${golinter_path}" ]]; then
    go get -u github.com/golangci/golangci-lint/cmd/golangci-lint
fi

SOURCE_DIRECTORY=$(dirname "${BASH_SOURCE[0]}")
cd "${SOURCE_DIRECTORY}/.."

if [[ -n $1 ]]; then
    golangci-lint run "$1"
else
    golangci-lint run ./...
fi

Be readable

Give a short but explicit name to your variables, functions and use lowerCamelCase representation.

// bad: too short
a := "[email protected]"

// bad: too long
userEmailWithRootPermissions := "[email protected]"

// good
userEmail := "[email protected]"
email := "[email protected]"

Group your logic within the same block

Short and pure functions, avoid a function which will contain more than 100 lines.

When your code is too complex:

  1. Try to separate things
  2. Try again
  3. Add comments

Name your functions based on their behavior, for example:

A function which will panic and not return an error will be prefixed by Must: regexp.MustCompile

Import

Group packages import, new line between each

  1. Standard library
  2. External packages
  3. Internal packages

In common cases, if you need to rename packages when you are importing them, you are doing it wrong.

import (
	"net/http"
	"strconv"
	"time"

	"github.com/gin-gonic/gin"
	"github.com/mholt/binding"
	"github.com/ulule/deepcopier"
	redis "gopkg.in/redis.v5"
	redisLocker "github.com/ulule/kyu/locker/redis"

	"github.com/ulule/foo/bar"
)

DO NOT rely on . import.

Be secure

Always check users input by using validators (govalidator) and sanitize them.

Don't sanitize an input which is already validated, it's useless and you can introduce unexpected behaviors.

validators.go

var PaymentMethods = map[string]string{
	PaymentMethodCreditCard:  "Credit card",
	PaymentMethodCheck:       "Check",
	PaymentMethodPaypal:      "Paypal",
	PaymentMethodDirectDebit: "Direct debit",
	PaymentMethodMaestro:     "Maestro",
}

const (
	ValueError                = "ValueError"
	PaymentMethodErrorMessage = "Payment method is invalid"
)

func PaymentMethod(paymentMethod string) binding.Errors {
	errs := binding.Errors{}

	if _, ok := PaymentMethods[paymentMethod]; !ok {
		errs = append(errs, binding.Error{
			FieldNames:     []string{"payment_method"},
			Classification: ValueError,
			Message:        PaymentMethodErrorMessage,
		})
	}

	return errs
}

There is no trusted sources.

Do not assume, verify your output.

Always rand.Seed(time.Now().Unix()) before using rand.

Be realistic

Your code will fail, use recover, sentry, alerting, etc.

Internal / external services will also fail, rely on services degradation as much as possible.

  • caching: disable cache if your redis server is down
  • messaging: rely on a in-memory broker when rabbitmq is down

Code organization

Be explicit when naming thing.

// bad
package user

func Get()
// good
package models

func GetUser()

Avoid doing useless tests when your method return a bool or the same output of a previously called method.

In general, else clause can be avoided by returning first.

// bad
func UserExists(ctx context.Context, id int) (bool, error) {
	exists, err := store.UserExists(ctx, id)

	if err == nil {
		if exists {
			return true, nil
		} else {
			return false, nil
		}
	}

	return false, err
}

// good
func UserExists(ctx context.Context, id int) (bool, error) {
	exists, err := store.UserExists(ctx, id)
	if err != nil {
		return false, err
	}

	return exists, nil
}

// better
func UserExists(ctx context.Context, id int) (bool, error) {
	return store.UserExists(ctx, id)
}
// not so bad
users := map[int]*User{}
exists := false

// not so bad either
var (
	users  = map[int]*User{}
	exists = false
)

Don't rely on zero values, be explicit as much as possible, you can apply The Zen of Python.

// very bad
var exists bool // false
var counter int // 0

// good
var (
    exists  = false
    counter = 0
)

// also good
exists  := false
counter := 0

Rely on context everywhere.

We should use named result parameters when the type of at least two successive returned variables is the same.

However, we must avoid naked return statement.

bad

package main

import "fmt"

func split(sum int) (x, y int) {
	x = sum * 4 / 9
	y = sum - x
	return
}

func main() {
	fmt.Println(split(17))
}

good

package main

import "fmt"

func split(sum int) (x int, y int) {
	x = sum * 4 / 9
	return x, sum - x
}

func main() {
	fmt.Println(split(17))
}

Types

Types should be explicit as much as possible:

  • Avoid interface{} type.
  • Don't be shy and/or skinflint about interface.
  • Visitor pattern is your best-friend if you can't use an interface, in order to avoid interface{} type.

Use generic litterals (int, float, etc...) unless you are defining a model, which should follow these conventions:

  • Use int64 type for primary, foreign key and counter.
  • Use Enumerated type for enumeration.
// Foobar is ...
type Foobar struct {
    ID        int64 `db:"id"`
    UserID    int64 `db:"user_id"`
    AccountID int64 `db:"account_id"`
    HitCount  int64 `db:"hit_count"`
    MissCount int64 `db:"miss_count"`
    Status    Enumerated `db:"status"`
    Type      Enumerated `db:"type"`
}

Also, don't hesitate to use type definitions to create a subtype in order to specify and restrict what you expect:

type Email string

Logging

Only use one of these levels:

  • Debug: A verbose entry usually used for future debugging.
  • Info: A general entry about what's going on inside the application.
  • Warning: A non-critical entry that deserve review.
  • Error: A critical and high-priority entry that deserve reaction.

Use structured logging:

// bad
log.Infof("User %d has contributed to Project %d", user.ID, project.ID)

// good
log.Info("New contribution", log.Int("user_id", user.ID), log.Int("project_id", project.ID))

DO NOT log passwords, emails, credentials, credit cards, personals informations or any other sensitives informations.

However, log everything that indicates how the application behaves and responds, like seeing a story of the user’s experience as they used your application. Also, a profusion of log is better than a lack of them: filter on structured entries are painless.

Example:

  • An attempt to authenticate a user was made.
    • Login has succeeded.
    • Login has failed because of an invalid paswword.
    • Login has failed because of an invalid email.
    • etc...
  • A password reset was requested for a user.
  • A password update was requested for a user.
    • Password was updated.
    • Change was discarded because it didn't match.
  • A user has
    • requested a list of available projects.
    • created a new project.
    • deleted a project.
    • uploaded a new media.
    • contributed to a project.
  • A user tried to consult a comment
    • and couldn't read it because he/she didn't have read permissions.
    • and couldn't read it because the comment is no longer available in the database.
    • and successfully read it.
  • A user tried to create a news but an error has occurred
    • because the payload was invalid.
    • because a network error has occurred.
    • because a database error has occurred.

Context

context.Context should be immutable, don't rely on context.Background() only to initialize it.

If your context is global don't store too much things in it, keep it simple:

  • connection pool (redis, postgresql, rabbitmq, etc.)
  • configuration

For the request context include all keys from the application context and add it request information:

  • user
  • resource for the dedicated endpoint
  • user lang

Error handling

If your method can fail, either handle errors nicely with a fallback or any recover mechanism. Or propagate them to the root level by wrapping them with context.

Moreover panic and recover is meant for exceptions, not common errors.

Use thr as variable name (ie: throwable) for silent errors.

defer func() {
	thr := recover()
	if thr != nil {
		tracer.Capture(thr)
	}
}()

Also, always check for errors.

// bad
foo()
val, _ := foo()

// good
_, err := foo()
val, err := foo()

And group your logic when checking an error.

// bad
result, err := thisMethodWillFail()

if err != nil {
	return err
}

// bad
if err := thisMethodWillFail(); err != nil {
    return err
}

// good
result, err := thisMethodWillFail()
if err != nil {
	return err
}

Database

  • Insert must return created id and date.
  • Update must return updated fields and date.
  • Archive must return archived date.

Try to avoid using SQL JOIN. Use preloads instead.

Use transactions as much as possible. Rollback if an error occured.

Transactions may contain non SQL code like messaging events.

Functional options

Try to adopt the functional options philosophy when a component requires configurations.

This pattern is divided by three principles:

  1. ComponentOptions
  2. Option
  3. Component

If you are not familiar with it, please read this introduction.

ComponentOptions

A ComponentOptions is a struct that will configure a Component instance.

For example, let's define a ServerOptions:

package server

type ServerOptions struct {
   Host string
   Port int
   Logger Logger
}

In order to mutate this struct, you'll have to use Option function. (However, defining default values are allowed)

Option

An Option is an interface that will configure a ComponentOptions using higher-order function.

Because interface can (and should) be public, we have decided to restrict the Option visibility so other packages and/or applications can't define new Option:

package server

type Option interface {
   apply(*ComponentOptions) error
}

type option func(*ComponentOptions) error

func (o option) apply(options *ComponentOptions) error {
   return o(options)
}

With that being said, and with our previous example, we can define Option function like this:

func Host(host string) Option {
   return option(func(options *ServerOptions) error {
      options.host = host
      return nil
   })
}

func Port(port int) Option {
   return option(func(options *ServerOptions) error {
      options.port = port
      return nil
   })
}

func WithLogger(logger Logger) Option {
	return option(func(options *ServerOptions) error {
		if logger == nil {
			return errors.New("server: a logger instance is required")
		}
		options.logger = logger
		return nil
	})
}

Component

Once every options are defined, what's left is to implement the Component instantiation process.

type Component struct {
    // ...
}

func New(options ...Option) (*Component, error) {
    // ...
}

Using our previous example, we should have something like this:

package server

import (
    "net"
    "bytes"
)

type Server struct {
    listener net.Listener
    logger Logger
}

func New(options ...Option) (*Server, error) {

    opts := &ServerOptions{
        host:   "localhost",
        port:   8080,
        logger: &bytes.Buffer{}
    }

    for _, option := range options {
        err := option.apply(opts)
        if err != nil {
            return nil, err
        }
    }

    listener, err := listen(opts.host, opts.port)
    if err != nil {
      return err
    }

    server := &Server {
        listener: listener,
        logger:   opts.logger,
    }

    return server, nil
}

Tests

Don't rely on managers to create your fixtures.

Keep your tests in the same package of your logic. Make them independent, fast and avoid a complex logic.

Keep functional and unit tests separate: you don't need to test a behavior from the HTTP handler.

Prefer writing multiple small tests than an unique integration test which will be slower:

  • test your payloads separately
  • test your managers using payloads not with an entire http request

Don't mock too much, avoid mocking the main datastore or your application will fail badly.

Scenario

Table driven tests are great, you should use them:

package foobar_test

func TestFoobar(t *testing.T) {

  scenarios := []struct {
    number   int
    expected int
  }{
    {1, 1},
    {2, 2},
    {3, 6},
    {4, 24},
    {5, 120},
    {6, 720},
    {7, 5040},
  }

  for _, scenario := range scenarios {
    actual := Foobar(scenario.number)
    if actual != scenario.expected {
      t.Errorf("Foobar(%d): expected %d, received %d", scenario.number, scenario.expected, actual)
    }
  }
}

Naming

Name your tests as follows: Test<Package>_<Component>_<Handler>.

Example: TestViews_Order_Display means we are testing the Display function from Order components inside views package.

NOTE: To run Order related tests, execute: go test -run Order

Vendor

Sorry you are doomed, we need to find a proper solution for this :)

Project architecture

This is an initial draft:

/application
	/commands
/configuration
	configuration.go
/constants
	constants.go
/events
	users.go
/gimme
	store.go
/failures
	errors.go
	handlers.go
/managers
	users.go
	users_test.go
/models
	user.go
    	user_test.go
/payments
	/backends
/store
    	users.go
    	users_queries.go
/rpc
	/validators
		user.go
	/payloads
		user.go
	/resources
		user.go
/tasks
	users.go
/web
	/authentication
		facebook.go
		ulule.go
	/handlers
		permissions.go
		resources.go
		users.go
	/middleware
		authentication.go
	router.go
	server.go
/worker
	/handlers
		users.go
	worker.go

Naming convention

Variables

type Category struct{}
type Media struct{}

// slices
mediaList := []Media{}
categories := []Category{}

// maps
categoriesByID := map[int][]Category{}
mediaListByID := map[int][]Media{}
categoryByID := map[int]Category{}

Store methods and managers

func GetCategoryByID(ctx context.Context, id int) (*models.Category, error)
func FindCategoriesByID(ctx context.Context, id int) ([]models.Category, error)
func FindCategories(ctx context.Context, opts PaginationOptions) ([]models.Category, *Cursor, error)
func FindCategoriesByUserID(ctx context.Context, userID int, opts PaginationOptions) ([]models.Category, *Cursor, error)
func UpdateCategory(ctx context.Context, category *models.Category) error
// hard delete
func DeleteCategory(ctx context.Context, category *models.Category) error
// soft delete
func ArchiveCategory(ctx context.Context, category *models.Category) error
func CreateCategory(ctx context.Context, category *models.Category) error

Web components

When you need to inject a fully loaded model to your web context, use Resource suffix.

// resources.go
// inject category in context
func CategoryResource() gin.HandlerFunc

Rely on behavior for permissions check.

func CanReadNews() gin.HandlerFunc
func CanCreateComments() gin.HandlerFunc
func CanCreateProjects() gin.HandlerFunc
func CanReadProject() gin.HandlerFunc
func CanUpdateProject() gin.HandlerFunc
func CanDeleteProject() gin.HandlerFunc
func isStaffMember() gin.HandlerFunc
func isSuperUser() gin.HandlerFunc
func isAuthenticated() gin.HandlerFunc
func isAnonymous() gin.HandlerFunc