swift: fix initializers with properties named self

When a type has a field `self`, the synthesized initializers need to
avoid using `self` as the internal parameter name, instead permuting it.
By default we pick `_self`, but we also ensure this doesn't conflict
with any other declared properties; in the event there is a conflict, we
append `_` until it's unique.

This makes the test introduced by the previous commit start passing.

Fixes #1530.

Author: lilyball

CHANGELOG.md

@@ -9,6 +9,7 @@
 - `apollo-codegen-scala`
   - <First `apollo-codegen-scala` related entry goes here>
 - `apollo-codegen-swift`
+  - Ensure fields named `self` don't cause compilation errors in the generated code [#1533](https://github.com/apollographql/apollo-tooling/pull/1533)
   - Preserve leading/trailing underscores on field names [#1533](https://github.com/apollographql/apollo-tooling/pull/1533)
 - `apollo-codegen-typescript`
   - <First `apollo-codegen-typescript` related entry goes here>

packages/apollo-codegen-swift/src/codeGeneration.ts

@@ -643,7 +643,9 @@ export class SwiftAPIGenerator extends SwiftGenerator<CompilerContext> {
                 swift`"__typename": ${SwiftSource.string(
                   variant.possibleTypes[0].toString()
                 )}`,
-                ...properties.map(this.propertyAssignmentForField, this)
+                ...properties.map(p =>
+                  this.propertyAssignmentForField(p, properties)
+                )
               ],
               ", "
             ) || ":",
@@ -683,7 +685,9 @@ export class SwiftAPIGenerator extends SwiftGenerator<CompilerContext> {
                   swift`"__typename": ${SwiftSource.string(
                     possibleType.toString()
                   )}`,
-                  ...properties.map(this.propertyAssignmentForField, this)
+                  ...properties.map(p =>
+                    this.propertyAssignmentForField(p, properties)
+                  )
                 ],
                 ", "
               ) || ":",
@@ -695,30 +699,37 @@ export class SwiftAPIGenerator extends SwiftGenerator<CompilerContext> {
     }
   }
 
