Skip to content

Latest commit

 

History

History
395 lines (308 loc) · 10.8 KB

File metadata and controls

395 lines (308 loc) · 10.8 KB

Go Coding Standard

This standard captures the preferred Go style used in dawgs. Follow gofmt first, then apply the conventions below when choosing code shape.

Receiver Names

Use s as the receiver name for methods, regardless of the concrete receiver type. The goal is to remove per-type receiver-name churn and reduce cognitive load while reading method bodies.

// Start begins serving requests.
func (s *Server) Start() error {
	go s.loop()
	return nil
}

// Validate reports whether the configuration is supported.
func (s Config) Validate() error {
	if s.Firewall.Backend != "nftables" {
		return fmt.Errorf("unsupported firewall backend %q", s.Firewall.Backend)
	}

	return nil
}

Avoid type-derived receiver names such as srv, cfg, db, or wm unless a local collision makes s unusable.

Vertical Spacing

Use single blank lines inside functions and closures to separate logical paragraphs. The driver packages are the reference style: setup, validation, resource acquisition, side-effecting work, mutation, and final return each get their own visual space when they are independent steps.

Keep validation blocks at the top of a function compact when they are a single gate into the rest of the operation.

if len(kindIDs) == 0 {
	return graph.Kinds{}, true
}

if mappedKinds, err := kindMapper.MapKindIDs(ctx, kindIDs); err == nil {
	return mappedKinds, true
}

return nil, false

Separate a multi-line call from the next independent operation. This includes logging, progress emission, database calls, and other side-effecting calls.

slog.Info("retriever load node phase started",
	slog.String("graph", graphEntry.Name),
	slog.Int64("node_count", graphEntry.NodeCount),
)
progress.emit(ProgressEvent{
	Operation: OperationLoad,
	Message:   "retriever load node phase started",
	Graph:     graphEntry.Name,
})

nodeMap, nodeCount, err := loadGraphNodes(ctx, db, graphEntry)
if err != nil {
	return 0, 0, err
}

Within loops and callbacks, give each filter, transformation, mutation, and checkpoint its own paragraph when the block does more than a couple of trivial statements.

for _, node := range nodes {
	kinds := node.Kinds.Strings()
	sort.Strings(kinds)
	addKindsToSet(nodeKinds, kinds)

	properties := node.Properties.MapOrEmpty()
	if activeScrubber != nil {
		properties = activeScrubber.scrubProperties(properties)
	}

	item := FragmentNode{
		ID:         node.ID.String(),
		Kinds:      kinds,
		Properties: properties,
	}

	items = append(items, item)
	if len(items) >= shardSize {
		if err := flush(); err != nil {
			return err
		}
	}
}

Do not add blank lines between statements that are one tight operation, such as creating a value and immediately returning it, assigning a field and returning the receiver, or a short if/else if chain that represents one decision.

Error Handling and Scope

Prefer initializer-backed if statements for operations that can fail. Handle the error at the point it is raised, and keep successful values scoped to the branch that uses them.

if cfg, err := server.ReadConfiguration(cfgPath); err != nil {
	log.Fatalf("Error reading config: %v", err)
} else if dbInst, err := db.NewDatabase(cfg.DBPath); err != nil {
	log.Fatalf("Error opening database: %v", err)
} else {
	// cfg and dbInst are only available where they are valid.
}

Use else if chains when each later step depends on the successful output of the previous step. This keeps the happy path close to the failure path and prevents partially initialized variables from leaking into wider scopes.

if content, err := os.ReadFile(path); err != nil {
	return cfg, err
} else if err := toml.Unmarshal(content, &cfg); err != nil {
	return cfg, err
} else {
	cfg = cfg.withDefaults()
	return cfg, nil
}

When an error requires a special-case branch, keep that branch nested at the error site.

if record, err := s.db.GetHostRecord(match.IPAddress); err != nil {
	if !errors.Is(err, db.ErrNotFound) {
		return err
	}

	// Create the missing record here.
} else {
	// Update the existing record here.
}

Use plain early returns when there is no useful success value to scope or when the initializer chain would make the code harder to follow.

Variable Grouping

Group related local variables aggressively with var blocks. This is preferred when multiple locals establish the state for the same operation, especially when some values are initialized and others are intentionally zero-valued.

var (
	records HostRecords

	txn    = s.db.NewTransaction(false)
	iter   = txn.NewIterator(badger.DefaultIteratorOptions)
	prefix = []byte(recordKeyPrefix)
)

Use blank lines inside the group to separate conceptual clusters. Prefer a single grouped declaration over several adjacent var statements.

For short-lived values used immediately, := is still appropriate.

line := scanner.Text()

Logical Spacing

Use blank lines inside functions to separate logical phases. This is guidance, not a hard formatting rule: prefer readability over mechanically inserting empty lines.

Common phase boundaries include setup before control flow, guard checks before work, resource acquisition before deferred cleanup and use, and mutation before the final return.

transaction, err := s.renderFirewallTransaction(state)
if err != nil {
	return err
}

