|
| 1 | +--- |
| 2 | +description: |
| 3 | +globs: *.go |
| 4 | +alwaysApply: false |
| 5 | +--- |
| 6 | +# Go Best Practices |
| 7 | + |
| 8 | +Follow these best practices when writing Go code in this repository. |
| 9 | + |
| 10 | +## Core Principles |
| 11 | + |
| 12 | +- **Clarity and Simplicity**: Write code that is easy to read, understand, and maintain. Favor explicit over implicit. |
| 13 | +- **Dependency Injection**: Use dependency injection to decouple components and enable testing. The functional options pattern is the standard approach. |
| 14 | +- **Interface-Driven Design**: Define behavior through interfaces to enable mocking and loose coupling. |
| 15 | +- **Explicit Error Handling**: Handle all errors explicitly. Use structured error types when appropriate. |
| 16 | + |
| 17 | +## Architecture Patterns |
| 18 | + |
| 19 | +### Functional Options Pattern |
| 20 | + |
| 21 | +Use the functional options pattern for component initialization. This is the standard across controllers and main components. |
| 22 | + |
| 23 | +### Interface Design |
| 24 | + |
| 25 | +- **Small, Focused Interfaces**: Keep interfaces small and focused on specific behavior |
| 26 | +- **Interface Segregation**: Prefer multiple small interfaces over large ones |
| 27 | +- **Testability**: All external dependencies should be behind interfaces for mocking |
| 28 | +- **Naming**: Use descriptive names that indicate the behavior (e.g., `InstallationManager`, `NetUtils`) |
| 29 | + |
| 30 | +## Error Handling |
| 31 | + |
| 32 | +### Error Wrapping and Context |
| 33 | + |
| 34 | +- **Wrap Errors**: Always add context when propagating errors using `fmt.Errorf("operation failed: %w", err)` |
| 35 | +- **Use %w verb for error wrapping**: Use `%w` instead of `%v` when wrapping errors to maintain the error chain |
| 36 | + ```go |
| 37 | + return fmt.Errorf("processing config: %w", err) // Good - contextual, no prefix |
| 38 | + return fmt.Errorf("reading config file: %w", err) // Good - specific context |
| 39 | + ``` |
| 40 | +- **Preserve Original**: Store the original error for unwrapping when using custom error types |
| 41 | +- **Meaningful Messages**: Error messages should be actionable and include relevant context without redundant prefixes |
| 42 | +- **Use type-safe error handling**: Check error types instead of string matching to avoid brittle code |
| 43 | + ```go |
| 44 | + if errors.Is(err, context.DeadlineExceeded) { ... } // Good |
| 45 | + if strings.Contains(err.Error(), "deadline") { ... } // Bad |
| 46 | + ``` |
| 47 | + |
| 48 | +### Error Message Consistency |
| 49 | + |
| 50 | +- **Go error strings should start with lowercase letter and not end with punctuation**: Follow Go conventions |
| 51 | + ```go |
| 52 | + return errors.New("failed to connect") // Good |
| 53 | + return errors.New("Failed to connect.") // Bad |
| 54 | + ``` |
| 55 | + |
| 56 | +## Naming Conventions |
| 57 | + |
| 58 | +### Package Names |
| 59 | +- Use short, concise, all-lowercase names |
| 60 | +- Avoid stuttering (don't repeat package name in exported types) |
| 61 | +- Examples: `types`, `utils`, `install`, `auth` |
| 62 | + |
| 63 | +### Types and Functions |
| 64 | +- **Exported Types**: Use `PascalCase` (e.g., `InstallController`, `NetUtils`) |
| 65 | +- **Exported Functions**: Use `PascalCase` (e.g., `NewInstallController`, `ValidateConfig`) |
| 66 | +- **Internal Functions**: Use `camelCase` (e.g., `processRequest`, `validateInput`) |
| 67 | +- **Variables**: Use `camelCase` for all variables |
| 68 | + |
| 69 | +### Interface Naming |
| 70 | +- **Behavior-Based**: Name interfaces after what they do (e.g., `Controller`, `Manager`, `NetUtils`) |
| 71 | +- **Avoid Generic Names**: Don't use generic suffixes like `Interface` unless necessary |
| 72 | +- **Single Method**: For single-method interfaces, consider the "-er" suffix pattern |
| 73 | + |
| 74 | +### Variable Naming Clarity |
| 75 | + |
| 76 | +- **Use distinct variable names for file paths vs configuration objects**: Distinguish between file locations, raw data, and parsed objects |
| 77 | + ```go |
| 78 | + configPath := "/etc/config.yaml" // file location |
| 79 | + configBytes := []byte{} // raw data |
| 80 | + config := &Config{} // parsed object |
| 81 | + ``` |
| 82 | + |
| 83 | +## Configuration Management |
| 84 | + |
| 85 | +### Input Validation and Defaults |
| 86 | + |
| 87 | +- **Check before setting defaults**: Always verify if user-provided fields are empty before setting defaults to avoid overriding user input |
| 88 | + ```go |
| 89 | + if config.DataDirectory == "" { |
| 90 | + config.DataDirectory = "/opt/default" |
| 91 | + } |
| 92 | + ``` |
| 93 | + |
| 94 | +## CLI Design |
| 95 | + |
| 96 | +### Flag Naming and Help |
| 97 | + |
| 98 | +- **Ensure all CLI flags appear in help menu with consistent naming patterns**: Use kebab-case for multi-word flags |
| 99 | + ```go |
| 100 | + installCmd.Flags().String("admin-console-namespace", "", "Namespace for admin console") // Good |
| 101 | + installCmd.Flags().String("adminconsolenamespace", "", "Namespace for admin console") // Bad |
| 102 | + ``` |
| 103 | + |
| 104 | +## Concurrency and Thread Safety |
| 105 | + |
| 106 | +### Mutex Usage |
| 107 | +- Use `sync.RWMutex` for read-heavy workloads |
| 108 | +- Keep critical sections small |
| 109 | +- Always use defer for unlocking: `defer mu.Unlock()` |
| 110 | + |
| 111 | +### Context Usage |
| 112 | +- Pass `context.Context` as the first parameter to functions that may block or need cancellation |
| 113 | +- Use `ctx context.Context` as the parameter name |
| 114 | +- Don't store contexts in structs; pass them through function calls |
| 115 | + |
| 116 | +## Logging |
| 117 | + |
| 118 | +### Structured Logging with Logrus |
| 119 | + |
| 120 | +- Use `logrus.FieldLogger` interface for dependency injection |
| 121 | +- Add contextual fields to log entries for better debugging |
| 122 | +- Use appropriate log levels: `Debug`, `Info`, `Warn`, `Error` |
| 123 | + |
| 124 | +### Logging Patterns |
| 125 | + |
| 126 | +- **Error Logging**: Always log errors at the outermost caller (bottom of the stack). All the context from the trace should be included in the message wrapping. |
| 127 | +- **Discard Logger**: Use `logger.NewDiscardLogger()` for tests |
0 commit comments