From 386a890bd92bdd990bf0057668c8f9a04076cbe6 Mon Sep 17 00:00:00 2001 From: Calvin Cestari Date: Wed, 21 Dec 2022 00:04:00 -0800 Subject: [PATCH 1/8] Accept spaces in link command paths --- Plugins/InstallCLI/InstallCLIPluginCommand.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Plugins/InstallCLI/InstallCLIPluginCommand.swift b/Plugins/InstallCLI/InstallCLIPluginCommand.swift index 5a6207390c..6d536940b5 100644 --- a/Plugins/InstallCLI/InstallCLIPluginCommand.swift +++ b/Plugins/InstallCLI/InstallCLIPluginCommand.swift @@ -13,7 +13,7 @@ struct InstallCLIPluginCommand: CommandPlugin { let task = Process() task.standardInput = nil task.environment = ProcessInfo.processInfo.environment - task.arguments = ["-c", "ln -f -s \(from.string) \(to.string)"] + task.arguments = ["-c", "ln -f -s '\(from.string)' '\(to.string)'"] task.executableURL = URL(fileURLWithPath: "/bin/zsh") try task.run() task.waitUntilExit() From b1ddd98a02a270a487eef54d06ad597b7a21fcaf Mon Sep 17 00:00:00 2001 From: Calvin Cestari Date: Fri, 6 Jan 2023 15:03:41 -0800 Subject: [PATCH 2/8] Move schema name check to earlier validation --- Sources/ApolloCodegenLib/ApolloCodegen.swift | 7 ++++++- Tests/ApolloCodegenTests/ApolloCodegenTests.swift | 6 ++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Sources/ApolloCodegenLib/ApolloCodegen.swift b/Sources/ApolloCodegenLib/ApolloCodegen.swift index ed19d15ce7..f4285ebd08 100644 --- a/Sources/ApolloCodegenLib/ApolloCodegen.swift +++ b/Sources/ApolloCodegenLib/ApolloCodegen.swift @@ -132,6 +132,12 @@ public class ApolloCodegen { /// Performs validation against deterministic errors that will cause code generation to fail. static func validate(config: ConfigurationContext) throws { + guard + !SwiftKeywords.DisallowedSchemaNamespaceNames.contains(config.schemaName.lowercased()) + else { + throw Error.schemaNameConflict(name: config.schemaName) + } + if case .swiftPackage = config.output.testMocks, config.output.schemaTypes.moduleType != .swiftPackageManager { throw Error.testMocksInvalidSwiftPackageConfiguration @@ -162,7 +168,6 @@ public class ApolloCodegen { static func validate(schemaName: String, compilationResult: CompilationResult) throws { guard - !SwiftKeywords.DisallowedSchemaNamespaceNames.contains(schemaName.lowercased()), !compilationResult.referencedTypes.contains(where: { namedType in namedType.swiftName == schemaName.firstUppercased }), diff --git a/Tests/ApolloCodegenTests/ApolloCodegenTests.swift b/Tests/ApolloCodegenTests/ApolloCodegenTests.swift index 234b62f3a0..1dea9f341a 100644 --- a/Tests/ApolloCodegenTests/ApolloCodegenTests.swift +++ b/Tests/ApolloCodegenTests/ApolloCodegenTests.swift @@ -2169,10 +2169,8 @@ class ApolloCodegenTests: XCTestCase { ), rootURL: nil) // then - expect(try ApolloCodegen.validate( - schemaName: configContext.schemaName, - compilationResult: CompilationResult.mock())) - .to(throwError(ApolloCodegen.Error.schemaNameConflict(name: configContext.schemaName))) + expect(try ApolloCodegen.validate(config: configContext)) + .to(throwError(ApolloCodegen.Error.schemaNameConflict(name: configContext.schemaName))) } } From 9f7b9552f5d605133c1fdfa06637dd1ecbdfd566 Mon Sep 17 00:00:00 2001 From: Calvin Cestari Date: Fri, 6 Jan 2023 17:04:15 -0800 Subject: [PATCH 3/8] Refactor the flow of configuration validation --- Sources/ApolloCodegenLib/ApolloCodegen.swift | 40 ++++++++++++------- .../ApolloCodegenTests.swift | 40 +++++++++---------- 2 files changed, 45 insertions(+), 35 deletions(-) diff --git a/Sources/ApolloCodegenLib/ApolloCodegen.swift b/Sources/ApolloCodegenLib/ApolloCodegen.swift index f4285ebd08..260872f6d2 100644 --- a/Sources/ApolloCodegenLib/ApolloCodegen.swift +++ b/Sources/ApolloCodegenLib/ApolloCodegen.swift @@ -76,14 +76,14 @@ public class ApolloCodegen { rootURL: rootURL ) - try validate(config: configContext) + try validate(context: configContext) let compilationResult = try compileGraphQLResult( configContext, experimentalFeatures: configuration.experimentalFeatures ) - try validate(schemaName: configContext.schemaName, compilationResult: compilationResult) + try validate(context: configContext, compilationResult: compilationResult) let ir = IR(compilationResult: compilationResult) @@ -130,21 +130,29 @@ public class ApolloCodegen { } } + + /// Performs validation against deterministic errors that will cause code generation to fail. + /// + /// - Parameter config: Code generation configuration settings. + public static func validate(config: ApolloCodegenConfiguration) throws { + try validate(context: ConfigurationContext(config: config)) + } + /// Performs validation against deterministic errors that will cause code generation to fail. - static func validate(config: ConfigurationContext) throws { + static func validate(context: ConfigurationContext) throws { guard - !SwiftKeywords.DisallowedSchemaNamespaceNames.contains(config.schemaName.lowercased()) + !SwiftKeywords.DisallowedSchemaNamespaceNames.contains(context.schemaName.lowercased()) else { - throw Error.schemaNameConflict(name: config.schemaName) + throw Error.schemaNameConflict(name: context.schemaName) } - if case .swiftPackage = config.output.testMocks, - config.output.schemaTypes.moduleType != .swiftPackageManager { + if case .swiftPackage = context.output.testMocks, + context.output.schemaTypes.moduleType != .swiftPackageManager { throw Error.testMocksInvalidSwiftPackageConfiguration } - if case .swiftPackageManager = config.output.schemaTypes.moduleType, - config.options.cocoapodsCompatibleImportStatements == true { + if case .swiftPackageManager = context.output.schemaTypes.moduleType, + context.options.cocoapodsCompatibleImportStatements == true { throw Error.invalidConfiguration(message: """ cocoapodsCompatibleImportStatements cannot be set to 'true' when the output schema types \ module type is Swift Package Manager. Change the cocoapodsCompatibleImportStatements \ @@ -152,10 +160,10 @@ public class ApolloCodegen { """) } - for searchPath in config.input.schemaSearchPaths { + for searchPath in context.input.schemaSearchPaths { try validate(inputSearchPath: searchPath) } - for searchPath in config.input.operationSearchPaths { + for searchPath in context.input.operationSearchPaths { try validate(inputSearchPath: searchPath) } } @@ -166,16 +174,18 @@ public class ApolloCodegen { } } - static func validate(schemaName: String, compilationResult: CompilationResult) throws { + /// Validates the configuration context against the GraphQL compilation result, checking for + /// configuration errors that are dependent on the schema and operations. + static func validate(context: ConfigurationContext, compilationResult: CompilationResult) throws { guard !compilationResult.referencedTypes.contains(where: { namedType in - namedType.swiftName == schemaName.firstUppercased + namedType.swiftName == context.schemaName.firstUppercased }), !compilationResult.fragments.contains(where: { fragmentDefinition in - fragmentDefinition.name == schemaName.firstUppercased + fragmentDefinition.name == context.schemaName.firstUppercased }) else { - throw Error.schemaNameConflict(name: schemaName) + throw Error.schemaNameConflict(name: context.schemaName) } } diff --git a/Tests/ApolloCodegenTests/ApolloCodegenTests.swift b/Tests/ApolloCodegenTests/ApolloCodegenTests.swift index 1dea9f341a..1a53ee761a 100644 --- a/Tests/ApolloCodegenTests/ApolloCodegenTests.swift +++ b/Tests/ApolloCodegenTests/ApolloCodegenTests.swift @@ -1940,7 +1940,7 @@ class ApolloCodegenTests: XCTestCase { ), rootURL: nil) // then - expect(try ApolloCodegen.validate(config: configContext)) + expect(try ApolloCodegen.validate(context: configContext)) .to(throwError(ApolloCodegen.Error.testMocksInvalidSwiftPackageConfiguration)) } @@ -1955,7 +1955,7 @@ class ApolloCodegenTests: XCTestCase { ), rootURL: nil) // then - expect(try ApolloCodegen.validate(config: configContext)) + expect(try ApolloCodegen.validate(context: configContext)) .to(throwError(ApolloCodegen.Error.testMocksInvalidSwiftPackageConfiguration)) } @@ -1966,7 +1966,7 @@ class ApolloCodegenTests: XCTestCase { ), rootURL: nil) // then - expect(try ApolloCodegen.validate(config: configContext)) + expect(try ApolloCodegen.validate(context: configContext)) .notTo(throwError()) } @@ -1977,7 +1977,7 @@ class ApolloCodegenTests: XCTestCase { ), rootURL: nil) // then - expect(try ApolloCodegen.validate(config: configContext)) + expect(try ApolloCodegen.validate(context: configContext)) .to(throwError(ApolloCodegen.Error.inputSearchPathInvalid(path: "operations/*"))) } @@ -1988,7 +1988,7 @@ class ApolloCodegenTests: XCTestCase { ), rootURL: nil) // then - expect(try ApolloCodegen.validate(config: configContext)) + expect(try ApolloCodegen.validate(context: configContext)) .to(throwError(ApolloCodegen.Error.inputSearchPathInvalid(path: "operations/*."))) } @@ -1999,7 +1999,7 @@ class ApolloCodegenTests: XCTestCase { ), rootURL: nil) // then - expect(try ApolloCodegen.validate(config: configContext)) + expect(try ApolloCodegen.validate(context: configContext)) .to(throwError(ApolloCodegen.Error.inputSearchPathInvalid(path: "schema/*"))) } @@ -2010,7 +2010,7 @@ class ApolloCodegenTests: XCTestCase { ), rootURL: nil) // then - expect(try ApolloCodegen.validate(config: configContext)) + expect(try ApolloCodegen.validate(context: configContext)) .to(throwError(ApolloCodegen.Error.inputSearchPathInvalid(path: "schema/*."))) } @@ -2030,7 +2030,7 @@ class ApolloCodegenTests: XCTestCase { ), rootURL: nil) expect(try ApolloCodegen.validate( - schemaName: configContext.schemaName, + context: configContext, compilationResult: compilationResult)) .to(throwError(ApolloCodegen.Error.schemaNameConflict(name: configContext.schemaName))) } @@ -2050,7 +2050,7 @@ class ApolloCodegenTests: XCTestCase { ), rootURL: nil) expect(try ApolloCodegen.validate( - schemaName: configContext.schemaName, + context: configContext, compilationResult: compilationResult)) .to(throwError(ApolloCodegen.Error.schemaNameConflict(name: configContext.schemaName))) } @@ -2070,7 +2070,7 @@ class ApolloCodegenTests: XCTestCase { ), rootURL: nil) expect(try ApolloCodegen.validate( - schemaName: configContext.schemaName, + context: configContext, compilationResult: compilationResult)) .to(throwError(ApolloCodegen.Error.schemaNameConflict(name: configContext.schemaName))) } @@ -2090,7 +2090,7 @@ class ApolloCodegenTests: XCTestCase { ), rootURL: nil) expect(try ApolloCodegen.validate( - schemaName: configContext.schemaName, + context: configContext, compilationResult: compilationResult)) .to(throwError(ApolloCodegen.Error.schemaNameConflict(name: configContext.schemaName))) } @@ -2110,7 +2110,7 @@ class ApolloCodegenTests: XCTestCase { ), rootURL: nil) expect(try ApolloCodegen.validate( - schemaName: configContext.schemaName, + context: configContext, compilationResult: compilationResult)) .to(throwError(ApolloCodegen.Error.schemaNameConflict(name: configContext.schemaName))) } @@ -2130,7 +2130,7 @@ class ApolloCodegenTests: XCTestCase { ), rootURL: nil) expect(try ApolloCodegen.validate( - schemaName: configContext.schemaName, + context: configContext, compilationResult: compilationResult)) .to(throwError(ApolloCodegen.Error.schemaNameConflict(name: configContext.schemaName))) } @@ -2152,7 +2152,7 @@ class ApolloCodegenTests: XCTestCase { ), rootURL: nil) expect(try ApolloCodegen.validate( - schemaName: configContext.schemaName, + context: configContext, compilationResult: compilationResult)) .to(throwError(ApolloCodegen.Error.schemaNameConflict(name: configContext.schemaName))) } @@ -2169,7 +2169,7 @@ class ApolloCodegenTests: XCTestCase { ), rootURL: nil) // then - expect(try ApolloCodegen.validate(config: configContext)) + expect(try ApolloCodegen.validate(context: configContext)) .to(throwError(ApolloCodegen.Error.schemaNameConflict(name: configContext.schemaName))) } } @@ -2203,7 +2203,7 @@ class ApolloCodegenTests: XCTestCase { ), rootURL: nil) expect(try ApolloCodegen.validate( - schemaName: configContext.schemaName, + context: configContext, compilationResult: compilationResult)) .notTo(throwError()) } @@ -2216,7 +2216,7 @@ class ApolloCodegenTests: XCTestCase { )) // then - expect(try ApolloCodegen.validate(config: configContext)) + expect(try ApolloCodegen.validate(context: configContext)) .to(throwError(ApolloCodegen.Error.invalidConfiguration(message: """ cocoapodsCompatibleImportStatements cannot be set to 'true' when the output schema types \ module type is Swift Package Manager. Change the cocoapodsCompatibleImportStatements \ @@ -2232,7 +2232,7 @@ class ApolloCodegenTests: XCTestCase { )) // then - expect(try ApolloCodegen.validate(config: configContext)).notTo(throwError()) + expect(try ApolloCodegen.validate(context: configContext)).notTo(throwError()) } func test__validation__givenSchemaTypesModule_embeddedInTarget_withCocoapodsCompatibleImportStatements_true_shouldNotThrow() throws { @@ -2243,7 +2243,7 @@ class ApolloCodegenTests: XCTestCase { )) // then - expect(try ApolloCodegen.validate(config: configContext)).notTo(throwError()) + expect(try ApolloCodegen.validate(context: configContext)).notTo(throwError()) } func test__validation__givenSchemaTypesModule_other_withCocoapodsCompatibleImportStatements_true_shouldNotThrow() throws { @@ -2254,7 +2254,7 @@ class ApolloCodegenTests: XCTestCase { )) // then - expect(try ApolloCodegen.validate(config: configContext)).notTo(throwError()) + expect(try ApolloCodegen.validate(context: configContext)).notTo(throwError()) } // MARK: Path Match Exclusion Tests From c38e85eda92581d3301551b8543006bb087be9ee Mon Sep 17 00:00:00 2001 From: Calvin Cestari Date: Fri, 6 Jan 2023 20:23:30 -0800 Subject: [PATCH 4/8] Refactor validation interface --- Sources/ApolloCodegenLib/ApolloCodegen.swift | 10 +-- .../ApolloCodegenTests.swift | 74 +++++++++---------- 2 files changed, 41 insertions(+), 43 deletions(-) diff --git a/Sources/ApolloCodegenLib/ApolloCodegen.swift b/Sources/ApolloCodegenLib/ApolloCodegen.swift index 260872f6d2..0bc6ba6adf 100644 --- a/Sources/ApolloCodegenLib/ApolloCodegen.swift +++ b/Sources/ApolloCodegenLib/ApolloCodegen.swift @@ -130,16 +130,16 @@ public class ApolloCodegen { } } - - /// Performs validation against deterministic errors that will cause code generation to fail. + /// Validates the configuration against deterministic errors that will cause code generation to + /// fail. This validation step does not take into account schema and operation specific types, it + /// is only a static analysis of the configuration. /// /// - Parameter config: Code generation configuration settings. - public static func validate(config: ApolloCodegenConfiguration) throws { + public static func _validate(config: ApolloCodegenConfiguration) throws { try validate(context: ConfigurationContext(config: config)) } - /// Performs validation against deterministic errors that will cause code generation to fail. - static func validate(context: ConfigurationContext) throws { + static private func validate(context: ConfigurationContext) throws { guard !SwiftKeywords.DisallowedSchemaNamespaceNames.contains(context.schemaName.lowercased()) else { diff --git a/Tests/ApolloCodegenTests/ApolloCodegenTests.swift b/Tests/ApolloCodegenTests/ApolloCodegenTests.swift index 1a53ee761a..5093ff3e45 100644 --- a/Tests/ApolloCodegenTests/ApolloCodegenTests.swift +++ b/Tests/ApolloCodegenTests/ApolloCodegenTests.swift @@ -1931,86 +1931,86 @@ class ApolloCodegenTests: XCTestCase { func test_validation_givenTestMockConfiguration_asSwiftPackage_withSchemaTypesModule_asEmbeddedInTarget_shouldThrow() throws { // given - let configContext = ApolloCodegen.ConfigurationContext(config: .mock( + let config = ApolloCodegenConfiguration.mock( input: .init(schemaPath: "path"), output: .mock( moduleType: .embeddedInTarget(name: "ModuleTarget"), testMocks: .swiftPackage(targetName: nil) ) - ), rootURL: nil) + ) // then - expect(try ApolloCodegen.validate(context: configContext)) + expect(try ApolloCodegen._validate(config: config)) .to(throwError(ApolloCodegen.Error.testMocksInvalidSwiftPackageConfiguration)) } func test_validation_givenTestMockConfiguration_asSwiftPackage_withSchemaTypesModule_asOther_shouldThrow() throws { // given - let configContext = ApolloCodegen.ConfigurationContext(config: .mock( + let config = ApolloCodegenConfiguration.mock( input: .init(schemaPath: "path"), output: .mock( moduleType: .other, testMocks: .swiftPackage(targetName: nil) ) - ), rootURL: nil) + ) // then - expect(try ApolloCodegen.validate(context: configContext)) + expect(try ApolloCodegen._validate(config: config)) .to(throwError(ApolloCodegen.Error.testMocksInvalidSwiftPackageConfiguration)) } func test_validation_givenTestMockConfiguration_asSwiftPackage_withSchemaTypesModule_asSwiftPackage_shouldNotThrow() throws { // given - let configContext = ApolloCodegen.ConfigurationContext(config: .mock( + let config = ApolloCodegenConfiguration.mock( input: .init(schemaPath: "path.graphqls") - ), rootURL: nil) + ) // then - expect(try ApolloCodegen.validate(context: configContext)) + expect(try ApolloCodegen._validate(config: config)) .notTo(throwError()) } func test_validation_givenOperationSearchPathWithoutFileExtensionComponent_shouldThrow() throws { // given - let configContext = ApolloCodegen.ConfigurationContext(config: .mock( + let config = ApolloCodegenConfiguration.mock( input: .init(schemaPath: "path.graphqls", operationSearchPaths: ["operations/*"]) - ), rootURL: nil) + ) // then - expect(try ApolloCodegen.validate(context: configContext)) + expect(try ApolloCodegen._validate(config: config)) .to(throwError(ApolloCodegen.Error.inputSearchPathInvalid(path: "operations/*"))) } func test_validation_givenOperationSearchPathEndingInPeriod_shouldThrow() throws { // given - let configContext = ApolloCodegen.ConfigurationContext(config: .mock( + let config = ApolloCodegenConfiguration.mock( input: .init(schemaPath: "path.graphqls", operationSearchPaths: ["operations/*."]) - ), rootURL: nil) + ) // then - expect(try ApolloCodegen.validate(context: configContext)) + expect(try ApolloCodegen._validate(config: config)) .to(throwError(ApolloCodegen.Error.inputSearchPathInvalid(path: "operations/*."))) } func test_validation_givenSchemaSearchPathWithoutFileExtensionComponent_shouldThrow() throws { // given - let configContext = ApolloCodegen.ConfigurationContext(config: .mock( + let config = ApolloCodegenConfiguration.mock( input: .init(schemaSearchPaths: ["schema/*"]) - ), rootURL: nil) + ) // then - expect(try ApolloCodegen.validate(context: configContext)) + expect(try ApolloCodegen._validate(config: config)) .to(throwError(ApolloCodegen.Error.inputSearchPathInvalid(path: "schema/*"))) } func test_validation_givenSchemaSearchPathEndingInPeriod_shouldThrow() throws { // given - let configContext = ApolloCodegen.ConfigurationContext(config: .mock( + let config = ApolloCodegenConfiguration.mock( input: .init(schemaSearchPaths: ["schema/*."]) - ), rootURL: nil) + ) // then - expect(try ApolloCodegen.validate(context: configContext)) + expect(try ApolloCodegen._validate(config: config)) .to(throwError(ApolloCodegen.Error.inputSearchPathInvalid(path: "schema/*."))) } @@ -2164,13 +2164,11 @@ class ApolloCodegenTests: XCTestCase { // when for name in disallowedNames { - let configContext = ApolloCodegen.ConfigurationContext(config: .mock( - schemaName: name - ), rootURL: nil) + let config = ApolloCodegenConfiguration.mock(schemaName: name) // then - expect(try ApolloCodegen.validate(context: configContext)) - .to(throwError(ApolloCodegen.Error.schemaNameConflict(name: configContext.schemaName))) + expect(try ApolloCodegen._validate(config: config)) + .to(throwError(ApolloCodegen.Error.schemaNameConflict(name: config.schemaName))) } } @@ -2210,13 +2208,13 @@ class ApolloCodegenTests: XCTestCase { func test__validation__givenSchemaTypesModule_swiftPackageManager_withCocoapodsCompatibleImportStatements_true_shouldThrow() throws { // given - let configContext = ApolloCodegen.ConfigurationContext(config: .mock( + let config = ApolloCodegenConfiguration.mock( .swiftPackageManager, options: .init(cocoapodsCompatibleImportStatements: true) - )) + ) // then - expect(try ApolloCodegen.validate(context: configContext)) + expect(try ApolloCodegen._validate(config: config)) .to(throwError(ApolloCodegen.Error.invalidConfiguration(message: """ cocoapodsCompatibleImportStatements cannot be set to 'true' when the output schema types \ module type is Swift Package Manager. Change the cocoapodsCompatibleImportStatements \ @@ -2226,35 +2224,35 @@ class ApolloCodegenTests: XCTestCase { func test__validation__givenSchemaTypesModule_swiftPackageManager_withCocoapodsCompatibleImportStatements_false_shouldNotThrow() throws { // given - let configContext = ApolloCodegen.ConfigurationContext(config: .mock( + let config = ApolloCodegenConfiguration.mock( .swiftPackageManager, options: .init(cocoapodsCompatibleImportStatements: false) - )) + ) // then - expect(try ApolloCodegen.validate(context: configContext)).notTo(throwError()) + expect(try ApolloCodegen._validate(config: config)).notTo(throwError()) } func test__validation__givenSchemaTypesModule_embeddedInTarget_withCocoapodsCompatibleImportStatements_true_shouldNotThrow() throws { // given - let configContext = ApolloCodegen.ConfigurationContext(config: .mock( + let config = ApolloCodegenConfiguration.mock( .embeddedInTarget(name: "TestTarget"), options: .init(cocoapodsCompatibleImportStatements: true) - )) + ) // then - expect(try ApolloCodegen.validate(context: configContext)).notTo(throwError()) + expect(try ApolloCodegen._validate(config: config)).notTo(throwError()) } func test__validation__givenSchemaTypesModule_other_withCocoapodsCompatibleImportStatements_true_shouldNotThrow() throws { // given - let configContext = ApolloCodegen.ConfigurationContext(config: .mock( + let config = ApolloCodegenConfiguration.mock( .other, options: .init(cocoapodsCompatibleImportStatements: true) - )) + ) // then - expect(try ApolloCodegen.validate(context: configContext)).notTo(throwError()) + expect(try ApolloCodegen._validate(config: config)).notTo(throwError()) } // MARK: Path Match Exclusion Tests From 631cac93aa421c40debda5c7f627bc64e98db457 Mon Sep 17 00:00:00 2001 From: Calvin Cestari Date: Fri, 6 Jan 2023 20:24:25 -0800 Subject: [PATCH 5/8] Use validation API from CLI initialize command --- Sources/CodegenCLI/Commands/Initialize.swift | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Sources/CodegenCLI/Commands/Initialize.swift b/Sources/CodegenCLI/Commands/Initialize.swift index ebfc551f75..8548367c50 100644 --- a/Sources/CodegenCLI/Commands/Initialize.swift +++ b/Sources/CodegenCLI/Commands/Initialize.swift @@ -86,11 +86,11 @@ public struct Initialize: ParsableCommand { func _run(fileManager: ApolloFileManager = .default, output: OutputClosure? = nil) throws { let encoded = try ApolloCodegenConfiguration - .minimalJSON( - schemaName: schemaName, - moduleType: moduleType, - targetName: targetName - ).asData() + .minimalJSON(schemaName: schemaName, moduleType: moduleType, targetName: targetName) + .asData() + + let decoded = try JSONDecoder().decode(ApolloCodegenConfiguration.self, from: encoded) + try ApolloCodegen._validate(config: decoded) if print { try print(data: encoded, output: output) From a62370908805ab8b99efc2baad52d3ed0464ac0c Mon Sep 17 00:00:00 2001 From: Calvin Cestari Date: Sat, 7 Jan 2023 21:20:47 -0800 Subject: [PATCH 6/8] Moves schema name validation logic to ApolloCodegen --- Sources/ApolloCodegenLib/ApolloCodegen.swift | 13 ++++++++++ Sources/CodegenCLI/Commands/Initialize.swift | 4 ---- .../ApolloCodegenTests.swift | 24 +++++++++++++++++++ .../Commands/InitializeTests.swift | 21 ++++++++++++---- 4 files changed, 54 insertions(+), 8 deletions(-) diff --git a/Sources/ApolloCodegenLib/ApolloCodegen.swift b/Sources/ApolloCodegenLib/ApolloCodegen.swift index 0bc6ba6adf..57824493b4 100644 --- a/Sources/ApolloCodegenLib/ApolloCodegen.swift +++ b/Sources/ApolloCodegenLib/ApolloCodegen.swift @@ -19,6 +19,7 @@ public class ApolloCodegen { case cannotLoadSchema case cannotLoadOperations case invalidConfiguration(message: String) + case invalidSchemaName(_ name: String, message: String) public var errorDescription: String? { switch self { @@ -47,6 +48,8 @@ public class ApolloCodegen { return "No GraphQL operations could be found. Please verify the operation search paths." case let .invalidConfiguration(message): return "The codegen configuration has conflicting values: \(message)" + case let .invalidSchemaName(name, message): + return "The schema name `\(name)` is invalid: \(message)" } } } @@ -140,6 +143,16 @@ public class ApolloCodegen { } static private func validate(context: ConfigurationContext) throws { + guard + !context.schemaName.trimmingCharacters(in: .whitespaces).isEmpty, + !context.schemaName.contains(where: { $0.isWhitespace }) + else { + throw Error.invalidSchemaName(context.schemaName, message: """ + Cannot be empty nor contain spaces. If your schema name has spaces consider replacing them \ + with the underscore character. + """) + } + guard !SwiftKeywords.DisallowedSchemaNamespaceNames.contains(context.schemaName.lowercased()) else { diff --git a/Sources/CodegenCLI/Commands/Initialize.swift b/Sources/CodegenCLI/Commands/Initialize.swift index 8548367c50..8017feb9c2 100644 --- a/Sources/CodegenCLI/Commands/Initialize.swift +++ b/Sources/CodegenCLI/Commands/Initialize.swift @@ -64,10 +64,6 @@ public struct Initialize: ParsableCommand { public init() { } public func validate() throws { - guard !schemaName.trimmingCharacters(in: .whitespaces).isEmpty else { - throw ValidationError("--schema-name value cannot be empty.") - } - switch (moduleType, targetName?.isEmpty) { case (.embeddedInTarget, nil), (.embeddedInTarget, true): throw ValidationError(""" diff --git a/Tests/ApolloCodegenTests/ApolloCodegenTests.swift b/Tests/ApolloCodegenTests/ApolloCodegenTests.swift index 5093ff3e45..47a8fb6ee7 100644 --- a/Tests/ApolloCodegenTests/ApolloCodegenTests.swift +++ b/Tests/ApolloCodegenTests/ApolloCodegenTests.swift @@ -2172,6 +2172,30 @@ class ApolloCodegenTests: XCTestCase { } } + func test__validation__givenEmptySchemaName_shouldThrow() throws { + let config = ApolloCodegenConfiguration.mock(schemaName: "") + + // then + expect(try ApolloCodegen._validate(config: config)) + .to(throwError(ApolloCodegen.Error.invalidSchemaName("", message: ""))) + } + + func test__validation__givenWhitespaceOnlySchemaName_shouldThrow() throws { + let config = ApolloCodegenConfiguration.mock(schemaName: " ") + + // then + expect(try ApolloCodegen._validate(config: config)) + .to(throwError(ApolloCodegen.Error.invalidSchemaName(" ", message: ""))) + } + + func test__validation__givenSchemaNameContainingWhitespace_shouldThrow() throws { + let config = ApolloCodegenConfiguration.mock(schemaName: "My Schema") + + // then + expect(try ApolloCodegen._validate(config: config)) + .to(throwError(ApolloCodegen.Error.invalidSchemaName("My Schema", message: ""))) + } + func test__validation__givenUniqueSchemaName_shouldNotThrow() throws { // given let object = GraphQLObjectType.mock("MockObject") diff --git a/Tests/CodegenCLITests/Commands/InitializeTests.swift b/Tests/CodegenCLITests/Commands/InitializeTests.swift index 2fd4efd8fb..8cde73ef17 100644 --- a/Tests/CodegenCLITests/Commands/InitializeTests.swift +++ b/Tests/CodegenCLITests/Commands/InitializeTests.swift @@ -44,17 +44,30 @@ class InitializeTests: XCTestCase { // MARK: - Validation Tests - func test__validation__givenWhitespaceSchemaName_shouldThrowValidationError() throws { + func test__validation__givenWhitespaceOnlySchemaName_shouldThrowError() throws { // given let options = [ "--schema-name= ", "--module-type=swiftPackageManager", ] + let subject = try self.parse(options) + // then - expect { try self.parse(options) }.to(throwUserValidationError( - ValidationError("--schema-name value cannot be empty.") - )) + expect(try subject._run()).to(throwError()) + } + + func test__validation__givenSchemaNameContainingWhitespace_shouldThrowError() throws { + // given + let options = [ + "--schema-name=\"My Schema\"", + "--module-type=swiftPackageManager", + ] + + let subject = try self.parse(options) + + // then + expect(try subject._run()).to(throwError()) } func test__validation__givenModuleType_embeddedInTarget_withNoTargetName_shouldThrowValidationError() throws { From 9dfe91bff5a327dbf945bbd5abebadc1a0eab03e Mon Sep 17 00:00:00 2001 From: Calvin Cestari Date: Mon, 9 Jan 2023 10:31:19 -0800 Subject: [PATCH 7/8] Simplify schema name validation logic --- Sources/ApolloCodegenLib/ApolloCodegen.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/ApolloCodegenLib/ApolloCodegen.swift b/Sources/ApolloCodegenLib/ApolloCodegen.swift index 57824493b4..796f4930c3 100644 --- a/Sources/ApolloCodegenLib/ApolloCodegen.swift +++ b/Sources/ApolloCodegenLib/ApolloCodegen.swift @@ -144,7 +144,7 @@ public class ApolloCodegen { static private func validate(context: ConfigurationContext) throws { guard - !context.schemaName.trimmingCharacters(in: .whitespaces).isEmpty, + !context.schemaName.isEmpty, !context.schemaName.contains(where: { $0.isWhitespace }) else { throw Error.invalidSchemaName(context.schemaName, message: """ From a492d548b002f33c62af0ec2ab30633e45b5d0fb Mon Sep 17 00:00:00 2001 From: Calvin Cestari Date: Mon, 9 Jan 2023 14:18:57 -0800 Subject: [PATCH 8/8] Improves function parameter naming --- Sources/ApolloCodegenLib/ApolloCodegen.swift | 10 +++--- .../ApolloCodegenTests.swift | 32 +++++-------------- 2 files changed, 13 insertions(+), 29 deletions(-) diff --git a/Sources/ApolloCodegenLib/ApolloCodegen.swift b/Sources/ApolloCodegenLib/ApolloCodegen.swift index 796f4930c3..6853a42ae3 100644 --- a/Sources/ApolloCodegenLib/ApolloCodegen.swift +++ b/Sources/ApolloCodegenLib/ApolloCodegen.swift @@ -79,14 +79,14 @@ public class ApolloCodegen { rootURL: rootURL ) - try validate(context: configContext) + try validate(configContext) let compilationResult = try compileGraphQLResult( configContext, experimentalFeatures: configuration.experimentalFeatures ) - try validate(context: configContext, compilationResult: compilationResult) + try validate(configContext, with: compilationResult) let ir = IR(compilationResult: compilationResult) @@ -139,10 +139,10 @@ public class ApolloCodegen { /// /// - Parameter config: Code generation configuration settings. public static func _validate(config: ApolloCodegenConfiguration) throws { - try validate(context: ConfigurationContext(config: config)) + try validate(ConfigurationContext(config: config)) } - static private func validate(context: ConfigurationContext) throws { + static private func validate(_ context: ConfigurationContext) throws { guard !context.schemaName.isEmpty, !context.schemaName.contains(where: { $0.isWhitespace }) @@ -189,7 +189,7 @@ public class ApolloCodegen { /// Validates the configuration context against the GraphQL compilation result, checking for /// configuration errors that are dependent on the schema and operations. - static func validate(context: ConfigurationContext, compilationResult: CompilationResult) throws { + static func validate(_ context: ConfigurationContext, with compilationResult: CompilationResult) throws { guard !compilationResult.referencedTypes.contains(where: { namedType in namedType.swiftName == context.schemaName.firstUppercased diff --git a/Tests/ApolloCodegenTests/ApolloCodegenTests.swift b/Tests/ApolloCodegenTests/ApolloCodegenTests.swift index 47a8fb6ee7..4e51153f64 100644 --- a/Tests/ApolloCodegenTests/ApolloCodegenTests.swift +++ b/Tests/ApolloCodegenTests/ApolloCodegenTests.swift @@ -2029,9 +2029,7 @@ class ApolloCodegenTests: XCTestCase { schemaName: name ), rootURL: nil) - expect(try ApolloCodegen.validate( - context: configContext, - compilationResult: compilationResult)) + expect(try ApolloCodegen.validate(configContext, with: compilationResult)) .to(throwError(ApolloCodegen.Error.schemaNameConflict(name: configContext.schemaName))) } } @@ -2049,9 +2047,7 @@ class ApolloCodegenTests: XCTestCase { schemaName: name ), rootURL: nil) - expect(try ApolloCodegen.validate( - context: configContext, - compilationResult: compilationResult)) + expect(try ApolloCodegen.validate(configContext, with: compilationResult)) .to(throwError(ApolloCodegen.Error.schemaNameConflict(name: configContext.schemaName))) } } @@ -2069,9 +2065,7 @@ class ApolloCodegenTests: XCTestCase { schemaName: name ), rootURL: nil) - expect(try ApolloCodegen.validate( - context: configContext, - compilationResult: compilationResult)) + expect(try ApolloCodegen.validate(configContext, with: compilationResult)) .to(throwError(ApolloCodegen.Error.schemaNameConflict(name: configContext.schemaName))) } } @@ -2089,9 +2083,7 @@ class ApolloCodegenTests: XCTestCase { schemaName: name ), rootURL: nil) - expect(try ApolloCodegen.validate( - context: configContext, - compilationResult: compilationResult)) + expect(try ApolloCodegen.validate(configContext, with: compilationResult)) .to(throwError(ApolloCodegen.Error.schemaNameConflict(name: configContext.schemaName))) } } @@ -2109,9 +2101,7 @@ class ApolloCodegenTests: XCTestCase { schemaName: name ), rootURL: nil) - expect(try ApolloCodegen.validate( - context: configContext, - compilationResult: compilationResult)) + expect(try ApolloCodegen.validate(configContext, with: compilationResult)) .to(throwError(ApolloCodegen.Error.schemaNameConflict(name: configContext.schemaName))) } } @@ -2129,9 +2119,7 @@ class ApolloCodegenTests: XCTestCase { schemaName: name ), rootURL: nil) - expect(try ApolloCodegen.validate( - context: configContext, - compilationResult: compilationResult)) + expect(try ApolloCodegen.validate(configContext, with: compilationResult)) .to(throwError(ApolloCodegen.Error.schemaNameConflict(name: configContext.schemaName))) } } @@ -2151,9 +2139,7 @@ class ApolloCodegenTests: XCTestCase { schemaName: name ), rootURL: nil) - expect(try ApolloCodegen.validate( - context: configContext, - compilationResult: compilationResult)) + expect(try ApolloCodegen.validate(configContext, with: compilationResult)) .to(throwError(ApolloCodegen.Error.schemaNameConflict(name: configContext.schemaName))) } } @@ -2224,9 +2210,7 @@ class ApolloCodegenTests: XCTestCase { schemaName: "MySchema" ), rootURL: nil) - expect(try ApolloCodegen.validate( - context: configContext, - compilationResult: compilationResult)) + expect(try ApolloCodegen.validate(configContext, with: compilationResult)) .notTo(throwError()) }