Skip to content

Commit 3a070ff

Browse files
ostermanclaudeaknysh
authored
refactor(utils): drop dead helpers and hand-rolled SliceContainsString (#2608)
* refactor(utils): drop dead helpers and hand-rolled SliceContainsString Begin dismantling the pkg/utils dumping ground. - Replace hand-rolled SliceContainsString / SliceContainsStringHasPrefix / SliceContainsStringStartsWith with stdlib slices.Contains / slices.ContainsFunc across ~39 call sites; remove the helpers (slices.Contains is the identical O(n) scan, and the hot path in yaml_utils.go already uses a map). - Delete nine dead exported functions (zero callers anywhere): ExtractAtmosConfig, GetGitHubRepoReleases, GetGitHubReleaseByTag, GetGitHubLatestRelease, PrintAsHcl, NewHighlightWriter (+ HighlightWriter type/method), GetAtmosConfigJSON, PrintAsJSONToFileDescriptor, PrintAsYAMLWithConfig; remove the now-empty config_utils.go and cascaded-unused imports/aliases. - Convert two depends_on dynamic errors in stack_utils.go to wrapped static errors (ErrDependencyResolution) — required to keep the lint gate green after restructuring a flagged if/else chain into early returns. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: remove dead code across pkg/utils and the wider repo Continue dismantling the pkg/utils dumping ground and extend the sweep repo-wide, driven by `go run golang.org/x/tools/cmd/deadcode@latest -test ./...` (authoritative for reachability; -test keeps test-only-used funcs out of the dead set) and cross-checked with the compiler. pkg/utils (~35 funcs + their tests): - array_cleanup, cty_utils, jsonpath, file_utils, json_utils, log_utils, map_utils, slice_utils, string_utils (intern wrappers), yaml_utils (cache-clear/print + GetUserHomeDir/ObfuscateSensitivePaths); delete type_utils.go and json_test.go. Slim the write-only YAML cache-stats to the hits/misses counters a live test actually reads. - Keep Intern, MatchWildcard, HasYAMLPosition, HasAppendTag, IsSocket and the unexported clearParsedYAMLCache - they back tests of live code or have live callers elsewhere. Repo-wide (~53 funcs + tests): internal/exec command-registry orphans (ExecuteHelmfileCmd, ExecuteTerraformGenerate*Cmd, vendor URI helpers, ...), the errors sentry-shutdown cluster, pkg/auth test_helpers + superseded kube map-equal helpers, pkg/provenance/tree_renderer, and assorted dead funcs in cmd, pkg/aws, pkg/logger, pkg/perf, pkg/telemetry, pkg/ui/theme, pkg/stack, pkg/runner/step, pkg/list/flags, pkg/toolchain/installer, tests/preconditions. Deliberately kept (deadcode still lists them): interface-conformance methods, Go Example_* docs, and recently-merged feature scaffolding (Atmos Git DefaultWorkdir/WithStderr, CI constructors, etc.). Verified: go build ./..., go vet ./..., changed-package tests, and the custom-gcl PR lint gate (--new-from-rev=origin/main) all green; deadcode -test reduced from 82 to 28 (the kept set above). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(exec): cover BuildDependentStackNameFromDependsOn{,Legacy} Add table-driven unit tests for the two depends-on resolution functions in internal/exec/stack_utils.go. They have a real production caller (pkg/spacelift/spacelift_stack_processor.go) but no unit test, so the refactor that rewrote them (early returns + wrapped ErrDependencyResolution) showed up as the bulk of the PR's uncovered patch lines. Each test covers all branches: resolves-to-stack, resolves-to-component (Legacy only), and the not-found error path (asserted via errors.Is on ErrDependencyResolution). No production code changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: cover refactor-touched lines to lift patch coverage past 80% Add focused unit tests for three pure functions whose refactor-modified lines (u.SliceContainsString → slices.Contains swaps and a gofumpt reformat) were the remaining uncovered patch lines: - internal/exec: handleDeploySubcommand — all deploy/apply/auto-approve branches. - internal/exec: validateTagsAndComponents — component present/absent, tag match/no-match, duplicate components (errors asserted via errors.Is). - pkg/provenance: formatProvenanceCommentWithStackFile — nil, no-color, and the previously-uncovered color path (computed entry, depth 3). These exercise real, live code paths (no production changes) and cover the mechanical refactor lines that integration tests hit but Go coverage didn't see. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(exec): cover apply+ApplyAutoApprove+UseTerraformPlan guard Address CodeRabbit review (PR #2608): add the missing TestHandleDeploySubcommand case for apply with ApplyAutoApprove=true AND UseTerraformPlan=true, which exercises the `!info.UseTerraformPlan` guard so the second if-block's condition is fully covered (no auto-approve is added when a plan is in use). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>
1 parent aec29bd commit 3a070ff

72 files changed

Lines changed: 247 additions & 3712 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎cmd/devcontainer/service.go‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,6 @@ type Service struct {
1919
logOutput io.Writer // Writer for log output (defaults to os.Stdout).
2020
}
2121

22-
// NewService creates a service with default providers.
23-
func NewService() *Service {
24-
return &Service{
25-
config: &DefaultConfigProvider{},
26-
runtime: NewDockerRuntimeProvider(),
27-
ui: &DefaultUIProvider{},
28-
logOutput: os.Stdout,
29-
}
30-
}
31-
3222
// NewTestableService creates a Service configured with the provided config, runtime, and UI providers for use in tests.
3323
func NewTestableService(
3424
config ConfigProvider,
@@ -58,14 +48,6 @@ func NewTestableServiceWithLogOutput(
5848
}
5949
}
6050

61-
// SetLogOutput sets the writer for log output.
62-
// This allows tests and alternative frontends to capture or redirect log output.
63-
func (s *Service) SetLogOutput(w io.Writer) {
64-
if w != nil {
65-
s.logOutput = w
66-
}
67-
}
68-
6951
// ensureInitialized checks if the service has been properly initialized.
7052
// Returns an error if atmosConfig is nil.
7153
func (s *Service) ensureInitialized() error {

‎cmd/identity_helpers.go‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -120,25 +120,6 @@ func extractIdentityFromArgs(args []string) string {
120120
return ""
121121
}
122122

123-
// CreateAuthManagerFromIdentity creates and authenticates an AuthManager from an identity name.
124-
// Returns nil if identityName is empty (no authentication requested).
125-
// Returns error if identityName is provided but auth is not configured in atmos.yaml.
126-
// This helper reduces nested complexity in describe commands.
127-
//
128-
// This function delegates to auth.CreateAndAuthenticateManager to ensure consistent
129-
// authentication behavior across CLI commands and internal execution logic.
130-
//
131-
// Note: This function does not load stack configs for default identities.
132-
// Use CreateAuthManagerFromIdentityWithAtmosConfig if you need stack-level default identity resolution.
133-
func CreateAuthManagerFromIdentity(
134-
identityName string,
135-
authConfig *schema.AuthConfig,
136-
) (auth.AuthManager, error) {
137-
defer perf.Track(nil, "cmd.CreateAuthManagerFromIdentity")()
138-
139-
return auth.CreateAndAuthenticateManager(identityName, authConfig, cfg.IdentityFlagSelectValue)
140-
}
141-
142123
// CreateAuthManagerFromIdentityWithAtmosConfig creates and authenticates an AuthManager from an
143124
// identity name using a pre-merged auth config.
144125
//

‎cmd/internal/validation.go‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,6 @@ func WithStackValidation(check bool) ValidateOption {
2323
}
2424
}
2525

26-
// WithConfigInfo sets the config info for validation, respecting config selection flags.
27-
func WithConfigInfo(info *schema.ConfigAndStacksInfo) ValidateOption {
28-
return func(cfg *ValidateConfig) {
29-
if info != nil {
30-
cfg.ConfigInfo = *info
31-
}
32-
}
33-
}
34-
3526
// ValidateAtmosConfig checks the Atmos configuration and returns an error instead of exiting.
3627
// This makes the function testable by allowing errors to be handled by the caller.
3728
//

‎errors/error_funcs.go‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -69,28 +69,6 @@ func InitializeMarkdown(config *schema.AtmosConfiguration) {
6969
}
7070
}
7171

72-
// GetMarkdownRenderer returns the package-level markdown renderer and may return nil
73-
// if the renderer has not been initialized via InitializeMarkdown or has been cleared.
74-
// This function is not safe for concurrent access during initialization.
75-
func GetMarkdownRenderer() *markdown.Renderer {
76-
return render
77-
}
78-
79-
// printPlainError writes a plain-text error to stderr without Markdown formatting.
80-
// This is used as a fallback when the markdown renderer is not available.
81-
func printPlainError(title string, err error, suggestion string) {
82-
maskedStderr := os.Stderr
83-
if title != "" {
84-
title = cases.Title(language.English).String(title)
85-
fmt.Fprintf(maskedStderr, "\n%s: %v\n", title, err)
86-
} else {
87-
fmt.Fprintf(maskedStderr, "\nError: %v\n", err)
88-
}
89-
if suggestion != "" {
90-
fmt.Fprintf(maskedStderr, "%s\n", suggestion)
91-
}
92-
}
93-
9472
// printStructuredPlainError extracts ErrorBuilder enrichments and prints them
9573
// in a structured plain text format. This is used when the markdown renderer
9674
// is not available (e.g., during early startup errors before config is loaded).

‎errors/sentry.go‎

Lines changed: 0 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,10 @@ func InitializeSentry(config *schema.SentryConfig) error {
5050
}
5151

5252
// CloseSentry flushes any pending Sentry events and closes the client.
53-
// Deprecated: Use CloseAllSentry() instead to properly close all component-specific clients.
5453
func CloseSentry() {
5554
sentry.Flush(CloseSentryTimeout)
5655
}
5756

58-
// CloseAllSentry closes all Sentry clients in the registry.
59-
func CloseAllSentry() {
60-
// Close global client.
61-
CloseSentry()
62-
63-
// Close all component-specific clients.
64-
GetRegistry().CloseAll()
65-
}
66-
6757
// CaptureError captures an error and sends it to Sentry using cockroachdb/errors native support.
6858
// This uses BuildSentryReport which automatically handles PII-free reporting, stack traces, and safe details.
6959
func CaptureError(err error) {
@@ -154,99 +144,3 @@ func CaptureErrorWithContext(err error, context map[string]string) {
154144
hub.CaptureEvent(event)
155145
})
156146
}
157-
158-
// CaptureErrorWithComponentConfig captures an error using component-specific Sentry configuration.
159-
// It uses the merged error configuration from component settings, falling back to global config.
160-
func CaptureErrorWithComponentConfig(err error, info *schema.ConfigAndStacksInfo, context map[string]string) {
161-
if err == nil {
162-
return
163-
}
164-
165-
// Get component error configuration.
166-
componentErrorConfig, configErr := GetComponentErrorConfig(info)
167-
if configErr != nil {
168-
// Log error but continue with global config.
169-
if atmosConfig != nil {
170-
componentErrorConfig = &atmosConfig.Errors
171-
} else {
172-
// Can't get any config - use global Sentry hub.
173-
CaptureErrorWithContext(err, context)
174-
return
175-
}
176-
}
177-
178-
// Merge with global config if available.
179-
var finalConfig *schema.ErrorsConfig
180-
if atmosConfig != nil {
181-
finalConfig = MergeErrorConfigs(&atmosConfig.Errors, componentErrorConfig)
182-
} else if componentErrorConfig != nil {
183-
finalConfig = componentErrorConfig
184-
} else {
185-
// No config available - use global hub.
186-
CaptureErrorWithContext(err, context)
187-
return
188-
}
189-
190-
// Get or create Sentry client for this configuration.
191-
hub, hubErr := GetRegistry().GetOrCreateClient(&finalConfig.Sentry)
192-
if hubErr != nil {
193-
// Failed to create client - fall back to global hub.
194-
CaptureErrorWithContext(err, context)
195-
return
196-
}
197-
198-
// If Sentry is disabled for this component, return early.
199-
if hub == nil {
200-
return
201-
}
202-
203-
// Build Sentry report using cockroachdb/errors native support.
204-
event, extraDetails := errors.BuildSentryReport(err)
205-
206-
hub.WithScope(func(scope *sentry.Scope) {
207-
// Add extra details from cockroachdb/errors as context.
208-
for key, value := range extraDetails {
209-
if contextMap, ok := value.(map[string]interface{}); ok {
210-
scope.SetContext(key, contextMap)
211-
}
212-
}
213-
214-
// Set Atmos context as tags with "atmos." prefix.
215-
for key, value := range context {
216-
event.Tags["atmos."+key] = value
217-
}
218-
219-
// Add component identification tags.
220-
if info != nil {
221-
if info.Component != "" {
222-
event.Tags["atmos.component"] = info.Component
223-
}
224-
if info.Stack != "" {
225-
event.Tags["atmos.stack"] = info.Stack
226-
}
227-
if info.ComponentType != "" {
228-
event.Tags["atmos.component_type"] = info.ComponentType
229-
}
230-
}
231-
232-
// Extract and set hints as breadcrumbs.
233-
hints := errors.GetAllHints(err)
234-
for _, hint := range hints {
235-
scope.AddBreadcrumb(&sentry.Breadcrumb{
236-
Type: "info",
237-
Category: "hint",
238-
Message: hint,
239-
Level: sentry.LevelInfo,
240-
}, 100)
241-
}
242-
243-
// Extract exit code if present.
244-
exitCode := GetExitCode(err)
245-
if exitCode != 0 && exitCode != 1 {
246-
event.Tags["atmos.exit_code"] = fmt.Sprintf("%d", exitCode)
247-
}
248-
249-
// Capture the pre-built event.
250-
hub.CaptureEvent(event)
251-
})
252-
}

