apollographql / apollo-ios

📱  A strongly-typed, caching GraphQL client for iOS, written in Swift.
https://www.apollographql.com/docs/ios/
MIT License
3.87k stars 717 forks source link

`SelectionSet` Generated Initializers Don't Compile with `self` Parameter #3330

Closed grantjbutler closed 7 months ago

grantjbutler commented 8 months ago

Summary

If a schema has a field named self, and you turn on the selectionSetInitializers option for codegen, the code that's generated will not compile, as the parameter generated for the self field in the initializer will take precedence over the self keyword that refers to the instance that conforms to SelectionSet that's being created. The compiler emits the error Incorrect argument label in call (have '_dataDict:', expected '_fieldData:') for the call to self.init(_dataDict: ...) that's generated.

Version

1.8

Steps to reproduce the behavior

  1. Have a schema with some model with a field named self, and reference that field in the structure of a response to query in some way.
  2. Turn on the selectionSetInitializers option for code gen, to generate initializers for all data types from the schema.
  3. Run the code generator.
  4. Build the target with the generated code.

Logs

No response

Anything else?

The code generator that shipped with the 0.x set of releases properly handled this scenario, if you're looking for a reference implementation to adapt. That code would generate the parameter as _self while keeping the argument label as self to keep the naming correct.

parametersForProperties created the list of arguments for a generated initializer. To determine what to call the parameters, it would call internalParameterName for each argument, which in turn would call isValidParameterName to check to see if using the field's name would cause issues and would need special handling. It's this function that says "if the field is named self, then that's going to cause issues", with would tell internalParameterName to prefix the name of the parameter with an underscore to avoid conflicts.

calvincestari commented 8 months ago

Thanks for raising the issue @grantjbutler, it looks like a particularly tricky one to resolve. 🤔

calvincestari commented 7 months ago

@grantjbutler I believe I have a solution for this but it won't make it into the release today. We'll issue a patch release after that to include this fix.

grantjbutler commented 7 months ago

Sounds good. Thanks for looking into this so quickly and coming up with a fix for this bug!

github-actions[bot] commented 7 months ago

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo iOS usage and allow us to serve you better.

calvincestari commented 7 months ago

@grantjbutler - this is fixed and merged. It's available in the main branch if you can target that otherwise it'll go out in the next release.