Skip to content

Conversation

@clinta
Copy link
Contributor

@clinta clinta commented Apr 13, 2025

This builds on top of #376 by adding a configuration option to automatically convert all snake_case type and field names to CamelCase.

This PR is a draft until #376 is merged.

I have:

  • Written a clear PR title and description (above)
  • Signed the Khan Academy CLA
  • Added tests covering my changes, if applicable
  • Included a link to the issue fixed, if applicable
  • Included documentation, for new features
  • Added an entry to the changelog

Copy link
Collaborator

@benjaminjkraft benjaminjkraft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, this looks pretty good. The one thing I'd like to have is for it to integrate with existing config better -- the goal being to avoid the situation where we have a bunch of overlapping but distinct config options in the future. I think that won't be too much work; it's mostly just plumbing things differently I suspect. But if it turns out to be too much trouble we could do a subset of that for now.

# - All original names are preserved in the GraphQL request
#
# Defaults to false.
auto_camel_case: false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's put this under casing below? I think the thing that makes sense is to add a casing.default with the same values that controls what we do "everywhere", and then support auto_camel_case as a value for each. (I'm guessing that's not too much work?) So it would look like

casing:
  default: auto_camel_case

for what you have, but then someone could also do

casing:
  default: auto_camel_case
  all_enums: raw

or whatever and have them work together correctly

Comment on lines 174 to 178
goName := arg.Variable
if g.Config.AutoCamelCase {
goName = snakeToCamel(goName)
}
goName = upperFirst(goName)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may be worth breaking this into a method now? especially since that will make it easy to add support for default: raw as I describe in the config

true,
}, {
// With nested snake_case fields
"ServiceIPsObjectSnake_case_fieldSnake_case_type",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are very convincing!

clinta added 6 commits April 13, 2025 18:20
This implements a feature for automatically converting snake_case field names to camelCase in Go structs. This is especially useful when working with GraphQL APIs that use snake_case field naming.

The feature works by:
1. Adding an auto_camel_case boolean option to Config
2. Adding a snakeToCamel function that converts snake_case to camelCase
3. Applying the conversion to field names before applying upperFirst
4. Adding full documentation and tests

The original field names are preserved in the GraphQL query.
This extends the auto_camel_case feature to apply to GraphQL type names in addition to field names. With this change, GraphQL types using snake_case (like 'user_profile') will be automatically converted to camelCase and then capitalized for Go (like 'UserProfile').

The feature works by:
1. Extending the existing auto_camel_case functionality to type names
2. Adding the snakeToCamel conversion to type names in various places:
   - In typeNameParts to handle nested type paths
   - In InputObject and Enum type name generation
3. Updating documentation and adding test cases with snake_case type names

The original type names are preserved in the GraphQL query for server compatibility.
This commit fixes an issue with the auto_camel_case configuration option
where snake_case wasn't being properly converted in nested field paths.
For example, with a query containing `object { snake_case_field { ... } }`,
the generated type names would incorrectly include underscores like
`ObjectSnake_case_fieldType` instead of `ObjectSnakeCaseFieldType`.

The fix applies the snakeToCamel conversion to field aliases in the nextPrefix
function, which affects how type names are constructed from nested field paths.
Added comprehensive tests to verify snake_case conversion works correctly
for both simple and nested path components.
- Remove auto_camel_case from Config struct
- Add auto_camel_case as a CasingAlgorithm
- Add Default field to Casing struct
- Add IsAutoCamelCase() helper method
- Use the same casing system for enum values and all types/fields
- Update test fixtures and documentation
- Add ApplyCasing utility function to process strings with a single casing algorithm
- Change function signatures to use CasingAlgorithm instead of boolean flags
- Add GetDefaultCasingAlgorithm to Config to simplify casing algorithm usage
- Update all code to use the unified casing approach
@clinta
Copy link
Contributor Author

clinta commented Apr 13, 2025

Take a look at this, I've tired to refactor to a more unified approach of applying casing algorithms. If this looks good, I can squash commits before you merge.

@benjaminjkraft benjaminjkraft marked this pull request as ready for review April 14, 2025 04:41
@benjaminjkraft
Copy link
Collaborator

This is great, thank you! I already squash when merging normally so it should be fine.

@benjaminjkraft benjaminjkraft merged commit e2e0ef0 into Khan:main Apr 14, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants