chore: Dropping more utils

[yhakbar]

Description

Drops utilities that are supported in the standard library.

TODOs

Read the Gruntwork contribution guidelines.

• I authored this code entirely myself
• I am submitting code based on open source software (e.g. MIT, MPL-2.0, Apache)]
• I am adding or upgrading a dependency or adapted code and confirm it has a compatible open source license
• Update the docs.
• Run the relevant tests successfully, including pre-commit checks.
• Include release notes. If this PR is backward incompatible, include a migration guide.

Release Notes (draft)

Added / Removed / Updated [X].

Migration Guide

Summary by CodeRabbit

• Refactor

  • Consolidated internal utility functions for slice operations including deduplication and merging, leveraging standard library patterns for improved maintainability and consistency.

• Tests

  • Updated test suite to align with refactored utility function signatures.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
terragrunt-docs	Ready	Preview, Comment	Jan 13, 2026 9:48pm

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR refactors utility functions in util/collections.go, removing legacy helpers like RemoveElementFromList, RemoveDuplicatesFromList, and FirstElement, while introducing replacements such as RemoveDuplicates, FirstNonEmpty, and MergeSlices. All call sites are updated to use the new APIs and stdlib alternatives like slices.DeleteFunc. Additionally, utility files util/min.go and util/strings.go are deleted with usages replaced by built-in min and strings.TrimSpace.

Changes

Cohort / File(s)	Summary
Utility function refactoring util/collections.go, util/collections_test.go	Core utility functions added (RemoveDuplicates, FirstNonEmpty, MergeSlices), removed (RemoveElementFromList, FirstElement, RemoveEmptyElements), and signature changes (RemoveDuplicatesKeepLast). Tests updated to reflect new API surface and behavioral changes including sorting behavior.
Utility file deletions util/min.go, util/min_test.go, util/strings.go	Removed Min integer comparison function and CleanString string normalization utility with associated test coverage.
Duplicate removal API migration config/config.go, internal/runner/runnerpool/runner.go, internal/services/catalog/catalog.go, tflint/tflint.go, tf/cliconfig/provider_installation.go	Replaced RemoveDuplicatesFromList with RemoveDuplicates and RemoveDuplicatesFromListKeepLast with RemoveDuplicatesKeepLast across var-file and repository URL deduplication sites.
Element removal refactoring cli/commands/commands.go, config/dependency.go, internal/discovery/discovery.go	Replaced RemoveElementFromList and RemoveSublistFromList calls with slices.DeleteFunc using predicate-based filtering.
First element extraction simplification cli/flags/error_handler.go, internal/strict/controls/deprecated_env_var.go, internal/strict/controls/deprecated_flag_name.go	Replaced FirstElement(RemoveEmptyElements(...)) pattern with direct FirstNonEmpty(...) calls.
Slice merging API update config/errors_block.go	Replaced MergeStringSlices with generic MergeSlices for RetryableErrors and IgnorableErrors merging.
Stdlib function adoption options/options.go, test/integration_debug_test.go	Replaced util.Min with built-in min function and util.CleanString with strings.TrimSpace.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~28 minutes

Possibly related PRs

• chore: Dropping usage of MapToSlice and StringListInsert #5297: Both PRs remove/replace multiple util helpers and transition call sites to stdlib functions like slices and maps packages for consistency.
• chore: Dropping util.JoinPath #5219: Both PRs involve deletions and replacements of utility wrappers in the util/ package, updating their call sites throughout the codebase.

Suggested reviewers

• denis256
• ThisGuyCodes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 76.19% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'chore: Dropping more utils' accurately summarizes the main objective of removing utility functions supported by the standard library, aligning with the changeset.
Description check	✅ Passed	The description is mostly complete with a clear explanation of changes, checked migration/compatibility items, and completed TODOs, though the release notes section lacks specific detail about what was removed.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

🧹 Recent nitpick comments

util/collections.go (1)

75-98: Consider optimizing index updates for large lists.

The current implementation has O(n²) worst-case complexity due to iterating over the entire seen map to update indices after each deletion (lines 86-90). For small lists this is fine, but if this function is used with large slices, consider an alternative approach that avoids the index update loop.