-  propertyAssignmentForField(field: {
-    responseKey: string;
-    propertyName: string;
-    type: GraphQLType;
-    isConditional?: boolean;
-    structName?: string;
-  }): SwiftSource {
+  propertyAssignmentForField(
+    field: {
+      responseKey: string;
+      propertyName: string;
+      type: GraphQLType;
+      isConditional?: boolean;
+      structName?: string;
+    },
+    properties: { propertyName: string }[]
+  ): SwiftSource {
     const {
       responseKey,
       propertyName,
       type,
       isConditional,
       structName
     } = field;
+    const parameterName = this.helpers.internalParameterName(
+      propertyName,
+      properties
+    );
     const valueExpression = isCompositeType(getNamedType(type))
       ? this.helpers.mapExpressionForType(
           type,
           isConditional,
           expression => swift`${expression}.resultMap`,
-          SwiftSource.identifier(propertyName),
+          SwiftSource.identifier(parameterName),
           structName!,
           "ResultMap"
         )
-      : SwiftSource.identifier(propertyName);
+      : SwiftSource.identifier(parameterName);
     return swift`${SwiftSource.string(responseKey)}: ${valueExpression}`;
   }
 
@@ -882,7 +893,12 @@ export class SwiftAPIGenerator extends SwiftGenerator<CompilerContext> {
 
     this.withinBlock(() => {
       properties.forEach(({ propertyName }) => {
-        this.printOnNewline(swift`self.${propertyName} = ${propertyName}`);
+        this.printOnNewline(
+          swift`self.${propertyName} = ${this.helpers.internalParameterName(
+            propertyName,
+            properties
+          )}`
+        );
       });
     });
   }
@@ -891,12 +907,20 @@ export class SwiftAPIGenerator extends SwiftGenerator<CompilerContext> {
     this.print(swift`(`);
     this.print(
       join(
-        properties.map(({ propertyName, typeName, isOptional }) =>
-          join([
-            swift`${propertyName}: ${typeName}`,
+        properties.map(({ propertyName, typeName, isOptional }) => {
+          const internalName = this.helpers.internalParameterName(
+            propertyName,
+            properties
+          );
+          const decl =
+            internalName === propertyName
+              ? propertyName
+              : swift`${propertyName} ${internalName}`;
+          return join([
+            swift`${decl}: ${typeName}`,
             isOptional ? swift` = nil` : undefined
-          ])
-        ),
+          ]);
+        }),
         ", "
       )
     );
@@ -1168,12 +1192,20 @@ export class SwiftAPIGenerator extends SwiftGenerator<CompilerContext> {
         this.print(swift`(`);
         this.print(
           join(
-            properties.map(({ propertyName, typeName, isOptional }) =>
-              join([
-                swift`${propertyName}: ${typeName}`,
+            properties.map(({ propertyName, typeName, isOptional }) => {
+              const internalName = this.helpers.internalParameterName(
+                propertyName,
+                properties
+              );
+              const decl =
+                internalName === propertyName
+                  ? propertyName
+                  : swift`${propertyName} ${internalName}`;
+              return join([
+                swift`${decl}: ${typeName}`,
                 isOptional ? swift` = nil` : undefined
-              ])
-            ),
+              ]);
+            }),
             ", "
           )
         );
@@ -1186,7 +1218,12 @@ export class SwiftAPIGenerator extends SwiftGenerator<CompilerContext> {
               join(
                 properties.map(
                   ({ name, propertyName }) =>
-                    swift`${SwiftSource.string(name)}: ${propertyName}`
+                    swift`${SwiftSource.string(
+                      name
+                    )}: ${this.helpers.internalParameterName(
+                      propertyName,
+                      properties
+                    )}`
                 ),
                 ", "
               ) || ":",

packages/apollo-codegen-swift/src/helpers.ts

@@ -135,6 +135,24 @@ export class Helpers {
     );
   }
 
+  /**
+   * Returns the internal parameter name for a given property name.
+   *
+   * If the property name is valid to use, it's returned directly. Otherwise it's prefixed with an
+   * underscore and modified until it's unique among the given property set.
+   * @param propertyName The name of the property.
+   * @param properties A list of properties that should be consulted when producing a unique name.
+   * @returns The name to use for the internal parameter name for the property.
+   */
+  internalParameterName(
+    propertyName: string,
+    properties: { propertyName: string }[]
+  ): string {
+    return SwiftSource.isValidParameterName(propertyName)
+      ? propertyName
+      : makeUniqueName(`_${propertyName}`, properties);
+  }
+
   // Properties
 
   propertyFromField(
@@ -351,6 +369,25 @@ function makeClosureSignature(
   return closureSignature;
 }
 
+/**
+ * Takes a proposed name and modifies it to be unique given a list of properties.
+ * @param proposedName The proposed name that shouldn't conflict with any property.
+ * @param properties A list of properties the name shouldn't conflict with.
+ * @returns A name based on `proposedName` that doesn't match any existing property name.
+ */
+function makeUniqueName(
+  proposedName: string,
+  properties: { propertyName: string }[]
+): string {
+  // Assume conflicts are very rare and property lists are short, and just do a linear search. If
+  // we find a conflict, start over with the modified name.
+  for (let name = proposedName; ; name += "_") {
+    if (properties.every(prop => prop.propertyName != name)) {
+      return name;
+    }
+  }
+}
+
 /**
  * Converts a value from "underscore_case" to "camelCase".
  *

packages/apollo-codegen-swift/src/language.ts

@@ -215,6 +215,21 @@ export class SwiftSource {
     );
   }
 
+  /**
+   * Returns whether the given name is valid as a method parameter name.
+   *
+   * Certain tokens aren't valid as method parameter names, even when escaped with backticks, as
+   * the compiler interprets the keyword and identifier as the same thing. In particular, `self`
+   * works this way.
+   * @param input The proposed parameter name.
+   * @returns `true` if the name can be used, or `false` if it needs a separate internal parameter
+   * name.
+   */
+  static isValidParameterName(input: string): boolean {
+    // Right now `self` is the only known token that we can't use with escaping.
+    return input !== "self";
+  }
+
   /**
    * Template tag for producing a `SwiftSource` value without performing escaping.
    *