Fix: Generate top-level getters in getter mode #7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
📊 Code Coverage ReportCoverage by file |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the getter mode code generation to create top-level getter functions for simple variables instead of variable declarations, while maintaining variable declarations only for structs and arrays of structs. This provides a consistent interface where all values (simple types and structs) are accessed through function calls that support environment variable overrides.
- Introduced top-level getter functions for primitive types and arrays of primitives
- Refactored common getter body logic into a shared
writeGetterBodymethod - Updated variable declarations to only include structs and arrays of structs
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/generator/struct_gen.go | Implements top-level getter function generation and refactors getter body logic to avoid code duplication |
| internal/generator/generator_test.go | Adds comprehensive test coverage for the new top-level getter functionality |
| example/getter_config/config.go | Demonstrates the change where Name is now a getter function instead of a variable |
| example/gen.go | Simplifies go:generate commands by removing redundant path references |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+424
to
+435
| case []any: | ||
| // Check if it's an array of maps (structs) | ||
| if len(val) > 0 { | ||
| if _, ok := val[0].(map[string]any); ok { | ||
| // Skip array of structs | ||
| continue | ||
| } | ||
| } | ||
| // Generate getter for array of primitives | ||
| if err := g.generateTopLevelGetter(buf, key, value); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Empty arrays of primitives ([]any) are not handled. When len(val) == 0, the code neither generates a getter nor adds a var declaration, causing these arrays to be silently omitted from the generated code. Consider generating a getter for empty arrays or handling them explicitly.
Comment on lines
+459
to
+463
| if len(val) > 0 { | ||
| if _, ok := val[0].(map[string]any); ok { | ||
| structName := sx.CamelCase(key) + "Item" | ||
| fmt.Fprintf(buf, "\t%s []%s\n", varName, structName) | ||
| } |
There was a problem hiding this comment.
Empty arrays of structs ([]any with length 0) are not added to the var block. When an empty array is encountered, no variable declaration is generated, which could cause compilation errors if the code expects this variable to exist. Consider adding a var declaration for empty arrays with a type comment or default struct type.
Suggested change
| if len(val) > 0 { | |
| if _, ok := val[0].(map[string]any); ok { | |
| structName := sx.CamelCase(key) + "Item" | |
| fmt.Fprintf(buf, "\t%s []%s\n", varName, structName) | |
| } | |
| structName := sx.CamelCase(key) + "Item" | |
| if len(val) > 0 { | |
| if _, ok := val[0].(map[string]any); ok { | |
| fmt.Fprintf(buf, "\t%s []%s\n", varName, structName) | |
| } | |
| } else { | |
| // Emit declaration for empty array of structs | |
| fmt.Fprintf(buf, "\t%s []%s\n", varName, structName) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix: Generate top-level getters in getter mode
This PR fixes a bug where top-level variables in getter mode were generated as simple
vardeclarations instead of getter functions, preventing environment variable overrides for top-level configuration values.Changes
writeGetterBody()to share getter logic between struct methods and top-level functionsTestGenerator_GetterMode_TopLevelVariables)getter_config/config.goto demonstrate the fix -Nameis now a function with env override supportExample
Before (incorrect):
After (correct):
Structs and arrays of structs remain as var declarations as intended.