Skip to content

chore: remove dead validation/map helper functions (batch 5) #18813

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dsyme merged 3 commits into from
Feb 28, 2026
+0 −720
Merged

chore: remove dead validation/map helper functions (batch 5) #18813

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cff131c
dead code removal
dsyme Feb 28, 2026
ba7e27b
Merge branch 'main' of https://github.com/github/gh-aw into dead5
dsyme Feb 28, 2026
73a5420
chore: remove dead validation/map helper functions (batch 5)
dsyme Feb 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
225 changes: 0 additions & 225 deletions pkg/workflow/map_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,16 @@
//
// Type Conversion:
// - parseIntValue() - Safely parse numeric types to int with truncation warnings
// - isEmptyOrNil() - Check if a value is empty, nil, or zero
//
// Map Operations:
// - filterMapKeys() - Create new map excluding specified keys
// - getMapFieldAsString() - Safely extract a string field from a map[string]any
// - getMapFieldAsMap() - Safely extract a nested map from a map[string]any
// - getMapFieldAsBool() - Safely extract a boolean field from a map[string]any
// - getMapFieldAsInt() - Safely extract an integer field from a map[string]any
//
// These utilities handle common type conversion and map manipulation patterns that
// occur frequently during YAML-to-struct parsing and configuration processing.

package workflow

import (
"strings"

"github.com/github/gh-aw/pkg/logger"
)

Expand Down Expand Up @@ -83,221 +76,3 @@ func filterMapKeys(original map[string]any, excludeKeys ...string) map[string]an
}
return result
}

// isEmptyOrNil evaluates whether a value represents an empty or absent state.
// This consolidates various emptiness checks across the codebase into a single
// reusable function. The function handles multiple value types with appropriate
// emptiness semantics for each.
//
// Returns true when encountering:
// - nil values (representing absence)
// - strings that are empty or contain only whitespace
// - numeric types equal to zero
// - boolean false
// - collections (slices, maps) with no elements
//
// Usage pattern:
//
// if isEmptyOrNil(configValue) {
// return NewValidationError("fieldName", "", "required field missing", "provide a value")
// }
func isEmptyOrNil(candidate any) bool {
// Handle nil case first
if candidate == nil {
return true
}

// Type-specific emptiness checks using reflection-free approach
switch typedValue := candidate.(type) {
case string:
// String is empty if blank after trimming whitespace
return len(strings.TrimSpace(typedValue)) == 0
case int:
return typedValue == 0
case int8:
return typedValue == 0
case int16:
return typedValue == 0
case int32:
return typedValue == 0
case int64:
return typedValue == 0
case uint:
return typedValue == 0
case uint8:
return typedValue == 0
case uint16:
return typedValue == 0
case uint32:
return typedValue == 0
case uint64:
return typedValue == 0
case float32:
return typedValue == 0.0
case float64:
return typedValue == 0.0
case bool:
// false represents empty boolean state
return !typedValue
case []any:
return len(typedValue) == 0
case map[string]any:
return len(typedValue) == 0
}

// Non-nil values of unrecognized types are considered non-empty
return false
}

// getMapFieldAsString retrieves a string value from a configuration map with safe type handling.
// This function wraps the common pattern of extracting string fields from map[string]any structures
// that result from YAML parsing, providing consistent error behavior and logging.
//
// The function returns the fallback value in these scenarios:
// - Source map is nil
// - Requested key doesn't exist in map
// - Value at key is not a string type
//
// Parameters:
// - source: The configuration map to query
// - fieldKey: The key to look up in the map
// - fallback: Value returned when extraction fails
//
// Example usage:
//
// titleValue := getMapFieldAsString(frontmatter, "title", "")
// if titleValue == "" {
// return NewValidationError("title", "", "title required", "provide a title")
// }
func getMapFieldAsString(source map[string]any, fieldKey string, fallback string) string {
// Early return for nil map
if source == nil {
return fallback
}

// Attempt to retrieve value
retrievedValue, keyFound := source[fieldKey]
if !keyFound {
return fallback
}

// Verify type before returning
stringValue, isString := retrievedValue.(string)
if !isString {
mapHelpersLog.Printf("Type mismatch for key %q: expected string, found %T", fieldKey, retrievedValue)
return fallback
}

return stringValue
}

