-
-
Notifications
You must be signed in to change notification settings - Fork 137
Open
Description
Describe the Bug
The template processing logic in internal/exec/stack_processor_utils.go:279 has high complexity (nestif: 6) and required a //nolint:nestif directive to pass linting. This indicates the function has deeply nested conditional logic that should be refactored.
Current State
File: internal/exec/stack_processor_utils.go
Line: 279
Function: Template processing in stack manifest imports
Complexity: 6 (nestif)
//nolint:nestif // Acceptable complexity for error handling with merge context
if !skipTemplatesProcessingInImports && len(context) > 0 {
stackManifestTemplatesProcessed, err = ProcessTmpl(relativeFilePath, stackYamlConfig, context, ignoreMissingTemplateValues)
if err != nil {
if atmosConfig.Logs.Level == u.LogLevelTrace || atmosConfig.Logs.Level == u.LogLevelDebug {
stackManifestTemplatesErrorMessage = fmt.Sprintf("\n\n%s", stackYamlConfig)
}
// Check if we have merge context to provide enhanced error formatting
if mergeContext != nil {
// Wrap the error with the sentinel first to preserve it
wrappedErr := fmt.Errorf("%w: %v", errUtils.ErrInvalidStackManifest, err)
// Then format it with context information
e := mergeContext.FormatError(wrappedErr, fmt.Sprintf("stack manifest '%s'%s", relativeFilePath, stackManifestTemplatesErrorMessage))
return nil, nil, nil, nil, nil, nil, nil, e
} else {
e := fmt.Errorf("%w: stack manifest '%s'\n%v%s", errUtils.ErrInvalidStackManifest, relativeFilePath, err, stackManifestTemplatesErrorMessage)
return nil, nil, nil, nil, nil, nil, nil, e
}
}
}Why This Needs Refactoring
- Testability: Deeply nested logic is harder to unit test in isolation
- Maintainability: Complex conditionals make future modifications error-prone
- Readability: Multiple levels of nesting reduce code clarity
- Code Quality: Linter warnings indicate code smell that should be addressed
Proposed Solution
Refactor the template processing logic into smaller, focused functions:
Example Approach
// processStackManifestTemplate processes Go templates in the imported stack manifest.
func processStackManifestTemplate(
atmosConfig *schema.AtmosConfiguration,
relativeFilePath string,
stackYamlConfig string,
context map[string]any,
ignoreMissingTemplateValues bool,
mergeContext *MergeContext,
) (string, error) {
processed, err := ProcessTmpl(relativeFilePath, stackYamlConfig, context, ignoreMissingTemplateValues)
if err != nil {
return "", formatTemplateError(atmosConfig, relativeFilePath, stackYamlConfig, err, mergeContext)
}
return processed, nil
}
// formatTemplateError formats template processing errors with appropriate context.
func formatTemplateError(
atmosConfig *schema.AtmosConfiguration,
relativeFilePath string,
stackYamlConfig string,
err error,
mergeContext *MergeContext,
) error {
errorMessage := buildTemplateErrorMessage(atmosConfig, stackYamlConfig)
if mergeContext != nil {
return formatErrorWithMergeContext(mergeContext, relativeFilePath, errorMessage, err)
}
return formatErrorWithoutMergeContext(relativeFilePath, errorMessage, err)
}
// buildTemplateErrorMessage constructs the error message based on log level.
func buildTemplateErrorMessage(atmosConfig *schema.AtmosConfiguration, stackYamlConfig string) string {
if atmosConfig.Logs.Level == u.LogLevelTrace || atmosConfig.Logs.Level == u.LogLevelDebug {
return fmt.Sprintf("\n\n%s", stackYamlConfig)
}
return ""
}
// formatErrorWithMergeContext formats error using merge context.
func formatErrorWithMergeContext(
mergeContext *MergeContext,
relativeFilePath string,
errorMessage string,
err error,
) error {
wrappedErr := fmt.Errorf("%w: %v", errUtils.ErrInvalidStackManifest, err)
return mergeContext.FormatError(wrappedErr, fmt.Sprintf("stack manifest '%s'%s", relativeFilePath, errorMessage))
}
// formatErrorWithoutMergeContext formats error without merge context.
func formatErrorWithoutMergeContext(
relativeFilePath string,
errorMessage string,
err error,
) error {
return fmt.Errorf("%w: stack manifest '%s'\n%v%s", errUtils.ErrInvalidStackManifest, relativeFilePath, err, errorMessage)
}Acceptance Criteria
- Remove
//nolint:nestifdirective from line 279 - Refactor template processing into smaller, single-purpose functions
- Each extracted function should have a clear, testable responsibility
- Add unit tests for each new helper function
- Ensure existing tests continue to pass
- golangci-lint passes without suppressions
- Code coverage maintained or improved
Additional Context
This issue was identified during PR #1504 when adding Windows CI path normalization fixes. The nolint directive was added as a temporary measure to unblock the PR, but the underlying complexity issue should be addressed.
Related
- Commit: 7ba97f3
- File:
internal/exec/stack_processor_utils.go - Lines: 279-297
Metadata
Metadata
Assignees
Labels
No labels