♻️ Alternative approach using backward iteration

 func RemoveDuplicatesKeepLast[S ~[]E, E comparable](list S) S {
-	seen := make(map[E]int, len(list))
-	result := make(S, 0, len(list))
-
-	for _, item := range list {
-		if idx, exists := seen[item]; exists {
-			// Remove the previous occurrence
-			result = slices.Delete(result, idx, idx+1)
-			// Update indices for items that were shifted
-			for k, v := range seen {
-				if v > idx {
-					seen[k] = v - 1
-				}
-			}
-		}
-
-		seen[item] = len(result)
-		result = append(result, item)
-	}
-
-	return result
+	seen := make(map[E]struct{}, len(list))
+	result := make(S, 0, len(list))
+
+	// Iterate backwards to keep last occurrence
+	for i := len(list) - 1; i >= 0; i-- {
+		if _, exists := seen[list[i]]; !exists {
+			seen[list[i]] = struct{}{}
+			result = append(result, list[i])
+		}
+	}
+
+	// Reverse to restore original order
+	slices.Reverse(result)
+	return result
 }

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5c2dc07 and b1af297.

📒 Files selected for processing (19)

• cli/commands/commands.go
• cli/flags/error_handler.go
• config/config.go
• config/dependency.go
• config/errors_block.go
• internal/discovery/discovery.go
• internal/runner/runnerpool/runner.go
• internal/services/catalog/catalog.go
• internal/strict/controls/deprecated_env_var.go
• internal/strict/controls/deprecated_flag_name.go
• options/options.go
• test/integration_debug_test.go
• tf/cliconfig/provider_installation.go
• tflint/tflint.go
• util/collections.go
• util/collections_test.go
• util/min.go
• util/min_test.go
• util/strings.go

💤 Files with no reviewable changes (3)

• util/min.go
• util/min_test.go
• util/strings.go

🧰 Additional context used

📓 Path-based instructions (1)