// getMapFieldAsMap retrieves a nested map value from a configuration map with safe type handling.
// This consolidates the pattern of extracting nested configuration sections while handling
// type mismatches gracefully. Returns nil when the field cannot be extracted as a map.
//
// Parameters:
// - source: The parent configuration map
// - fieldKey: The key identifying the nested map
//
// Example usage:
//
// toolsSection := getMapFieldAsMap(config, "tools")
// if toolsSection != nil {
// playwrightConfig := getMapFieldAsMap(toolsSection, "playwright")
// }
func getMapFieldAsMap(source map[string]any, fieldKey string) map[string]any {
// Guard against nil source
if source == nil {
return nil
}

// Look up the field
retrievedValue, keyFound := source[fieldKey]
if !keyFound {
return nil
}

// Type assert to nested map
mapValue, isMap := retrievedValue.(map[string]any)
if !isMap {
mapHelpersLog.Printf("Type mismatch for key %q: expected map[string]any, found %T", fieldKey, retrievedValue)
return nil
}

return mapValue
}

// getMapFieldAsBool retrieves a boolean value from a configuration map with safe type handling.
// This wraps the pattern of extracting boolean configuration flags while providing consistent
// fallback behavior when the value is missing or has an unexpected type.
//
// Parameters:
// - source: The configuration map to query
// - fieldKey: The key to look up
// - fallback: Value returned when extraction fails
//
// Example usage:
//
// sandboxEnabled := getMapFieldAsBool(config, "sandbox", false)
// if sandboxEnabled {
// // Enable sandbox mode
// }
func getMapFieldAsBool(source map[string]any, fieldKey string, fallback bool) bool {
// Handle nil source
if source == nil {
return fallback
}

// Retrieve value from map
retrievedValue, keyFound := source[fieldKey]
if !keyFound {
return fallback
}

// Verify boolean type
booleanValue, isBoolean := retrievedValue.(bool)
if !isBoolean {
mapHelpersLog.Printf("Type mismatch for key %q: expected bool, found %T", fieldKey, retrievedValue)
return fallback
}

return booleanValue
}

// getMapFieldAsInt retrieves an integer value from a configuration map with automatic numeric type conversion.
// This function handles the common pattern of extracting numeric config values that may be represented
// as various numeric types in YAML (int, int64, float64, uint64). It delegates to parseIntValue for
// the actual type conversion logic.
//
// Parameters:
// - source: The configuration map to query
// - fieldKey: The key to look up
// - fallback: Value returned when extraction or conversion fails
//
// Example usage:
//
// retentionDays := getMapFieldAsInt(config, "retention-days", 30)
// if err := validateIntRange(retentionDays, 1, 90, "retention-days"); err != nil {
// return err
// }
func getMapFieldAsInt(source map[string]any, fieldKey string, fallback int) int {
// Guard against nil source
if source == nil {
return fallback
}

// Look up the value
retrievedValue, keyFound := source[fieldKey]
if !keyFound {
return fallback
}

// Attempt numeric conversion using existing utility
convertedInt, conversionOk := parseIntValue(retrievedValue)
if !conversionOk {
mapHelpersLog.Printf("Failed to convert key %q to int: got %T", fieldKey, retrievedValue)
return fallback
}

return convertedInt
}
89 changes: 0 additions & 89 deletions pkg/workflow/validation_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,6 @@
// # Available Helper Functions
//
// - validateIntRange() - Validates that an integer value is within a specified range
// - ValidateRequired() - Validates that a required field is not empty
// - ValidateMaxLength() - Validates that a field does not exceed maximum length
// - ValidateMinLength() - Validates that a field meets minimum length requirement
// - ValidateInList() - Validates that a value is in an allowed list
// - ValidatePositiveInt() - Validates that a value is a positive integer
// - ValidateNonNegativeInt() - Validates that a value is a non-negative integer
// - validateMountStringFormat() - Parses and validates a "source:dest:mode" mount string
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "Available Helper Functions" list was updated to remove the deleted exported validators, but it now omits other helpers that still exist in this file (e.g., formatList() and validateTargetRepoSlug()). Please update this section (or remove it) so the header documentation reflects the current contents of validation_helpers.go.

