Embedded Structs in Code Generation #1909
Conversation
WalkthroughThe changes update several function signatures and type definitions. In the Go code generation packages, functions such as GenerateMetaMapConsts and generateTSTypeInternal now include an extra boolean parameter, which alters behavior when processing embedded structs. Calls to these functions have been updated accordingly to pass a default value (false) in non-embedded contexts. In the frontend TypeScript definitions, additional optional properties have been added to telemetry types to capture client and location information. Furthermore, a new type for AI configuration (AiSettingsType) has been introduced and embedded into the existing SettingsType, expanding the configuration structure. These modifications adjust how constants and type declarations are generated and enrich the type definitions with new fields without changing the underlying core logic. Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/gogen/gogen.go (1)
36-39: LGTM! Well-structured handling of embedded types.The implementation correctly handles embedded structs by:
- Conditionally wrapping const declarations
- Recursively processing anonymous fields
- Preventing duplicate declarations for embedded types
Consider adding a doc comment to explain the
embeddedparameter's purpose:+// GenerateMetaMapConsts generates constant declarations for struct field names. +// The embedded parameter controls whether the type is being processed as an embedded struct, +// affecting the generation of const declaration blocks. func GenerateMetaMapConsts(buf *strings.Builder, constPrefix string, rtype reflect.Type, embedded bool) {Also applies to: 47-52, 69-71
frontend/types/gotypes.d.ts (1)
809-818: Consider using type composition to avoid property duplication.The same properties are duplicated between
TEventPropsandTEventUserProps. Consider extracting these common properties into a shared interface to follow the DRY principle.+type TelemetryCommonProps = { + "client:arch"?: string; + "client:version"?: string; + "client:initial_version"?: string; + "client:buildtime"?: string; + "client:osrelease"?: string; + "client:isdev"?: boolean; + "autoupdate:channel"?: string; + "autoupdate:enabled"?: boolean; + "loc:countrycode"?: string; + "loc:regioncode"?: string; +}; -type TEventProps = { +type TEventProps = TelemetryCommonProps & { - "client:arch"?: string; - "client:version"?: string; - "client:initial_version"?: string; - "client:buildtime"?: string; - "client:osrelease"?: string; - "client:isdev"?: boolean; - "autoupdate:channel"?: string; - "autoupdate:enabled"?: boolean; - "loc:countrycode"?: string; - "loc:regioncode"?: string; // ... rest of the properties }; -type TEventUserProps = { +type TEventUserProps = TelemetryCommonProps & { - "client:arch"?: string; - "client:version"?: string; - "client:initial_version"?: string; - "client:buildtime"?: string; - "client:osrelease"?: string; - "client:isdev"?: boolean; - "autoupdate:channel"?: string; - "autoupdate:enabled"?: boolean; - "loc:countrycode"?: string; - "loc:regioncode"?: string; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cmd/generatego/main-generatego.go(2 hunks)frontend/types/gotypes.d.ts(1 hunks)pkg/gogen/gogen.go(2 hunks)pkg/tsgen/tsgen.go(3 hunks)pkg/wconfig/metaconsts.go(0 hunks)pkg/wconfig/settingsconfig.go(2 hunks)
💤 Files with no reviewable changes (1)
- pkg/wconfig/metaconsts.go
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (go)
- GitHub Check: Build for TestDriver.ai
🔇 Additional comments (5)
cmd/generatego/main-generatego.go (1)
59-59: LGTM! Function calls updated correctly.The calls to
gogen.GenerateMetaMapConstshave been properly updated to include the newembeddedparameter, withfalseas the default value for top-level type generation.Also applies to: 72-72
pkg/tsgen/tsgen.go (1)
179-179: LGTM! Consistent implementation with gogen.go.The TypeScript type generation correctly handles embedded types using the same pattern as
gogen.go:
- Conditionally wrapping type declarations
- Recursively processing anonymous fields
- Preventing duplicate declarations for embedded types
Also applies to: 186-194, 201-206, 236-238
pkg/wconfig/settingsconfig.go (2)
35-49: LGTM! Well-structured AI settings type.The new
AiSettingsTypefollows good practices:
- Consistent field naming convention
- Proper use of
omitemptytags- Clear JSON tag prefixes with
ai:
51-57: LGTM! Clean embedding of AI settings.The
AiSettingsTypeis properly embedded inSettingsType, which aligns with the PR's objective of improving struct embedding.frontend/types/gotypes.d.ts (1)
771-780: LGTM! The new telemetry properties follow consistent naming and typing patterns.The new properties are well-organized into logical groups (client info, autoupdate settings, location data) and use appropriate types for their purposes.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/gogen/gogen.go (1)
37-41: Well-structured implementation for embedded struct handling!The changes effectively handle embedded structs by:
- Conditionally wrapping const blocks only for top-level types
- Recursively processing embedded fields while maintaining proper formatting
- Using an efficient string builder approach for embedded content
The implementation is clean, efficient, and aligns perfectly with the PR objectives.
Consider adding a comment explaining the embedded struct handling behavior for future maintainers, as this is a significant architectural feature.
Also applies to: 49-54, 71-73
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/gogen/gogen.go(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: Build for TestDriver.ai
🔇 Additional comments (1)
pkg/gogen/gogen.go (1)
36-36: LGTM! Clear and purposeful signature update.The addition of the
embedded boolparameter is well-aligned with the PR's objective to support embedded struct handling.
This allows code generation to properly embed structs when embedded in the original types. It affects the generation of
gotypes.d.tsandmetaconsts.go.Additionally, the
AiSettingsTypehas been split off and embedded into the originalSettingsTypeto make schema generation easier.