**/*.go

⚙️ CodeRabbit configuration file

Review the Go code for quality and correctness. Make sure that the Go code follows best practices, is performant, and is easy to understand and maintain.

Files:

• cli/flags/error_handler.go
• internal/strict/controls/deprecated_env_var.go
• cli/commands/commands.go
• internal/discovery/discovery.go
• config/errors_block.go
• test/integration_debug_test.go
• internal/runner/runnerpool/runner.go
• options/options.go
• config/dependency.go
• internal/services/catalog/catalog.go
• config/config.go
• tf/cliconfig/provider_installation.go
• tflint/tflint.go
• internal/strict/controls/deprecated_flag_name.go
• util/collections_test.go
• util/collections.go

🧠 Learnings (3)

📚 Learning: 2025-04-17T13:02:28.098Z

Learnt from: yhakbar
Repo: gruntwork-io/terragrunt PR: 4169
File: cli/commands/hcl/validate/cli.go:29-60
Timestamp: 2025-04-17T13:02:28.098Z
Learning: Avoid shadowing imported packages with local variables in Go code, such as using a variable named `flags` when the `github.com/gruntwork-io/terragrunt/cli/flags` package is imported. Use more specific variable names like `flagSet` instead.

Applied to files:

• cli/flags/error_handler.go
• cli/commands/commands.go
• options/options.go
• internal/strict/controls/deprecated_flag_name.go

📚 Learning: 2025-08-19T16:05:54.723Z

Learnt from: Resonance1584
Repo: gruntwork-io/terragrunt PR: 4683
File: go.mod:86-90
Timestamp: 2025-08-19T16:05:54.723Z
Learning: When analyzing Go module dependencies for removal, always check for both direct imports and API usage across all Go files in the repository, not just a quick search. The github.com/mattn/go-zglob library is used for filesystem walking and glob expansion in multiple Terragrunt files including util/file.go, format commands, and AWS provider patch functionality.

Applied to files:

• internal/discovery/discovery.go

📚 Learning: 2025-02-10T13:36:19.542Z

Learnt from: levkohimins
Repo: gruntwork-io/terragrunt PR: 3723
File: cli/commands/stack/action.go:160-160
Timestamp: 2025-02-10T13:36:19.542Z
Learning: The project uses a custom error package `github.com/gruntwork-io/terragrunt/internal/errors` which provides similar functionality to `fmt.Errorf` but includes stack traces. Prefer using this package's error functions (e.g., `errors.Errorf`, `errors.New`) over the standard library's error handling.

Applied to files:

• tf/cliconfig/provider_installation.go

🧬 Code graph analysis (12)

cli/flags/error_handler.go (1)

util/collections.go (1)

• FirstNonEmpty (101-110)

internal/strict/controls/deprecated_env_var.go (1)

util/collections.go (1)

• FirstNonEmpty (101-110)

cli/commands/commands.go (1)

tf/tf.go (1)

• FlagNameDestroy (61-61)

internal/discovery/discovery.go (1)

util/collections.go (1)

• RemoveDuplicates (55-60)

config/errors_block.go (1)

util/collections.go (1)

• MergeSlices (64-73)

internal/runner/runnerpool/runner.go (2)

util/collections.go (1)

• RemoveDuplicates (55-60)

internal/component/component.go (1)

• Component (35-59)

internal/services/catalog/catalog.go (1)

util/collections.go (1)

• RemoveDuplicates (55-60)

config/config.go (1)

util/collections.go (1)

• RemoveDuplicatesKeepLast (77-98)

tf/cliconfig/provider_installation.go (2)

cli/commands/find/cli.go (2)

• Include (30-30)
• Exclude (29-29)

util/collections.go (2)

• RemoveDuplicates (55-60)
• RemoveDuplicatesKeepLast (77-98)

tflint/tflint.go (1)

util/collections.go (1)

• RemoveDuplicatesKeepLast (77-98)

internal/strict/controls/deprecated_flag_name.go (1)

util/collections.go (1)

• FirstNonEmpty (101-110)

util/collections_test.go (1)

util/collections.go (3)

• RemoveDuplicates (55-60)
• RemoveDuplicatesKeepLast (77-98)
• MergeSlices (64-73)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Pull Request has non-contributor approval

🔇 Additional comments (37)

test/integration_debug_test.go (1)

204-204: LGTM!

Good refactor replacing the custom util.CleanString with the standard library strings.TrimSpace. This correctly trims whitespace before JSON unmarshaling and aligns with the PR goal of using stdlib utilities.

options/options.go (1)

545-546: LGTM!

The replacement of util.Min with Go's built-in min function is correct. The slices package import confirms Go 1.21+ is in use, where the min builtin is available. The logic for avoiding out-of-bounds slice access remains intact.

internal/services/catalog/catalog.go (1)

113-116: LGTM!

The switch to util.RemoveDuplicates is correct. While this function now sorts the result (unlike the previous RemoveDuplicatesFromList), the order of repository URLs being processed in the subsequent loop doesn't affect functionality since each URL is handled independently.

config/dependency.go (1)

436-436: LGTM!

The replacement of the custom util.RemoveElementFromList with slices.DeleteFunc is correct and idiomatic. The predicate-based deletion properly removes the current dependencyPath from the traversal stack during DFS backtracking.

cli/commands/commands.go (1)

288-292: LGTM!

The transformation of apply -destroy to destroy is correctly implemented. Using slices.DeleteFunc to remove the -destroy flag after changing the command to destroy is appropriate—it removes all occurrences of the flag, ensuring no redundant flags remain in the args.

internal/runner/runnerpool/runner.go (2)

727-727: LGTM!

The use of util.RemoveDuplicates for deduplicating dependents is correct. While the result is now sorted, the order of dependent units doesn't affect the transitive dependency computation logic.

---

738-740: LGTM!

The combination of util.RemoveDuplicates for merging dependent lists and slices.DeleteFunc to exclude self-references correctly implements the transitive closure computation for unit dependencies.

util/collections.go (3)

53-60: LGTM!

The RemoveDuplicates implementation is clean and idiomatic—cloning the input to avoid mutation, then using slices.Sort + slices.Compact for efficient deduplication. The cmp.Ordered constraint correctly restricts usage to sortable types.

---

62-73: LGTM!

The MergeSlices implementation correctly handles the nil case from slices.Concat by returning an empty slice instead of nil, which is good defensive practice for callers.

---

100-110: LGTM!

The FirstNonEmpty function is a clean utility that follows Go idioms for generic programming with the comparable constraint.

config/errors_block.go (2)

166-198: LGTM for the IgnoreBlock merge refactor.

The same util.MergeSlices change is applied consistently here. The same sorting consideration from the RetryBlock merge applies.

---

147-147: No actionable issue. The util.MergeSlices function explicitly documents that it sorts the result, which is appropriate for this use case. Error pattern matching via regex does not depend on the order of patterns in the list, making the sorting behavior semantically irrelevant for RetryableErrors and IgnorableErrors.

cli/flags/error_handler.go (1)

25-25: LGTM!

The refactor from util.FirstElement(util.RemoveEmptyElements(flag.Names())) to util.FirstNonEmpty(flag.Names()) is semantically equivalent, cleaner, and avoids an intermediate slice allocation.

tflint/tflint.go (1)

226-226: LGTM!

The rename from util.RemoveDuplicatesFromListKeepLast to util.RemoveDuplicatesKeepLast is a straightforward API consolidation. The "keep last" semantics are preserved, which is correct for var-file ordering where later files should take precedence.

internal/strict/controls/deprecated_flag_name.go (1)

35-36: LGTM!

The refactor to util.FirstNonEmpty is semantically equivalent to the previous util.FirstElement(util.RemoveEmptyElements(...)) pattern and is more concise.

internal/strict/controls/deprecated_env_var.go (1)

36-37: LGTM!

Consistent with the refactor in deprecated_flag_name.go. The util.FirstNonEmpty replacement is semantically equivalent and more concise.

config/config.go (2)

933-934: LGTM!

The transition to util.RemoveDuplicatesKeepLast maintains the correct semantics for var files—duplicates are removed while preserving the last occurrence, ensuring later var files take precedence.

---

940-940: LGTM!

Consistent with the change above for RequiredVarFiles.

internal/discovery/discovery.go (3)

11-11: LGTM!

Standard library slices import for DeleteFunc usage below.

---

1215-1215: LGTM!

The new util.RemoveDuplicates function sorts the result, but this doesn't affect correctness here since the order of dependents in the index map doesn't matter for the graph traversal logic.

---

1242-1243: LGTM!

Clean replacement using stdlib slices.DeleteFunc. The two-step approach (deduplicate then filter out self-references) is clear and correct. The sorting from RemoveDuplicates doesn't impact correctness for transitive dependent propagation.

tf/cliconfig/provider_installation.go (13)

6-6: LGTM!

Standard library import for slices.DeleteFunc and slices.Contains.

---

138-138: Note: Behavior change - output is now sorted.

util.RemoveDuplicates sorts the result, whereas the previous implementation likely preserved insertion order. For provider include patterns, order typically doesn't affect matching semantics, but this is a subtle behavior change worth noting.

---

150-150: Consistent with the AppendInclude change above.

---

158-158: LGTM!

Clean idiomatic use of slices.DeleteFunc with slices.Contains predicate. This is O(n*m) where n is the slice length and m is addrs length, but both are typically small for provider patterns.

---

170-170: LGTM!

Consistent with RemoveExclude.

---

251-251: LGTM!

Consistent with ProviderInstallationDirect.

---

263-263: LGTM!

Consistent pattern.

---

271-271: LGTM!

Consistent use of slices.DeleteFunc.

---

283-283: LGTM!

---

364-364: LGTM!

Consistent with other AppendInclude implementations.

---

376-376: Verify: Intentional use of RemoveDuplicatesKeepLast vs RemoveDuplicates?

This is the only AppendExclude method using RemoveDuplicatesKeepLast (preserves order, keeps last occurrence) while all other AppendInclude/AppendExclude methods use RemoveDuplicates (sorts output). Is this inconsistency intentional for NetworkMirror excludes, or should this also use RemoveDuplicates for consistency?

---

384-384: LGTM!

---

396-396: LGTM!

util/collections_test.go (3)

173-194: LGTM!

Good test coverage for RemoveDuplicates. The test cases correctly verify:

• Empty slice handling
• Single element
• Sorting behavior (line 182: {"foo", "bar"} → {"bar", "foo"})
• Deduplication with sorting

---

196-218: LGTM!

Excellent test coverage for RemoveDuplicatesKeepLast. The test cases clearly demonstrate the "keep last occurrence" semantics:

• Line 206: {"foo", "bar", "foobar", "bar", "foo"} → {"foobar", "bar", "foo"} - duplicates removed, last positions kept
• Line 207: Additional case confirming the behavior

---

220-243: LGTM!

Test cases properly verify MergeSlices behavior including:

• Empty slice handling
• Merging with sorting (line 231: {"foo"} + {"bar"} → {"bar", "foo"})
• Deduplication across merged slices (line 232)

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}