‎errors/sentry_registry.go‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,6 @@ type SentryClientRegistry struct {
2020
configs map[string]*schema.SentryConfig
2121
}
2222

23-
// globalRegistry is the singleton registry instance.
24-
var globalRegistry = &SentryClientRegistry{
25-
clients: make(map[string]*sentry.Hub),
26-
configs: make(map[string]*schema.SentryConfig),
27-
}
28-
29-
// GetRegistry returns the global Sentry client registry.
30-
func GetRegistry() *SentryClientRegistry {
31-
return globalRegistry
32-
}
33-
3423
// configKey generates a unique key for a Sentry configuration.
3524
// This allows reusing clients for identical configurations.
3625
func configKey(config *schema.SentryConfig) (string, error) {
@@ -138,20 +127,6 @@ func (r *SentryClientRegistry) GetOrCreateClient(config *schema.SentryConfig) (*
138127
return hub, nil
139128
}
140129

141-
// CloseAll flushes and closes all Sentry clients in the registry.
142-
func (r *SentryClientRegistry) CloseAll() {
143-
r.mu.Lock()
144-
defer r.mu.Unlock()
145-
146-
for _, hub := range r.clients {
147-
hub.Flush(CloseSentryTimeout)
148-
}
149-
150-
// Clear the registry.
151-
r.clients = make(map[string]*sentry.Hub)
152-
r.configs = make(map[string]*schema.SentryConfig)
153-
}
154-
155130
// componentErrorConfigWithMetadata holds both the decoded config and metadata about which fields were explicitly set.
156131
type componentErrorConfigWithMetadata struct {
157132
config *schema.ErrorsConfig

‎internal/exec/atlantis_generate_repo_config.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package exec
33
import (
44
"path/filepath"
55
"reflect"
6+
"slices"
67
"strings"
78

89
"github.com/go-viper/mapstructure/v2"
@@ -292,8 +293,7 @@ func ExecuteAtlantisGenerateRepoConfig(
292293

293294
// Check if the 'components' filter is provided
294295
if len(components) == 0 ||
295-
u.SliceContainsString(components, componentName) {
296-
296+
slices.Contains(components, componentName) {
297297
// Component vars
298298
if varsSection, ok = componentSection["vars"].(map[string]any); !ok {
299299
continue
@@ -418,10 +418,10 @@ func ExecuteAtlantisGenerateRepoConfig(
418418
if len(stacks) == 0 ||
419419
// 'stacks' filter can contain the names of the top-level stack config files:
420420
// atmos terraform generate varfiles --stacks=orgs/cp/tenant1/staging/us-east-2,orgs/cp/tenant2/dev/us-east-2
421-
u.SliceContainsString(stacks, stackConfigFileName) ||
421+
slices.Contains(stacks, stackConfigFileName) ||
422422
// 'stacks' filter can also contain the logical stack names (derived from the context vars):
423423
// atmos terraform generate varfiles --stacks=tenant1-ue2-staging,tenant1-ue2-prod
424-
u.SliceContainsString(stacks, stackSlug) {
424+
slices.Contains(stacks, stackSlug) {
425425
// Generate an atlantis project for the component in the stack
426426
// Replace the context tokens
427427
var whenModified []string

‎internal/exec/atmos.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package exec
22

33
import (
44
"fmt"
5+
"slices"
56
"strings"
67

78
"github.com/cloudposse/atmos/pkg/perf"
@@ -71,7 +72,7 @@ func ExecuteAtmosCmd() error {
7172
lo.ForEach(componentsSet, func(c string, _ int) {
7273
var stacksForComponent []string
7374
for k, v := range stacksComponentsMap {
74-
if u.SliceContainsString(v, c) {
75+
if slices.Contains(v, c) {
7576
stacksForComponent = append(stacksForComponent, k)
7677
}
7778
}

‎internal/exec/cli_utils.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"errors"
55
"fmt"
66
"os"
7+
"slices"
78
"strings"
89

910
"github.com/spf13/cobra"
@@ -359,15 +360,15 @@ func parseQuotedCompoundSubcommand(arg string) *compoundSubcommandResult {
359360
return &compoundSubcommandResult{subCommand: cmdWrite, subCommand2: cmdVarfile, argCount: 1}
360361
}
361362
case cmdWorkspace:
362-
if u.SliceContainsString(workspaceSubcommands, second) {
363+
if slices.Contains(workspaceSubcommands, second) {
363364
return &compoundSubcommandResult{subCommand: cmdWorkspace, subCommand2: second, argCount: 1}
364365
}
365366
case cmdState:
366-
if u.SliceContainsString(stateSubcommands, second) {
367+
if slices.Contains(stateSubcommands, second) {
367368
return &compoundSubcommandResult{subCommand: fmt.Sprintf(cmdFmtSpaced, cmdState, second), argCount: 1}
368369
}
369370
case cmdProviders:
370-
if u.SliceContainsString(providersSubcommands, second) {
371+
if slices.Contains(providersSubcommands, second) {
371372
return &compoundSubcommandResult{subCommand: fmt.Sprintf(cmdFmtSpaced, cmdProviders, second), argCount: 1}
372373
}
373374
}
@@ -452,15 +453,15 @@ func parseSeparateCompoundSubcommand(first, second string) *compoundSubcommandRe
452453
return &compoundSubcommandResult{subCommand: cmdWrite, subCommand2: cmdVarfile, argCount: 2}
453454
}
454455
case cmdWorkspace:
455-
if u.SliceContainsString(workspaceSubcommands, second) {
456+
if slices.Contains(workspaceSubcommands, second) {
456457
return &compoundSubcommandResult{subCommand: cmdWorkspace, subCommand2: second, argCount: 2}
457458
}
458459
case cmdState:
459-
if u.SliceContainsString(stateSubcommands, second) {
460+
if slices.Contains(stateSubcommands, second) {
460461
return &compoundSubcommandResult{subCommand: fmt.Sprintf(cmdFmtSpaced, cmdState, second), argCount: 2}
461462
}
462463
case cmdProviders:
463-
if u.SliceContainsString(providersSubcommands, second) {
464+
if slices.Contains(providersSubcommands, second) {
464465
return &compoundSubcommandResult{subCommand: fmt.Sprintf(cmdFmtSpaced, cmdProviders, second), argCount: 2}
465466
}
466467
}

‎internal/exec/describe_stacks_component_processor.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package exec
44
import (
55
"errors"
66
"fmt"
7+
"slices"
78
"strings"
89

910
"github.com/go-viper/mapstructure/v2"
@@ -232,7 +233,7 @@ func (p *describeStacksProcessor) processStackFile(stackFileName string, stackMa
232233
}
233234

234235
for _, te := range typeEntries {
235-
if len(p.componentTypes) > 0 && !u.SliceContainsString(p.componentTypes, te.name) {
236+
if len(p.componentTypes) > 0 && !slices.Contains(p.componentTypes, te.name) {
236237
continue
237238
}
238239
typeSection, ok := componentsSection[te.name].(map[string]any)
@@ -351,8 +352,8 @@ func (p *describeStacksProcessor) processComponentEntry( //nolint:gocognit,reviv
351352
// This check is performed before any mutations to componentSection so that
352353
// the live stacksMap data is not modified for filtered-out components.
353354
componentIncluded := len(p.components) == 0 ||
354-
u.SliceContainsString(p.components, componentName) ||
355-
u.SliceContainsString(derivedComponents, componentName)
355+
slices.Contains(p.components, componentName) ||
356+
slices.Contains(derivedComponents, componentName)
356357
if !componentIncluded {
357358
return nil
358359
}
@@ -647,7 +648,7 @@ func addSectionsToComponentEntry(
647648
continue
648649
}
649650
}
650-
if len(sections) == 0 || u.SliceContainsString(sections, sectionName) {
651+
if len(sections) == 0 || slices.Contains(sections, sectionName) {
651652
destMap[sectionName] = section
652653
}
653654
}

0 commit comments

Comments
 (0)