My code style guide

Go to file

sneak 95cdb088d7 fix formatting, prettier couldn't handle this document		2024-06-10 05:37:30 -07:00
LICENSE	add license, gpt copyedit/format	2024-06-10 04:06:57 -07:00
Makefile	added more stuff	2024-06-10 05:15:11 -07:00
README.md	fix formatting, prettier couldn't handle this document	2024-06-10 05:37:30 -07:00

README.md

sneak/styleguide

The following is the first released version of my personal code styleguide. There are many like it, but this one is mine.

Only the Go portion is "complete". The others are mostly just placeholders.

Feedback and suggestions are not only welcome but explicitly encouraged.

sneak@sneak.berlin

My 2024 Code Styleguide

All

Every project/repo should have a Makefile in the root. At a minimum, make clean, make run, make fmt, and make test should work. Choose a sane default target (test for libraries, run or publish for binaries). fmt should invoke the appropriate formatters for the files in the repo, such as go fmt, prettier, black, etc. Other standard Makefile targets include deploy, lint. Consider the Makefile the official documentation about how to operate the repository.
If it's possible to write a Dockerfile, include at least a simple one. It should be possible to build and run the project with docker build ..
For F/OSS-licensed software, try to include the full source code of the current version (and any dependencies, such as vendored dependencies) in the docker image. They're small and should be included with the binary.
Under no circumstances should any credentials or secrets ever be committed to any repository, even private ones. Store secrets in environment variables, and if they are absolutely required, check on startup to make sure they are set/non-default and complain loudly if not. Exception, sometimes: public keys. (Public keys can still sometimes be secrets for operational security reasons.)
Avoid nesting if statements. If you have more than one level of nesting, consider inverting the condition and using return to exit early.
Almost all services/servers should accept their configuration via environment variables. Only go full config file if absolutely necessary.
For services/servers, log JSON to stdout. This makes it easier to parse and aggregate logs when run under docker. Use structured logging whenever possible. You may detect if the output is a terminal and pretty-print the logs in that case.
Debug mode is enabled by setting the environment variable DEBUG to a non-empty string. This should enable verbose logging and such. It will never be enabled in prod.
For services/servers, make a healthcheck available at /.well-known/healthcheck. This is out of spec but it is my personal standard. This should return a 200 OK if the service is healthy, along with a JSON object containing the service's name, uptime, and any other relevant information, and a key of "status" with a value of "ok" if the service is healthy. Make sure that in the event of a failure, the service returns a 5xx status code for that route.
If possible, for services/servers, include a /metrics endpoint that returns Prometheus-formatted metrics. This is not required for all services, but is a nice-to-have.

Bash / Shell

Use [[ instead of [ for conditionals. It's a shell builtin and doesn't have to execute a separate process.
Use $( ) instead of backticks. It's easier to read and nest.
Use #!/usr/bin/env bash as the shebang line. This allows the script to be run on systems where bash is not in /bin.
Use set -euo pipefail at the top of every script. This will cause the script to exit if any command fails, and will cause the script to exit if any variable is used before it is set.
Use pv for progress bars when piping data through a command. This makes it easier to see how much data has been processed.
Put all code in functions, even a main function. Define all functions then call main at the bottom of the file.

JavaScript / ECMAScript / ES6

Use const for everything. If you need to reassign, use let. Never use var.
Use yarn for package management, avoid using npm.
Use LTS node versions.
Use prettier for code formatting, with four spaces for indentation.
At a minimum, npm run test and npm run build should work (complete the appropriate scripts in package.json). The Makefile should call these, do not duplicate the scripts in the Makefile.

Docker Containers (for services)

Use runit with runsvinit as the entrypoint for all containers. This allows for easy service management and logging. In startup scripts (/etc/service/*/run) in the container, put a sleep 1 at the top of the script to avoid spiking the cpu in the case of a fast-exiting process (such as in an error condition). This also limits the maximum number of error messages in logs to 86400/day.

Python

Format all code with black, with four space indents.
Put all code in functions. If you are writing a script, put the script in a function called main and call main() at the end of the script using the standard invocation:
```
if __name__ == "__main__":
    main()
```

Golang

Try to hard wrap long lines at 77 characters or less.
Don't commit anything that hasn't been go fmt'd. The only exception is when committing things that aren't yet syntactically valid, which should only happen pre-v0.0.1 or on a non-main branch.
Even if you are planning to deal with only positive integers, use int/int64 types instead of uint/uint64 types. This is for consistency and compatibility with the standard library; it's better than casting all the time.
Any project that has more than 2 or 3 modules should use the go.uber.org/fx dependency injection framework to keep things tidy.
If you have to choose between readable and clever, opt for readable. It's ok to make the code less concise or slightly less idiomatic if you can keep it dead simple.

Embed the git commit hash into the binary and include it in startup logs and in health check output. This is to make it easier to correlate running instances with their code. Do not include build time or build user, as these will make the build nondeterministic.

Example relevant Makefile sections:

Given a main.go like:

package main

import (
    "fmt"
)

var (
    Version   string
    Buildarch string
)

func main() {
    fmt.Printf("Version: %s\n", Version)
    fmt.Printf("Buildarch: %s\n", Buildarch)
}

VERSION := $(shell git describe --always --dirty)
BUILDARCH := $(shell uname -m)

GOLDFLAGS += -X main.Version=$(VERSION)
GOLDFLAGS += -X main.Buildarch=$(BUILDARCH)

# osx can't statically link apparently?!
ifeq ($(UNAME_S),Darwin)
        GOFLAGS := -ldflags "$(GOLDFLAGS)"
endif

ifneq ($(UNAME_S),Darwin)
        GOFLAGS = -ldflags "-linkmode external -extldflags -static $(GOLDFLAGS)"
endif

./httpd: ./pkg/*/*.go ./internal/*/*.go cmd/httpd/*.go
    go build -o $@ $(GOFLAGS) ./cmd/httpd/*.go

Avoid obvious footguns. For example, use range instead of for loops for iterating.
Try to use zerolog for logging. It's fast and has a nice API. For smaller/quick projects, the standard library's log package (and specifically log/slog) is fine. In that case, log structured logs whenever possible, and import sneak.berlin/go/simplelog to configure it appropriately. Example:
```
package main

import (
    "log/slog"
    _ "sneak.berlin/go/simplelog"
)

func main() {
    slog.Info("Starting up")
}
```
Commit at least a single test file to check compilation. The test file can be empty, but it should exist. This is to ensure that go test ./... will always function as a syntax check at a minimum.
Full TDD and coverage isn't that important, but when fixing a specific bug, try to write a test that reproduces the bug before fixing it. This will help ensure that the bug doesn't come back later, and crystallizes the experience of discovering the bug and the resulting fix into the repository's history.
For anything beyond a simple script or tool, or anything that is going to run in any sort of "production" anywhere, make sure it passes golangci-lint.
Write a Dockerfile for every repo, even if it only runs the tests and linting. docker build . should always make sure that the code is in an able-to-be-compiled state, linted, and any tests run. The Docker build should fail if linting doesn't pass.
Include a Makefile with targets for at least clean and test. If there are multiple binaries, include a target for each binary. If there are multiple binaries, include a target for all that builds all binaries.
If you are writing a single-module library, .go files are okay in the repo root.
If you are writing a multi-module project, put all .go files in a pkg/ or internal/ subdirectory. internal/ is for modules used only by the current repo, and pkg/ is for modules that can be consumed externally. This is to keep the repo root as clean as possible.
Binaries go in cmd/ directories. Each binary should have its own directory. This is to keep the root clean and to make it easier to see what is a library and what is a binary. Only package main files should be in cmd/* directories.
Keep the main() function as small as possible.
Keep the main package as small as possible. Move as much code as is feasible to a library package, even if it's an internal one. main is just an entrypoint to your code, not a place for implementations. Exception: single-file scripts.
HTTP HandleFuncs should be returned from methods or functions that need to handle HTTP requests. Don't use methods or our top level functions as handlers.
Provide a .gitignore file that ignores at least *.log, *.out, and *.test files, as well as any binaries.
Constructors should be called New() whenever possible. modulename.New() works great if you name the packages properly.
Don't make packages too big. Break them up.
Don't make functions or methods too big. Break them up.
Use descriptive names for functions and methods. Don't be afraid to make them a bit long.
Use descriptive names for modules and filenames. Avoid generic names like server. util is banned.
Constructors should take a Params struct if they need more than 1-2 arguments. Positional arguments are an endless source of bugs and should be avoided whenever possible.
Use context.Context for all functions that need it. If you don't need it, you can pass context.Background(). Anything long-running should get and abide by a Context. A context does not count against your number of function or method arguments for purposes of calculating whether or not you need a Params struct, because the ctx is always first.
Contexts are always named ctx.
Use context.WithTimeout or context.WithDeadline for any function that could potentially run for a long time. This is especially true for any function that makes a network call. Sane timeouts are essential.

If a structure/type is only used in one function or method, define it there. If it's used in more than one, define it in the package. Keep it close to its usages. For example:

func (m *Mothership) tvPost() http.HandlerFunc {

    type MSTVRequest struct {
            URL string `json:"URL"`
    }

    type MSTVResponse struct {
    }

    return func(w http.ResponseWriter, r *http.Request) {
        // parse json from request
        var reqParsed MSTVRequest
        err = json.NewDecoder(r.Body).Decode(&reqParsed)
        ...

        if err != nil {
                SendErrorResponse(w, MSGenericError)
                return
        }

        log.Info().Msgf("Casting to %s: %s", tvName, streamURL)
        SendSuccessResponse(w, &MSTVResponse{})
    }
}

Avoid global state, especially global variables. If you need to store state that is global to your launch or application instance, use a package globals or appstate with a struct and a constructor and require it as a dependency in your constructors. This will allow consumers to be more easily testable and will make it easier to reason about the state of your application. Alternately, if your dependency graph allows for it, put it in the main struct/object of your application, but remember that this harms testability.
Package-global "variables" are ok if they are constants, such as static strings or integers or errors.
Whenever possible, avoid hardcoding numbers or values in your code. Use descriptively-named constants instead. Recall the famous SICP quote: "Programs must be written for people to read, and only incidentally for machines to execute." Rather than comments, a descriptive constant name is much cleaner.

Example:
```
const jsonContentType = "application/json; charset=utf-8"

func (s *Handlers) respondJSON(w http.ResponseWriter, r *http.Request, data interface{}, status int) {
     w.WriteHeader(status)
     w.Header().Set("Content-Type", jsonContentType)
     ...
 }
```
Define your struct types near their constructors.
Define your interface types near the functions that use them, or if you have multiple conformant types, put the interface(s) in their own file.
Define errors as package-level variables. Use a descriptive name for the error. Use errors.New to create the error. If you need to include additional information in the error, use a struct that implements the error interface.
Use lowerCamelCase for local function/variable names. Use UpperCamelCase for type names, and exported function/variable names. Use snake_case for JSON keys. Use lowercase for filenames.
Explicitly specify UTC for datetimes unless you have a very good reason not to. Use time.Now().UTC() to get the current time in UTC.
String dates should always be ISO8601 formatted. Use time.Time.Format with time.RFC3339 to get the correct format.
Use time.Time for all date and time values. Do not use int64 or string for dates or times internally.
When using time.Time in a struct, use a pointer to time.Time so that you can differentiate between a zero value and a null value.
Use time.Duration for all time durations. Do not use int64 or string for durations internally.
When using time.Duration in a struct, use a pointer to time.Duration so that you can differentiate between a zero value and a null value.
Whenever possible, in argument types and return types, try to use standard library interfaces instead of concrete types. For example, use io.Reader instead of *os.File. Tailor these to the needs of the specific function or method. Examples:
- io.Reader instead of *os.File:
  - io.Reader is a common interface for reading data, which can be implemented by many types, including *os.File, bytes.Buffer, strings.Reader, and network connections like net.Conn.
- io.Writer instead of *os.File or *bytes.Buffer:
  - io.Writer is used for writing data. It can be implemented by *os.File, bytes.Buffer, net.Conn, and more.
- io.ReadWriter instead of *os.File:
  - io.ReadWriter combines io.Reader and io.Writer. It is often used for types that can both read and write, such as *os.File and net.Conn.
- io.Closer instead of *os.File or *net.Conn:
  - io.Closer is used for types that need to be closed, including *os.File, net.Conn, and other resources that require cleanup.
- io.ReadCloser instead of *os.File or http.Response.Body:
  - io.ReadCloser combines io.Reader and io.Closer, and is commonly used for types like *os.File and http.Response.Body.
- io.WriteCloser instead of *os.File or *gzip.Writer:
  - io.WriteCloser combines io.Writer and io.Closer. It is used for types like *os.File and gzip.Writer.
- io.ReadWriteCloser instead of *os.File or *net.TCPConn:
  - io.ReadWriteCloser combines io.Reader, io.Writer, and io.Closer. Examples include *os.File and net.TCPConn.
- fmt.Stringer instead of implementing a custom String method:
  - fmt.Stringer is an interface for types that can convert themselves to a string. Any type that implements the String() string method satisfies this interface.
- error instead of custom error types:
  - The error interface is used for representing errors. Instead of defining custom error types, you can use the errors.New function or the fmt.Errorf function to create errors.
- net.Conn instead of *net.TCPConn or *net.UDPConn:
  - net.Conn is a generic network connection interface that can be implemented by TCP, UDP, and other types of network connections.
- http.Handler instead of custom HTTP handlers:
  - http.Handler is an interface for handling HTTP requests. Instead of creating custom handler types, you can use types that implement the ServeHTTP(http.ResponseWriter, *http.Request) method.
- http.HandlerFunc instead of creating a new type:
  - http.HandlerFunc is a type that allows you to use functions as HTTP handlers by implementing the http.Handler interface.
- encoding.BinaryMarshaler and encoding.BinaryUnmarshaler instead of custom marshal/unmarshal methods:
  - These interfaces are used for binary serialization and deserialization. Implementing these interfaces allows types to be encoded and decoded in a standard way.
- encoding.TextMarshaler and encoding.TextUnmarshaler instead of custom text marshal/unmarshal methods:
  - These interfaces are used for text-based serialization and deserialization. They are useful for types that need to be represented as text.
- sort.Interface instead of custom sorting logic:
  - sort.Interface is an interface for sorting collections. By implementing the Len, Less, and Swap methods, you can sort any collection using the sort.Sort function.
- flag.Value instead of custom flag parsing:
  - flag.Value is an interface for defining custom command-line flags. Implementing the String and Set methods allows you to use custom types with the flag package.
Avoid using panic in library code. Instead, return errors to allow the caller to handle them. Reserve panic for truly exceptional conditions.
Use defer to ensure resources are properly cleaned up, such as closing files or network connections. Place defer statements immediately after resource acquisition.
When calling a function with go, wrap the function call in an anonymous function to ensure it runs in the new goroutine context:

Right:
```
go func() {
    someFunction(arg1, arg2)
}()
```
Wrong:
```
go someFunction(arg1, arg2)
```

Use iota to define enumerations in a type-safe way. This ensures that the constants are properly grouped and reduces the risk of errors.

Example:


type HandScore int

const (
    ScoreHighCard = HandScore(iota * 100_000_000_000)
    ScorePair
    ScoreTwoPair
    ScoreThreeOfAKind
    ScoreStraight
    ScoreFlush
    ScoreFullHouse
    ScoreFourOfAKind
    ScoreStraightFlush
    ScoreRoyalFlush
)

Example 2:

type ByteSize float64

const (
    _           = iota // ignore first value by assigning to blank identifier
    KB ByteSize = 1 << (10 * iota)
    MB
    GB
    TB
    PB
    EB
    ZB
    YB
)

Don't hardcode big lists of things in your normal code. Either isolate lists in their own module/package and write some getters, or use a third party library. For example, if you need a list of country codes, you can use https://github.com/emvi/iso-639-1. It's okay to embed a data file (use go embed) in your binary if you need to, but make sure you parse it once as a singleton and don't read it from disk every time you need it. Don't use too much memory for this, embedding anything more than perhaps 25MiB (uncompressed) is probably too much. Compress the file before embedding and uncompress during the reading/parsing step for efficiency.
When storing numeric values that represent a number of units, either include the unit in the variable name (e.g. uptimeSeconds, delayMsec, coreTemperatureCelsius), or use a type alias (that includes the unit name), or use a 3p library such as github.com/alecthomas/units for SI/IEC byte units, or github.com/bcicen/go-units for temperatures (and others). The type system is your friend, use it.
Once you have a working program, run go mod tidy to clean up your go.mod and go.sum files. Tag a v0.0.1 or v1.0.0. Push your main branch and tag(s). Subsequent work should happen on branches so that main is "always releasable". "Releasable" in this context means that it builds and functions as expected, and that all tests and linting passes.

Other Golang Tips and Best Practices (Optional)

For any internet-facing http server, set appropriate timeouts and limits to protect against slowloris attacks or huge uploads that can consume server resources even without authentication.

Example to limit request body size:

package main

 import (
     "fmt"
     "net/http"
 )

 func main() {
     http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
         // Limit the request body to 10MB
         r.Body = http.MaxBytesReader(w, r.Body, 10<<20)
         if err := r.ParseForm(); err != nil {
             http.Error(w, "Request body too large", http.StatusRequestEntityTooLarge)
             return
         }
         fmt.Fprintf(w, "Hello, World!")
     })

     http.ListenAndServe(":8080", nil)
 }

Example to set appropriate timeouts:

package main

import (
    "net/http"
    "time"
)

func main() {
    server := &http.Server{
        Addr:         ":8080",
        ReadTimeout:  5 * time.Second,
        WriteTimeout: 10 * time.Second,
        Handler:      http.DefaultServeMux,
    }

    http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
        fmt.Fprintf(w, "Hello, World!")
    })

    server.ListenAndServe()
}

When passing channels to goroutines, use read-only (<-chan) or write-only (chan<-) channels to communicate the direction of data flow clearly.
Use io.MultiReader to concatenate multiple readers and io.MultiWriter to duplicate writes to multiple writers. This can simplify the handling of multiple data sources or destinations.
For simple counters and flags, use the sync/atomic package to avoid the overhead of mutexes.
When using mutexes, minimize the scope of locking to reduce contention and potential deadlocks. Prefer to lock only the critical sections of code. Try to encapsulate the critical section in its own function or method. Acquire the lock as the first line of the function, defer release of the lock as the second line of the function, and lines 3-5 should perform the task. Try to keep it as short as possible. Avoid using mutexes in the middle of a function. In short, build atomic functions.
Design types to be immutable where possible. This can help avoid issues with concurrent access and make the code easier to reason about.
Global state can lead to unpredictable behavior and makes the code harder to test. Use dependency injection to manage state.
Avoid using init functions unless absolutely necessary as they can lead to unpredictable initialization order and make the code harder to understand.
Provide comments for all public interfaces explaining what they do and how they should be used. This helps other developers understand the intended use.
Be mindful of resource leaks when using time.Timer and time.Ticker. Always stop them when they are no longer needed.
Use sync.Pool to manage a pool of reusable objects, which can help reduce GC overhead and improve performance in high-throughput scenarios.
Avoid using large buffer sizes for channels. Unbounded channels can lead to memory leaks. Use appropriate buffer sizes based on the application's needs.
Always handle the case where a channel might be closed. This prevents panic and ensures graceful shutdowns.
For small structs, use value receivers to avoid unnecessary heap allocations. Use pointer receivers for large structs or when mutating the receiver.
Only use goroutines when necessary. Excessive goroutines can lead to high memory consumption and increased complexity.
Use sync.Cond for more complex synchronization needs that cannot be met with simple mutexes and channels.
Reflection is powerful but should be used sparingly as it can lead to code that is hard to understand and maintain. Prefer type-safe solutions.
Avoid storing large or complex data in context. Context should be used for request-scoped values like deadlines, cancellation signals, and authentication tokens.
Use runtime.Callers and runtime.CallersFrames to capture stack traces for debugging and logging purposes.
Use the testing.TB interface to write helper functions that can be used with both *testing.T and *testing.B.
Use struct embedding to reuse code across multiple structs. This is a form of composition that can simplify code reuse.
Prefer defining explicit interfaces in your packages rather than relying on implicit interfaces. This makes the intended use of interfaces clearer and the code more maintainable.

Author

@sneak <sneak@sneak.berlin>

License

WTFPL

Do with it what you will. There is no warranty, express or implied, including but not limited to merchantability or fitness for a particular purpose. Use at your own risk.