Skip to content

Commit 0ca325e

Browse files
lilyballlilyball
committed
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.
1 parent 3ce759e commit 0ca325e

4 files changed

Lines changed: 113 additions & 23 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
- `apollo-codegen-scala`
1212
- <First `apollo-codegen-scala` related entry goes here>
1313
- `apollo-codegen-swift`
14+
- Ensure fields named `self` don't cause compilation errors in the generated code [#1533](https://github.com/apollographql/apollo-tooling/pull/1533)
1415
- Preserve leading/trailing underscores on field names [#1533](https://github.com/apollographql/apollo-tooling/pull/1533)
1516
- `apollo-codegen-typescript`
1617
- <First `apollo-codegen-typescript` related entry goes here>

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

Lines changed: 60 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,9 @@ export class SwiftAPIGenerator extends SwiftGenerator<CompilerContext> {
624624
swift`"__typename": ${SwiftSource.string(
625625
variant.possibleTypes[0].toString()
626626
)}`,
627-
...properties.map(this.propertyAssignmentForField, this)
627+
...properties.map(p =>
628+
this.propertyAssignmentForField(p, properties)
629+
)
628630
],
629631
", "
630632
) || ":",
@@ -664,7 +666,9 @@ export class SwiftAPIGenerator extends SwiftGenerator<CompilerContext> {
664666
swift`"__typename": ${SwiftSource.string(
665667
possibleType.toString()
666668
)}`,
667-
...properties.map(this.propertyAssignmentForField, this)
669+
...properties.map(p =>
670+
this.propertyAssignmentForField(p, properties)
671+
)
668672
],
669673
", "
670674
) || ":",
@@ -676,30 +680,37 @@ export class SwiftAPIGenerator extends SwiftGenerator<CompilerContext> {
676680
}
677681
}
678682

