diff --git a/AGENTS.md b/AGENTS.md index b7d802ad45..410ef14a59 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -247,6 +247,49 @@ DEBUG_COLORS=0 DEBUG=* gh aw compile ### Go Code Style - **ALWAYS use `any` instead of `interface{}`** - Use the modern `any` type alias (Go 1.18+) for consistency across the codebase +### Type Patterns and Best Practices + +Use appropriate type patterns to improve code clarity, maintainability, and type safety: + +**Semantic Type Aliases** - Use for domain-specific primitives: +```go +// ✅ GOOD - Semantic meaning +type LineLength int +type Version string +type FeatureFlag string + +const MaxExpressionLineLength LineLength = 120 +const DefaultCopilotVersion Version = "0.0.369" +const MCPGatewayFeatureFlag FeatureFlag = "mcp-gateway" +``` + +**Dynamic Types** - Use `map[string]any` for truly dynamic data: +```go +// ✅ GOOD - Unknown structure at compile time +func ProcessFrontmatter(frontmatter map[string]any) error { + // YAML/JSON with dynamic structure +} + +// ✅ GOOD - Document why any is needed +// githubTool uses any because tool configuration structure +// varies based on engine and toolsets +func ValidatePermissions(permissions *Permissions, githubTool any) +``` + +**When to use each pattern**: +- **Semantic type aliases**: Domain concepts (lengths, versions, durations) +- **`map[string]any`**: YAML/JSON parsing, dynamic configurations +- **Interfaces**: Multiple implementations, polymorphism, testing +- **Concrete types**: Known structure, type safety + +**Avoid**: +- Using `any` when the type is known +- Creating unnecessary type aliases that don't add clarity +- Large "god" interfaces with many methods +- Type name collisions (use descriptive, domain-qualified names) + +**See**: specs/go-type-patterns.md for detailed guidance and examples + ### GitHub Actions Integration For JavaScript files in `pkg/workflow/js/*.cjs`: - Use `core.info`, `core.warning`, `core.error` (not console.log) diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 7a04c0cff9..84c1206391 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -9,11 +9,39 @@ import ( const CLIExtensionPrefix = "gh aw" // Semantic types for measurements and identifiers - -// LineLength represents a line length in characters for expression formatting +// +// These type aliases provide meaningful names for primitive types, improving code clarity +// and type safety. They follow the semantic type alias pattern where the type name +// indicates both what the value represents and how it should be used. +// +// Benefits of semantic type aliases: +// - Self-documenting: The type name explains the purpose +// - Type safety: Prevents mixing different concepts with the same underlying type +// - Clear intent: Signals to readers what the value represents +// - Easy refactoring: Can change implementation without affecting API +// +// See specs/go-type-patterns.md for detailed guidance on type patterns. + +// LineLength represents a line length in characters for expression formatting. +// This semantic type distinguishes line lengths from arbitrary integers, +// making formatting code more readable and preventing accidental misuse. +// +// Example usage: +// +// if len(expression) > int(constants.MaxExpressionLineLength) { +// // Break into multiple lines +// } type LineLength int -// Version represents a software version string +// Version represents a software version string. +// This semantic type distinguishes version strings from arbitrary strings, +// enabling future validation logic (e.g., semver parsing) and making +// version requirements explicit in function signatures. +// +// Example usage: +// +// const DefaultCopilotVersion Version = "0.0.369" +// func InstallTool(name string, version Version) error { ... } type Version string // MaxExpressionLineLength is the maximum length for a single line expression before breaking into multiline diff --git a/pkg/parser/frontmatter.go b/pkg/parser/frontmatter.go index 9c03735fd5..f311311923 100644 --- a/pkg/parser/frontmatter.go +++ b/pkg/parser/frontmatter.go @@ -70,21 +70,25 @@ func ParseImportDirective(line string) *ImportDirectiveMatch { // ImportsResult holds the result of processing imports from frontmatter type ImportsResult struct { - MergedTools string // Merged tools configuration from all imports - MergedMCPServers string // Merged mcp-servers configuration from all imports - MergedEngines []string // Merged engine configurations from all imports - MergedSafeOutputs []string // Merged safe-outputs configurations from all imports - MergedSafeInputs []string // Merged safe-inputs configurations from all imports - MergedMarkdown string // Merged markdown content from all imports - MergedSteps string // Merged steps configuration from all imports - MergedRuntimes string // Merged runtimes configuration from all imports - MergedServices string // Merged services configuration from all imports - MergedNetwork string // Merged network configuration from all imports - MergedPermissions string // Merged permissions configuration from all imports - MergedSecretMasking string // Merged secret-masking steps from all imports - ImportedFiles []string // List of imported file paths (for manifest) - AgentFile string // Path to custom agent file (if imported) - ImportInputs map[string]any // Aggregated input values from all imports (key = input name, value = input value) + MergedTools string // Merged tools configuration from all imports + MergedMCPServers string // Merged mcp-servers configuration from all imports + MergedEngines []string // Merged engine configurations from all imports + MergedSafeOutputs []string // Merged safe-outputs configurations from all imports + MergedSafeInputs []string // Merged safe-inputs configurations from all imports + MergedMarkdown string // Merged markdown content from all imports + MergedSteps string // Merged steps configuration from all imports + MergedRuntimes string // Merged runtimes configuration from all imports + MergedServices string // Merged services configuration from all imports + MergedNetwork string // Merged network configuration from all imports + MergedPermissions string // Merged permissions configuration from all imports + MergedSecretMasking string // Merged secret-masking steps from all imports + ImportedFiles []string // List of imported file paths (for manifest) + AgentFile string // Path to custom agent file (if imported) + // ImportInputs uses map[string]any because input values can be different types (string, number, boolean). + // This is parsed from YAML frontmatter where the structure is dynamic and not known at compile time. + // This is an appropriate use of 'any' for dynamic YAML/JSON data. + // See specs/go-type-patterns.md for guidance on when to use map[string]any. + ImportInputs map[string]any // Aggregated input values from all imports (key = input name, value = input value) } // ImportInputDefinition defines an input parameter for a shared workflow import. @@ -94,19 +98,26 @@ type ImportsResult struct { type ImportInputDefinition struct { Description string `yaml:"description,omitempty" json:"description,omitempty"` Required bool `yaml:"required,omitempty" json:"required,omitempty"` - Default any `yaml:"default,omitempty" json:"default,omitempty"` // Can be string, number, or boolean + Default any `yaml:"default,omitempty" json:"default,omitempty"` // Can be string, number, or boolean (dynamic type from YAML) Type string `yaml:"type,omitempty" json:"type,omitempty"` // "string", "choice", "boolean", "number" Options []string `yaml:"options,omitempty" json:"options,omitempty"` // Options for choice type } // ImportSpec represents a single import specification (either a string path or an object with path and inputs) type ImportSpec struct { - Path string // Import path (required) + Path string // Import path (required) + // Inputs uses map[string]any because input values can be different types (string, number, boolean). + // This is parsed from YAML frontmatter and validated against the imported workflow's input definitions. + // This is an appropriate use of 'any' for dynamic YAML data. See specs/go-type-patterns.md. Inputs map[string]any // Optional input values to pass to the imported workflow (values are string, number, or boolean) } // ProcessImportsFromFrontmatter processes imports field from frontmatter // Returns merged tools and engines from imported files +// +// Type Pattern Note: frontmatter uses map[string]any because it represents parsed YAML with +// dynamic structure that varies by workflow. This is the appropriate pattern for parsing +// user-provided configuration files. See specs/go-type-patterns.md for guidance. func ProcessImportsFromFrontmatter(frontmatter map[string]any, baseDir string) (mergedTools string, mergedEngines []string, err error) { result, err := ProcessImportsFromFrontmatterWithManifest(frontmatter, baseDir, nil) if err != nil { diff --git a/pkg/workflow/permissions_validator.go b/pkg/workflow/permissions_validator.go index d99908ab20..a432b4cfdb 100644 --- a/pkg/workflow/permissions_validator.go +++ b/pkg/workflow/permissions_validator.go @@ -93,6 +93,20 @@ type PermissionsValidationResult struct { // This is the general-purpose permission validator used during workflow compilation to check // that the declared permissions are sufficient for the GitHub MCP toolsets being used. // +// Parameters: +// - permissions: The workflow's declared permissions +// - githubTool: The GitHub tool configuration (uses any type because the structure varies +// based on the engine and toolsets being used. This is an appropriate use of any for +// dynamic configuration data that cannot be known at compile time.) +// +// Returns: +// - A validation result indicating any missing permissions and which toolsets require them +// +// Type Pattern Note: githubTool uses 'any' because tool configuration structure is dynamic +// and varies based on engine (copilot, claude, codex) and mode (local, remote). This is +// parsed from YAML frontmatter where the structure is not known until runtime. +// See specs/go-type-patterns.md for guidance on when to use 'any' types. +// // Use ValidatePermissions (this function) for general permission validation against GitHub MCP toolsets. // Use ValidateIncludedPermissions (in imports.go) when validating permissions from included/imported workflow files. func ValidatePermissions(permissions *Permissions, githubTool any) *PermissionsValidationResult { diff --git a/specs/README.md b/specs/README.md index dc7cd95690..298dbfb65a 100644 --- a/specs/README.md +++ b/specs/README.md @@ -8,6 +8,7 @@ This directory contains design specifications and implementation documentation f |----------|--------|----------------| | [Code Organization Patterns](./code-organization.md) | ✅ Documented | Code organization guidelines and patterns | | [Validation Architecture](./validation-architecture.md) | ✅ Documented | `pkg/workflow/validation.go` and domain-specific files | +| [Go Type Patterns and Best Practices](./go-type-patterns.md) | ✅ Documented | `pkg/constants/constants.go`, `pkg/workflow/permissions_validator.go`, `pkg/parser/frontmatter.go` | ## Specifications @@ -43,4 +44,4 @@ When adding new specifications: --- -**Last Updated**: 2025-12-05 +**Last Updated**: 2025-12-18 diff --git a/specs/go-type-patterns.md b/specs/go-type-patterns.md new file mode 100644 index 0000000000..136548029c --- /dev/null +++ b/specs/go-type-patterns.md @@ -0,0 +1,658 @@ +--- +description: Type patterns and best practices for GitHub Agentic Workflows +--- + +# Go Type Patterns and Best Practices + +This document describes the type patterns used throughout the GitHub Agentic Workflows codebase and provides guidance on when and how to use different typing approaches. + +## Table of Contents + +- [Semantic Type Aliases](#semantic-type-aliases) +- [Dynamic YAML/JSON Handling](#dynamic-yamljson-handling) +- [Interface Patterns](#interface-patterns) +- [Type Safety Guidelines](#type-safety-guidelines) +- [Anti-Patterns](#anti-patterns) + +--- + +## Semantic Type Aliases + +Semantic type aliases provide meaningful names for primitive types, improving code clarity and preventing mistakes through type safety. + +### Pattern: Semantic Type Alias + +**Purpose**: Distinguish different uses of the same primitive type with meaningful names + +**Implementation**: +```go +// LineLength represents a line length in characters for expression formatting +type LineLength int + +// Version represents a software version string +type Version string +``` + +**Benefits**: +- Self-documenting code - the type name explains the purpose +- Type safety - prevents mixing different concepts that share the same underlying type +- Clear intent - signals to readers what the value represents +- Easy refactoring - can change underlying implementation without affecting API + +### Examples in Codebase + +#### LineLength Type + +**Location**: `pkg/constants/constants.go` + +**Purpose**: Represents character counts for formatting decisions + +```go +// LineLength represents a line length in characters for expression formatting +type LineLength int + +// MaxExpressionLineLength is the maximum length for a single line expression +const MaxExpressionLineLength LineLength = 120 + +// ExpressionBreakThreshold is the threshold for breaking long lines +const ExpressionBreakThreshold LineLength = 100 +``` + +**Usage**: +```go +// Clear intent - these are lengths, not arbitrary integers +if len(expression) > int(constants.MaxExpressionLineLength) { + // Break into multiple lines +} +``` + +#### Version Type + +**Location**: `pkg/constants/constants.go` + +**Purpose**: Represents software version strings + +```go +// Version represents a software version string +type Version string + +// DefaultCopilotVersion is the default version of the GitHub Copilot CLI +const DefaultCopilotVersion Version = "0.0.369" + +// DefaultClaudeCodeVersion is the default version of the Claude Code CLI +const DefaultClaudeCodeVersion Version = "2.0.71" +``` + +**Benefits**: +- Distinguishes version strings from arbitrary strings +- Makes version requirements explicit in function signatures +- Enables future validation logic (e.g., semver parsing) + +#### Feature Flag Constants + +**Location**: `pkg/workflow/gateway.go`, `pkg/workflow/safe_inputs.go` + +**Purpose**: Named constants for feature flag identifiers + +```go +// MCPGatewayFeatureFlag is the feature flag name for enabling MCP gateway +const MCPGatewayFeatureFlag = "mcp-gateway" + +// SafeInputsFeatureFlag is the name of the feature flag for safe-inputs +const SafeInputsFeatureFlag = "safe-inputs" +``` + +**Benefits**: +- Single source of truth for feature flag names +- Prevents typos when checking feature flags +- Easy to find all usages through IDE navigation + +### When to Use Semantic Type Aliases + +✅ **Use semantic type aliases when:** + +- You have a primitive type that represents a specific concept (e.g., `LineLength`, `Version`, `Duration`) +- Multiple unrelated concepts share the same primitive type (prevents confusion) +- You want to prevent mixing incompatible values (type safety) +- The type name adds clarity that a comment alone wouldn't provide +- Future validation logic might be needed + +❌ **Don't use semantic type aliases when:** + +- The primitive type is already clear from context +- It's a one-off usage without reuse +- The type would be overly specific (prefer composition) +- It adds ceremony without clarity + +### Best Practices for Type Aliases + +1. **Document the purpose**: Always include a comment explaining what the type represents + +```go +// ✅ GOOD - Clear purpose +// LineLength represents a line length in characters for expression formatting +type LineLength int + +// ❌ BAD - No explanation +type LineLength int +``` + +2. **Use descriptive names**: The name should indicate both what it is and how it's used + +```go +// ✅ GOOD - Indicates both content and purpose +type Version string +type LineLength int + +// ❌ BAD - Too generic +type String string +type Number int +``` + +3. **Provide constants with the type**: Define common values using the type + +```go +// ✅ GOOD - Constants use the semantic type +const MaxExpressionLineLength LineLength = 120 + +// ❌ BAD - Constants use primitive type +const MaxExpressionLineLength = 120 // type: int, should be LineLength +``` + +4. **Convert explicitly**: Make type conversions explicit in code + +```go +// ✅ GOOD - Explicit conversion +if len(line) > int(constants.MaxExpressionLineLength) { + // ... +} + +// ❌ BAD - Implicit conversion won't compile +if len(line) > constants.MaxExpressionLineLength { // Type mismatch + // ... +} +``` + +--- + +## Dynamic YAML/JSON Handling + +When parsing YAML or JSON with dynamic/unknown structures, `map[string]any` is the appropriate choice. + +### Pattern: Dynamic Data Structures + +**Purpose**: Handle configuration or data with unknown structure at compile time + +**When to Use**: +- Parsing YAML/JSON frontmatter from markdown files +- Processing user-provided configuration +- Working with GitHub Actions workflow YAML (dynamic fields) +- Intermediate representation during compilation + +### Examples in Codebase + +#### Frontmatter Parsing + +**Location**: `pkg/parser/frontmatter.go` + +**Purpose**: Parse workflow frontmatter which has dynamic structure + +```go +// ImportInputs aggregates input values from all imports +// Uses map[string]any because input values can be string, number, or boolean +ImportInputs map[string]any // key = input name, value = input value + +// ImportSpec represents a single import with optional inputs +type ImportSpec struct { + Path string // Import path (required) + Inputs map[string]any // Optional input values (string, number, or boolean) +} + +// ProcessImportsFromFrontmatter processes dynamic imports field +func ProcessImportsFromFrontmatter( + frontmatter map[string]any, // Dynamic frontmatter structure + baseDir string, +) (mergedTools string, mergedEngines []string, err error) { + // Parse dynamic YAML structure... +} +``` + +**Why `map[string]any`**: +- Frontmatter structure varies by workflow +- Input values can be different types (string, number, boolean) +- Schema validation happens separately +- Allows flexible configuration without code changes + +#### Tool Configuration + +**Location**: `pkg/workflow/permissions_validator.go` + +**Purpose**: Handle dynamic GitHub MCP tool configuration + +```go +// ValidatePermissions accepts any type for GitHub tool config +// because the structure varies based on tool configuration +func ValidatePermissions( + permissions *Permissions, + githubTool any, // Could be map, struct, or nil +) *PermissionsValidationResult { + // Extract toolsets from dynamic configuration... +} +``` + +**Why `any`**: +- Tool configuration structure not known at compile time +- Different tools have different configuration schemas +- Enables runtime type inspection and extraction + +### Best Practices for Dynamic Types + +1. **Document why `any` is used**: Explain the dynamic nature + +```go +// ✅ GOOD - Explains why any is necessary +// githubTool uses any because the tool configuration structure +// varies based on the engine and toolsets being used +func ValidatePermissions(permissions *Permissions, githubTool any) + +// ❌ BAD - No explanation +func ValidatePermissions(permissions *Permissions, githubTool any) +``` + +2. **Validate early**: Convert from `any` to typed structures ASAP + +```go +// ✅ GOOD - Extract and validate immediately +func ProcessConfig(config any) error { + configMap, ok := config.(map[string]any) + if !ok { + return fmt.Errorf("expected map, got %T", config) + } + + // Now work with typed data + name, _ := configMap["name"].(string) + // ... +} +``` + +3. **Use type assertions safely**: Always check the boolean return + +```go +// ✅ GOOD - Check assertion success +value, ok := data["key"].(string) +if !ok { + return fmt.Errorf("expected string") +} + +// ❌ BAD - Panic on type mismatch +value := data["key"].(string) // Can panic! +``` + +4. **Prefer specific types when structure is known**: Only use `any` when truly dynamic + +```go +// ✅ GOOD - Known structure uses typed struct +type ToolConfig struct { + Name string + Version string + Options map[string]any // Only options are dynamic +} + +// ❌ BAD - Using any when structure is known +type ToolConfig map[string]any +``` + +--- + +## Interface Patterns + +Interfaces define behavior contracts and enable polymorphism. Several interface patterns exist in the codebase. + +### Pattern: Behavior Contract Interface + +**Purpose**: Define what a type can do, not what it is + +**Example**: `CodingAgentEngine` Interface + +**Location**: `pkg/workflow/agentic_engine.go` + +```go +// CodingAgentEngine defines the interface for AI coding engines +type CodingAgentEngine interface { + // GetName returns the engine name (e.g., "copilot", "claude", "codex") + GetName() string + + // GenerateSteps creates workflow steps for this engine + GenerateSteps(config EngineConfig) ([]Step, error) +} +``` + +**Benefits**: +- Multiple engine implementations (Copilot, Claude, Codex) +- Easy to add new engines +- Testable with mock implementations +- Clear contract for engine behavior + +### Pattern: Configuration Interface + +**Purpose**: Allow different configuration sources with common interface + +**Example**: `ToolConfig` Interface + +**Location**: `pkg/workflow/mcp-config.go` + +```go +// ToolConfig represents the common interface for tool configurations +type ToolConfig interface { + GetName() string + GetType() string + Validate() error +} +``` + +**Benefits**: +- Different tools can have different configuration structures +- Common validation interface +- Type-safe tool access through interface + +### When to Use Interfaces + +✅ **Use interfaces when:** + +- Multiple types need to implement the same behavior +- You want to enable testing with mocks +- You need polymorphism (different implementations of same contract) +- You want to decouple implementation from usage +- The behavior is more important than the data structure + +❌ **Don't use interfaces when:** + +- Only one implementation exists and no others are planned +- The data structure itself is the interface (use structs) +- It adds indirection without benefit +- A simple function would suffice + +### Interface Best Practices + +1. **Keep interfaces small**: Prefer many small interfaces over large ones + +```go +// ✅ GOOD - Small, focused interface +type Validator interface { + Validate() error +} + +type Namer interface { + GetName() string +} + +// ❌ BAD - Kitchen sink interface +type Tool interface { + Validate() error + GetName() string + GetVersion() string + Execute() error + Cleanup() error + // ... many more methods +} +``` + +2. **Define interfaces where they're used**: Consumers define interfaces they need + +```go +// ✅ GOOD - Interface defined where used +// pkg/workflow/compiler.go +type Validator interface { + Validate() error +} + +func Compile(v Validator) error { + if err := v.Validate(); err != nil { + return err + } + // ... +} + +// ❌ BAD - Interface defined far from usage +// pkg/types/interfaces.go +type Validator interface { + Validate() error +} +``` + +3. **Document interface contracts**: Explain what implementations must do + +```go +// ✅ GOOD - Clear documentation +// CodingAgentEngine defines the interface for AI coding engines. +// Implementations must: +// - Return a unique lowercase name +// - Generate valid GitHub Actions workflow steps +// - Handle errors gracefully +type CodingAgentEngine interface { + GetName() string + GenerateSteps(config EngineConfig) ([]Step, error) +} +``` + +--- + +## Type Safety Guidelines + +### Use `any` Sparingly + +The type `any` (alias for `interface{}`) should be used only when necessary: + +**Valid uses of `any`**: +- Parsing dynamic YAML/JSON structures +- Generic utility functions that work with multiple types +- Reflection-based code +- Interfacing with external libraries that require `interface{}` + +**Avoid `any` for**: +- Function parameters when the type is known +- Return values when the type is known +- Struct fields when the structure is fixed +- Map values when the value type is consistent + +### Type Conversion Safety + +Always use safe type assertions: + +```go +// ✅ GOOD - Safe type assertion with check +value, ok := data["key"].(string) +if !ok { + return fmt.Errorf("expected string, got %T", data["key"]) +} + +// ❌ BAD - Unsafe assertion (can panic) +value := data["key"].(string) +``` + +### Avoid Type Name Collisions + +When creating new types, check for existing types with similar names: + +```go +// ✅ GOOD - Distinct, descriptive names +type WorkflowPermissions struct { /* ... */ } +type UserPermissions struct { /* ... */ } +type RepositoryPermissions struct { /* ... */ } + +// ❌ BAD - Generic names that might collide +type Permissions struct { /* ... */ } // Which permissions? +type Config struct { /* ... */ } // Which config? +``` + +**Best practices**: +- Use package-qualified access when importing types +- Prefix types with their domain/purpose +- Run `go build` to catch name collisions early +- Use IDE tools to search for existing type names + +--- + +## Anti-Patterns + +### ❌ Anti-Pattern 1: Overusing `any` + +**Problem**: Using `any` when the type is known leads to runtime errors and poor maintainability + +```go +// ❌ BAD - Using any when type is known +func ProcessConfig(config any) error { + // Have to type assert everywhere + name := config.(map[string]any)["name"].(string) + version := config.(map[string]any)["version"].(string) + // ... +} + +// ✅ GOOD - Use typed struct +type Config struct { + Name string + Version string +} + +func ProcessConfig(config Config) error { + // Type-safe access + name := config.Name + version := config.Version + // ... +} +``` + +### ❌ Anti-Pattern 2: Using Primitive Types for Domain Concepts + +**Problem**: Primitive types don't convey meaning and enable mistakes + +```go +// ❌ BAD - Easy to mix up timeout and retry count +func CallAPI(url string, timeout int, retries int) error { + // Which int is which? + CallAPI("https://api.example.com", 3, 5) // Wrong order! +} + +// ✅ GOOD - Semantic types prevent confusion +type Timeout time.Duration +type RetryCount int + +func CallAPI(url string, timeout Timeout, retries RetryCount) error { + // Type mismatch caught at compile time + CallAPI("https://api.example.com", Timeout(3), RetryCount(5)) +} +``` + +### ❌ Anti-Pattern 3: God Interfaces + +**Problem**: Large interfaces with many methods are hard to implement and test + +```go +// ❌ BAD - Too many responsibilities +type WorkflowProcessor interface { + Parse() error + Validate() error + Compile() error + Deploy() error + Monitor() error + Rollback() error +} + +// ✅ GOOD - Small, focused interfaces +type Parser interface { + Parse() error +} + +type Validator interface { + Validate() error +} + +type Compiler interface { + Compile() error +} +``` + +### ❌ Anti-Pattern 4: Unnecessary Type Aliases + +**Problem**: Creating type aliases that don't add value + +```go +// ❌ BAD - Doesn't add clarity +type String string +type Integer int +type MyError error + +// ✅ GOOD - Semantic meaning +type URL string +type Port int +type ValidationError error +``` + +### ❌ Anti-Pattern 5: Using `interface{}` Instead of `any` + +**Problem**: Using `interface{}` when `any` is clearer and more modern (Go 1.18+) + +```go +// ❌ BAD - Using old interface{} syntax +func Process(data interface{}) error { + // ... +} + +// ✅ GOOD - Using modern any alias +func Process(data any) error { + // ... +} +``` + +**Note**: The codebase standard is to **always use `any` instead of `interface{}`** + +--- + +## Summary + +### Quick Decision Tree + +``` +Need to represent a value? +│ +├─ Is structure known at compile time? +│ ├─ YES → Use typed struct or specific type +│ └─ NO → Use map[string]any or any +│ +├─ Is it a primitive with semantic meaning? +│ ├─ YES → Use semantic type alias (e.g., LineLength, Version) +│ └─ NO → Use primitive type directly +│ +├─ Need polymorphism? +│ ├─ YES → Define interface for behavior +│ └─ NO → Use concrete type +│ +└─ Handling external data (YAML/JSON)? + ├─ YES → Use map[string]any initially, validate and convert + └─ NO → Use specific types +``` + +### Key Principles + +1. **Prefer specific types** over generic ones +2. **Use semantic type aliases** for domain concepts +3. **Use `any` only for truly dynamic data** (YAML/JSON parsing) +4. **Keep interfaces small and focused** +5. **Document type choices**, especially when using `any` +6. **Always use `any` instead of `interface{}`** (Go 1.18+ standard) +7. **Validate and convert** from dynamic types early +8. **Avoid type name collisions** with descriptive names + +### Common Patterns Summary + +| Pattern | When to Use | Example | +|---------|-------------|---------| +| Semantic Type Alias | Domain-specific primitives | `type LineLength int` | +| `map[string]any` | Dynamic YAML/JSON parsing | Frontmatter, tool configs | +| Behavior Interface | Multiple implementations | `CodingAgentEngine` | +| Configuration Interface | Varied config structures | `ToolConfig` | +| Named Constants | Feature flags, identifiers | `MCPGatewayFeatureFlag` | + +--- + +**Last Updated**: 2025-12-18