Skip to content

[fp-enhancer] Improve pkg/cli - functional/immutability enhancements (round 1)#23658

Closed
github-actions[bot] wants to merge 1 commit intomainfrom
fp-enhancer/pkg-cli-round-1-d775483f3b2e16de
Closed

[fp-enhancer] Improve pkg/cli - functional/immutability enhancements (round 1)#23658
github-actions[bot] wants to merge 1 commit intomainfrom
fp-enhancer/pkg-cli-round-1-d775483f3b2e16de

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

This PR applies moderate, tasteful functional/immutability techniques to the pkg/cli package to improve code clarity and maintainability.

Round-Robin Progress: First package processed. Next run will process: pkg/console

Summary of Changes

1. Declarative Slice Initialization (tool_graph.go)

Replaced a manual 4-line loop + sort.Strings with a single declarative expression:

// Before
var tools []string
for tool := range g.Tools {
    tools = append(tools, tool)
}
sort.Strings(tools)

// After
tools := slices.Sorted(maps.Keys(g.Tools))

This pattern is already used elsewhere in pkg/cli (e.g., run_workflow_validation.go), making this consistent with existing codebase style.

2. Pre-allocated Slice (tool_graph.go)

Pre-allocate transitions slice with known capacity to avoid incremental growth:

// Before
var transitions []ToolTransition

// After
transitions := make([]ToolTransition, 0, len(g.Transitions))

3. Type-safe Sort (tool_graph.go)

Replaced sort.Slice (index-based, untyped) with slices.SortFunc (typed comparison):

// Before
sort.Slice(transitions, func(i, j int) bool {
    if transitions[i].Count != transitions[j].Count {
        return transitions[i].Count > transitions[j].Count
    }
    ...
})

// After
slices.SortFunc(transitions, func(a, b ToolTransition) int {
    if a.Count != b.Count {
        return b.Count - a.Count // descending by count
    }
    ...
})

Side effect: the "sort" import is replaced with "maps" and "slices" (standard library packages).

4. Pre-allocated Slice (actionlint.go)

Pre-allocate relPaths with known capacity from len(lockFiles):

// Before
var relPaths []string

// After
relPaths := make([]string, 0, len(lockFiles))

Testing

  • TestToolGraph, TestToolGraphMultipleSequences, TestToolGraphEmptySequences all pass
  • ✅ All actionlint tests pass
  • go build ./pkg/cli/ succeeds
  • make fmt passes
  • ✅ No behavioral changes — functionality is identical

Benefits

  • Clarity: Declarative slices.Sorted(maps.Keys(...)) communicates intent in one line
  • Consistency: Matches patterns already used in run_workflow_validation.go
  • Safety: slices.SortFunc is type-safe vs index-based sort.Slice
  • Performance: Pre-allocation avoids slice growth reallocations

Automated by Functional Pragmatist — processing pkg/cli (round 1 of N). Next: pkg/console

Generated by Functional Pragmatist ·

  • expires on Apr 1, 2026, 9:34 AM UTC

- tool_graph.go: replace manual map-to-slice loop + sort.Strings with
  slices.Sorted(maps.Keys(...)) for declarative, immutable initialization
- tool_graph.go: pre-allocate transitions slice with make([]T, 0, len(map))
  to avoid unnecessary reallocations
- tool_graph.go: replace sort.Slice with slices.SortFunc using typed
  comparison function (eliminates index-based access, more readable)
- actionlint.go: pre-allocate relPaths slice with known capacity

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@pelikhan
Copy link
Copy Markdown
Collaborator

/q update prompt to avoid calling make with a length, it triggers codeql violations.

@pelikhan pelikhan closed this Mar 31, 2026
@github-actions
Copy link
Copy Markdown
Contributor Author

github-actions bot commented Mar 31, 2026

🎩 Mission equipment ready! Q has optimized your workflow. Use wisely, 007! 🔫

@github-actions
Copy link
Copy Markdown
Contributor Author

Issue created: #23684

🎩 Equipped by Q

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.

1 participant