679-
propertyAssignmentForField(field: {
680-
responseKey: string;
681-
propertyName: string;
682-
type: GraphQLType;
683-
isConditional?: boolean;
684-
structName?: string;
685-
}): SwiftSource {
683+
propertyAssignmentForField(
684+
field: {
685+
responseKey: string;
686+
propertyName: string;
687+
type: GraphQLType;
688+
isConditional?: boolean;
689+
structName?: string;
690+
},
691+
properties: { propertyName: string }[]
692+
): SwiftSource {
686693
const {
687694
responseKey,
688695
propertyName,
689696
type,
690697
isConditional,
691698
structName
692699
} = field;
700+
const parameterName = this.helpers.internalParameterName(
701+
propertyName,
702+
properties
703+
);
693704
const valueExpression = isCompositeType(getNamedType(type))
694705
? this.helpers.mapExpressionForType(
695706
type,
696707
isConditional,
697708
expression => swift`${expression}.resultMap`,
698-
SwiftSource.identifier(propertyName),
709+
SwiftSource.identifier(parameterName),
699710
structName!,
700711
"ResultMap"
701712
)
702-
: SwiftSource.identifier(propertyName);
713+
: SwiftSource.identifier(parameterName);
703714
return swift`${SwiftSource.string(responseKey)}: ${valueExpression}`;
704715
}
705716

@@ -863,7 +874,12 @@ export class SwiftAPIGenerator extends SwiftGenerator<CompilerContext> {
863874

864875
this.withinBlock(() => {
865876
properties.forEach(({ propertyName }) => {
866-
this.printOnNewline(swift`self.${propertyName} = ${propertyName}`);
877+
this.printOnNewline(
878+
swift`self.${propertyName} = ${this.helpers.internalParameterName(
879+
propertyName,
880+
properties
881+
)}`
882+
);
867883
});
868884
});
869885
}
@@ -872,12 +888,20 @@ export class SwiftAPIGenerator extends SwiftGenerator<CompilerContext> {
872888
this.print(swift`(`);
873889
this.print(
874890
join(
875-
properties.map(({ propertyName, typeName, isOptional }) =>
876-
join([
877-
swift`${propertyName}: ${typeName}`,
891+
properties.map(({ propertyName, typeName, isOptional }) => {
892+
const internalName = this.helpers.internalParameterName(
893+
propertyName,
894+
properties
895+
);
896+
const decl =
897+
internalName === propertyName
898+
? propertyName
899+
: swift`${propertyName} ${internalName}`;
900+
return join([
901+
swift`${decl}: ${typeName}`,
878902
isOptional ? swift` = nil` : undefined
879-
])
880-
),
903+
]);
904+
}),
881905
", "
882906
)
883907
);
@@ -1149,12 +1173,20 @@ export class SwiftAPIGenerator extends SwiftGenerator<CompilerContext> {
11491173
this.print(swift`(`);
11501174
this.print(
11511175
join(
1152-
properties.map(({ propertyName, typeName, isOptional }) =>
1153-
join([
1154-
swift`${propertyName}: ${typeName}`,
1176+
properties.map(({ propertyName, typeName, isOptional }) => {
1177+
const internalName = this.helpers.internalParameterName(
1178+
propertyName,
1179+
properties
1180+
);
1181+
const decl =
1182+
internalName === propertyName
1183+
? propertyName
1184+
: swift`${propertyName} ${internalName}`;
1185+
return join([
1186+
swift`${decl}: ${typeName}`,
11551187
isOptional ? swift` = nil` : undefined
1156-
])
1157-
),
1188+
]);
1189+
}),
11581190
", "
11591191
)
11601192
);
@@ -1167,7 +1199,12 @@ export class SwiftAPIGenerator extends SwiftGenerator<CompilerContext> {
11671199
join(
11681200
properties.map(
11691201
({ name, propertyName }) =>
1170-
swift`${SwiftSource.string(name)}: ${propertyName}`
1202+
swift`${SwiftSource.string(
1203+
name
1204+
)}: ${this.helpers.internalParameterName(
1205+
propertyName,
1206+
properties
1207+
)}`
11711208
),
11721209
", "
11731210
) || ":",

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,24 @@ export class Helpers {
135135
);
136136
}
137137

138+
/**
139+
* Returns the internal parameter name for a given property name.
140+
*
141+
* If the property name is valid to use, it's returned directly. Otherwise it's prefixed with an
142+
* underscore and modified until it's unique among the given property set.
143+
* @param propertyName The name of the property.
144+
* @param properties A list of properties that should be consulted when producing a unique name.
145+
* @returns The name to use for the internal parameter name for the property.
146+
*/
147+
internalParameterName(
148+
propertyName: string,
149+
properties: { propertyName: string }[]
150+
): string {
151+
return SwiftSource.isValidParameterName(propertyName)
152+
? propertyName
153+
: makeUniqueName(`_${propertyName}`, properties);
154+
}
155+
138156
// Properties
139157

140158
propertyFromField(
@@ -351,6 +369,25 @@ function makeClosureSignature(
351369
return closureSignature;
352370
}
353371

372+
/**
373+
* Takes a proposed name and modifies it to be unique given a list of properties.
374+
* @param proposedName The proposed name that shouldn't conflict with any property.
375+
* @param properties A list of properties the name shouldn't conflict with.
376+
* @returns A name based on `proposedName` that doesn't match any existing property name.
377+
*/
378+
function makeUniqueName(
379+
proposedName: string,
380+
properties: { propertyName: string }[]
381+
): string {
382+
// Assume conflicts are very rare and property lists are short, and just do a linear search. If
383+
// we find a conflict, start over with the modified name.
384+
for (let name = proposedName; ; name += "_") {
385+
if (properties.every(prop => prop.propertyName != name)) {
386+
return name;
387+
}
388+
}
389+
}
390+
354391
/**
355392
* Converts a value from "underscore_case" to "camelCase".
356393
*

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,21 @@ export class SwiftSource {
177177
);
178178
}
179179

180+
/**
181+
* Returns whether the given name is valid as a method parameter name.
182+
*
183+
* Certain tokens aren't valid as method parameter names, even when escaped with backticks, as
184+
* the compiler interprets the keyword and identifier as the same thing. In particular, `self`
185+
* works this way.
186+
* @param input The proposed parameter name.
187+
* @returns `true` if the name can be used, or `false` if it needs a separate internal parameter
188+
* name.
189+
*/
190+
static isValidParameterName(input: string): boolean {
191+
// Right now `self` is the only known token that we can't use with escaping.
192+
return input !== "self";
193+
}
194+
180195
/**
181196
* Template tag for producing a `SwiftSource` value without performing escaping.
182197
*

0 commit comments

Comments
 (0)
⚡