log.Infof("Applying nftables transaction. Banned %d hosts.", state.Count())
if output, err := s.runNftCommand(s.nftPath, transaction.Payload, "-j", "-f", "-"); err != nil {
	return fmt.Errorf("backend apply failed running %s: %s: %w", s.nftPath, output, err)
}

s.firewallState = state
return nil

Within loops, use blank lines to make each filtering or transformation step stand on its own.

for _, record := range hostRecords {
	if !now.Before(record.NextRefresh) {
		continue
	}

	ipAddress := net.ParseIP(record.IPAddress)
	if ipAddress == nil || ipAddress.To4() == nil {
		continue
	}

	if staticCIDRAllows(cfg.StaticAllowCIDRs, ipAddress) {
		continue
	}

	state.BannedIPv4[record.IPAddress] = record
}

In select statements, separate cases with blank lines when each case performs distinct work.

select {
case <-refreshTicker.C:
	s.update()

case <-s.updateC:
	s.update()

case <-s.joiner.StopC:
	return
}

Avoid splitting tightly coupled statements when the second line is the immediate effect of the first.

Documentation Comments

Every function and method declaration and every struct or interface definition must have a semantically relevant Go doc comment, whether it is exported or unexported. Document every struct field and interface member individually, including embedded fields and embedded interface elements. Follow Go doc form by starting each comment with the declared identifier when applicable.

Comments must explain the declaration's purpose, meaning, behavior, or contract. Merely restating the identifier without adding useful information does not satisfy this requirement.

// RecordStore retrieves and persists records.
type RecordStore interface {
	// Find returns the record identified by key.
	Find(ctx context.Context, key string) (Record, error)

	// Save persists record and returns any write failure.
	Save(ctx context.Context, record Record) error
}

Function Ordering

Prefer ordering functions in the same file so dependencies appear before the functions that call them, when that makes the file read naturally. This is guidance, not a hard formatting rule: public entry points, framework conventions, and established local ordering may take precedence.

Struct Definitions

Write struct type definitions across multiple lines, with one field per line. Align naturally with gofmt; do not compress structs onto one line.

// FirewallConfig controls how the firewall backend manages bans.
type FirewallConfig struct {
	// Backend selects the firewall implementation.
	Backend string `toml:"backend"`

	// Table identifies the firewall table managed by the backend.
	Table string `toml:"table"`

	// BanSet identifies the set containing banned addresses.
	BanSet string `toml:"ban_set"`

	// Family selects the address family managed by the backend.
	Family string `toml:"family"`

	// DryRunSummaryOnly limits dry-run output to a summary.
	DryRunSummaryOnly bool `toml:"dry_run_summary_only"`
}

Use field names in struct literals, especially for exported types, config types, tests, and any literal with more than one field.

return Server{
	cfg:           cfg,
	db:            dbInst,
	firewallState: NewFirewallState(),
	updateC:       updateC,
	joiner:        NewJoinChannelPair(),
	nftPath:       nftPath,
	nftBackend:    NewNftablesBackend(cfg.Firewall),
	runNftCommand: defaultNftCommandRunner,
}, nil

Prefer multi-line keyed literals even when a literal is currently small, because they remain stable as fields are added and make diffs easier to read.

freshWatch := db.File{
	Path:   event.Name,
	Offset: 0,
}

Practical Defaults

Keep code direct and local. Favor readable control flow over abstractions that only hide one or two calls.

Return zero values explicitly on error for multi-return functions.

if watcher, err := fsnotify.NewWatcher(); err != nil {
	return FileWatcher{}, err
} else {
	return FileWatcher{
		cfg:     cfg,
		watcher: watcher,
	}, nil
}

Defer cleanup immediately after acquiring a resource.

fin, err := os.Open(fileWatch.Path)
if err != nil {
	return uniqueBanHosts, uniqueAllowHosts
}
defer fin.Close()

Use package-level grouped const and var declarations for related values. Every package-level (global) var and const entity must have a semantically relevant Go doc comment, whether it is exported or unexported. Document each member of a grouped declaration individually. Comments on function-local var declarations are optional and left to the author's discretion.

// ErrNotFound indicates that the requested record does not exist.
var ErrNotFound = errors.New("not found")

const (
	// fileWatchKey formats a file watch key.
	fileWatchKey KeyFormat = "file_watch.%s"

	// hostRecordKey formats a host record key.
	hostRecordKey KeyFormat = "hosts.%s"

	// hostRecordKeyPrefix identifies the host record key namespace.
	hostRecordKeyPrefix KeyFormat = "hosts."
)

In grouped var and const declarations, treat each leading comment and the member definition it documents as one unit. Separate that unit from the next comment and member definition with exactly one blank line. The final member definition may instead be followed directly by the closing ).

var (
	// expansionRootFilter identifies the recursive traversal root filter.
	expansionRootFilter = pgsql.Identifier("traversal_root_filter")

	// expansionTerminalFilter identifies the recursive traversal terminal filter.
	expansionTerminalFilter = pgsql.Identifier("traversal_terminal_filter")

	// expansionPairFilter identifies the recursive traversal pair filter.
	expansionPairFilter = pgsql.Identifier("traversal_pair_filter")
)