Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Escape deprecation messages #2951

Merged
merged 12 commits into from
Apr 12, 2023
4 changes: 4 additions & 0 deletions Apollo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,7 @@
E608A52B280905C2001BE656 /* WSProtocolTestsBase.swift in Sources */ = {isa = PBXBuildFile; fileRef = E608A528280905C2001BE656 /* WSProtocolTestsBase.swift */; };
E608A52D280905E9001BE656 /* SubscriptionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E608A52C280905E9001BE656 /* SubscriptionTests.swift */; };
E60AE2EE27E3FC6C003C093A /* TemplateRenderer.swift in Sources */ = {isa = PBXBuildFile; fileRef = E60AE2ED27E3FC6C003C093A /* TemplateRenderer.swift */; };
E60F457A29E4CFB800E60A04 /* TemplateString_DeprecationMessage_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E60F457929E4CFB800E60A04 /* TemplateString_DeprecationMessage_Tests.swift */; };
E610D8D7278EA2390023E495 /* EnumFileGenerator.swift in Sources */ = {isa = PBXBuildFile; fileRef = E610D8D6278EA2390023E495 /* EnumFileGenerator.swift */; };
E610D8D9278EA2560023E495 /* EnumFileGeneratorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E610D8D8278EA2560023E495 /* EnumFileGeneratorTests.swift */; };
E610D8DB278EB0900023E495 /* InterfaceFileGenerator.swift in Sources */ = {isa = PBXBuildFile; fileRef = E610D8DA278EB0900023E495 /* InterfaceFileGenerator.swift */; };
Expand Down Expand Up @@ -1878,6 +1879,7 @@
E608A528280905C2001BE656 /* WSProtocolTestsBase.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = WSProtocolTestsBase.swift; sourceTree = "<group>"; };
E608A52C280905E9001BE656 /* SubscriptionTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SubscriptionTests.swift; sourceTree = "<group>"; };
E60AE2ED27E3FC6C003C093A /* TemplateRenderer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TemplateRenderer.swift; sourceTree = "<group>"; };
E60F457929E4CFB800E60A04 /* TemplateString_DeprecationMessage_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TemplateString_DeprecationMessage_Tests.swift; sourceTree = "<group>"; };
E610D8D6278EA2390023E495 /* EnumFileGenerator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EnumFileGenerator.swift; sourceTree = "<group>"; };
E610D8D8278EA2560023E495 /* EnumFileGeneratorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EnumFileGeneratorTests.swift; sourceTree = "<group>"; };
E610D8DA278EB0900023E495 /* InterfaceFileGenerator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InterfaceFileGenerator.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2949,6 +2951,7 @@
DE9CEAF2282C6AC300959AF9 /* TemplateRenderer_TestMockFile_Tests.swift */,
E64F7EB927A085D90059C021 /* UnionTemplateTests.swift */,
DED5B358286CEA0900AE6BFF /* TemplateString_Documentation_Tests.swift */,
E60F457929E4CFB800E60A04 /* TemplateString_DeprecationMessage_Tests.swift */,
);
path = Templates;
sourceTree = "<group>";
Expand Down Expand Up @@ -5097,6 +5100,7 @@
E6D90D0D278FFE35009CAC5D /* SchemaMetadataFileGeneratorTests.swift in Sources */,
E6B42D0B27A4746800A3BD58 /* SchemaModuleFileGeneratorTests.swift in Sources */,
9B68F0552416B33300E97318 /* LineByLineComparison.swift in Sources */,
E60F457A29E4CFB800E60A04 /* TemplateString_DeprecationMessage_Tests.swift in Sources */,
DE09066F27A4713F00211300 /* OperationDefinitionTemplateTests.swift in Sources */,
DE09F9C6270269F800795949 /* OperationDefinitionTemplate_DocumentType_Tests.swift in Sources */,
9B4751AD2575B5070001FB87 /* PluralizerTests.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,44 @@ extension TemplateString.StringInterpolation {
) {
calvincestari marked this conversation as resolved.
Show resolved Hide resolved
guard
config.options.warningsOnDeprecatedUsage == .include,
let deprecationReason = deprecationReason
let escapedDeprecationReason = deprecationReason?.escapedDoubleQuotes()
else {
removeLineIfEmpty()
return
}

let deprecationReasonLines = deprecationReason
.split(separator: "\n", omittingEmptySubsequences: false)
if escapedDeprecationReason.firstIndex(of: "\n") != nil {
calvincestari marked this conversation as resolved.
Show resolved Hide resolved
calvincestari marked this conversation as resolved.
Show resolved Hide resolved
let splitReasonLines = escapedDeprecationReason
.split(separator: "\n", omittingEmptySubsequences: false)

if deprecationReasonLines.count > 1 {
appendInterpolation("""
@available(*, deprecated, message: \"\"\"
\(deprecationReasonLines.joinedAsLines(withIndent: " "))
\(splitReasonLines.joinedAsLines(withIndent: " "))
\"\"\")
""")
} else {
appendInterpolation("""
@available(*, deprecated, message: \"\(deprecationReason)\")
@available(*, deprecated, message: \"\(escapedDeprecationReason)\")
""")
}
}

mutating func appendInterpolation(
field: String,
argument: String,
warningReason: String
) {
let escapedWarningReason = warningReason.escapedDoubleQuotes()

appendInterpolation("""
#warning("Argument '\(argument)' of field '\(field)' is deprecated. \
Reason: '\(escapedWarningReason)'")
""")
}
}

fileprivate extension String {
func escapedDoubleQuotes() -> String {
replacingOccurrences(of: "\"", with: "\\\"")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,7 @@ struct SelectionSetTemplate {
return """
\(if: deprecatedArguments != nil && !deprecatedArguments.unsafelyUnwrapped.isEmpty, """
\(deprecatedArguments.unsafelyUnwrapped.map { """
#warning("Argument '\($0.arg)' of field '\($0.field)' is deprecated. \
Reason: '\($0.reason)'")
\(field: $0.field, argument: $0.arg, warningReason: $0.reason)
"""})
""")
\(selectionsTemplate)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,230 @@
import XCTest
import Nimble
@testable import ApolloCodegenLib

final class TemplateString_DeprecationMessage_Tests: XCTestCase {

let config = ApolloCodegen.ConfigurationContext(config: .mock(
options: .init(
warningsOnDeprecatedUsage: .include
)
))

// MARK: - Swift @available Attribute Tests

func test__availableAttribute__givenSingleLineDeprecationMessageWithInnerDoubleQuotes_shouldEscapeDoubleQuotes() throws {
// given
let subject = TemplateString("""
\(deprecationReason: "not supported, use \"another thing\" instead.", config: config)
""")

let expected = #"""
@available(*, deprecated, message: "not supported, use \"another thing\" instead.")
"""#

// then
let actual = subject.description

expect(actual).to(equalLineByLine(expected))
}

func test__availableAttribute__givenMultiLineDeprecationMessageWithInnerDoubleQuotes_shouldEscapeDoubleQuotes() throws {
calvincestari marked this conversation as resolved.
Show resolved Hide resolved
// given
let subject = TemplateString("""
\(deprecationReason: "not supported\nuse \"another thing\" instead.", config: config)
""")

let expected = #"""
@available(*, deprecated, message: """
not supported
use \"another thing\" instead.
""")
"""#

// then
let actual = subject.description

expect(actual).to(equalLineByLine(expected))
}

// MARK: Swift #warning Directive Tests

func test__warningDirective__givenSingleLineDeprecationMessageWithInnerDoubleQuotes_shouldEscapeDoubleQuotes() throws {
// given
let subject = TemplateString("""
\(field: "fieldOne", argument: "argOne", warningReason: "not supported, use \"another thing\" instead.")
""")

let expected = #"""
#warning("Argument 'argOne' of field 'fieldOne' is deprecated. Reason: 'not supported, use \"another thing\" instead.'")
"""#

// then
let actual = subject.description

expect(actual).to(equalLineByLine(expected))
}

// MARK: SDL-to-Generation Test
//
// These tests ensure that when given double quotes in SDL the generation of Swift code works as
calvincestari marked this conversation as resolved.
Show resolved Hide resolved
// expected from frontend parsing all the way to rendering of attributes, comments andwarnings.
calvincestari marked this conversation as resolved.
Show resolved Hide resolved
// There is a test here for all the places that the GraphQL schema supports the @deprecated
// directive.
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for adding these tests. Super critical b/c if we change the back end to SwiftGraphQL someday we want to ensure it still works all the way through!

Copy link
Member Author

Choose a reason for hiding this comment

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

These tests don't include the characters \0 (null character), nor \' (single quote) because they're caught as syntax errors by graphql-js.

failed: caught error: "Syntax Error: Invalid character escape sequence: "\0".

failed: caught error: "Syntax Error: Invalid character escape sequence: "'".

I think this is because graphql-js is validating strings against the GraphQL spec definition of allowed escape characters. Those two characters are not in that list.

Copy link
Member Author

@calvincestari calvincestari Apr 12, 2023

Choose a reason for hiding this comment

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

@AnthonyMDev similarly graphql-js does not allow \( either so there is no need to check and replace that character. What we spoke about yesterday.

failed: caught error: "Syntax Error: Invalid character escape sequence: "(".


func test__field__givenSDLDeprecationMessageWithInnerDoubleQuotes_shouldEscapeDoubleQuotes() throws {
// given
let schemaSDL = #"""
type Query {
animal: Animal
}

type Animal {
genus: String @deprecated(reason: "not supported, use \"species\" instead.")
species: String
}
"""#

let document = """
query GetAnimal {
animal {
genus
}
}
"""

let ir = try IR.mock(schema: schemaSDL, document: document)
let operation = ir.build(operation: try XCTUnwrap(ir.compilationResult[operation: "GetAnimal"]))
let subject = SelectionSetTemplate(generateInitializers: true, config: config)

let expected = #"""
@available(*, deprecated, message: "not supported, use \"species\" instead.")
"""#

// then
let actual = subject.render(for: operation)

expect(actual).to(equalLineByLine(expected, atLine: 37, ignoringExtraLines: true))
}

func test__inputField_givenSDLDeprecationMessageWithInnerDoubleQuotes_shouldEscapeDoubleQuotes() throws {
// given
let schemaSDL = #"""
type Query {
animal(filter: Filter): Animal
}

type Animal {
name: String
}

input Filter {
genus: String @deprecated(reason: "not supported, use \"species\" instead.")
}
"""#

let document = """
query GetAnimal($filter: Filter) {
animal(filter: $filter) {
name
}
}
"""

let ir = try IR.mock(schema: schemaSDL, document: document)
let inputObject = ir.schema.referencedTypes.inputObjects[0]

let subject = InputObjectTemplate(graphqlInputObject: inputObject, config: config)

let expected = #"""
@available(*, deprecated, message: "not supported, use \"species\" instead.")
"""#

// then
let actual = subject.render()

expect(actual).to(equalLineByLine(expected, atLine: 23, ignoringExtraLines: true))
}

func test__enum__givenSDLDeprecationMessageWithInnerDoubleQuotes_shouldNotEscapeDoubleQuotes() throws {
// given
let schemaSDL = #"""
type Query {
animal: Animal
}

type Animal {
name: String
size: Size
}

enum Size {
tiny @deprecated(reason: "not supported, use \"small\" instead.")
small
large
}
"""#

let document = """
query GetAnimal {
animal {
name
size
}
}
"""

let ir = try IR.mock(schema: schemaSDL, document: document)
let `enum` = ir.schema.referencedTypes.enums[0]

let subject = EnumTemplate(graphqlEnum: `enum`, config: config)

let expected = #"""
/// **Deprecated**: not supported, use "small" instead.
"""#

// then
let actual = subject.render()

expect(actual).to(equalLineByLine(expected, atLine: 8, ignoringExtraLines: true))
}

func test__argument__givenSDLDeprecationMessageWithInnerDoubleQuotes_shouldEscapeDoubleQuotes() throws {
// given
let schemaSDL = #"""
type Query {
animal: Animal
}

type Animal {
species: String
predators(genus: String @deprecated(reason: "not supported, use \"species\" instead."), species: String): Animal
}
"""#

let document = """
query GetAnimal($genus: String) {
animal {
species
predators(genus: $genus) {
species
}
}
}
"""

let ir = try IR.mock(schema: schemaSDL, document: document)
let operation = ir.build(operation: try XCTUnwrap(ir.compilationResult[operation: "GetAnimal"]))
let subject = SelectionSetTemplate(generateInitializers: true, config: config)

let expected = #"""
#warning("Argument 'genus' of field 'predators' is deprecated. Reason: 'not supported, use \"species\" instead.'")
"""#

// then
let actual = subject.render(for: operation)

expect(actual).to(equalLineByLine(expected, atLine: 32, ignoringExtraLines: true))
}

}