Skip to content

[fp-enhancer] Improve pkg/cli: add sliceutil.Any and replace for-break patterns#20651

Merged
pelikhan merged 1 commit intomainfrom
fp-enhancer/pkg-cli-any-pattern-d7a2157a44f7c272
Mar 12, 2026
Merged

[fp-enhancer] Improve pkg/cli: add sliceutil.Any and replace for-break patterns#20651
pelikhan merged 1 commit intomainfrom
fp-enhancer/pkg-cli-any-pattern-d7a2157a44f7c272

Conversation

@github-actions
Copy link
Contributor

Applies moderate functional/immutability improvements to the pkg/cli package as part of a round-robin systematic refactoring pass.

Round-Robin Progress: Run 1 of N — processed pkg/cli. Next run will process pkg/console.

Summary of Changes

New: sliceutil.Any — Pure Predicate Function

Added Any[T any](slice []T, predicate func(T) bool) bool to pkg/sliceutil:

  • Returns true if any element satisfies the predicate (short-circuits on first match)
  • Returns false for nil or empty slices
  • Pure function — does not modify the input slice
  • Comprehensive tests: 7 table-driven cases + early-termination test + string scenario test

Applied in pkg/cli/engine_secrets.go — 2 Locations

Two identical exists = true; break imperative patterns replaced with sliceutil.Any:

// Before: imperative for-break
exists := existingSecrets[req.Name]
if !exists {
    for _, alt := range req.AlternativeEnvVars {
        if existingSecrets[alt] {
            exists = true
            break
        }
    }
}

// After: declarative, intent-first
exists := existingSecrets[req.Name] || sliceutil.Any(req.AlternativeEnvVars, func(alt string) bool {
    return existingSecrets[alt]
})

Applied in pkg/cli/add_workflow_resolution.go — 2 Improvements

1. Wildcard detection replaced with sliceutil.Any:

// Before
hasWildcard := false
for _, spec := range parsedSpecs {
    if spec.IsWildcard {
        hasWildcard = true
        break
    }
}

// After
hasWildcard := sliceutil.Any(parsedSpecs, func(spec *WorkflowSpec) bool {
    return spec.IsWildcard
})

2. Slice pre-allocation with known capacity:

// Before: zero-capacity composite literal
parsedSpecs := []*WorkflowSpec{}

// After: pre-allocated with known capacity
parsedSpecs := make([]*WorkflowSpec, 0, len(workflows))

Benefits

  • Clarity: sliceutil.Any(...) immediately expresses intent ("does any element satisfy X?") vs reading through a loop to understand it
  • Safety: Removes mutable flag variable (hasWildcard := false; ... hasWildcard = true) in favour of an immutable assignment
  • Reusability: Any is a general-purpose function; 3 immediate uses in pkg/cli confirm it's worth having
  • Consistency: The codebase already uses sliceutil.Map, sliceutil.Filter, sliceutil.ContainsAny fits naturally alongside them
  • Performance: Any short-circuits on first match (verified by TestAny_StopsEarly)

Testing

  • TestAny — 7 table-driven test cases covering all edge cases (empty, nil, match, no-match, all-match, single)
  • TestAny_Strings — mirrors the real-world existingSecrets pattern from engine_secrets.go
  • TestAny_StopsEarly — verifies short-circuit behaviour
  • ✅ All existing engine_secrets tests pass (TestGetMissingRequiredSecrets, TestGetEngineSecretNameAndValue, etc.)
  • ✅ Full unit test suite passes (make test-unit)
  • ✅ Code formatted (make fmt)

References:

Generated by Functional Pragmatist ·

  • expires on Mar 13, 2026, 9:32 AM UTC

Add Any() function to pkg/sliceutil for checking if any element
satisfies a predicate. Apply it in pkg/cli to replace imperative
for-break patterns with a functional, declarative style.

Changes:
- pkg/sliceutil/sliceutil.go: Add Any[T any] generic function
- pkg/sliceutil/sliceutil_test.go: Add comprehensive tests for Any
- pkg/cli/engine_secrets.go: Use sliceutil.Any for AlternativeEnvVars
  exists checks (2 locations), eliminating for+break patterns
- pkg/cli/add_workflow_resolution.go: Use sliceutil.Any for hasWildcard
  detection; pre-allocate parsedSpecs slice with known capacity

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@pelikhan pelikhan marked this pull request as ready for review March 12, 2026 13:38
Copilot AI review requested due to automatic review settings March 12, 2026 13:38
@pelikhan pelikhan merged commit 82eafb1 into main Mar 12, 2026
@pelikhan pelikhan deleted the fp-enhancer/pkg-cli-any-pattern-d7a2157a44f7c272 branch March 12, 2026 13:38
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new generic sliceutil.Any helper and refactors several pkg/cli loops into a more declarative, short-circuiting predicate style.

Changes:

  • Added sliceutil.Any[T] to centralize “does any element match?” logic with early termination.
  • Replaced two for { ... break } existence checks in pkg/cli/engine_secrets.go with sliceutil.Any.
  • Updated ResolveWorkflows to (a) pre-allocate parsedSpecs capacity and (b) detect wildcards via sliceutil.Any.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
pkg/sliceutil/sliceutil.go Introduces Any[T] predicate helper with short-circuit behavior.
pkg/sliceutil/sliceutil_test.go Adds unit tests covering match/no-match, nil/empty, strings scenario, and early termination.
pkg/cli/engine_secrets.go Simplifies alternative-secret existence checks using sliceutil.Any.
pkg/cli/add_workflow_resolution.go Uses sliceutil.Any for wildcard detection and pre-allocates parsedSpecs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants