Skip to content

Commit

Permalink
swift: Revamp source printing to ensure escaping everywhere
Browse files Browse the repository at this point in the history
The source generator now operates using a string wrapper type called
`SwiftSource`. Identifiers are escaped when converting into
`SwiftSource` values, with an alternative method for producing escaped
string literals. An escape hatch is provided for disabling escapes, but
the default behavior is to escape all identifiers in the dynamic input.

Most `SwiftSource` values are produced by a tagged template literal,
which allows for easy mixing of string literals containing Swift
keywords and dynamic input that needs escaping.

As part of this, rewrite string escaping such that it actually escapes
string contents properly. This required fixing a test that had a bad
spec enforcing broken behavior.

This commit makes the test added earlier pass.

Fixes apollographql/apollo-ios#193.
Fixes apollographql/apollo-ios#752.
  • Loading branch information
lilyball committed Sep 10, 2019
1 parent fb2e573 commit f295ac3
Show file tree
Hide file tree
Showing 7 changed files with 689 additions and 292 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
- Replace instanceof checks with their respective predicates [#1518](https://github.com/apollographql/apollo-tooling/pull/1518)
- `apollo-codegen-swift`
- Replace instanceof checks with their respective predicates [#1518](https://github.com/apollographql/apollo-tooling/pull/1518)
- Ensure types and strings are properly escaped in all generated code [#1515](https://github.com/apollographql/apollo-tooling/pull/1515)
- `apollo-codegen-typescript`
- Replace instanceof checks with their respective predicates [#1518](https://github.com/apollographql/apollo-tooling/pull/1518)
- `apollo-env`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ exports[`Swift code generation #classDeclarationForOperation() should correctly
/// }
/// }
public let operationDefinition =
\\"mutation CreateReview($episode: Episode) {\\\\n createReview(episode: $episode, review: {stars: 5, commentary: \\\\\\"\\\\\\"\\\\\\"\\\\n Wow!\\\\n I thought\\\\n This movie \\\\ ROCKED!\\\\n \\\\\\"\\\\\\"\\\\\\"}) {\\\\n stars\\\\n commentary\\\\n }\\\\n}\\"
\\"mutation CreateReview($episode: Episode) {\\\\n createReview(episode: $episode, review: {stars: 5, commentary: \\\\\\"\\\\\\"\\\\\\"\\\\n Wow!\\\\n I thought\\\\n This movie \\\\\\\\ ROCKED!\\\\n \\\\\\"\\\\\\"\\\\\\"}) {\\\\n stars\\\\n commentary\\\\n }\\\\n}\\"

public let operationName = \\"CreateReview\\"

Expand Down
26 changes: 13 additions & 13 deletions packages/apollo-codegen-swift/src/__tests__/codeGeneration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ describe("Swift code generation", () => {
responseKey: "response_key",
propertyName: "propertyName",
type: GraphQLString
})
}).source
).toBe('"response_key": propertyName');
});

Expand All @@ -232,7 +232,7 @@ describe("Swift code generation", () => {
responseKey: "response_key",
propertyName: "propertyName",
type: new GraphQLNonNull(GraphQLString)
})
}).source
).toBe('"response_key": propertyName');
});

Expand All @@ -242,7 +242,7 @@ describe("Swift code generation", () => {
responseKey: "response_key",
propertyName: "propertyName",
type: new GraphQLList(GraphQLString)
})
}).source
).toBe('"response_key": propertyName');
});

Expand All @@ -252,7 +252,7 @@ describe("Swift code generation", () => {
responseKey: "response_key",
propertyName: "propertyName",
type: new GraphQLList(new GraphQLNonNull(GraphQLString))
})
}).source
).toBe('"response_key": propertyName');
});

Expand All @@ -262,7 +262,7 @@ describe("Swift code generation", () => {
responseKey: "response_key",
propertyName: "propertyName",
type: new GraphQLNonNull(new GraphQLList(GraphQLString))
})
}).source
).toBe('"response_key": propertyName');
});

Expand All @@ -274,7 +274,7 @@ describe("Swift code generation", () => {
type: new GraphQLNonNull(
new GraphQLList(new GraphQLNonNull(GraphQLString))
)
})
}).source
).toBe('"response_key": propertyName');
});

Expand All @@ -284,7 +284,7 @@ describe("Swift code generation", () => {
responseKey: "response_key",
propertyName: "propertyName",
type: schema.getType("Droid")
})
}).source
).toBe(
'"response_key": propertyName.flatMap { (value: Droid) -> ResultMap in value.resultMap }'
);
Expand All @@ -296,7 +296,7 @@ describe("Swift code generation", () => {
responseKey: "response_key",
propertyName: "propertyName",
type: new GraphQLNonNull(schema.getType("Droid"))
})
}).source
).toBe('"response_key": propertyName.resultMap');
});

Expand All @@ -306,7 +306,7 @@ describe("Swift code generation", () => {
responseKey: "response_key",
propertyName: "propertyName",
type: new GraphQLList(schema.getType("Droid"))
})
}).source
).toBe(
'"response_key": propertyName.flatMap { (value: [Droid?]) -> [ResultMap?] in value.map { (value: Droid?) -> ResultMap? in value.flatMap { (value: Droid) -> ResultMap in value.resultMap } } }'
);
Expand All @@ -318,7 +318,7 @@ describe("Swift code generation", () => {
responseKey: "response_key",
propertyName: "propertyName",
type: new GraphQLList(new GraphQLNonNull(schema.getType("Droid")))
})
}).source
).toBe(
'"response_key": propertyName.flatMap { (value: [Droid]) -> [ResultMap] in value.map { (value: Droid) -> ResultMap in value.resultMap } }'
);
Expand All @@ -330,7 +330,7 @@ describe("Swift code generation", () => {
responseKey: "response_key",
propertyName: "propertyName",
type: new GraphQLNonNull(new GraphQLList(schema.getType("Droid")))
})
}).source
).toBe(
'"response_key": propertyName.map { (value: Droid?) -> ResultMap? in value.flatMap { (value: Droid) -> ResultMap in value.resultMap } }'
);
Expand All @@ -344,7 +344,7 @@ describe("Swift code generation", () => {
type: new GraphQLNonNull(
new GraphQLList(new GraphQLNonNull(schema.getType("Droid")))
)
})
}).source
).toBe(
'"response_key": propertyName.map { (value: Droid) -> ResultMap in value.resultMap }'
);
Expand Down Expand Up @@ -664,7 +664,7 @@ describe("Swift code generation", () => {
.selectionSet.selections[0] as Field).args as Argument[];
const dictionaryLiteral = generator.helpers.dictionaryLiteralForFieldArguments(
fieldArguments
);
).source;

expect(dictionaryLiteral).toBe(
'["episode": "JEDI", "review": ["stars": 2, "commentary": GraphQLVariable("commentary"), "favorite_color": ["red": GraphQLVariable("red"), "blue": 100, "green": 50]]]'
Expand Down
113 changes: 112 additions & 1 deletion packages/apollo-codegen-swift/src/__tests__/language.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { stripIndent } from "common-tags";

import { SwiftGenerator } from "../language";
import { SwiftGenerator, SwiftSource, swift } from "../language";
import { valueFromAST } from "graphql";

describe("Swift code generation: Basic language constructs", () => {
let generator: SwiftGenerator<any>;
Expand Down Expand Up @@ -264,3 +265,113 @@ describe("Swift code generation: Basic language constructs", () => {
expect(generator.output).toMatchSnapshot();
});
});

describe("Swift code generation: Escaping", () => {
describe("using SwiftSource", () => {
it(`should escape identifiers`, () => {
expect(SwiftSource.identifier("self").source).toBe("`self`");
expect(SwiftSource.identifier("public").source).toBe("`public`");
expect(SwiftSource.identifier("Array<Type>").source).toBe(
"Array<`Type`>"
);
expect(SwiftSource.identifier("[Self?]?").source).toBe("[`Self`?]?");
});

it(`should not escape other words`, () => {
expect(SwiftSource.identifier("me").source).toBe("me");
expect(SwiftSource.identifier("_Self").source).toBe("_Self");
expect(SwiftSource.identifier("classes").source).toBe("classes");
});

it(`should escape fewer words in member position`, () => {
expect(SwiftSource.identifier(".self").source).toBe(".`self`");
expect(SwiftSource.identifier(".public").source).toBe(".public");
expect(SwiftSource.identifier("Foo.Self.Type.self.class").source).toBe(
"Foo.Self.`Type`.`self`.class"
);
});

it(`should escape fewer words at offset 0 with member escaping`, () => {
expect(SwiftSource.memberName("self").source).toBe("`self`");
expect(SwiftSource.memberName("public").source).toBe("public");
expect(SwiftSource.memberName(" public").source).toBe(" `public`");
expect(SwiftSource.memberName("Foo.Self.Type.self.class").source).toBe(
"Foo.Self.`Type`.`self`.class"
);
});

it(`should escape strings`, () => {
expect(SwiftSource.string("foobar").source).toBe('"foobar"');
expect(SwiftSource.string("foo\n bar ").source).toBe('"foo\\n bar "');
expect(SwiftSource.string("one'two\"three\\four\tfive").source).toBe(
'"one\'two\\"three\\\\four\\tfive"'
);
});

it(`should trim strings when asked`, () => {
expect(SwiftSource.string("foobar", true).source).toBe('"foobar"');
expect(SwiftSource.string("foo\n bar ", true).source).toBe('"foo bar"');
});

it(`should support concatenation`, () => {
expect(swift`one`.concat().source).toBe("one");
expect(swift`one`.concat(swift`two`).source).toBe("onetwo");
expect(swift`one`.concat(swift`two`, swift`three`).source).toBe(
"onetwothree"
);
});

it(`should support appending`, () => {
let value = swift`one`;
value.append();
expect(value.source).toBe("one");
value.append(swift`foo`);
expect(value.source).toBe("onefoo");
value.append(swift`bar`, swift`baz`, swift`qux`);
expect(value.source).toBe("onefoobarbazqux");
});
});
describe("using SwiftGenerator", () => {
let generator: SwiftGenerator<any>;

beforeEach(() => {
generator = new SwiftGenerator({});
});

it(`should trim with multilineString`, () => {
generator.multilineString("foo\n bar ");

expect(generator.output).toBe('"foo bar"');
});

it(`shouldn't trim with multilineString when using """`, () => {
generator.multilineString('"""\nfoo\n bar \n"""');
expect(generator.output).toBe('"\\"\\"\\"\\nfoo\\n bar \\n\\"\\"\\""');
});
});
describe("using template strings", () => {
it(`should escape interpolated strings but not string literals`, () => {
expect(swift`self`.source).toBe("self");
expect(swift`${"self"}`.source).toBe("`self`");
expect(swift`class ${"Foo.Type.self"}: ${"Protocol?"}`.source).toBe(
"class Foo.`Type`.`self`: `Protocol`?"
);
expect(swift`${["Self", "Foo.Self.self"]}`.source).toBe(
"`Self`,Foo.Self.`self`"
);
expect(swift`${true} ${"true"}`.source).toBe("true `true`");
expect(swift`${{ toString: () => "self" }}`.source).toBe("`self`");
});

it(`should not escape already-escaped interpolated strings`, () => {
expect(swift`${swift`${"self"}`}`.source).toBe("`self`");
expect(swift`${"public"} ${new SwiftSource("public")}`.source).toBe(
"`public` public"
);
});

it(`should not escape with the raw tag`, () => {
expect(SwiftSource.raw`${"self"}`.source).toBe("self");
});
});
});
Loading

0 comments on commit f295ac3

Please sign in to comment.