Suggested change
// - validateMountStringFormat() - Parses and validates a "source:dest:mode" mount string
// - validateMountStringFormat() - Parses and validates a "source:dest:mode" mount string
// - formatList() - Formats a slice of strings into a human-readable comma-separated list
// - validateTargetRepoSlug() - Validates that a target-repo slug is not a wildcard

Copilot uses AI. Check for mistakes.
//
// # Design Rationale
Expand All @@ -31,8 +25,6 @@ package workflow
import (
"errors"
"fmt"
"slices"
"strconv"
"strings"

"github.com/github/gh-aw/pkg/logger"
Expand Down Expand Up @@ -68,87 +60,6 @@ func validateIntRange(value, min, max int, fieldName string) error {
return nil
}

// ValidateRequired validates that a required field is not empty
func ValidateRequired(field, value string) error {
if strings.TrimSpace(value) == "" {
validationHelpersLog.Printf("Required field validation failed: field=%s", field)
return NewValidationError(
field,
value,
"field is required and cannot be empty",
fmt.Sprintf("Provide a non-empty value for '%s'", field),
)
}
return nil
}

// ValidateMaxLength validates that a field does not exceed maximum length
func ValidateMaxLength(field, value string, maxLength int) error {
if len(value) > maxLength {
return NewValidationError(
field,
value,
fmt.Sprintf("field exceeds maximum length of %d characters (actual: %d)", maxLength, len(value)),
fmt.Sprintf("Shorten '%s' to %d characters or less", field, maxLength),
)
}
return nil
}

// ValidateMinLength validates that a field meets minimum length requirement
func ValidateMinLength(field, value string, minLength int) error {
if len(value) < minLength {
return NewValidationError(
field,
value,
fmt.Sprintf("field is shorter than minimum length of %d characters (actual: %d)", minLength, len(value)),
fmt.Sprintf("Ensure '%s' is at least %d characters long", field, minLength),
)
}
return nil
}

// ValidateInList validates that a value is in an allowed list
func ValidateInList(field, value string, allowedValues []string) error {
if slices.Contains(allowedValues, value) {
return nil
}

validationHelpersLog.Printf("List validation failed: field=%s, value=%s not in allowed list", field, value)
return NewValidationError(
field,
value,
fmt.Sprintf("value is not in allowed list: %v", allowedValues),
fmt.Sprintf("Choose one of the allowed values for '%s': %s", field, strings.Join(allowedValues, ", ")),
)
}

// ValidatePositiveInt validates that a value is a positive integer
func ValidatePositiveInt(field string, value int) error {
if value <= 0 {
return NewValidationError(
field,
strconv.Itoa(value),
"value must be a positive integer",
fmt.Sprintf("Provide a positive integer value for '%s'", field),
)
}
return nil
}

// ValidateNonNegativeInt validates that a value is a non-negative integer
func ValidateNonNegativeInt(field string, value int) error {
if value < 0 {
return NewValidationError(
field,
strconv.Itoa(value),
"value must be a non-negative integer",
fmt.Sprintf("Provide a non-negative integer value for '%s'", field),
)
}
return nil
}

// validateMountStringFormat parses a mount string and validates its basic format.
// Expected format: "source:destination:mode" where mode is "ro" or "rw".
// Returns (source, dest, mode, nil) on success, or ("", "", "", error) on failure.
Expand Down
Loading
Loading