From 829e6ac581c844d88d8c4f4a91cca5fd67ba7afc Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 19 Nov 2023 13:38:08 -0700 Subject: [PATCH 01/34] fix: Adds all graphql-js validation rules --- .../GraphQL/Validation/SpecifiedRules.swift | 44 +++++++++++-------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index 521abe68..45f6220a 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -2,28 +2,36 @@ * This set includes all validation rules defined by the GraphQL spec. */ public let specifiedRules: [(ValidationContext) -> Visitor] = [ - // UniqueOperationNames, -// LoneAnonymousOperation, -// KnownTypeNames, -// FragmentsOnCompositeTypes, -// VariablesAreInputTypes, +// ExecutableDefinitionsRule, +// UniqueOperationNamesRule, +// LoneAnonymousOperationRule, +// SingleFieldSubscriptionsRule, +// KnownTypeNamesRule, +// FragmentsOnCompositeTypesRule, +// VariablesAreInputTypesRule, ScalarLeafsRule, FieldsOnCorrectTypeRule, -// UniqueFragmentNames, -// KnownFragmentNames, -// NoUnusedFragments, +// UniqueFragmentNamesRule, +// KnownFragmentNamesRule, +// NoUnusedFragmentsRule, PossibleFragmentSpreadsRule, -// NoFragmentCycles, -// UniqueVariableNames, -// NoUndefinedVariables, +// NoFragmentCyclesRule, +// UniqueVariableNamesRule, +// NoUndefinedVariablesRule, NoUnusedVariablesRule, -// KnownDirectives, +// KnownDirectivesRule, +// UniqueDirectivesPerLocationRule, +// DeferStreamDirectiveOnRootFieldRule, +// DeferStreamDirectiveOnValidOperationsRule, +// DeferStreamDirectiveLabelRule, KnownArgumentNamesRule, -// UniqueArgumentNames, -// ArgumentsOfCorrectType, +// UniqueArgumentNamesRule, +// ArgumentsOfCorrectTypeRule, +// ValuesOfCorrectTypeRule, +// ProvidedRequiredArgumentsRule, ProvidedNonNullArgumentsRule, -// DefaultValuesOfCorrectType, -// VariablesInAllowedPosition, -// OverlappingFieldsCanBeMerged, -// UniqueInputFieldNames, +// DefaultValuesOfCorrectTypeRule, +// VariablesInAllowedPositionRule, +// OverlappingFieldsCanBeMergedRule, +// UniqueInputFieldNamesRule, ] From 37969495a52385347e45904b1fad81f909111e39 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 12 Nov 2023 12:11:10 -0700 Subject: [PATCH 02/34] test: Adds multi-location comparison function --- .../ValidationTests/ValidationTests.swift | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/Tests/GraphQLTests/ValidationTests/ValidationTests.swift b/Tests/GraphQLTests/ValidationTests/ValidationTests.swift index fa960664..b8b7aff5 100644 --- a/Tests/GraphQLTests/ValidationTests/ValidationTests.swift +++ b/Tests/GraphQLTests/ValidationTests/ValidationTests.swift @@ -79,4 +79,44 @@ class ValidationTestCase: XCTestCase { let errorPath = error.path.elements.map { $0.description }.joined(separator: " ") XCTAssertEqual(errorPath, path, "Unexpected error path", file: testFile, line: testLine) } + + func assertValidationError( + error: GraphQLError?, + locations: [(line: Int, column: Int)], + path: String = "", + message: String, + testFile: StaticString = #file, + testLine: UInt = #line + ) throws { + guard let error = error else { + return XCTFail("Error was not provided") + } + + XCTAssertEqual( + error.message, + message, + "Unexpected error message", + file: testFile, + line: testLine + ) + for (index, actualLocation) in error.locations.enumerated() { + let expectedLocation = locations[index] + XCTAssertEqual( + actualLocation.line, + expectedLocation.line, + "Unexpected line location", + file: testFile, + line: testLine + ) + XCTAssertEqual( + actualLocation.column, + expectedLocation.column, + "Unexpected column location", + file: testFile, + line: testLine + ) + } + let errorPath = error.path.elements.map { $0.description }.joined(separator: " ") + XCTAssertEqual(errorPath, path, "Unexpected error path", file: testFile, line: testLine) + } } From 2bcecfe04042aff955c6016e3b56dab1d819734c Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Mon, 6 Nov 2023 00:21:14 -0700 Subject: [PATCH 03/34] feature: Adds NoUnusedFragmentsRule --- .../Rules/NoUnusedFragmentsRule.swift | 47 +++++ .../GraphQL/Validation/SpecifiedRules.swift | 2 +- .../NoUnusedFragmentsRuleTests.swift | 171 ++++++++++++++++++ 3 files changed, 219 insertions(+), 1 deletion(-) create mode 100644 Sources/GraphQL/Validation/Rules/NoUnusedFragmentsRule.swift create mode 100644 Tests/GraphQLTests/ValidationTests/NoUnusedFragmentsRuleTests.swift diff --git a/Sources/GraphQL/Validation/Rules/NoUnusedFragmentsRule.swift b/Sources/GraphQL/Validation/Rules/NoUnusedFragmentsRule.swift new file mode 100644 index 00000000..05e2a868 --- /dev/null +++ b/Sources/GraphQL/Validation/Rules/NoUnusedFragmentsRule.swift @@ -0,0 +1,47 @@ + +/** + * No unused fragments + * + * A GraphQL document is only valid if all fragment definitions are spread + * within operations, or spread within other fragments spread within operations. + * + * See https://spec.graphql.org/draft/#sec-Fragments-Must-Be-Used + */ +func NoUnusedFragmentsRule(context: ValidationContext) -> Visitor { + var fragmentNameUsed = Set() + var fragmentDefs = [FragmentDefinition]() + + return Visitor( + enter: { node, _, _, _, _ in + if let operation = node as? OperationDefinition { + for fragment in context.getRecursivelyReferencedFragments(operation: operation) { + fragmentNameUsed.insert(fragment.name.value) + } + return .continue + } + + if let fragment = node as? FragmentDefinition { + fragmentDefs.append(fragment) + return .continue + } + return .continue + }, + leave: { node, _, _, _, _ -> VisitResult in + // Use Document as proxy for the end of the visitation + if node is Document { + for fragmentDef in fragmentDefs { + let fragName = fragmentDef.name.value + if !fragmentNameUsed.contains(fragName) { + context.report( + error: GraphQLError( + message: "Fragment \"\(fragName)\" is never used.", + nodes: [fragmentDef] + ) + ) + } + } + } + return .continue + } + ) +} diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index 45f6220a..3643ac2e 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -13,7 +13,7 @@ public let specifiedRules: [(ValidationContext) -> Visitor] = [ FieldsOnCorrectTypeRule, // UniqueFragmentNamesRule, // KnownFragmentNamesRule, -// NoUnusedFragmentsRule, + NoUnusedFragmentsRule, PossibleFragmentSpreadsRule, // NoFragmentCyclesRule, // UniqueVariableNamesRule, diff --git a/Tests/GraphQLTests/ValidationTests/NoUnusedFragmentsRuleTests.swift b/Tests/GraphQLTests/ValidationTests/NoUnusedFragmentsRuleTests.swift new file mode 100644 index 00000000..eaeb82e6 --- /dev/null +++ b/Tests/GraphQLTests/ValidationTests/NoUnusedFragmentsRuleTests.swift @@ -0,0 +1,171 @@ +@testable import GraphQL +import XCTest + +class NoUnusedFragmentsRuleTests: ValidationTestCase { + override func setUp() { + rule = NoUnusedFragmentsRule + } + + func testAllFragmentNamesAreUsed() throws { + try assertValid( + """ + { + human(id: 4) { + ...HumanFields1 + ... on Human { + ...HumanFields2 + } + } + } + fragment HumanFields1 on Human { + name + ...HumanFields3 + } + fragment HumanFields2 on Human { + name + } + fragment HumanFields3 on Human { + name + } + """ + ) + } + + func testAllFragmentNamesAreUsedByMultipleOperations() throws { + try assertValid( + """ + query Foo { + human(id: 4) { + ...HumanFields1 + } + } + query Bar { + human(id: 4) { + ...HumanFields2 + } + } + fragment HumanFields1 on Human { + name + ...HumanFields3 + } + fragment HumanFields2 on Human { + name + } + fragment HumanFields3 on Human { + name + } + """ + ) + } + + func testContainsUnknownFragments() throws { + let errors = try assertInvalid( + errorCount: 2, + query: """ + query Foo { + human(id: 4) { + ...HumanFields1 + } + } + query Bar { + human(id: 4) { + ...HumanFields2 + } + } + fragment HumanFields1 on Human { + name + ...HumanFields3 + } + fragment HumanFields2 on Human { + name + } + fragment HumanFields3 on Human { + name + } + fragment Unused1 on Human { + name + } + fragment Unused2 on Human { + name + } + """ + ) + + try assertValidationError( + error: errors[0], line: 21, column: 1, + message: "Fragment \"Unused1\" is never used." + ) + + try assertValidationError( + error: errors[1], line: 24, column: 1, + message: "Fragment \"Unused2\" is never used." + ) + } + + func testContainsUnknownFragmentsWithRefCycle() throws { + let errors = try assertInvalid( + errorCount: 2, + query: """ + query Foo { + human(id: 4) { + ...HumanFields1 + } + } + query Bar { + human(id: 4) { + ...HumanFields2 + } + } + fragment HumanFields1 on Human { + name + ...HumanFields3 + } + fragment HumanFields2 on Human { + name + } + fragment HumanFields3 on Human { + name + } + fragment Unused1 on Human { + name + ...Unused2 + } + fragment Unused2 on Human { + name + ...Unused1 + } + """ + ) + + try assertValidationError( + error: errors[0], line: 21, column: 1, + message: "Fragment \"Unused1\" is never used." + ) + + try assertValidationError( + error: errors[1], line: 25, column: 1, + message: "Fragment \"Unused2\" is never used." + ) + } + + func testContainsUnknownAndUndefFragments() throws { + let errors = try assertInvalid( + errorCount: 1, + query: """ + query Foo { + human(id: 4) { + ...bar + } + } + fragment foo on Human { + name + } + """ + ) + + try assertValidationError( + error: errors[0], line: 6, column: 1, + message: "Fragment \"foo\" is never used." + ) + } +} From c2b52db95904b8858208113a5f3f115cb4527ad4 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 12 Nov 2023 11:12:48 -0700 Subject: [PATCH 04/34] feature: Adds UniqueOperationNamesRule --- .../Rules/UniqueOperationNamesRule.swift | 30 ++++ .../GraphQL/Validation/SpecifiedRules.swift | 2 +- .../UniqueOperationNamesRuleTests.swift | 152 ++++++++++++++++++ 3 files changed, 183 insertions(+), 1 deletion(-) create mode 100644 Sources/GraphQL/Validation/Rules/UniqueOperationNamesRule.swift create mode 100644 Tests/GraphQLTests/ValidationTests/UniqueOperationNamesRuleTests.swift diff --git a/Sources/GraphQL/Validation/Rules/UniqueOperationNamesRule.swift b/Sources/GraphQL/Validation/Rules/UniqueOperationNamesRule.swift new file mode 100644 index 00000000..60b91438 --- /dev/null +++ b/Sources/GraphQL/Validation/Rules/UniqueOperationNamesRule.swift @@ -0,0 +1,30 @@ + +/** + * Unique operation names + * + * A GraphQL document is only valid if all defined operations have unique names. + * + * See https://spec.graphql.org/draft/#sec-Operation-Name-Uniqueness + */ +func UniqueOperationNamesRule(context: ValidationContext) -> Visitor { + var knownOperationNames = [String: Name]() + return Visitor( + enter: { node, _, _, _, _ in + if let operation = node as? OperationDefinition { + if let operationName = operation.name { + if let knownOperationName = knownOperationNames[operationName.value] { + context.report( + error: GraphQLError( + message: "There can be only one operation named \"\(operationName.value)\".", + nodes: [knownOperationName, operationName] + ) + ) + } else { + knownOperationNames[operationName.value] = operationName + } + } + } + return .continue + } + ) +} diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index 3643ac2e..2aa9b67a 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -3,7 +3,7 @@ */ public let specifiedRules: [(ValidationContext) -> Visitor] = [ // ExecutableDefinitionsRule, -// UniqueOperationNamesRule, + UniqueOperationNamesRule, // LoneAnonymousOperationRule, // SingleFieldSubscriptionsRule, // KnownTypeNamesRule, diff --git a/Tests/GraphQLTests/ValidationTests/UniqueOperationNamesRuleTests.swift b/Tests/GraphQLTests/ValidationTests/UniqueOperationNamesRuleTests.swift new file mode 100644 index 00000000..fae39fbb --- /dev/null +++ b/Tests/GraphQLTests/ValidationTests/UniqueOperationNamesRuleTests.swift @@ -0,0 +1,152 @@ +@testable import GraphQL +import XCTest + +class UniqueOperationNamesRuleTests: ValidationTestCase { + override func setUp() { + rule = UniqueOperationNamesRule + } + + func testNoOperations() throws { + try assertValid( + """ + fragment fragA on Type { + field + } + """ + ) + } + + func testOneAnonOperation() throws { + try assertValid( + """ + { + field + } + """ + ) + } + + func testOneNamedOperation() throws { + try assertValid( + """ + query Foo { + field + } + """ + ) + } + + func testMultipleOperations() throws { + try assertValid( + """ + query Foo { + field + } + + query Bar { + field + } + """ + ) + } + + func testMultipleOperationsOfDifferentTypes() throws { + try assertValid( + """ + query Foo { + field + } + + mutation Bar { + field + } + + subscription Baz { + field + } + """ + ) + } + + func testFragmentAndOperationNamedTheSame() throws { + try assertValid( + """ + query Foo { + ...Foo + } + fragment Foo on Type { + field + } + """ + ) + } + + func testMultipleOperationsOfSameName() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + query Foo { + fieldA + } + query Foo { + fieldB + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 1, column: 7), + (line: 4, column: 7), + ], + message: "There can be only one operation named \"Foo\"." + ) + } + + func testMultipleOperationsOfDifferentTypesMutation() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + query Foo { + fieldA + } + mutation Foo { + fieldB + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 1, column: 7), + (line: 4, column: 10), + ], + message: "There can be only one operation named \"Foo\"." + ) + } + + func testMultipleOperationsOfDifferentTypesSubscription() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + query Foo { + fieldA + } + subscription Foo { + fieldB + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 1, column: 7), + (line: 4, column: 14), + ], + message: "There can be only one operation named \"Foo\"." + ) + } +} From 79c7ca08f218f9cad6d9dffde0a1c59f41efda0b Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 12 Nov 2023 11:27:34 -0700 Subject: [PATCH 05/34] feature: Adds LoneAnonymousOperationRule --- .../Rules/LoneAnonymousOperationRule.swift | 32 +++++ .../GraphQL/Validation/SpecifiedRules.swift | 2 +- .../LoneAnonymousOperationRuleTests.swift | 120 ++++++++++++++++++ 3 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 Sources/GraphQL/Validation/Rules/LoneAnonymousOperationRule.swift create mode 100644 Tests/GraphQLTests/ValidationTests/LoneAnonymousOperationRuleTests.swift diff --git a/Sources/GraphQL/Validation/Rules/LoneAnonymousOperationRule.swift b/Sources/GraphQL/Validation/Rules/LoneAnonymousOperationRule.swift new file mode 100644 index 00000000..783327ce --- /dev/null +++ b/Sources/GraphQL/Validation/Rules/LoneAnonymousOperationRule.swift @@ -0,0 +1,32 @@ + +/** + * Lone anonymous operation + * + * A GraphQL document is only valid if when it contains an anonymous operation + * (the query short-hand) that it contains only that one operation definition. + * + * See https://spec.graphql.org/draft/#sec-Lone-Anonymous-Operation + */ +func LoneAnonymousOperationRule(context: ValidationContext) -> Visitor { + var operationCount = 0 + return Visitor( + enter: { node, _, _, _, _ in + if let document = node as? Document { + operationCount = document.definitions.filter { $0 is OperationDefinition }.count + return .continue + } + if let operation = node as? OperationDefinition { + if operation.name == nil, operationCount > 1 { + context.report( + error: GraphQLError( + message: "This anonymous operation must be the only defined operation.", + nodes: [operation] + ) + ) + } + return .continue + } + return .continue + } + ) +} diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index 2aa9b67a..0b0c10cf 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -4,7 +4,7 @@ public let specifiedRules: [(ValidationContext) -> Visitor] = [ // ExecutableDefinitionsRule, UniqueOperationNamesRule, -// LoneAnonymousOperationRule, + LoneAnonymousOperationRule, // SingleFieldSubscriptionsRule, // KnownTypeNamesRule, // FragmentsOnCompositeTypesRule, diff --git a/Tests/GraphQLTests/ValidationTests/LoneAnonymousOperationRuleTests.swift b/Tests/GraphQLTests/ValidationTests/LoneAnonymousOperationRuleTests.swift new file mode 100644 index 00000000..29810040 --- /dev/null +++ b/Tests/GraphQLTests/ValidationTests/LoneAnonymousOperationRuleTests.swift @@ -0,0 +1,120 @@ +@testable import GraphQL +import XCTest + +class LoneAnonymousOperationRuleTests: ValidationTestCase { + override func setUp() { + rule = LoneAnonymousOperationRule + } + + func testNoOperations() throws { + try assertValid( + """ + fragment fragA on Type { + field + } + """ + ) + } + + func testOneAnonOperation() throws { + try assertValid( + """ + { + field + } + """ + ) + } + + func testMultipleNamedOperations() throws { + try assertValid( + """ + query Foo { + field + } + + query Bar { + field + } + """ + ) + } + + func testAnonOperationWithFragment() throws { + try assertValid( + """ + { + ...Foo + } + fragment Foo on Type { + field + } + """ + ) + } + + func testMultipleAnonOperations() throws { + let errors = try assertInvalid( + errorCount: 2, + query: + """ + { + fieldA + } + { + fieldB + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 1, column: 1)], + message: "This anonymous operation must be the only defined operation." + ) + try assertValidationError( + error: errors[1], + locations: [(line: 4, column: 1)], + message: "This anonymous operation must be the only defined operation." + ) + } + + func testAnonOperationWithAMutation() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + fieldA + } + mutation Foo { + fieldB + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 1, column: 1)], + message: "This anonymous operation must be the only defined operation." + ) + } + + func testAnonOperationWithASubscription() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + fieldA + } + subscription Foo { + fieldB + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 1, column: 1)], + message: "This anonymous operation must be the only defined operation." + ) + } +} From 787b92d024af30638cc00668735fb129371c2dcb Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 12 Nov 2023 12:07:43 -0700 Subject: [PATCH 06/34] fix: suggestionList ordering is deterministic --- Sources/GraphQL/SwiftUtilities/SuggestionList.swift | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Sources/GraphQL/SwiftUtilities/SuggestionList.swift b/Sources/GraphQL/SwiftUtilities/SuggestionList.swift index 5eea5a95..8208735d 100644 --- a/Sources/GraphQL/SwiftUtilities/SuggestionList.swift +++ b/Sources/GraphQL/SwiftUtilities/SuggestionList.swift @@ -20,7 +20,12 @@ func suggestionList( } return optionsByDistance.keys.sorted { // Values are guaranteed non-nil since the keys come from the object itself - optionsByDistance[$0]! - optionsByDistance[$1]! != 0 + let distanceDiff = optionsByDistance[$0]! - optionsByDistance[$1]! + if distanceDiff != 0 { + return distanceDiff < 0 + } else { + return $0.lexicographicallyPrecedes($1) + } } } From 7a73e8576a6fab72b4b776c85c3314a5faefc01d Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 12 Nov 2023 11:56:43 -0700 Subject: [PATCH 07/34] feature: Adds KnownTypeNamesRule --- .../Validation/Rules/KnownTypeNamesRule.swift | 52 ++++++++++++++++ .../GraphQL/Validation/SpecifiedRules.swift | 2 +- .../KnownTypeNamesRuleTests.swift | 61 +++++++++++++++++++ 3 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 Sources/GraphQL/Validation/Rules/KnownTypeNamesRule.swift create mode 100644 Tests/GraphQLTests/ValidationTests/KnownTypeNamesRuleTests.swift diff --git a/Sources/GraphQL/Validation/Rules/KnownTypeNamesRule.swift b/Sources/GraphQL/Validation/Rules/KnownTypeNamesRule.swift new file mode 100644 index 00000000..08c73a05 --- /dev/null +++ b/Sources/GraphQL/Validation/Rules/KnownTypeNamesRule.swift @@ -0,0 +1,52 @@ + +/** + * Known type names + * + * A GraphQL document is only valid if referenced types (specifically + * variable definitions and fragment conditions) are defined by the type schema. + * + * See https://spec.graphql.org/draft/#sec-Fragment-Spread-Type-Existence + */ +func KnownTypeNamesRule(context: ValidationContext) -> Visitor { + let definitions = context.ast.definitions + let existingTypesMap = context.schema.typeMap + + var typeNames = Set() + for typeName in existingTypesMap.keys { + typeNames.insert(typeName) + } + for definition in definitions { + if + let type = definition as? TypeDefinition, + let nameResult = type.get(key: "name"), + case let .node(nameNode) = nameResult, + let name = nameNode as? Name + { + typeNames.insert(name.value) + } + } + + return Visitor( + enter: { node, _, _, _, _ in + if let type = node as? NamedType { + let typeName = type.name.value + if !typeNames.contains(typeName) { + // TODO: Add SDL support + + let suggestedTypes = suggestionList( + input: typeName, + options: Array(typeNames) + ) + context.report( + error: GraphQLError( + message: "Unknown type \"\(typeName)\"." + + didYouMean(suggestions: suggestedTypes), + nodes: [node] + ) + ) + } + } + return .continue + } + ) +} diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index 0b0c10cf..ca28e832 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -6,7 +6,7 @@ public let specifiedRules: [(ValidationContext) -> Visitor] = [ UniqueOperationNamesRule, LoneAnonymousOperationRule, // SingleFieldSubscriptionsRule, -// KnownTypeNamesRule, + KnownTypeNamesRule, // FragmentsOnCompositeTypesRule, // VariablesAreInputTypesRule, ScalarLeafsRule, diff --git a/Tests/GraphQLTests/ValidationTests/KnownTypeNamesRuleTests.swift b/Tests/GraphQLTests/ValidationTests/KnownTypeNamesRuleTests.swift new file mode 100644 index 00000000..1dcc85b0 --- /dev/null +++ b/Tests/GraphQLTests/ValidationTests/KnownTypeNamesRuleTests.swift @@ -0,0 +1,61 @@ +@testable import GraphQL +import XCTest + +class KnownTypeNamesRuleTests: ValidationTestCase { + override func setUp() { + rule = KnownTypeNamesRule + } + + func testKnownTypeNamesAreValid() throws { + try assertValid( + """ + query Foo( + $var: String + $required: [Int!]! + $introspectionType: __EnumValue + ) { + user(id: 4) { + pets { ... on Pet { name }, ...PetFields, ... { name } } + } + } + + fragment PetFields on Pet { + name + } + """ + ) + } + + func testUnknownTypeNamesAreInvalid() throws { + let errors = try assertInvalid( + errorCount: 3, + query: + """ + query Foo($var: [JumbledUpLetters!]!) { + user(id: 4) { + name + pets { ... on Badger { name }, ...PetFields } + } + } + fragment PetFields on Peat { + name + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 1, column: 18)], + message: "Unknown type \"JumbledUpLetters\"." + ) + try assertValidationError( + error: errors[1], + locations: [(line: 4, column: 19)], + message: "Unknown type \"Badger\"." + ) + try assertValidationError( + error: errors[2], + locations: [(line: 7, column: 23)], + message: "Unknown type \"Peat\". Did you mean \"Pet\" or \"Cat\"?" + ) + } +} From 9ab0dfe4d9e315150ae7de4bc3125601ed507d14 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 12 Nov 2023 18:45:19 -0700 Subject: [PATCH 08/34] test: Adds ComplexInput & OneOfInput to example schema --- .../ValidationTests/ExampleSchema.swift | 193 ++++++++++++++++++ 1 file changed, 193 insertions(+) diff --git a/Tests/GraphQLTests/ValidationTests/ExampleSchema.swift b/Tests/GraphQLTests/ValidationTests/ExampleSchema.swift index 1defddde..803a4997 100644 --- a/Tests/GraphQLTests/ValidationTests/ExampleSchema.swift +++ b/Tests/GraphQLTests/ValidationTests/ExampleSchema.swift @@ -376,6 +376,198 @@ let ValidationExampleHumanOrAlien = try! GraphQLUnionType( types: [ValidationExampleHuman, ValidationExampleAlien] ) +// input ComplexInput { +// requiredField: Boolean! +// nonNullField: Boolean! = false +// intField: Int +// stringField: String +// booleanField: Boolean +// stringListField: [String] +// } +let ValidationExampleComplexInput = try! GraphQLInputObjectType( + name: "ComplexInput", + fields: [ + "requiredField": InputObjectField(type: GraphQLNonNull(GraphQLBoolean)), + "nonNullField": InputObjectField(type: GraphQLNonNull(GraphQLBoolean), defaultValue: false), + "intField": InputObjectField(type: GraphQLInt), + "stringField": InputObjectField(type: GraphQLString), + "booleanField": InputObjectField(type: GraphQLBoolean), + "stringListField": InputObjectField(type: GraphQLList(GraphQLString)), + ] +) + +// input OneOfInput @oneOf { +// stringField: String +// intField: Int +// } +let ValidationExampleOneOfInput = try! GraphQLInputObjectType( + name: "OneOfInput", + // TODO: Add @oneOf directive + fields: [ + "stringField": InputObjectField(type: GraphQLBoolean), + "intField": InputObjectField(type: GraphQLInt), + ] +) + +// type ComplicatedArgs { +// # TODO List +// # TODO Coercion +// # TODO NotNulls +// intArgField(intArg: Int): String +// nonNullIntArgField(nonNullIntArg: Int!): String +// stringArgField(stringArg: String): String +// booleanArgField(booleanArg: Boolean): String +// enumArgField(enumArg: FurColor): String +// floatArgField(floatArg: Float): String +// idArgField(idArg: ID): String +// stringListArgField(stringListArg: [String]): String +// stringListNonNullArgField(stringListNonNullArg: [String!]): String +// complexArgField(complexArg: ComplexInput): String +// oneOfArgField(oneOfArg: OneOfInput): String +// multipleReqs(req1: Int!, req2: Int!): String +// nonNullFieldWithDefault(arg: Int! = 0): String +// multipleOpts(opt1: Int = 0, opt2: Int = 0): String +// multipleOptAndReq(req1: Int!, req2: Int!, opt1: Int = 0, opt2: Int = 0): String +// } +let ValidationExampleComplicatedArgs = try! GraphQLObjectType( + name: "ComplicatedArgs", + fields: [ + "intArgField": GraphQLField( + type: GraphQLString, + args: ["intArg": GraphQLArgument(type: GraphQLInt)], + resolve: { inputValue, _, _, _ -> String? in + print(type(of: inputValue)) + return nil + } + ), + "nonNullIntArgField": GraphQLField( + type: GraphQLString, + args: ["nonNullIntArg": GraphQLArgument(type: GraphQLNonNull(GraphQLInt))], + resolve: { inputValue, _, _, _ -> String? in + print(type(of: inputValue)) + return nil + } + ), + "stringArgField": GraphQLField( + type: GraphQLString, + args: ["stringArg": GraphQLArgument(type: GraphQLString)], + resolve: { inputValue, _, _, _ -> String? in + print(type(of: inputValue)) + return nil + } + ), + "booleanArgField": GraphQLField( + type: GraphQLString, + args: ["booleanArg": GraphQLArgument(type: GraphQLBoolean)], + resolve: { inputValue, _, _, _ -> String? in + print(type(of: inputValue)) + return nil + } + ), + "enumArgField": GraphQLField( + type: GraphQLString, + args: ["enumArg": GraphQLArgument(type: ValidationExampleFurColor)], + resolve: { inputValue, _, _, _ -> String? in + print(type(of: inputValue)) + return nil + } + ), + "floatArgField": GraphQLField( + type: GraphQLString, + args: ["floatArg": GraphQLArgument(type: GraphQLFloat)], + resolve: { inputValue, _, _, _ -> String? in + print(type(of: inputValue)) + return nil + } + ), + "idArgField": GraphQLField( + type: GraphQLString, + args: ["idArg": GraphQLArgument(type: GraphQLID)], + resolve: { inputValue, _, _, _ -> String? in + print(type(of: inputValue)) + return nil + } + ), + "stringListArgField": GraphQLField( + type: GraphQLString, + args: ["stringListArg": GraphQLArgument(type: GraphQLList(GraphQLString))], + resolve: { inputValue, _, _, _ -> String? in + print(type(of: inputValue)) + return nil + } + ), + "stringListNonNullArgField": GraphQLField( + type: GraphQLString, + args: [ + "stringListNonNullArg": GraphQLArgument(type: GraphQLList(GraphQLNonNull(GraphQLString))), + ], + resolve: { inputValue, _, _, _ -> String? in + print(type(of: inputValue)) + return nil + } + ), + "complexArgField": GraphQLField( + type: GraphQLString, + args: ["complexArg": GraphQLArgument(type: ValidationExampleComplexInput)], + resolve: { inputValue, _, _, _ -> String? in + print(type(of: inputValue)) + return nil + } + ), + "oneOfArgField": GraphQLField( + type: GraphQLString, + args: ["oneOfArg": GraphQLArgument(type: ValidationExampleOneOfInput)], + resolve: { inputValue, _, _, _ -> String? in + print(type(of: inputValue)) + return nil + } + ), + "multipleReqs": GraphQLField( + type: GraphQLString, + args: [ + "req1": GraphQLArgument(type: GraphQLNonNull(GraphQLInt)), + "req2": GraphQLArgument(type: GraphQLNonNull(GraphQLInt)), + ], + resolve: { inputValue, _, _, _ -> String? in + print(type(of: inputValue)) + return nil + } + ), + "nonNullFieldWithDefault": GraphQLField( + type: GraphQLString, + args: ["arg": GraphQLArgument(type: GraphQLNonNull(GraphQLInt), defaultValue: 0)], + resolve: { inputValue, _, _, _ -> String? in + print(type(of: inputValue)) + return nil + } + ), + "multipleOpts": GraphQLField( + type: GraphQLString, + args: [ + "opt1": GraphQLArgument(type: GraphQLInt, defaultValue: 0), + "opt2": GraphQLArgument(type: GraphQLInt, defaultValue: 0), + ], + resolve: { inputValue, _, _, _ -> String? in + print(type(of: inputValue)) + return nil + } + ), + "multipleOptAndReq": GraphQLField( + type: GraphQLString, + args: [ + "req1": GraphQLArgument(type: GraphQLNonNull(GraphQLInt)), + "req2": GraphQLArgument(type: GraphQLNonNull(GraphQLInt)), + "opt1": GraphQLArgument(type: GraphQLInt, defaultValue: 0), + "opt2": GraphQLArgument(type: GraphQLInt, defaultValue: 0), + ], + resolve: { inputValue, _, _, _ -> String? in + print(type(of: inputValue)) + return nil + } + ), + ] +) + // type QueryRoot { // dog: Dog // } @@ -391,6 +583,7 @@ let ValidationExampleQueryRoot = try! GraphQLObjectType( return nil }, "humanOrAlien": GraphQLField(type: ValidationExampleHumanOrAlien), + "complicatedArgs": GraphQLField(type: ValidationExampleComplicatedArgs), ] ) From 073718b5c036d213263af820812b575709a8ea9f Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 12 Nov 2023 18:46:05 -0700 Subject: [PATCH 09/34] feature: Adds FragmentsOnCompositeTypesRule --- .../Rules/FragmentsOnCompositeTypesRule.swift | 52 +++++++ .../GraphQL/Validation/SpecifiedRules.swift | 2 +- .../FragmentsOnCompositeTypesRuleTests.swift | 144 ++++++++++++++++++ 3 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 Sources/GraphQL/Validation/Rules/FragmentsOnCompositeTypesRule.swift create mode 100644 Tests/GraphQLTests/ValidationTests/FragmentsOnCompositeTypesRuleTests.swift diff --git a/Sources/GraphQL/Validation/Rules/FragmentsOnCompositeTypesRule.swift b/Sources/GraphQL/Validation/Rules/FragmentsOnCompositeTypesRule.swift new file mode 100644 index 00000000..c8b6eeb0 --- /dev/null +++ b/Sources/GraphQL/Validation/Rules/FragmentsOnCompositeTypesRule.swift @@ -0,0 +1,52 @@ + +/** + * Fragments on composite type + * + * Fragments use a type condition to determine if they apply, since fragments + * can only be spread into a composite type (object, interface, or union), the + * type condition must also be a composite type. + * + * See https://spec.graphql.org/draft/#sec-Fragments-On-Composite-Types + */ +func FragmentsOnCompositeTypesRule(context: ValidationContext) -> Visitor { + return Visitor( + enter: { node, _, _, _, _ in + if let fragment = node as? InlineFragment { + if let typeCondition = fragment.typeCondition { + if let type = typeFromAST(schema: context.schema, inputTypeAST: typeCondition) { + if type is GraphQLCompositeType { + return .continue + } + let typeStr = typeCondition.name.value + context.report( + error: GraphQLError( + message: + "Fragment cannot condition on non composite type \"\(typeStr)\".", + nodes: [typeCondition] + ) + ) + } + } + return .continue + } + if let fragment = node as? FragmentDefinition { + let typeCondition = fragment.typeCondition + if let type = typeFromAST(schema: context.schema, inputTypeAST: typeCondition) { + if type is GraphQLCompositeType { + return .continue + } + let typeStr = typeCondition.name.value + context.report( + error: GraphQLError( + message: + "Fragment \"\(fragment.name.value)\" cannot condition on non composite type \"\(typeStr)\".", + nodes: [typeCondition] + ) + ) + } + return .continue + } + return .continue + } + ) +} diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index ca28e832..2447c908 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -7,7 +7,7 @@ public let specifiedRules: [(ValidationContext) -> Visitor] = [ LoneAnonymousOperationRule, // SingleFieldSubscriptionsRule, KnownTypeNamesRule, -// FragmentsOnCompositeTypesRule, + FragmentsOnCompositeTypesRule, // VariablesAreInputTypesRule, ScalarLeafsRule, FieldsOnCorrectTypeRule, diff --git a/Tests/GraphQLTests/ValidationTests/FragmentsOnCompositeTypesRuleTests.swift b/Tests/GraphQLTests/ValidationTests/FragmentsOnCompositeTypesRuleTests.swift new file mode 100644 index 00000000..dac81579 --- /dev/null +++ b/Tests/GraphQLTests/ValidationTests/FragmentsOnCompositeTypesRuleTests.swift @@ -0,0 +1,144 @@ +@testable import GraphQL +import XCTest + +class FragmentsOnCompositeTypesRuleTests: ValidationTestCase { + override func setUp() { + rule = FragmentsOnCompositeTypesRule + } + + func testObjectIsValidFragmentType() throws { + try assertValid( + """ + fragment validFragment on Dog { + barks + } + """ + ) + } + + func testInterfaceIsValidFragmentType() throws { + try assertValid( + """ + fragment validFragment on Pet { + name + } + """ + ) + } + + func testObjectIsValidInlineFragmentType() throws { + try assertValid( + """ + fragment validFragment on Pet { + ... on Dog { + barks + } + } + """ + ) + } + + func testInterfaceIsValidInlineFragmentType() throws { + try assertValid( + """ + fragment validFragment on Mammal { + ... on Canine { + name + } + } + """ + ) + } + + func testInlineFragmentWithoutTypeIsValid() throws { + try assertValid( + """ + fragment validFragment on Pet { + ... { + name + } + } + """ + ) + } + + func testUnionIsValidFragmentType() throws { + try assertValid( + """ + fragment validFragment on CatOrDog { + __typename + } + """ + ) + } + + func testScalarIsInvalidFragmentType() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + fragment scalarFragment on Boolean { + bad + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 1, column: 28)], + message: "Fragment \"scalarFragment\" cannot condition on non composite type \"Boolean\"." + ) + } + + func testEnumIsInvalidFragmentType() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + fragment scalarFragment on FurColor { + bad + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 1, column: 28)], + message: "Fragment \"scalarFragment\" cannot condition on non composite type \"FurColor\"." + ) + } + + func testInputObjectIsInvalidFragmentType() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + fragment inputFragment on ComplexInput { + stringField + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 1, column: 27)], + message: "Fragment \"inputFragment\" cannot condition on non composite type \"ComplexInput\"." + ) + } + + func testScalarIsInvalidInlineFragmentType() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + fragment invalidFragment on Pet { + ... on String { + barks + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 2, column: 10)], + message: "Fragment cannot condition on non composite type \"String\"." + ) + } +} From e26affc2c249afc4a4e69783ac953069a6bb0801 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 12 Nov 2023 19:23:31 -0700 Subject: [PATCH 10/34] feature: Adds VariablesAreInputTypesRule --- .../Rules/VariablesAreInputTypesRule.swift | 33 +++++++++++ .../GraphQL/Validation/SpecifiedRules.swift | 2 +- .../VariablesAreInputTypesRuleTests.swift | 55 +++++++++++++++++++ 3 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 Sources/GraphQL/Validation/Rules/VariablesAreInputTypesRule.swift create mode 100644 Tests/GraphQLTests/ValidationTests/VariablesAreInputTypesRuleTests.swift diff --git a/Sources/GraphQL/Validation/Rules/VariablesAreInputTypesRule.swift b/Sources/GraphQL/Validation/Rules/VariablesAreInputTypesRule.swift new file mode 100644 index 00000000..847d576a --- /dev/null +++ b/Sources/GraphQL/Validation/Rules/VariablesAreInputTypesRule.swift @@ -0,0 +1,33 @@ + +/** + * Variables are input types + * + * A GraphQL operation is only valid if all the variables it defines are of + * input types (scalar, enum, or input object). + * + * See https://spec.graphql.org/draft/#sec-Variables-Are-Input-Types + */ +func VariablesAreInputTypesRule(context: ValidationContext) -> Visitor { + return Visitor( + enter: { node, _, _, _, _ in + if let variableDefinition = node as? VariableDefinition { + let variableType = variableDefinition.type + if let type = typeFromAST(schema: context.schema, inputTypeAST: variableType) { + guard !isInputType(type: type) else { + return .continue + } + + let variableName = variableDefinition.variable.name.value + let typeName = print(ast: variableType) + context.report( + error: GraphQLError( + message: "Variable \"$\(variableName)\" cannot be non-input type \"\(typeName)\".", + nodes: [variableType] + ) + ) + } + } + return .continue + } + ) +} diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index 2447c908..f96932f7 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -8,7 +8,7 @@ public let specifiedRules: [(ValidationContext) -> Visitor] = [ // SingleFieldSubscriptionsRule, KnownTypeNamesRule, FragmentsOnCompositeTypesRule, -// VariablesAreInputTypesRule, + VariablesAreInputTypesRule, ScalarLeafsRule, FieldsOnCorrectTypeRule, // UniqueFragmentNamesRule, diff --git a/Tests/GraphQLTests/ValidationTests/VariablesAreInputTypesRuleTests.swift b/Tests/GraphQLTests/ValidationTests/VariablesAreInputTypesRuleTests.swift new file mode 100644 index 00000000..024cc06a --- /dev/null +++ b/Tests/GraphQLTests/ValidationTests/VariablesAreInputTypesRuleTests.swift @@ -0,0 +1,55 @@ +@testable import GraphQL +import XCTest + +class VariablesAreInputTypesRuleTests: ValidationTestCase { + override func setUp() { + rule = VariablesAreInputTypesRule + } + + func testUnknownTypesAreIgnored() throws { + try assertValid( + """ + query Foo($a: Unknown, $b: [[Unknown!]]!) { + field(a: $a, b: $b) + } + """ + ) + } + + func testInputTypesAreValid() throws { + try assertValid( + """ + query Foo($a: String, $b: [Boolean!]!, $c: ComplexInput) { + field(a: $a, b: $b, c: $c) + } + """ + ) + } + + func testOutputTypesAreInvalid() throws { + let errors = try assertInvalid( + errorCount: 3, + query: + """ + query Foo($a: Dog, $b: [[CatOrDog!]]!, $c: Pet) { + field(a: $a, b: $b, c: $c) + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 1, column: 15)], + message: "Variable \"$a\" cannot be non-input type \"Dog\"." + ) + try assertValidationError( + error: errors[1], + locations: [(line: 1, column: 24)], + message: "Variable \"$b\" cannot be non-input type \"[[CatOrDog!]]!\"." + ) + try assertValidationError( + error: errors[2], + locations: [(line: 1, column: 44)], + message: "Variable \"$c\" cannot be non-input type \"Pet\"." + ) + } +} From 5fad1f45ae9e83d555861d00b3ad7e0a5443a858 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Mon, 13 Nov 2023 10:54:04 -0700 Subject: [PATCH 11/34] feature: Adds UniqueFragmentNamesRule --- .../Rules/UniqueFragmentNamesRule.swift | 29 ++++ .../GraphQL/Validation/SpecifiedRules.swift | 2 +- .../UniqueFragmentNamesRuleTests.swift | 130 ++++++++++++++++++ 3 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 Sources/GraphQL/Validation/Rules/UniqueFragmentNamesRule.swift create mode 100644 Tests/GraphQLTests/ValidationTests/UniqueFragmentNamesRuleTests.swift diff --git a/Sources/GraphQL/Validation/Rules/UniqueFragmentNamesRule.swift b/Sources/GraphQL/Validation/Rules/UniqueFragmentNamesRule.swift new file mode 100644 index 00000000..905b09a0 --- /dev/null +++ b/Sources/GraphQL/Validation/Rules/UniqueFragmentNamesRule.swift @@ -0,0 +1,29 @@ + +/** + * Unique fragment names + * + * A GraphQL document is only valid if all defined fragments have unique names. + * + * See https://spec.graphql.org/draft/#sec-Fragment-Name-Uniqueness + */ +func UniqueFragmentNamesRule(context: ValidationContext) -> Visitor { + var knownFragmentNames = [String: Name]() + return Visitor( + enter: { node, _, _, _, _ in + if let fragment = node as? FragmentDefinition { + let fragmentName = fragment.name + if let knownFragmentName = knownFragmentNames[fragmentName.value] { + context.report( + error: GraphQLError( + message: "There can be only one fragment named \"\(fragmentName.value)\".", + nodes: [knownFragmentName, fragmentName] + ) + ) + } else { + knownFragmentNames[fragmentName.value] = fragmentName + } + } + return .continue + } + ) +} diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index f96932f7..9dff3ab3 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -11,7 +11,7 @@ public let specifiedRules: [(ValidationContext) -> Visitor] = [ VariablesAreInputTypesRule, ScalarLeafsRule, FieldsOnCorrectTypeRule, -// UniqueFragmentNamesRule, + UniqueFragmentNamesRule, // KnownFragmentNamesRule, NoUnusedFragmentsRule, PossibleFragmentSpreadsRule, diff --git a/Tests/GraphQLTests/ValidationTests/UniqueFragmentNamesRuleTests.swift b/Tests/GraphQLTests/ValidationTests/UniqueFragmentNamesRuleTests.swift new file mode 100644 index 00000000..4733b47b --- /dev/null +++ b/Tests/GraphQLTests/ValidationTests/UniqueFragmentNamesRuleTests.swift @@ -0,0 +1,130 @@ +@testable import GraphQL +import XCTest + +class UniqueFragmentNamesRuleTests: ValidationTestCase { + override func setUp() { + rule = UniqueFragmentNamesRule + } + + func testNoFragments() throws { + try assertValid( + """ + { + field + } + """ + ) + } + + func testOneFragment() throws { + try assertValid( + """ + { + ...fragA + } + + fragment fragA on Type { + field + } + """ + ) + } + + func testManyFragments() throws { + try assertValid( + """ + { + ...fragA + ...fragB + ...fragC + } + fragment fragA on Type { + fieldA + } + fragment fragB on Type { + fieldB + } + fragment fragC on Type { + fieldC + } + """ + ) + } + + func testInlineFragmentsAreAlwaysUnique() throws { + try assertValid( + """ + { + ...on Type { + fieldA + } + ...on Type { + fieldB + } + } + """ + ) + } + + func testFragmentAndOperationNamedTheSame() throws { + try assertValid( + """ + query Foo { + ...Foo + } + fragment Foo on Type { + field + } + """ + ) + } + + func testFragmentsNamedTheSame() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + ...fragA + } + fragment fragA on Type { + fieldA + } + fragment fragA on Type { + fieldB + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 4, column: 10), + (line: 7, column: 10), + ], + message: "There can be only one fragment named \"fragA\"." + ) + } + + func testFragmentsNamedTheSameWithoutBeingReferenced() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + fragment fragA on Type { + fieldA + } + fragment fragA on Type { + fieldB + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 1, column: 10), + (line: 4, column: 10), + ], + message: "There can be only one fragment named \"fragA\"." + ) + } +} From c5131f64cb220e1b65ef6ac0305e29e522a93a03 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Mon, 13 Nov 2023 11:02:35 -0700 Subject: [PATCH 12/34] feature: Adds KnownFragmentNamesRule --- .../Rules/KnownFragmentNamesRule.swift | 29 ++++++++ .../GraphQL/Validation/SpecifiedRules.swift | 2 +- .../KnownFragmentNamesTests.swift | 73 +++++++++++++++++++ 3 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 Sources/GraphQL/Validation/Rules/KnownFragmentNamesRule.swift create mode 100644 Tests/GraphQLTests/ValidationTests/KnownFragmentNamesTests.swift diff --git a/Sources/GraphQL/Validation/Rules/KnownFragmentNamesRule.swift b/Sources/GraphQL/Validation/Rules/KnownFragmentNamesRule.swift new file mode 100644 index 00000000..a58591e4 --- /dev/null +++ b/Sources/GraphQL/Validation/Rules/KnownFragmentNamesRule.swift @@ -0,0 +1,29 @@ +import Foundation + +/** + * Known fragment names + * + * A GraphQL document is only valid if all `...Fragment` fragment spreads refer + * to fragments defined in the same document. + * + * See https://spec.graphql.org/draft/#sec-Fragment-spread-target-defined + */ +func KnownFragmentNamesRule(context: ValidationContext) -> Visitor { + return Visitor( + enter: { node, _, _, _, _ in + if let fragmentReference = node as? FragmentSpread { + let fragmentName = fragmentReference.name.value + let fragmentDefinition = context.getFragment(name: fragmentName) + + if fragmentDefinition == nil { + context.report(error: GraphQLError( + message: "Unknown fragment \"\(fragmentName)\".", + nodes: [fragmentReference.name] + )) + } + } + + return .continue + } + ) +} diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index 9dff3ab3..37a0e26a 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -12,7 +12,7 @@ public let specifiedRules: [(ValidationContext) -> Visitor] = [ ScalarLeafsRule, FieldsOnCorrectTypeRule, UniqueFragmentNamesRule, -// KnownFragmentNamesRule, + KnownFragmentNamesRule, NoUnusedFragmentsRule, PossibleFragmentSpreadsRule, // NoFragmentCyclesRule, diff --git a/Tests/GraphQLTests/ValidationTests/KnownFragmentNamesTests.swift b/Tests/GraphQLTests/ValidationTests/KnownFragmentNamesTests.swift new file mode 100644 index 00000000..981c1159 --- /dev/null +++ b/Tests/GraphQLTests/ValidationTests/KnownFragmentNamesTests.swift @@ -0,0 +1,73 @@ +@testable import GraphQL +import XCTest + +class KnownFragmentNamesTests: ValidationTestCase { + override func setUp() { + rule = KnownFragmentNamesRule + } + + func testKnownFragmentNamesAreValid() throws { + try assertValid( + """ + { + human(id: 4) { + ...HumanFields1 + ... on Human { + ...HumanFields2 + } + ... { + name + } + } + } + fragment HumanFields1 on Human { + name + ...HumanFields3 + } + fragment HumanFields2 on Human { + name + } + fragment HumanFields3 on Human { + name + } + """ + ) + } + + func testUnknownFragmentNamesAreInvalid() throws { + let errors = try assertInvalid( + errorCount: 3, + query: + """ + { + human(id: 4) { + ...UnknownFragment1 + ... on Human { + ...UnknownFragment2 + } + } + } + fragment HumanFields on Human { + name + ...UnknownFragment3 + } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 8)], + message: "Unknown fragment \"UnknownFragment1\"." + ) + try assertValidationError( + error: errors[1], + locations: [(line: 5, column: 10)], + message: "Unknown fragment \"UnknownFragment2\"." + ) + try assertValidationError( + error: errors[2], + locations: [(line: 11, column: 6)], + message: "Unknown fragment \"UnknownFragment3\"." + ) + } +} From 0049176f48c3da6862beb0ed56da7cdbec4cb043 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 19 Nov 2023 18:25:47 -0700 Subject: [PATCH 13/34] fix: visitor unskips nodes correctly --- Sources/GraphQL/Language/Visitor.swift | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Sources/GraphQL/Language/Visitor.swift b/Sources/GraphQL/Language/Visitor.swift index 27d7a604..1f0bcafd 100644 --- a/Sources/GraphQL/Language/Visitor.swift +++ b/Sources/GraphQL/Language/Visitor.swift @@ -304,9 +304,14 @@ func visitInParallel(visitors: [Visitor]) -> Visitor { } else if case .node = result { return result } - } // else if case let .node(skippedNode) = skipping[i], skippedNode == node { -// skipping[i] = nil -// } + } else if + case let .node(skippedNodeValue) = skipping[i], + let skippedNode = skippedNodeValue, + skippedNode.kind == node.kind, + skippedNode.loc == node.loc + { + skipping[i] = nil + } } return .continue From 062a97592a290f7ed36e05a6113fd8d9c801ff12 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Mon, 13 Nov 2023 12:46:54 -0700 Subject: [PATCH 14/34] feature: Adds NoFragmentCyclesRule --- .../Rules/NoFragmentCyclesRule.swift | 79 +++++ .../GraphQL/Validation/SpecifiedRules.swift | 2 +- .../NoFragmentCyclesRuleTests.swift | 312 ++++++++++++++++++ 3 files changed, 392 insertions(+), 1 deletion(-) create mode 100644 Sources/GraphQL/Validation/Rules/NoFragmentCyclesRule.swift create mode 100644 Tests/GraphQLTests/ValidationTests/NoFragmentCyclesRuleTests.swift diff --git a/Sources/GraphQL/Validation/Rules/NoFragmentCyclesRule.swift b/Sources/GraphQL/Validation/Rules/NoFragmentCyclesRule.swift new file mode 100644 index 00000000..acf56315 --- /dev/null +++ b/Sources/GraphQL/Validation/Rules/NoFragmentCyclesRule.swift @@ -0,0 +1,79 @@ + +/** + * No fragment cycles + * + * The graph of fragment spreads must not form any cycles including spreading itself. + * Otherwise an operation could infinitely spread or infinitely execute on cycles in the underlying data. + * + * See https://spec.graphql.org/draft/#sec-Fragment-spreads-must-not-form-cycles + */ +func NoFragmentCyclesRule(context: ValidationContext) -> Visitor { + // Tracks already visited fragments to maintain O(N) and to ensure that cycles + // are not redundantly reported. + var visitedFrags = Set() + + // Array of AST nodes used to produce meaningful errors + var spreadPath = [FragmentSpread]() + + // Position in the spread path + var spreadPathIndexByName = [String: Int]() + + // This does a straight-forward DFS to find cycles. + // It does not terminate when a cycle was found but continues to explore + // the graph to find all possible cycles. + func detectCycleRecursive(fragment: FragmentDefinition) { + if visitedFrags.contains(fragment.name.value) { + return + } + + let fragmentName = fragment.name.value + visitedFrags.insert(fragmentName) + + let spreadNodes = context.getFragmentSpreads(node: fragment.selectionSet) + if spreadNodes.count == 0 { + return + } + + spreadPathIndexByName[fragmentName] = spreadPath.count + + for spreadNode in spreadNodes { + let spreadName = spreadNode.name.value + let cycleIndex = spreadPathIndexByName[spreadName] + + spreadPath.append(spreadNode) + if let cycleIndex = cycleIndex { + let cyclePath = Array(spreadPath[cycleIndex ..< spreadPath.count]) + let viaPath = cyclePath[0 ..< max(cyclePath.count - 1, 0)] + .map { "\"\($0.name.value)\"" }.joined(separator: ", ") + + context.report( + error: GraphQLError( + message: "Cannot spread fragment \"\(spreadName)\" within itself" + + (viaPath != "" ? " via \(viaPath)." : "."), + nodes: cyclePath + ) + ) + } else { + if let spreadFragment = context.getFragment(name: spreadName) { + detectCycleRecursive(fragment: spreadFragment) + } + } + spreadPath.removeLast() + } + + spreadPathIndexByName[fragmentName] = nil + } + + return Visitor( + enter: { node, _, _, _, _ in + if node is OperationDefinition { + return .skip + } + if let fragmentDefinition = node as? FragmentDefinition { + detectCycleRecursive(fragment: fragmentDefinition) + return .skip + } + return .continue + } + ) +} diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index 37a0e26a..d2b3c2b5 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -15,7 +15,7 @@ public let specifiedRules: [(ValidationContext) -> Visitor] = [ KnownFragmentNamesRule, NoUnusedFragmentsRule, PossibleFragmentSpreadsRule, -// NoFragmentCyclesRule, + NoFragmentCyclesRule, // UniqueVariableNamesRule, // NoUndefinedVariablesRule, NoUnusedVariablesRule, diff --git a/Tests/GraphQLTests/ValidationTests/NoFragmentCyclesRuleTests.swift b/Tests/GraphQLTests/ValidationTests/NoFragmentCyclesRuleTests.swift new file mode 100644 index 00000000..9912e161 --- /dev/null +++ b/Tests/GraphQLTests/ValidationTests/NoFragmentCyclesRuleTests.swift @@ -0,0 +1,312 @@ +@testable import GraphQL +import XCTest + +class NoFragmentCyclesRuleTests: ValidationTestCase { + override func setUp() { + rule = NoFragmentCyclesRule + } + + func testSingleReferenceIsValid() throws { + try assertValid( + """ + fragment fragA on Dog { ...fragB } + fragment fragB on Dog { name } + """ + ) + } + + func testSpreadingTwiceIsNotCircular() throws { + try assertValid( + """ + fragment fragA on Dog { ...fragB, ...fragB } + fragment fragB on Dog { name } + """ + ) + } + + func testSpreadingTwiceIndirectlyIsNotCircular() throws { + try assertValid( + """ + fragment fragA on Dog { ...fragB, ...fragC } + fragment fragB on Dog { ...fragC } + fragment fragC on Dog { name } + """ + ) + } + + func testDoubleSpreadWithinAbstractTypes() throws { + try assertValid( + """ + fragment nameFragment on Pet { + ... on Dog { name } + ... on Cat { name } + } + + fragment spreadsInAnon on Pet { + ... on Dog { ...nameFragment } + ... on Cat { ...nameFragment } + } + """ + ) + } + + func testDoesNotFalsePositiveOnUnknownFragment() throws { + try assertValid( + """ + fragment nameFragment on Pet { + ...UnknownFragment + } + """ + ) + } + + func testSpreadingRecursivelyWithinFieldFails() throws { + let errors = try assertInvalid( + errorCount: 1, + query: """ + fragment fragA on Human { relatives { ...fragA } }, + """ + ) + + try assertValidationError( + error: errors[0], + locations: [(line: 1, column: 39)], + message: "Cannot spread fragment \"fragA\" within itself." + ) + } + + func testNoSpreadingItselfDirectly() throws { + let errors = try assertInvalid( + errorCount: 1, + query: """ + fragment fragA on Dog { ...fragA } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [(line: 1, column: 25)], + message: "Cannot spread fragment \"fragA\" within itself." + ) + } + + func testNoSpreadingItselfDirectlyWithinInlineFragment() throws { + let errors = try assertInvalid( + errorCount: 1, + query: """ + fragment fragA on Pet { + ... on Dog { + ...fragA + } + } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 5)], + message: "Cannot spread fragment \"fragA\" within itself." + ) + } + + func testNoSpreadingItselfIndirectly() throws { + let errors = try assertInvalid( + errorCount: 1, + query: """ + fragment fragA on Dog { ...fragB } + fragment fragB on Dog { ...fragA } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [ + (line: 1, column: 25), + (line: 2, column: 25), + ], + message: "Cannot spread fragment \"fragA\" within itself via \"fragB\"." + ) + } + + func testNoSpreadingItselfIndirectlyReportsOppositeOrder() throws { + let errors = try assertInvalid( + errorCount: 1, + query: """ + fragment fragB on Dog { ...fragA } + fragment fragA on Dog { ...fragB } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [ + (line: 1, column: 25), + (line: 2, column: 25), + ], + message: "Cannot spread fragment \"fragB\" within itself via \"fragA\"." + ) + } + + func testNoSpreadingItselfIndirectlyWithinInlineFragment() throws { + let errors = try assertInvalid( + errorCount: 1, + query: """ + fragment fragA on Pet { + ... on Dog { + ...fragB + } + } + fragment fragB on Pet { + ... on Dog { + ...fragA + } + } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [ + (line: 3, column: 5), + (line: 8, column: 5), + ], + message: "Cannot spread fragment \"fragA\" within itself via \"fragB\"." + ) + } + + func testNoSpreadingItselfDeeply() throws { + let errors = try assertInvalid( + errorCount: 2, + query: """ + fragment fragA on Dog { ...fragB } + fragment fragB on Dog { ...fragC } + fragment fragC on Dog { ...fragO } + fragment fragX on Dog { ...fragY } + fragment fragY on Dog { ...fragZ } + fragment fragZ on Dog { ...fragO } + fragment fragO on Dog { ...fragP } + fragment fragP on Dog { ...fragA, ...fragX } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [ + (line: 1, column: 25), + (line: 2, column: 25), + (line: 3, column: 25), + (line: 7, column: 25), + (line: 8, column: 25), + ], + message: #"Cannot spread fragment "fragA" within itself via "fragB", "fragC", "fragO", "fragP"."# + ) + + try assertValidationError( + error: errors[1], + locations: [ + (line: 7, column: 25), + (line: 8, column: 35), + (line: 4, column: 25), + (line: 5, column: 25), + (line: 6, column: 25), + ], + message: #"Cannot spread fragment "fragO" within itself via "fragP", "fragX", "fragY", "fragZ"."# + ) + } + + func testNoSpreadingItselfDeeplyTwoPaths() throws { + let errors = try assertInvalid( + errorCount: 2, + query: """ + fragment fragA on Dog { ...fragB, ...fragC } + fragment fragB on Dog { ...fragA } + fragment fragC on Dog { ...fragA } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [ + (line: 1, column: 25), + (line: 2, column: 25), + ], + message: #"Cannot spread fragment "fragA" within itself via "fragB"."# + ) + + try assertValidationError( + error: errors[1], + locations: [ + (line: 1, column: 35), + (line: 3, column: 25), + ], + message: #"Cannot spread fragment "fragA" within itself via "fragC"."# + ) + } + + func testNoSpreadingItselfDeeplyTwoPathsAltTraverseOrder() throws { + let errors = try assertInvalid( + errorCount: 2, + query: """ + fragment fragA on Dog { ...fragC } + fragment fragB on Dog { ...fragC } + fragment fragC on Dog { ...fragA, ...fragB } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [ + (line: 1, column: 25), + (line: 3, column: 25), + ], + message: #"Cannot spread fragment "fragA" within itself via "fragC"."# + ) + + try assertValidationError( + error: errors[1], + locations: [ + (line: 3, column: 35), + (line: 2, column: 25), + ], + message: #"Cannot spread fragment "fragC" within itself via "fragB"."# + ) + } + + func testNoSpreadingItselfDeeplyAndImmediately() throws { + let errors = try assertInvalid( + errorCount: 3, + query: """ + fragment fragA on Dog { ...fragB } + fragment fragB on Dog { ...fragB, ...fragC } + fragment fragC on Dog { ...fragA, ...fragB } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [ + (line: 2, column: 25), + ], + message: #"Cannot spread fragment "fragB" within itself."# + ) + + try assertValidationError( + error: errors[1], + locations: [ + (line: 1, column: 25), + (line: 2, column: 35), + (line: 3, column: 25), + ], + message: #"Cannot spread fragment "fragA" within itself via "fragB", "fragC"."# + ) + + try assertValidationError( + error: errors[2], + locations: [ + (line: 2, column: 35), + (line: 3, column: 35), + ], + message: #"Cannot spread fragment "fragB" within itself via "fragC"."# + ) + } +} From 14949c71abe17af0667a40a19dd2a90d1d59c07e Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Mon, 13 Nov 2023 20:52:20 -0700 Subject: [PATCH 15/34] feature: Adds UniqueVariableNamesRule --- .../Rules/UniqueVariableNamesRule.swift | 31 +++++++++++ .../GraphQL/Validation/SpecifiedRules.swift | 2 +- .../UniqueVariableNamesRuleTests.swift | 54 +++++++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 Sources/GraphQL/Validation/Rules/UniqueVariableNamesRule.swift create mode 100644 Tests/GraphQLTests/ValidationTests/UniqueVariableNamesRuleTests.swift diff --git a/Sources/GraphQL/Validation/Rules/UniqueVariableNamesRule.swift b/Sources/GraphQL/Validation/Rules/UniqueVariableNamesRule.swift new file mode 100644 index 00000000..1339304e --- /dev/null +++ b/Sources/GraphQL/Validation/Rules/UniqueVariableNamesRule.swift @@ -0,0 +1,31 @@ + +/** + * Unique variable names + * + * A GraphQL operation is only valid if all its variables are uniquely named. + */ +func UniqueVariableNamesRule(context: ValidationContext) -> Visitor { + return Visitor( + enter: { node, _, _, _, _ in + if let operation = node as? OperationDefinition { + let variableDefinitions = operation.variableDefinitions + + let seenVariableDefinitions = Dictionary(grouping: variableDefinitions) { node in + node.variable.name.value + } + + for (variableName, variableNodes) in seenVariableDefinitions { + if variableNodes.count > 1 { + context.report( + error: GraphQLError( + message: "There can be only one variable named \"$\(variableName)\".", + nodes: variableNodes.map { $0.variable.name } + ) + ) + } + } + } + return .continue + } + ) +} diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index d2b3c2b5..508ed8a4 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -16,7 +16,7 @@ public let specifiedRules: [(ValidationContext) -> Visitor] = [ NoUnusedFragmentsRule, PossibleFragmentSpreadsRule, NoFragmentCyclesRule, -// UniqueVariableNamesRule, + UniqueVariableNamesRule, // NoUndefinedVariablesRule, NoUnusedVariablesRule, // KnownDirectivesRule, diff --git a/Tests/GraphQLTests/ValidationTests/UniqueVariableNamesRuleTests.swift b/Tests/GraphQLTests/ValidationTests/UniqueVariableNamesRuleTests.swift new file mode 100644 index 00000000..6dbe2bce --- /dev/null +++ b/Tests/GraphQLTests/ValidationTests/UniqueVariableNamesRuleTests.swift @@ -0,0 +1,54 @@ +@testable import GraphQL +import XCTest + +class UniqueVariableNamesRuleTests: ValidationTestCase { + override func setUp() { + rule = UniqueVariableNamesRule + } + + func testUniqueVariableNames() throws { + try assertValid( + """ + query A($x: Int, $y: String) { __typename } + query B($x: String, $y: Int) { __typename } + """ + ) + } + + func testDuplicateVariableNames() throws { + let errors = try assertInvalid( + errorCount: 3, + query: + """ + query A($x: Int, $x: Int, $x: String) { __typename } + query B($x: String, $x: Int) { __typename } + query C($x: Int, $x: Int) { __typename } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 1, column: 10), + (line: 1, column: 19), + (line: 1, column: 28), + ], + message: #"There can be only one variable named "$x"."# + ) + try assertValidationError( + error: errors[1], + locations: [ + (line: 2, column: 10), + (line: 2, column: 22), + ], + message: #"There can be only one variable named "$x"."# + ) + try assertValidationError( + error: errors[2], + locations: [ + (line: 3, column: 10), + (line: 3, column: 19), + ], + message: #"There can be only one variable named "$x"."# + ) + } +} From 2c3abcc94e0a152f8bf7d893afa6cf9f25b5d214 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Mon, 13 Nov 2023 21:23:33 -0700 Subject: [PATCH 16/34] feature: Adds NoUndefinedVariablesRule --- .../Rules/NoUndefinedVariablesRule.swift | 42 ++ .../GraphQL/Validation/SpecifiedRules.swift | 2 +- .../NoUndefinedVariablesRuleTests.swift | 421 ++++++++++++++++++ 3 files changed, 464 insertions(+), 1 deletion(-) create mode 100644 Sources/GraphQL/Validation/Rules/NoUndefinedVariablesRule.swift create mode 100644 Tests/GraphQLTests/ValidationTests/NoUndefinedVariablesRuleTests.swift diff --git a/Sources/GraphQL/Validation/Rules/NoUndefinedVariablesRule.swift b/Sources/GraphQL/Validation/Rules/NoUndefinedVariablesRule.swift new file mode 100644 index 00000000..babe99ef --- /dev/null +++ b/Sources/GraphQL/Validation/Rules/NoUndefinedVariablesRule.swift @@ -0,0 +1,42 @@ + +/** + * No undefined variables + * + * A GraphQL operation is only valid if all variables encountered, both directly + * and via fragment spreads, are defined by that operation. + * + * See https://spec.graphql.org/draft/#sec-All-Variable-Uses-Defined + */ +func NoUndefinedVariablesRule(context: ValidationContext) -> Visitor { + return Visitor( + enter: { node, _, _, _, _ in + if let operation = node as? OperationDefinition { + let variableNameDefined = Set( + operation.variableDefinitions.map { $0.variable.name.value } + ) + + let usages = context.getRecursiveVariableUsages(operation: operation) + for usage in usages { + let node = usage.node + let varName = node.name.value + if !variableNameDefined.contains(varName) { + let message: String + if let operationName = operation.name { + message = + "Variable \"$\(varName)\" is not defined by operation \"\(operationName.value)\"." + } else { + message = "Variable \"$\(varName)\" is not defined." + } + context.report( + error: GraphQLError( + message: message, + nodes: [node, operation] + ) + ) + } + } + } + return .continue + } + ) +} diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index 508ed8a4..ce04bdf6 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -17,7 +17,7 @@ public let specifiedRules: [(ValidationContext) -> Visitor] = [ PossibleFragmentSpreadsRule, NoFragmentCyclesRule, UniqueVariableNamesRule, -// NoUndefinedVariablesRule, + NoUndefinedVariablesRule, NoUnusedVariablesRule, // KnownDirectivesRule, // UniqueDirectivesPerLocationRule, diff --git a/Tests/GraphQLTests/ValidationTests/NoUndefinedVariablesRuleTests.swift b/Tests/GraphQLTests/ValidationTests/NoUndefinedVariablesRuleTests.swift new file mode 100644 index 00000000..e23d6fcb --- /dev/null +++ b/Tests/GraphQLTests/ValidationTests/NoUndefinedVariablesRuleTests.swift @@ -0,0 +1,421 @@ +@testable import GraphQL +import XCTest + +class NoUndefinedVariablesRuleTests: ValidationTestCase { + override func setUp() { + rule = NoUndefinedVariablesRule + } + + func testAllVariablesDefined() throws { + try assertValid( + """ + query Foo($a: String, $b: String, $c: String) { + field(a: $a, b: $b, c: $c) + } + """ + ) + } + + func testAllVariablesDeeplyDefined() throws { + try assertValid( + """ + query Foo($a: String, $b: String, $c: String) { + field(a: $a) { + field(b: $b) { + field(c: $c) + } + } + } + """ + ) + } + + func testAllVariablesDeeplyInInlineFragmentsDefined() throws { + try assertValid( + """ + query Foo($a: String, $b: String, $c: String) { + ... on Type { + field(a: $a) { + field(b: $b) { + ... on Type { + field(c: $c) + } + } + } + } + } + """ + ) + } + + func testAllVariablesInFragmentsDeeplyDefined() throws { + try assertValid( + """ + query Foo($a: String, $b: String, $c: String) { + ...FragA + } + fragment FragA on Type { + field(a: $a) { + ...FragB + } + } + fragment FragB on Type { + field(b: $b) { + ...FragC + } + } + fragment FragC on Type { + field(c: $c) + } + """ + ) + } + + func testVariableWithinSingleFragmentDefinedInMultipleOperations() throws { + try assertValid( + """ + query Foo($a: String) { + ...FragA + } + query Bar($a: String) { + ...FragA + } + fragment FragA on Type { + field(a: $a) + } + """ + ) + } + + func testVariableWithinFragmentsDefinedInOperations() throws { + try assertValid( + """ + query Foo($a: String) { + ...FragA + } + query Bar($b: String) { + ...FragB + } + fragment FragA on Type { + field(a: $a) + } + fragment FragB on Type { + field(b: $b) + } + """ + ) + } + + func testVariableWithinRecursiveFragmentDefined() throws { + try assertValid( + """ + query Foo($a: String) { + ...FragA + } + fragment FragA on Type { + field(a: $a) { + ...FragA + } + } + """ + ) + } + + func testVariableNotDefined() throws { + let errors = try assertInvalid( + errorCount: 1, + query: """ + query Foo($a: String, $b: String, $c: String) { + field(a: $a, b: $b, c: $c, d: $d) + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 2, column: 33), + (line: 1, column: 1), + ], + message: #"Variable "$d" is not defined by operation "Foo"."# + ) + } + + func testVariableNotDefinedByUnNamedQuery() throws { + let errors = try assertInvalid( + errorCount: 1, + query: """ + { + field(a: $a) + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 2, column: 12), + (line: 1, column: 1), + ], + message: #"Variable "$a" is not defined."# + ) + } + + func testMultipleVariablesNotDefined() throws { + let errors = try assertInvalid( + errorCount: 2, + query: """ + query Foo($b: String) { + field(a: $a, b: $b, c: $c) + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 2, column: 12), + (line: 1, column: 1), + ], + message: #"Variable "$a" is not defined by operation "Foo"."# + ) + try assertValidationError( + error: errors[1], + locations: [ + (line: 2, column: 26), + (line: 1, column: 1), + ], + message: #"Variable "$c" is not defined by operation "Foo"."# + ) + } + + func testVariableInFragmentNotDefinedByUnNamedQuery() throws { + let errors = try assertInvalid( + errorCount: 1, + query: """ + { + ...FragA + } + fragment FragA on Type { + field(a: $a) + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 5, column: 12), + (line: 1, column: 1), + ], + message: #"Variable "$a" is not defined."# + ) + } + + func testVariableInFragmentNotDefinedByOperation() throws { + let errors = try assertInvalid( + errorCount: 1, + query: """ + query Foo($a: String, $b: String) { + ...FragA + } + fragment FragA on Type { + field(a: $a) { + ...FragB + } + } + fragment FragB on Type { + field(b: $b) { + ...FragC + } + } + fragment FragC on Type { + field(c: $c) + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 15, column: 12), + (line: 1, column: 1), + ], + message: #"Variable "$c" is not defined by operation "Foo"."# + ) + } + + func testMultipleVariablesInFragmentsNotDefined() throws { + let errors = try assertInvalid( + errorCount: 2, + query: """ + query Foo($b: String) { + ...FragA + } + fragment FragA on Type { + field(a: $a) { + ...FragB + } + } + fragment FragB on Type { + field(b: $b) { + ...FragC + } + } + fragment FragC on Type { + field(c: $c) + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 5, column: 12), + (line: 1, column: 1), + ], + message: #"Variable "$a" is not defined by operation "Foo"."# + ) + try assertValidationError( + error: errors[1], + locations: [ + (line: 15, column: 12), + (line: 1, column: 1), + ], + message: #"Variable "$c" is not defined by operation "Foo"."# + ) + } + + func testSingleVariableInFragmentNotDefinedByMultipleOperations() throws { + let errors = try assertInvalid( + errorCount: 2, + query: """ + query Foo($a: String) { + ...FragAB + } + query Bar($a: String) { + ...FragAB + } + fragment FragAB on Type { + field(a: $a, b: $b) + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 8, column: 19), + (line: 1, column: 1), + ], + message: #"Variable "$b" is not defined by operation "Foo"."# + ) + try assertValidationError( + error: errors[1], + locations: [ + (line: 8, column: 19), + (line: 4, column: 1), + ], + message: #"Variable "$b" is not defined by operation "Bar"."# + ) + } + + func testSingleVariableInFragmentUsedByOtherOperation() throws { + let errors = try assertInvalid( + errorCount: 2, + query: """ + query Foo($b: String) { + ...FragA + } + query Bar($a: String) { + ...FragB + } + fragment FragA on Type { + field(a: $a) + } + fragment FragB on Type { + field(b: $b) + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 8, column: 12), + (line: 1, column: 1), + ], + message: #"Variable "$a" is not defined by operation "Foo"."# + ) + try assertValidationError( + error: errors[1], + locations: [ + (line: 11, column: 12), + (line: 4, column: 1), + ], + message: #"Variable "$b" is not defined by operation "Bar"."# + ) + } + + func testMultipleUndefinedVariablesProduceMultipleErrors() throws { + let errors = try assertInvalid( + errorCount: 6, + query: """ + query Foo($b: String) { + ...FragAB + } + query Bar($a: String) { + ...FragAB + } + fragment FragAB on Type { + field1(a: $a, b: $b) + ...FragC + field3(a: $a, b: $b) + } + fragment FragC on Type { + field2(c: $c) + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 8, column: 13), + (line: 1, column: 1), + ], + message: #"Variable "$a" is not defined by operation "Foo"."# + ) + try assertValidationError( + error: errors[1], + locations: [ + (line: 10, column: 13), + (line: 1, column: 1), + ], + message: #"Variable "$a" is not defined by operation "Foo"."# + ) + try assertValidationError( + error: errors[2], + locations: [ + (line: 13, column: 13), + (line: 1, column: 1), + ], + message: #"Variable "$c" is not defined by operation "Foo"."# + ) + try assertValidationError( + error: errors[3], + locations: [ + (line: 8, column: 20), + (line: 4, column: 1), + ], + message: #"Variable "$b" is not defined by operation "Bar"."# + ) + try assertValidationError( + error: errors[4], + locations: [ + (line: 10, column: 20), + (line: 4, column: 1), + ], + message: #"Variable "$b" is not defined by operation "Bar"."# + ) + try assertValidationError( + error: errors[5], + locations: [ + (line: 13, column: 13), + (line: 4, column: 1), + ], + message: #"Variable "$c" is not defined by operation "Bar"."# + ) + } +} From 70ffab6c27b320f650adf39eaa481a139fff2281 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Mon, 13 Nov 2023 21:48:11 -0700 Subject: [PATCH 17/34] feature: Adds UniqueArgumentNamesRule --- .../Rules/UniqueArgumentNamesRule.swift | 37 ++++ .../GraphQL/Validation/SpecifiedRules.swift | 2 +- .../UniqueArgumentNamesRuleTests.swift | 181 ++++++++++++++++++ 3 files changed, 219 insertions(+), 1 deletion(-) create mode 100644 Sources/GraphQL/Validation/Rules/UniqueArgumentNamesRule.swift create mode 100644 Tests/GraphQLTests/ValidationTests/UniqueArgumentNamesRuleTests.swift diff --git a/Sources/GraphQL/Validation/Rules/UniqueArgumentNamesRule.swift b/Sources/GraphQL/Validation/Rules/UniqueArgumentNamesRule.swift new file mode 100644 index 00000000..eca433c0 --- /dev/null +++ b/Sources/GraphQL/Validation/Rules/UniqueArgumentNamesRule.swift @@ -0,0 +1,37 @@ + +/** + * Unique argument names + * + * A GraphQL field or directive is only valid if all supplied arguments are + * uniquely named. + * + * See https://spec.graphql.org/draft/#sec-Argument-Names + */ +func UniqueArgumentNamesRule(context: ValidationContext) -> Visitor { + return Visitor( + enter: { node, _, _, _, _ in + let argumentNodes: [Argument] + if let field = node as? Field { + argumentNodes = field.arguments + } else if let directive = node as? Directive { + argumentNodes = directive.arguments + } else { + return .continue + } + + let seenArgs = Dictionary(grouping: argumentNodes) { $0.name.value } + + for (argName, argNodes) in seenArgs { + if argNodes.count > 1 { + context.report( + error: GraphQLError( + message: "There can be only one argument named \"\(argName)\".", + nodes: argNodes.map { $0.name } + ) + ) + } + } + return .continue + } + ) +} diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index ce04bdf6..01c1ac77 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -25,7 +25,7 @@ public let specifiedRules: [(ValidationContext) -> Visitor] = [ // DeferStreamDirectiveOnValidOperationsRule, // DeferStreamDirectiveLabelRule, KnownArgumentNamesRule, -// UniqueArgumentNamesRule, + UniqueArgumentNamesRule, // ArgumentsOfCorrectTypeRule, // ValuesOfCorrectTypeRule, // ProvidedRequiredArgumentsRule, diff --git a/Tests/GraphQLTests/ValidationTests/UniqueArgumentNamesRuleTests.swift b/Tests/GraphQLTests/ValidationTests/UniqueArgumentNamesRuleTests.swift new file mode 100644 index 00000000..581c0f20 --- /dev/null +++ b/Tests/GraphQLTests/ValidationTests/UniqueArgumentNamesRuleTests.swift @@ -0,0 +1,181 @@ +@testable import GraphQL +import XCTest + +class UniqueArgumentNamesRuleTests: ValidationTestCase { + override func setUp() { + rule = UniqueArgumentNamesRule + } + + func testNoArgumentsOnField() throws { + try assertValid( + """ + { + field + } + """ + ) + } + + func testNoArgumentsOnDirective() throws { + try assertValid( + """ + { + field @directive + } + """ + ) + } + + func testArgumentOnField() throws { + try assertValid( + """ + { + field(arg: "value") + } + """ + ) + } + + func testArgumentOnDirective() throws { + try assertValid( + """ + { + field @directive(arg: "value") + } + """ + ) + } + + func testSameArgumentOnTwoFields() throws { + try assertValid( + """ + { + one: field(arg: "value") + two: field(arg: "value") + } + """ + ) + } + + func testSameArgumentOnFieldAndDirective() throws { + try assertValid( + """ + { + field(arg: "value") @directive(arg: "value") + } + """ + ) + } + + func testSameArgumentOnTwoDirectives() throws { + try assertValid( + """ + { + field @directive1(arg: "value") @directive2(arg: "value") + } + """ + ) + } + + func testMultipleFieldArguments() throws { + try assertValid( + """ + { + field(arg1: "value", arg2: "value", arg3: "value") + } + """ + ) + } + + func testMultipleDirectiveArguments() throws { + try assertValid( + """ + { + field @directive(arg1: "value", arg2: "value", arg3: "value") + } + """ + ) + } + + func testDuplicateFieldArguments() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + field(arg1: "value", arg1: "value") + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 2, column: 9), + (line: 2, column: 24), + ], + message: "There can be only one argument named \"arg1\"." + ) + } + + func testManyDuplicateFieldArguments() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + field(arg1: "value", arg1: "value", arg1: "value") + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 2, column: 9), + (line: 2, column: 24), + (line: 2, column: 39), + ], + message: "There can be only one argument named \"arg1\"." + ) + } + + func testDuplicateDirectiveArguments() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + field @directive(arg1: "value", arg1: "value") + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 2, column: 20), + (line: 2, column: 35), + ], + message: "There can be only one argument named \"arg1\"." + ) + } + + func testManyDuplicateDirectiveArguments() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + field @directive(arg1: "value", arg1: "value", arg1: "value") + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 2, column: 20), + (line: 2, column: 35), + (line: 2, column: 50), + ], + message: "There can be only one argument named \"arg1\"." + ) + } +} From 0c87951457ea1aba3efc5a58f9290f19abbd34ae Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Mon, 13 Nov 2023 22:12:26 -0700 Subject: [PATCH 18/34] feature: Adds UniqueInputFieldNamesRule --- .../Rules/UniqueInputFieldNamesRule.swift | 45 +++++++ .../GraphQL/Validation/SpecifiedRules.swift | 2 +- .../UniqueInputFieldNamesRuleTests.swift | 124 ++++++++++++++++++ 3 files changed, 170 insertions(+), 1 deletion(-) create mode 100644 Sources/GraphQL/Validation/Rules/UniqueInputFieldNamesRule.swift create mode 100644 Tests/GraphQLTests/ValidationTests/UniqueInputFieldNamesRuleTests.swift diff --git a/Sources/GraphQL/Validation/Rules/UniqueInputFieldNamesRule.swift b/Sources/GraphQL/Validation/Rules/UniqueInputFieldNamesRule.swift new file mode 100644 index 00000000..1b847217 --- /dev/null +++ b/Sources/GraphQL/Validation/Rules/UniqueInputFieldNamesRule.swift @@ -0,0 +1,45 @@ + +/** + * Unique input field names + * + * A GraphQL input object value is only valid if all supplied fields are + * uniquely named. + * + * See https://spec.graphql.org/draft/#sec-Input-Object-Field-Uniqueness + */ +func UniqueInputFieldNamesRule(context: ValidationContext) -> Visitor { + var knownNameStack = [[String: Name]]() + var knownNames = [String: Name]() + + return Visitor( + enter: { node, _, _, _, _ in + if node is ObjectValue { + knownNameStack.append(knownNames) + knownNames = [:] + return .continue + } + if let objectField = node as? ObjectField { + let fieldName = objectField.name.value + if let knownName = knownNames[fieldName] { + context.report( + error: GraphQLError( + message: "There can be only one input field named \"\(fieldName)\".", + nodes: [knownName, objectField.name] + ) + ) + } else { + knownNames[fieldName] = objectField.name + } + return .continue + } + return .continue + }, + leave: { node, _, _, _, _ in + if node is ObjectValue { + let prevKnownNames = knownNameStack.popLast() + knownNames = prevKnownNames ?? [:] + } + return .continue + } + ) +} diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index 01c1ac77..eefd78b7 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -33,5 +33,5 @@ public let specifiedRules: [(ValidationContext) -> Visitor] = [ // DefaultValuesOfCorrectTypeRule, // VariablesInAllowedPositionRule, // OverlappingFieldsCanBeMergedRule, -// UniqueInputFieldNamesRule, + UniqueInputFieldNamesRule, ] diff --git a/Tests/GraphQLTests/ValidationTests/UniqueInputFieldNamesRuleTests.swift b/Tests/GraphQLTests/ValidationTests/UniqueInputFieldNamesRuleTests.swift new file mode 100644 index 00000000..9802a759 --- /dev/null +++ b/Tests/GraphQLTests/ValidationTests/UniqueInputFieldNamesRuleTests.swift @@ -0,0 +1,124 @@ +@testable import GraphQL +import XCTest + +class UniqueInputFieldNamesRuleTests: ValidationTestCase { + override func setUp() { + rule = UniqueInputFieldNamesRule + } + + func testInputObjectWithFields() throws { + try assertValid( + """ + { + field(arg: { f: true }) + } + """ + ) + } + + func testSameInputObjectWithinTwoArgs() throws { + try assertValid( + """ + { + field(arg1: { f: true }, arg2: { f: true }) + } + """ + ) + } + + func testMultipleInputObjectFields() throws { + try assertValid( + """ + { + field(arg: { f1: "value", f2: "value", f3: "value" }) + } + """ + ) + } + + func testAllowsForNestedInputObjectsWithSimilarFields() throws { + try assertValid( + """ + { + field(arg: { + deep: { + deep: { + id: 1 + } + id: 1 + } + id: 1 + }) + } + """ + ) + } + + func testDuplicateInputObjectFields() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + field(arg: { f1: "value", f1: "value" }) + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 2, column: 16), + (line: 2, column: 29), + ], + message: #"There can be only one input field named "f1"."# + ) + } + + func testManyDuplicateInputObjectFields() throws { + let errors = try assertInvalid( + errorCount: 2, + query: + """ + { + field(arg: { f1: "value", f1: "value", f1: "value" }) + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 2, column: 16), + (line: 2, column: 29), + ], + message: #"There can be only one input field named "f1"."# + ) + try assertValidationError( + error: errors[1], + locations: [ + (line: 2, column: 16), + (line: 2, column: 42), + ], + message: #"There can be only one input field named "f1"."# + ) + } + + func testNestedDuplicateInputObjectFields() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + field(arg: { f1: {f2: "value", f2: "value" }}) + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 2, column: 21), + (line: 2, column: 34), + ], + message: #"There can be only one input field named "f2"."# + ) + } +} From db782bfb71cfe57543b6faa9b74e9706a48c4734 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Fri, 17 Nov 2023 18:21:48 -0700 Subject: [PATCH 19/34] fix: Standard scalars deliver correct error messages --- Sources/GraphQL/Type/Definition.swift | 12 ++++++------ Sources/GraphQL/Type/Scalars.swift | 25 ++++++++++++++++++++----- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/Sources/GraphQL/Type/Definition.swift b/Sources/GraphQL/Type/Definition.swift index 0daabd51..1a27715e 100644 --- a/Sources/GraphQL/Type/Definition.swift +++ b/Sources/GraphQL/Type/Definition.swift @@ -1018,7 +1018,7 @@ public final class GraphQLEnumType { let mapValue = try map(from: value) guard let enumValue = valueLookup[mapValue] else { throw GraphQLError( - message: "Enum '\(name)' cannot represent value '\(mapValue)'." + message: "Enum \"\(name)\" cannot represent value: \(mapValue)." ) } return .string(enumValue.name) @@ -1027,13 +1027,13 @@ public final class GraphQLEnumType { public func parseValue(value: Map) throws -> Map { guard let valueStr = value.string else { throw GraphQLError( - message: "Enum '\(name)' cannot represent non-string value '\(value)'." + + message: "Enum \"\(name)\" cannot represent non-string value: \(value)." + didYouMeanEnumValue(unknownValueStr: value.description) ) } guard let enumValue = nameLookup[valueStr] else { throw GraphQLError( - message: "Value '\(valueStr)' does not exist in '\(name)' enum." + + message: "Value \"\(valueStr)\" does not exist in \"\(name)\" enum." + didYouMeanEnumValue(unknownValueStr: valueStr) ) } @@ -1043,14 +1043,14 @@ public final class GraphQLEnumType { public func parseLiteral(valueAST: Value) throws -> Map { guard let enumNode = valueAST as? EnumValue else { throw GraphQLError( - message: "Enum '\(name)' cannot represent non-enum value '\(valueAST)'." + - didYouMeanEnumValue(unknownValueStr: "\(valueAST)"), + message: "Enum \"\(name)\" cannot represent non-enum value: \(print(ast: valueAST))." + + didYouMeanEnumValue(unknownValueStr: print(ast: valueAST)), nodes: [valueAST] ) } guard let enumValue = nameLookup[enumNode.value] else { throw GraphQLError( - message: "Value '\(enumNode)' does not exist in '\(name)' enum." + + message: "Value \"\(enumNode.value)\" does not exist in \"\(name)\" enum." + didYouMeanEnumValue(unknownValueStr: enumNode.value), nodes: [valueAST] ) diff --git a/Sources/GraphQL/Type/Scalars.swift b/Sources/GraphQL/Type/Scalars.swift index 256f94d1..92940bf7 100644 --- a/Sources/GraphQL/Type/Scalars.swift +++ b/Sources/GraphQL/Type/Scalars.swift @@ -10,7 +10,10 @@ public let GraphQLInt = try! GraphQLScalarType( return .int(int) } - return .null + throw GraphQLError( + message: "Int cannot represent non-integer value: \(print(ast: ast))", + nodes: [ast] + ) } ) @@ -31,7 +34,10 @@ public let GraphQLFloat = try! GraphQLScalarType( return .double(double) } - return .null + throw GraphQLError( + message: "Float cannot represent non-numeric value: \(print(ast: ast))", + nodes: [ast] + ) } ) @@ -48,7 +54,10 @@ public let GraphQLString = try! GraphQLScalarType( return .string(ast.value) } - return .null + throw GraphQLError( + message: "String cannot represent a non-string value: \(print(ast: ast))", + nodes: [ast] + ) } ) @@ -62,7 +71,10 @@ public let GraphQLBoolean = try! GraphQLScalarType( return .bool(ast.value) } - return .null + throw GraphQLError( + message: "Boolean cannot represent a non-boolean value: \(print(ast: ast))", + nodes: [ast] + ) } ) @@ -85,6 +97,9 @@ public let GraphQLID = try! GraphQLScalarType( return .string(ast.value) } - return .null + throw GraphQLError( + message: "ID cannot represent a non-string and non-integer value: \(print(ast: ast))", + nodes: [ast] + ) } ) From fe4ea0d41a4f6f40d21a625deb25b25b1b45a0e5 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Fri, 17 Nov 2023 17:42:40 -0700 Subject: [PATCH 20/34] fix: Failed parseLiteral gets null These issues are caught by separate validation rules --- Sources/GraphQL/Utilities/ValueFromAST.swift | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Sources/GraphQL/Utilities/ValueFromAST.swift b/Sources/GraphQL/Utilities/ValueFromAST.swift index 0a9288af..098ee1f6 100644 --- a/Sources/GraphQL/Utilities/ValueFromAST.swift +++ b/Sources/GraphQL/Utilities/ValueFromAST.swift @@ -103,7 +103,11 @@ func valueFromAST( } if let leafType = type as? GraphQLLeafType { - return try leafType.parseLiteral(valueAST: valueAST) + do { + return try leafType.parseLiteral(valueAST: valueAST) + } catch { + return .null + } } throw GraphQLError(message: "Provided type is not an input type") From c2888ff4f91a95d6bcaf8b2d517154cbc709f381 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Fri, 17 Nov 2023 18:19:31 -0700 Subject: [PATCH 21/34] fix: Scalar parsing defaults match and reject objects correctly --- Sources/GraphQL/Type/Definition.swift | 37 +++++----- .../Utilities/ValueFromASTUntyped.swift | 67 +++++++++++++++++++ 2 files changed, 83 insertions(+), 21 deletions(-) create mode 100644 Sources/GraphQL/Utilities/ValueFromASTUntyped.swift diff --git a/Sources/GraphQL/Type/Definition.swift b/Sources/GraphQL/Type/Definition.swift index 1a27715e..f7fb7bba 100644 --- a/Sources/GraphQL/Type/Definition.swift +++ b/Sources/GraphQL/Type/Definition.swift @@ -171,35 +171,22 @@ public final class GraphQLScalarType { public let kind: TypeKind = .scalar let serialize: (Any) throws -> Map - let parseValue: ((Map) throws -> Map)? - let parseLiteral: ((Value) throws -> Map)? - - public init( - name: String, - description: String? = nil, - serialize: @escaping (Any) throws -> Map - ) throws { - try assertValid(name: name) - self.name = name - self.description = description - self.serialize = serialize - parseValue = nil - parseLiteral = nil - } + let parseValue: (Map) throws -> Map + let parseLiteral: (Value) throws -> Map public init( name: String, description: String? = nil, serialize: @escaping (Any) throws -> Map, - parseValue: @escaping (Map) throws -> Map, - parseLiteral: @escaping (Value) throws -> Map + parseValue: ((Map) throws -> Map)? = nil, + parseLiteral: ((Value) throws -> Map)? = nil ) throws { try assertValid(name: name) self.name = name self.description = description self.serialize = serialize - self.parseValue = parseValue - self.parseLiteral = parseLiteral + self.parseValue = parseValue ?? defaultParseValue + self.parseLiteral = parseLiteral ?? defaultParseLiteral } // Serializes an internal value to include in a response. @@ -209,15 +196,23 @@ public final class GraphQLScalarType { // Parses an externally provided value to use as an input. public func parseValue(value: Map) throws -> Map { - return try parseValue?(value) ?? Map.null + return try parseValue(value) } // Parses an externally provided literal value to use as an input. public func parseLiteral(valueAST: Value) throws -> Map { - return try parseLiteral?(valueAST) ?? Map.null + return try parseLiteral(valueAST) } } +let defaultParseValue: ((Map) throws -> Map) = { value in + value +} + +let defaultParseLiteral: ((Value) throws -> Map) = { value in + try valueFromASTUntyped(valueAST: value) +} + extension GraphQLScalarType: Encodable { private enum CodingKeys: String, CodingKey { case name diff --git a/Sources/GraphQL/Utilities/ValueFromASTUntyped.swift b/Sources/GraphQL/Utilities/ValueFromASTUntyped.swift new file mode 100644 index 00000000..c1aa8bb6 --- /dev/null +++ b/Sources/GraphQL/Utilities/ValueFromASTUntyped.swift @@ -0,0 +1,67 @@ +import OrderedCollections + +/** + * Produces a JavaScript value given a GraphQL Value AST. + * + * Unlike `valueFromAST()`, no type is provided. The resulting map + * will reflect the provided GraphQL value AST. + * + * | GraphQL Value | Map Value | + * | -------------------- | ---------------- | + * | Input Object | .dictionary | + * | List | .array | + * | Boolean | .boolean | + * | String / Enum | .string | + * | Int | .int | + * | Float | .float | + * | Null | .null | + * + */ +public func valueFromASTUntyped( + valueAST: Value, + variables: [String: Map] = [:] +) throws -> Map { + switch valueAST { + case _ as NullValue: + return .null + case let value as IntValue: + guard let int = Int(value.value) else { + throw GraphQLError(message: "Int cannot represent non-integer value: \(value)") + } + return .int(int) + case let value as FloatValue: + guard let double = Double(value.value) else { + throw GraphQLError(message: "Float cannot represent non numeric value: \(value)") + } + return .double(double) + case let value as StringValue: + return .string(value.value) + case let value as EnumValue: + return .string(value.value) + case let value as BooleanValue: + return .bool(value.value) + case let value as ListValue: + let array = try value.values.map { try valueFromASTUntyped( + valueAST: $0, + variables: variables + ) } + return .array(array) + case let value as ObjectValue: + var dictionary = OrderedDictionary() + try value.fields.forEach { field in + dictionary[field.name.value] = try valueFromASTUntyped( + valueAST: field.value, + variables: variables + ) + } + return .dictionary(dictionary) + case let value as Variable: + if let variable = variables[value.name.value] { + return variable + } else { + return .undefined + } + default: + return .undefined + } +} From e13ef3111090f8ffc4a1a37147e7321a3be082f4 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Fri, 17 Nov 2023 23:21:17 -0700 Subject: [PATCH 22/34] fix: TypeInfo ListValue walking bug TypeInfo also matched to graphql-js --- Sources/GraphQL/Utilities/TypeInfo.swift | 82 ++++++++++++++++++----- Sources/GraphQL/Validation/Validate.swift | 5 +- 2 files changed, 67 insertions(+), 20 deletions(-) diff --git a/Sources/GraphQL/Utilities/TypeInfo.swift b/Sources/GraphQL/Utilities/TypeInfo.swift index ce8e1026..58a2c240 100644 --- a/Sources/GraphQL/Utilities/TypeInfo.swift +++ b/Sources/GraphQL/Utilities/TypeInfo.swift @@ -9,8 +9,10 @@ final class TypeInfo { var parentTypeStack: [GraphQLCompositeType?] var inputTypeStack: [GraphQLInputType?] var fieldDefStack: [GraphQLFieldDefinition?] + var defaultValueStack: [Map] var directive: GraphQLDirective? var argument: GraphQLArgumentDefinition? + var enumValue: GraphQLEnumValueDefinition? init(schema: GraphQLSchema) { self.schema = schema @@ -18,8 +20,10 @@ final class TypeInfo { parentTypeStack = [] inputTypeStack = [] fieldDefStack = [] + defaultValueStack = [] directive = nil argument = nil + enumValue = nil } var type: GraphQLOutputType? { @@ -50,6 +54,13 @@ final class TypeInfo { return nil } + var defaultValue: Map? { + if !defaultValueStack.isEmpty { + return defaultValueStack[defaultValueStack.count - 1] + } + return nil + } + func enter(node: Node) { switch node { case is SelectionSet: @@ -64,13 +75,17 @@ final class TypeInfo { case let node as Field: var fieldDef: GraphQLFieldDefinition? + var fieldType: GraphQLType? if let parentType = parentType { fieldDef = getFieldDef(schema: schema, parentType: parentType, fieldAST: node) + if let fieldDef = fieldDef { + fieldType = fieldDef.type + } } fieldDefStack.append(fieldDef) - typeStack.append(fieldDef?.type) + typeStack.append(fieldType as? GraphQLOutputType) case let node as Directive: directive = schema.getDirective(name: node.name.value) @@ -94,7 +109,7 @@ final class TypeInfo { if let typeConditionAST = node.typeCondition { outputType = typeFromAST(schema: schema, inputTypeAST: typeConditionAST) } else { - outputType = type + outputType = getNamedType(type: type) } typeStack.append(outputType as? GraphQLOutputType) @@ -107,33 +122,59 @@ final class TypeInfo { inputTypeStack.append(inputType as? GraphQLInputType) case let node as Argument: - var argType: GraphQLInputType? + var argDef: GraphQLArgumentDefinition? if let directive = directive { - if let argDef = directive.args.find({ $0.name == node.name.value }) { - argType = argDef.type - argument = argDef + if let argDefinition = directive.args.find({ $0.name == node.name.value }) { + argDef = argDefinition } } else if let fieldDef = fieldDef { - if let argDef = fieldDef.args.find({ $0.name == node.name.value }) { - argType = argDef.type - argument = argDef + if let argDefinition = fieldDef.args.find({ $0.name == node.name.value }) { + argDef = argDefinition } } - inputTypeStack.append(argType) + argument = argDef + defaultValueStack.append(argDef?.defaultValue ?? .undefined) + inputTypeStack.append(argDef?.type) + + case is ListType, is ListValue: + let listType = getNullableType(type: inputType) + let itemType: GraphQLType? - case is ListType: // could be ListValue - if let listType = getNullableType(type: inputType) as? GraphQLList { - inputTypeStack.append(listType.ofType as? GraphQLInputType) + if let listType = listType as? GraphQLList { + itemType = listType.ofType + } else { + itemType = listType } + defaultValueStack.append(.undefined) - inputTypeStack.append(nil) + if let itemType = itemType as? GraphQLInputType { + inputTypeStack.append(itemType) + } else { + inputTypeStack.append(nil) + } case let node as ObjectField: - if let objectType = getNamedType(type: inputType) as? GraphQLInputObjectType { - let inputField = objectType.fields[node.name.value] - inputTypeStack.append(inputField?.type) + let objectType = getNamedType(type: inputType) + var inputFieldType: GraphQLInputType? + var inputField: InputObjectFieldDefinition? + + if let objectType = objectType as? GraphQLInputObjectType { + inputField = objectType.fields[node.name.value] + if let inputField = inputField { + inputFieldType = inputField.type + } + } + + defaultValueStack.append(inputField?.defaultValue ?? .undefined) + inputTypeStack.append(inputFieldType) + + case let node as EnumValue: + if let enumType = getNamedType(type: inputType) as? GraphQLEnumType { + enumValue = enumType.nameLookup[node.value] + } else { + enumValue = nil } default: @@ -161,11 +202,16 @@ final class TypeInfo { case is Argument: argument = nil + _ = defaultValueStack.popLast() _ = inputTypeStack.popLast() - case is ListType /* could be listValue */, is ObjectField: + case is ListType, is ListValue, is ObjectField: + _ = defaultValueStack.popLast() _ = inputTypeStack.popLast() + case is EnumValue: + enumValue = nil + default: break } diff --git a/Sources/GraphQL/Validation/Validate.swift b/Sources/GraphQL/Validation/Validate.swift index 7559b16e..271c3aa1 100644 --- a/Sources/GraphQL/Validation/Validate.swift +++ b/Sources/GraphQL/Validation/Validate.swift @@ -110,7 +110,7 @@ extension HasSelectionSet: Hashable { } } -public typealias VariableUsage = (node: Variable, type: GraphQLInputType?) +public typealias VariableUsage = (node: Variable, type: GraphQLInputType?, defaultValue: Map?) /** * An instance of this class is passed as the "this" context to all validators, @@ -242,7 +242,8 @@ public final class ValidationContext { if let variable = node as? Variable { usages.append(VariableUsage( node: variable, - type: typeInfo.inputType + type: typeInfo.inputType, + defaultValue: typeInfo.defaultValue )) } From f7bb35a70a9d5eada86c4f58c73dbd286d86d337 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Tue, 14 Nov 2023 18:22:19 -0700 Subject: [PATCH 23/34] feature: Adds ValuesOfCorrectTypeRule --- Sources/GraphQL/Type/Definition.swift | 4 + Sources/GraphQL/Utilities/TypeInfo.swift | 7 + .../Rules/ValuesOfCorrectTypeRule.swift | 174 ++ .../GraphQL/Validation/SpecifiedRules.swift | 4 +- Sources/GraphQL/Validation/Validate.swift | 4 + .../ValuesOfCorrectTypeRuleTests.swift | 1448 +++++++++++++++++ 6 files changed, 1638 insertions(+), 3 deletions(-) create mode 100644 Sources/GraphQL/Validation/Rules/ValuesOfCorrectTypeRule.swift create mode 100644 Tests/GraphQLTests/ValidationTests/ValuesOfCorrectTypeRuleTests.swift diff --git a/Sources/GraphQL/Type/Definition.swift b/Sources/GraphQL/Type/Definition.swift index f7fb7bba..2f20d884 100644 --- a/Sources/GraphQL/Type/Definition.swift +++ b/Sources/GraphQL/Type/Definition.swift @@ -1379,6 +1379,10 @@ extension InputObjectFieldDefinition: KeySubscriptable { } } +public func isRequiredInputField(_ field: InputObjectFieldDefinition) -> Bool { + return field.type is GraphQLNonNull && field.defaultValue == nil +} + public typealias InputObjectFieldDefinitionMap = [String: InputObjectFieldDefinition] /** diff --git a/Sources/GraphQL/Utilities/TypeInfo.swift b/Sources/GraphQL/Utilities/TypeInfo.swift index 58a2c240..cff0fbcc 100644 --- a/Sources/GraphQL/Utilities/TypeInfo.swift +++ b/Sources/GraphQL/Utilities/TypeInfo.swift @@ -47,6 +47,13 @@ final class TypeInfo { return nil } + var parentInputType: GraphQLInputType? { + if inputTypeStack.count >= 2 { + return inputTypeStack[inputTypeStack.count - 2] + } + return nil + } + var fieldDef: GraphQLFieldDefinition? { if !fieldDefStack.isEmpty { return fieldDefStack[fieldDefStack.count - 1] diff --git a/Sources/GraphQL/Validation/Rules/ValuesOfCorrectTypeRule.swift b/Sources/GraphQL/Validation/Rules/ValuesOfCorrectTypeRule.swift new file mode 100644 index 00000000..9cfcb7bb --- /dev/null +++ b/Sources/GraphQL/Validation/Rules/ValuesOfCorrectTypeRule.swift @@ -0,0 +1,174 @@ + +/** + * Value literals of correct type + * + * A GraphQL document is only valid if all value literals are of the type + * expected at their position. + * + * See https://spec.graphql.org/draft/#sec-Values-of-Correct-Type + */ +func ValuesOfCorrectTypeRule(context: ValidationContext) -> Visitor { + var variableDefinitions = [String: VariableDefinition]() + + return Visitor( + enter: { node, _, _, _, _ in + if node is OperationDefinition { + variableDefinitions = [:] + return .continue + } + if let variableDefinition = node as? VariableDefinition { + variableDefinitions[variableDefinition.variable.name.value] = variableDefinition + return .continue + } + if let list = node as? ListValue { + guard let type = getNullableType(type: context.parentInputType) else { + return .continue + } + guard type is GraphQLList else { + isValidValueNode(context, list) + return .break // Don't traverse further. + } + return .continue + } + if let object = node as? ObjectValue { + let type = getNamedType(type: context.inputType) + guard let type = type as? GraphQLInputObjectType else { + isValidValueNode(context, object) + return .break // Don't traverse further. + } + // Ensure every required field exists. + let fieldNodeMap = Dictionary(grouping: object.fields) { field in + field.name.value + } + for (fieldName, fieldDef) in type.fields { + if fieldNodeMap[fieldName] == nil, isRequiredInputField(fieldDef) { + let typeStr = fieldDef.type + context.report( + error: GraphQLError( + message: "Field \"\(type.name).\(fieldDef.name)\" of required type \"\(typeStr)\" was not provided.", + nodes: [object] + ) + ) + } + } + + // TODO: Add oneOf support + return .continue + } + if let field = node as? ObjectField { + let parentType = getNamedType(type: context.parentInputType) + if + context.inputType == nil, + let parentType = parentType as? GraphQLInputObjectType + { + let suggestions = suggestionList( + input: field.name.value, + options: Array(parentType.fields.keys) + ) + context.report( + error: GraphQLError( + message: + "Field \"\(field.name.value)\" is not defined by type \"\(parentType.name)\"." + + didYouMean(suggestions: suggestions), + nodes: [field] + ) + ) + } + return .continue + } + if let null = node as? NullValue { + let type = context.inputType + if let type = type as? GraphQLNonNull { + context.report( + error: GraphQLError( + message: + "Expected value of type \"\(type)\", found \(print(ast: node)).", + nodes: [null] + ) + ) + } + return .continue + } + if let node = node as? EnumValue { + isValidValueNode(context, node) + return .continue + } + if let node = node as? IntValue { + isValidValueNode(context, node) + return .continue + } + if let node = node as? FloatValue { + isValidValueNode(context, node) + return .continue + } + if let node = node as? StringValue { + isValidValueNode(context, node) + return .continue + } + if let node = node as? BooleanValue { + isValidValueNode(context, node) + return .continue + } + return .continue + } + ) +} + +/** + * Any value literal may be a valid representation of a Scalar, depending on + * that scalar type. + */ +func isValidValueNode(_ context: ValidationContext, _ node: Value) { + // Report any error at the full type expected by the location. + guard let locationType = context.inputType else { + return + } + + let type = getNamedType(type: locationType) + + if !isLeafType(type: type) { + context.report( + error: GraphQLError( + message: "Expected value of type \"\(locationType)\", found \(print(ast: node)).", + nodes: [node] + ) + ) + return + } + + // Scalars and Enums determine if a literal value is valid via parseLiteral(), + // which may throw or return an invalid value to indicate failure. + do { + if let type = type as? GraphQLScalarType { + if try type.parseLiteral(valueAST: node) == .undefined { + context.report( + error: GraphQLError( + message: "Expected value of type \"\(locationType)\", found \(print(ast: node)).", + nodes: [node] + ) + ) + } + } + if let type = type as? GraphQLEnumType { + if try type.parseLiteral(valueAST: node) == .undefined { + context.report( + error: GraphQLError( + message: "Expected value of type \"\(locationType)\", found \(print(ast: node)).", + nodes: [node] + ) + ) + } + } + } catch { + if let graphQLError = error as? GraphQLError { + context.report(error: graphQLError) + } else { + context.report( + error: GraphQLError( + message: "Expected value of type \"\(locationType)\", found \(print(ast: node)).", + nodes: [node] + ) + ) + } + } +} diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index eefd78b7..16479de6 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -26,11 +26,9 @@ public let specifiedRules: [(ValidationContext) -> Visitor] = [ // DeferStreamDirectiveLabelRule, KnownArgumentNamesRule, UniqueArgumentNamesRule, -// ArgumentsOfCorrectTypeRule, -// ValuesOfCorrectTypeRule, + ValuesOfCorrectTypeRule, // ProvidedRequiredArgumentsRule, ProvidedNonNullArgumentsRule, -// DefaultValuesOfCorrectTypeRule, // VariablesInAllowedPositionRule, // OverlappingFieldsCanBeMergedRule, UniqueInputFieldNamesRule, diff --git a/Sources/GraphQL/Validation/Validate.swift b/Sources/GraphQL/Validation/Validate.swift index 271c3aa1..cd6e8232 100644 --- a/Sources/GraphQL/Validation/Validate.swift +++ b/Sources/GraphQL/Validation/Validate.swift @@ -285,6 +285,10 @@ public final class ValidationContext { return typeInfo.inputType } + public var parentInputType: GraphQLInputType? { + return typeInfo.parentInputType + } + public var fieldDef: GraphQLFieldDefinition? { return typeInfo.fieldDef } diff --git a/Tests/GraphQLTests/ValidationTests/ValuesOfCorrectTypeRuleTests.swift b/Tests/GraphQLTests/ValidationTests/ValuesOfCorrectTypeRuleTests.swift new file mode 100644 index 00000000..5755223c --- /dev/null +++ b/Tests/GraphQLTests/ValidationTests/ValuesOfCorrectTypeRuleTests.swift @@ -0,0 +1,1448 @@ +@testable import GraphQL +import XCTest + +class ValuesOfCorrectTypeRuleTests: ValidationTestCase { + override func setUp() { + rule = ValuesOfCorrectTypeRule + } + + // MARK: Valid values + + func testGoodIntValue() throws { + try assertValid( + """ + { + complicatedArgs { + intArgField(intArg: 2) + } + } + """ + ) + } + + func testGoodNegativeIntValue() throws { + try assertValid( + """ + { + complicatedArgs { + intArgField(intArg: -2) + } + } + """ + ) + } + + func testGoodBooleanValue() throws { + try assertValid( + """ + { + complicatedArgs { + booleanArgField(booleanArg: true) + } + } + """ + ) + } + + func testGoodStringValue() throws { + try assertValid( + """ + { + complicatedArgs { + stringArgField(stringArg: "foo") + } + } + """ + ) + } + + func testGoodFloatValue() throws { + try assertValid( + """ + { + complicatedArgs { + floatArgField(floatArg: 1.1) + } + } + """ + ) + } + + func testGoodNegativeFloatValue() throws { + try assertValid( + """ + { + complicatedArgs { + floatArgField(floatArg: -1.1) + } + } + """ + ) + } + + func testIntIntoFloat() throws { + try assertValid( + """ + { + complicatedArgs { + floatArgField(floatArg: 1) + } + } + """ + ) + } + + func testIntIntoID() throws { + try assertValid( + """ + { + complicatedArgs { + idArgField(idArg: 1) + } + } + """ + ) + } + + func testStringIntoID() throws { + try assertValid( + """ + { + complicatedArgs { + idArgField(idArg: "someIdString") + } + } + """ + ) + } + + func testGoodEnumValue() throws { + try assertValid( + """ + { + dog { + doesKnowCommand(dogCommand: SIT) + } + } + """ + ) + } + + func testEnumWithUndefinedValue() throws { + try assertValid( + """ + { + complicatedArgs { + enumArgField(enumArg: UNKNOWN) + } + } + """ + ) + } + + func testEnumWithNullValue() throws { + try assertValid( + """ + { + complicatedArgs { + enumArgField(enumArg: NO_FUR) + } + } + """ + ) + } + + func testNullIntoNullableType() throws { + try assertValid( + """ + { + complicatedArgs { + intArgField(intArg: null) + } + } + """ + ) + + try assertValid( + """ + { + dog(a: null, b: null, c:{ requiredField: true, intField: null }) { + name + } + } + """ + ) + } + + // MARK: Invalid String Values + + func testIntIntoString() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + stringArgField(stringArg: 1) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 31)], + message: "String cannot represent a non-string value: 1" + ) + } + + func testFloatIntoString() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + stringArgField(stringArg: 1.0) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 31)], + message: "String cannot represent a non-string value: 1.0" + ) + } + + func testBooleanIntoString() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + stringArgField(stringArg: true) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 31)], + message: "String cannot represent a non-string value: true" + ) + } + + func testUnquotedStringIntoString() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + stringArgField(stringArg: BAR) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 31)], + message: "String cannot represent a non-string value: BAR" + ) + } + + func testInvalidIntValues() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + intArgField(intArg: "3") + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 25)], + message: #"Int cannot represent non-integer value: "3""# + ) + } + + // Swift doesn't parse BigInt anyway +// func testBigIntIntoInt() throws { +// let errors = try assertInvalid( +// errorCount: 1, +// query: +// """ +// { +// complicatedArgs { +// intArgField(intArg: 829384293849283498239482938) +// } +// } +// """ +// ) +// try assertValidationError( +// error: errors[0], +// locations: [(line: 3, column: 25)], +// message: "Int cannot represent non-32-bit signed integer value: 829384293849283498239482938" +// ) +// } + + func testUnquotedStringIntoInt() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + intArgField(intArg: FOO) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 25)], + message: "Int cannot represent non-integer value: FOO" + ) + } + + func testSimpleFloatIntoInt() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + intArgField(intArg: 3.0) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 25)], + message: "Int cannot represent non-integer value: 3.0" + ) + } + + func testFloatIntoInt() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + intArgField(intArg: 3.333) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 25)], + message: "Int cannot represent non-integer value: 3.333" + ) + } + + // MARK: Invalid Float Values + + func testStringIntoFloat() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + floatArgField(floatArg: "3.333") + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 29)], + message: #"Float cannot represent non-numeric value: "3.333""# + ) + } + + func testBooleanIntoFloat() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + floatArgField(floatArg: true) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 29)], + message: "Float cannot represent non-numeric value: true" + ) + } + + func testUnquotedIntoFloat() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + floatArgField(floatArg: FOO) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 29)], + message: "Float cannot represent non-numeric value: FOO" + ) + } + + // MARK: Invalid Boolean Value + + func testIntIntoBoolean() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + booleanArgField(booleanArg: 2) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 33)], + message: "Boolean cannot represent a non-boolean value: 2" + ) + } + + func testFloatIntoBoolean() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + booleanArgField(booleanArg: 1.0) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 33)], + message: "Boolean cannot represent a non-boolean value: 1.0" + ) + } + + func testStringIntoBoolean() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + booleanArgField(booleanArg: "true") + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 33)], + message: #"Boolean cannot represent a non-boolean value: "true""# + ) + } + + func testUnquotedIntoBoolean() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + booleanArgField(booleanArg: TRUE) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 33)], + message: "Boolean cannot represent a non-boolean value: TRUE" + ) + } + + // MARK: Invalid ID Value + + func testFloatIntoID() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + idArgField(idArg: 1.0) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 23)], + message: "ID cannot represent a non-string and non-integer value: 1.0" + ) + } + + func testBooleanIntoID() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + idArgField(idArg: true) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 23)], + message: "ID cannot represent a non-string and non-integer value: true" + ) + } + + func testUnquotedIntoID() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + idArgField(idArg: SOMETHING) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 23)], + message: "ID cannot represent a non-string and non-integer value: SOMETHING" + ) + } + + // MARK: Invalid Enum Value + + func testIntIntoEnum() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + dog { + doesKnowCommand(dogCommand: 2) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 33)], + message: #"Enum "DogCommand" cannot represent non-enum value: 2."# + ) + } + + func testFloatIntoEnum() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + dog { + doesKnowCommand(dogCommand: 1.0) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 33)], + message: #"Enum "DogCommand" cannot represent non-enum value: 1.0."# + ) + } + + func testStringIntoEnum() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + dog { + doesKnowCommand(dogCommand: "SIT") + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 33)], + message: #"Enum "DogCommand" cannot represent non-enum value: "SIT". Did you mean the enum value "SIT"?"# + ) + } + + func testBooleanIntoEnum() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + dog { + doesKnowCommand(dogCommand: true) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 33)], + message: #"Enum "DogCommand" cannot represent non-enum value: true."# + ) + } + + func testUnknownEnumValueIntoEnum() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + dog { + doesKnowCommand(dogCommand: JUGGLE) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 33)], + message: #"Value "JUGGLE" does not exist in "DogCommand" enum."# + ) + } + + func testDifferentCaseEnumValueIntoEnum() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + dog { + doesKnowCommand(dogCommand: sit) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 33)], + message: #"Value "sit" does not exist in "DogCommand" enum."# + ) + } + + // MARK: Valid List Value + + func testGoodListValue() throws { + try assertValid( + """ + { + complicatedArgs { + stringListArgField(stringListArg: ["one", null, "two"]) + } + } + """ + ) + } + + func testEmptyListValue() throws { + try assertValid( + """ + { + complicatedArgs { + stringListArgField(stringListArg: []) + } + } + """ + ) + } + + func testNullValueIntoList() throws { + try assertValid( + """ + { + complicatedArgs { + stringListArgField(stringListArg: null) + } + } + """ + ) + } + + func testSingleValueIntoList() throws { + try assertValid( + """ + { + complicatedArgs { + stringListArgField(stringListArg: "one") + } + } + """ + ) + } + + // MARK: Invalid List Value + + func testIncorrectItemType() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + stringListArgField(stringListArg: ["one", 2]) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 47)], + message: "String cannot represent a non-string value: 2" + ) + } + + func testSingleValueOfIncorrectType() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + stringListArgField(stringListArg: 1) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 39)], + message: "String cannot represent a non-string value: 1" + ) + } + + // MARK: Valid Non-Nullable Value + + func testArgOnOptionalArg() throws { + try assertValid( + """ + { + dog { + isHouseTrained(atOtherHomes: true) + } + } + """ + ) + } + + func testNoArgOnOptionalArg() throws { + try assertValid( + """ + { + dog { + isHouseTrained + } + } + """ + ) + } + + func testMultipleArgs() throws { + try assertValid( + """ + { + complicatedArgs { + multipleReqs(req1: 1, req2: 2) + } + } + """ + ) + } + + func testMultipleArgsReverseOrder() throws { + try assertValid( + """ + { + complicatedArgs { + multipleReqs(req2: 2, req1: 1) + } + } + """ + ) + } + + func testNoArgsOnMultipleOptional() throws { + try assertValid( + """ + { + complicatedArgs { + multipleOpts + } + } + """ + ) + } + + func testOneArgOnMultipleOptional() throws { + try assertValid( + """ + { + complicatedArgs { + multipleOpts(opt1: 1) + } + } + """ + ) + } + + func testSecondArgOnMultipleOptional() throws { + try assertValid( + """ + { + complicatedArgs { + multipleOpts(opt2: 1) + } + } + """ + ) + } + + func testMultipleRequiredArgsOnMixedList() throws { + try assertValid( + """ + { + complicatedArgs { + multipleOptAndReq(req1: 3, req2: 4) + } + } + """ + ) + } + + func testMultipleRequiredAndOneOptionalArgOnMixedList() throws { + try assertValid( + """ + { + complicatedArgs { + multipleOptAndReq(req1: 3, req2: 4, opt1: 5) + } + } + """ + ) + } + + func testAllRequiredAndOptionalArgsOnMixedList() throws { + try assertValid( + """ + { + complicatedArgs { + multipleOptAndReq(req1: 3, req2: 4, opt1: 5, opt2: 6) + } + } + """ + ) + } + + // MARK: Invalid Non-Nullable Value + + func testIncorrectValueType() throws { + let errors = try assertInvalid( + errorCount: 2, + query: + """ + { + complicatedArgs { + multipleReqs(req2: "two", req1: "one") + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 24)], + message: #"Int cannot represent non-integer value: "two""# + ) + try assertValidationError( + error: errors[1], + locations: [(line: 3, column: 37)], + message: #"Int cannot represent non-integer value: "one""# + ) + } + + func testIncorrectValueAndMissingArgument() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + multipleReqs(req1: "one") + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 24)], + message: #"Int cannot represent non-integer value: "one""# + ) + } + + func testNullValue() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + multipleReqs(req1: null) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 24)], + message: #"Expected value of type "Int!", found null."# + ) + } + + // MARK: Valid Input Object Value + + func testOptionalArgDespiteRequiredFieldInType() throws { + try assertValid( + """ + { + complicatedArgs { + complexArgField + } + } + """ + ) + } + + func testPartialObjectOnlyRequired() throws { + try assertValid( + """ + { + complicatedArgs { + complexArgField(complexArg: { requiredField: true }) + } + } + """ + ) + } + + func testPartialObjectRequiredFieldCanBeFalsy() throws { + try assertValid( + """ + { + complicatedArgs { + complexArgField(complexArg: { requiredField: false }) + } + } + """ + ) + } + + func testPartialObjectIncludingRequired() throws { + try assertValid( + """ + { + complicatedArgs { + complexArgField(complexArg: { requiredField: true, intField: 4 }) + } + } + """ + ) + } + + func testFullObject() throws { + try assertValid( + """ + { + complicatedArgs { + complexArgField(complexArg: { + requiredField: true, + intField: 4, + stringField: "foo", + booleanField: false, + stringListField: ["one", "two"] + }) + } + } + """ + ) + } + + func testFullObjectWithFieldsInDifferentOrder() throws { + try assertValid( + """ + { + complicatedArgs { + complexArgField(complexArg: { + stringListField: ["one", "two"], + booleanField: false, + requiredField: true, + stringField: "foo", + intField: 4, + }) + } + } + """ + ) + } + + // MARK: Valid oneOf Object Value + +// func testExactlyOneField() throws { +// try assertValid( +// """ +// { +// complicatedArgs { +// oneOfArgField(oneOfArg: { stringField: "abc" }) +// } +// } +// """ +// ) +// } + + // MARK: Invalid input object value + + func testPartialObjectMissingRequired() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + complexArgField(complexArg: { intField: 4 }) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 3, column: 33)], + message: #"Field "ComplexInput.requiredField" of required type "Boolean!" was not provided."# + ) + } + + func testPartialObjectInvalidFieldType() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + complexArgField(complexArg: { + stringListField: ["one", 2], + requiredField: true, + }) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 4, column: 32)], + message: #"String cannot represent a non-string value: 2"# + ) + } + + func testPartialObjectNullToNonNullField() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + complexArgField(complexArg: { + requiredField: true, + nonNullField: null, + }) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 5, column: 21)], + message: #"Expected value of type "Boolean!", found null."# + ) + } + + func testPartialObjectUnknownFieldArg() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + complicatedArgs { + complexArgField(complexArg: { + requiredField: true, + invalidField: "value" + }) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [(line: 5, column: 7)], + message: #"Field "invalidField" is not defined by type "ComplexInput". Did you mean "intField" or "nonNullField"?"# + ) + } + + func testReportsOriginalErrorForCustomScalarWhichThrows() throws { + let customScalar = try GraphQLScalarType( + name: "Invalid", + serialize: { _ in + true + }, + parseValue: { value in + throw GraphQLError( + message: "Invalid scalar is always invalid: \(value)" + ) + }, + parseLiteral: { value in + throw GraphQLError( + message: "Invalid scalar is always invalid: \(print(ast: value))" + ) + } + ) + + let schema = try! GraphQLSchema( + query: GraphQLObjectType( + name: "Query", + fields: [ + "invalidArg": GraphQLField( + type: GraphQLString, + args: [ + "arg": GraphQLArgument(type: customScalar), + ] + ), + ] + ) + ) + + let doc = try parse(source: "{ invalidArg(arg: 123) }") + let errors = validate(schema: schema, ast: doc, rules: [ValuesOfCorrectTypeRule]) + + try assertValidationError( + error: errors[0], + locations: [(line: 1, column: 19)], + message: #"Invalid scalar is always invalid: 123"# + ) + } + + func testReportsOriginalErrorForCustomScalarThatReturnsUndefined() throws { + let customScalar = try GraphQLScalarType( + name: "CustomScalar", + serialize: { _ in + .undefined + }, + parseValue: { _ in + .undefined + }, + parseLiteral: { _ in + .undefined + } + ) + + let schema = try! GraphQLSchema( + query: GraphQLObjectType( + name: "Query", + fields: [ + "invalidArg": GraphQLField( + type: GraphQLString, + args: [ + "arg": GraphQLArgument(type: customScalar), + ] + ), + ] + ) + ) + + let doc = try parse(source: "{ invalidArg(arg: 123) }") + let errors = validate(schema: schema, ast: doc, rules: [ValuesOfCorrectTypeRule]) + + try assertValidationError( + error: errors[0], + locations: [(line: 1, column: 19)], + message: #"Expected value of type "CustomScalar", found 123."# + ) + } + + func testAllowsCustomScalarToAcceptComplexLiterals() throws { + let customScalar = try GraphQLScalarType( + name: "Any", + serialize: { value in + try Map(any: value) + } + ) + + let schema = try! GraphQLSchema( + query: GraphQLObjectType( + name: "Query", + fields: [ + "anyArg": GraphQLField( + type: GraphQLString, + args: [ + "arg": GraphQLArgument(type: customScalar), + ] + ), + ] + ), + types: [ + customScalar, + ] + ) + + let doc = try parse(source: """ + { + test1: anyArg(arg: 123) + test2: anyArg(arg: "abc") + test3: anyArg(arg: [123, "abc"]) + test4: anyArg(arg: {deep: [123, "abc"]}) + } + """) + let errors = validate(schema: schema, ast: doc, rules: [ValuesOfCorrectTypeRule]) + XCTAssertEqual(errors, []) + } + + // MARK: Invalid oneOf input object value TODO + + // MARK: Directive arguments + + func testWithDirectivesOfValidTypes() throws { + try assertValid( + """ + { + dog @include(if: true) { + name + } + human @skip(if: false) { + name + } + } + """ + ) + } + + func testWithDirectiveWithIncorrectTypes() throws { + let errors = try assertInvalid( + errorCount: 2, + query: + """ + { + dog @include(if: "yes") { + name @skip(if: ENUM) + } + } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [(line: 2, column: 20)], + message: #"Boolean cannot represent a non-boolean value: "yes""# + ) + + try assertValidationError( + error: errors[1], + locations: [(line: 3, column: 20)], + message: #"Boolean cannot represent a non-boolean value: ENUM"# + ) + } + + // MARK: Variable default values + + func testVariablesWithValidDefaultValues() throws { + try assertValid( + """ + query WithDefaultValues( + $a: Int = 1, + $b: String = "ok", + $c: ComplexInput = { requiredField: true, intField: 3 } + $d: Int! = 123 + ) { + dog { name } + } + """ + ) + } + + func testVariablesWithValidDefaultNullValues() throws { + try assertValid( + """ + query WithDefaultValues( + $a: Int = null, + $b: String = null, + $c: ComplexInput = { requiredField: true, intField: null } + ) { + dog { name } + } + """ + ) + } + + func testVariablesWithInvalidDefaultNullValues() throws { + let errors = try assertInvalid( + errorCount: 3, + query: + """ + query WithDefaultValues( + $a: Int! = null, + $b: String! = null, + $c: ComplexInput = { requiredField: null, intField: null } + ) { + dog { name } + } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [(line: 2, column: 14)], + message: #"Expected value of type "Int!", found null."# + ) + + try assertValidationError( + error: errors[1], + locations: [(line: 3, column: 17)], + message: #"Expected value of type "String!", found null."# + ) + + try assertValidationError( + error: errors[2], + locations: [(line: 4, column: 39)], + message: #"Expected value of type "Boolean!", found null."# + ) + } + + func testVariablesWithInvalidDefaultValues() throws { + let errors = try assertInvalid( + errorCount: 3, + query: + """ + query InvalidDefaultValues( + $a: Int = "one", + $b: String = 4, + $c: ComplexInput = "NotVeryComplex" + ) { + dog { name } + } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [(line: 2, column: 13)], + message: #"Int cannot represent non-integer value: "one""# + ) + + try assertValidationError( + error: errors[1], + locations: [(line: 3, column: 16)], + message: #"String cannot represent a non-string value: 4"# + ) + + try assertValidationError( + error: errors[2], + locations: [(line: 4, column: 22)], + message: #"Expected value of type "ComplexInput", found "NotVeryComplex"."# + ) + } + + func testVariablesWithComplexInvalidDefaultValues() throws { + let errors = try assertInvalid( + errorCount: 2, + query: + """ + query WithDefaultValues( + $a: ComplexInput = { requiredField: 123, intField: "abc" } + ) { + dog { name } + } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [(line: 2, column: 39)], + message: #"Boolean cannot represent a non-boolean value: 123"# + ) + + try assertValidationError( + error: errors[1], + locations: [(line: 2, column: 54)], + message: #"Int cannot represent non-integer value: "abc""# + ) + } + + func testComplexVariableMissingRequiredField() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + query MissingRequiredField($a: ComplexInput = {intField: 3}) { + dog { name } + } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [(line: 1, column: 47)], + message: #"Field "ComplexInput.requiredField" of required type "Boolean!" was not provided."# + ) + } + + func testListVariablesWithInvalidItem() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + query InvalidItem($a: [String] = ["one", 2]) { + dog { name } + } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [(line: 1, column: 42)], + message: #"String cannot represent a non-string value: 2"# + ) + } +} From 07d415fb3047fa3bdfff9393e1c28299b3539f6c Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sat, 18 Nov 2023 00:32:03 -0700 Subject: [PATCH 24/34] feature: Adds VariablesInAllowedPositionRule --- .../VariablesInAllowedPositionRule.swift | 90 ++++ .../GraphQL/Validation/SpecifiedRules.swift | 2 +- .../StarWarsTests/StarWarsQueryTests.swift | 2 +- .../VariablesInAllowedPositionRuleTests.swift | 404 ++++++++++++++++++ 4 files changed, 496 insertions(+), 2 deletions(-) create mode 100644 Sources/GraphQL/Validation/Rules/VariablesInAllowedPositionRule.swift create mode 100644 Tests/GraphQLTests/ValidationTests/VariablesInAllowedPositionRuleTests.swift diff --git a/Sources/GraphQL/Validation/Rules/VariablesInAllowedPositionRule.swift b/Sources/GraphQL/Validation/Rules/VariablesInAllowedPositionRule.swift new file mode 100644 index 00000000..c393430e --- /dev/null +++ b/Sources/GraphQL/Validation/Rules/VariablesInAllowedPositionRule.swift @@ -0,0 +1,90 @@ + +/** + * Variables in allowed position + * + * Variable usages must be compatible with the arguments they are passed to. + * + * See https://spec.graphql.org/draft/#sec-All-Variable-Usages-are-Allowed + */ +func VariablesInAllowedPositionRule(context: ValidationContext) -> Visitor { + var varDefMap: [String: VariableDefinition] = [:] + return Visitor( + enter: { node, _, _, _, _ in + switch node { + case _ as OperationDefinition: + varDefMap = [:] + case let variableDefinition as VariableDefinition: + varDefMap[variableDefinition.variable.name.value] = variableDefinition + default: + break + } + return .continue + }, + leave: { node, _, _, _, _ in + switch node { + case let operation as OperationDefinition: + let usages = context.getRecursiveVariableUsages(operation: operation) + + for usage in usages { + let varName = usage.node.name.value + let schema = context.schema + + if + let varDef = varDefMap[varName], + let type = usage.type, + let varType = typeFromAST(schema: schema, inputTypeAST: varDef.type) + { + // A var type is allowed if it is the same or more strict (e.g. is + // a subtype of) than the expected type. It can be more strict if + // the variable type is non-null when the expected type is nullable. + // If both are list types, the variable item type can be more strict + // than the expected item type (contravariant). + let isAllowed = (try? allowedVariableUsage( + schema: schema, + varType: varType, + varDefaultValue: varDef.defaultValue, + locationType: type, + locationDefaultValue: usage.defaultValue + )) ?? false + if !isAllowed { + context.report( + error: GraphQLError( + message: "Variable \"$\(varName)\" of type \"\(varType)\" used in position expecting type \"\(type)\".", + nodes: [varDef, usage.node] + ) + ) + } + } + } + default: + break + } + return .continue + } + ) +} + +/** + * Returns true if the variable is allowed in the location it was found, + * which includes considering if default values exist for either the variable + * or the location at which it is located. + */ +func allowedVariableUsage( + schema: GraphQLSchema, + varType: GraphQLType, + varDefaultValue: Value?, + locationType: GraphQLType, + locationDefaultValue: Map? +) throws -> Bool { + if let locationType = locationType as? GraphQLNonNull, !(varType is GraphQLNonNull) { + let hasNonNullVariableDefaultValue = varDefaultValue != nil && varDefaultValue? + .kind != .nullValue + let hasLocationDefaultValue = locationDefaultValue != .undefined + if !hasNonNullVariableDefaultValue && !hasLocationDefaultValue { + return false + } + let nullableLocationType = locationType.ofType + return try isTypeSubTypeOf(schema, varType, nullableLocationType) + } + return try isTypeSubTypeOf(schema, varType, locationType) +} diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index 16479de6..7299503c 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -29,7 +29,7 @@ public let specifiedRules: [(ValidationContext) -> Visitor] = [ ValuesOfCorrectTypeRule, // ProvidedRequiredArgumentsRule, ProvidedNonNullArgumentsRule, -// VariablesInAllowedPositionRule, + VariablesInAllowedPositionRule, // OverlappingFieldsCanBeMergedRule, UniqueInputFieldNamesRule, ] diff --git a/Tests/GraphQLTests/StarWarsTests/StarWarsQueryTests.swift b/Tests/GraphQLTests/StarWarsTests/StarWarsQueryTests.swift index 5e56da32..5d2d9eb7 100644 --- a/Tests/GraphQLTests/StarWarsTests/StarWarsQueryTests.swift +++ b/Tests/GraphQLTests/StarWarsTests/StarWarsQueryTests.swift @@ -186,7 +186,7 @@ class StarWarsQueryTests: XCTestCase { let query = """ - query FetchHeroByEpisodeQuery($episode: String) { + query FetchHeroByEpisodeQuery($episode: Episode) { hero(episode: $episode) { name } diff --git a/Tests/GraphQLTests/ValidationTests/VariablesInAllowedPositionRuleTests.swift b/Tests/GraphQLTests/ValidationTests/VariablesInAllowedPositionRuleTests.swift new file mode 100644 index 00000000..eddff05c --- /dev/null +++ b/Tests/GraphQLTests/ValidationTests/VariablesInAllowedPositionRuleTests.swift @@ -0,0 +1,404 @@ +@testable import GraphQL +import XCTest + +class VariablesInAllowedPositionRuleTests: ValidationTestCase { + override func setUp() { + rule = VariablesInAllowedPositionRule + } + + func testBooleanToBoolean() throws { + try assertValid( + """ + query Query($booleanArg: Boolean) + { + complicatedArgs { + booleanArgField(booleanArg: $booleanArg) + } + } + """ + ) + } + + func testBooleanToBooleanWithinFragment() throws { + try assertValid( + """ + fragment booleanArgFrag on ComplicatedArgs { + booleanArgField(booleanArg: $booleanArg) + } + query Query($booleanArg: Boolean) + { + complicatedArgs { + ...booleanArgFrag + } + } + """ + ) + + try assertValid( + """ + query Query($booleanArg: Boolean) + { + complicatedArgs { + ...booleanArgFrag + } + } + fragment booleanArgFrag on ComplicatedArgs { + booleanArgField(booleanArg: $booleanArg) + } + """ + ) + } + + func testNonNullBooleanToBoolean() throws { + try assertValid( + """ + query Query($nonNullBooleanArg: Boolean!) + { + complicatedArgs { + booleanArgField(booleanArg: $nonNullBooleanArg) + } + } + """ + ) + } + + func testNonNullBooleanToBooleanWithinFragment() throws { + try assertValid( + """ + fragment booleanArgFrag on ComplicatedArgs { + booleanArgField(booleanArg: $nonNullBooleanArg) + } + + query Query($nonNullBooleanArg: Boolean!) + { + complicatedArgs { + ...booleanArgFrag + } + } + """ + ) + } + + func testStringListToStringList() throws { + try assertValid( + """ + query Query($stringListVar: [String]) + { + complicatedArgs { + stringListArgField(stringListArg: $stringListVar) + } + } + """ + ) + } + + func testNonNullStringListToStringList() throws { + try assertValid( + """ + query Query($stringListVar: [String!]) + { + complicatedArgs { + stringListArgField(stringListArg: $stringListVar) + } + } + """ + ) + } + + func testStringToStringListInItemPosition() throws { + try assertValid( + """ + query Query($stringVar: String) + { + complicatedArgs { + stringListArgField(stringListArg: [$stringVar]) + } + } + """ + ) + } + + func testNonNullStringToStringListInItemPosition() throws { + try assertValid( + """ + query Query($stringVar: String!) + { + complicatedArgs { + stringListArgField(stringListArg: [$stringVar]) + } + } + """ + ) + } + + func testComplexInputToComplexInput() throws { + try assertValid( + """ + query Query($complexVar: ComplexInput) + { + complicatedArgs { + complexArgField(complexArg: $complexVar) + } + } + """ + ) + } + + func testComplexInputToComplexInputInFieldPosition() throws { + try assertValid( + """ + query Query($boolVar: Boolean = false) + { + complicatedArgs { + complexArgField(complexArg: {requiredArg: $boolVar}) + } + } + """ + ) + } + + func testNonNullBooleanToNonNullBooleanInDirective() throws { + try assertValid( + """ + query Query($boolVar: Boolean!) + { + dog @include(if: $boolVar) + } + """ + ) + } + + func testIntToIntNonNull() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + query Query($intArg: Int) { + complicatedArgs { + nonNullIntArgField(nonNullIntArg: $intArg) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 1, column: 13), + (line: 3, column: 39), + ], + message: #"Variable "$intArg" of type "Int" used in position expecting type "Int!"."# + ) + } + + func testIntToIntNonNullWithinFragment() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + fragment nonNullIntArgFieldFrag on ComplicatedArgs { + nonNullIntArgField(nonNullIntArg: $intArg) + } + + query Query($intArg: Int) { + complicatedArgs { + ...nonNullIntArgFieldFrag + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 5, column: 13), + (line: 2, column: 37), + ], + message: #"Variable "$intArg" of type "Int" used in position expecting type "Int!"."# + ) + } + + func testIntToIntNonNullWithinNestedFragment() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + fragment outerFrag on ComplicatedArgs { + ...nonNullIntArgFieldFrag + } + + fragment nonNullIntArgFieldFrag on ComplicatedArgs { + nonNullIntArgField(nonNullIntArg: $intArg) + } + + query Query($intArg: Int) { + complicatedArgs { + ...outerFrag + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 9, column: 13), + (line: 6, column: 37), + ], + message: #"Variable "$intArg" of type "Int" used in position expecting type "Int!"."# + ) + } + + func testStringToBoolean() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + query Query($stringVar: String) { + complicatedArgs { + booleanArgField(booleanArg: $stringVar) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 1, column: 13), + (line: 3, column: 33), + ], + message: #"Variable "$stringVar" of type "String" used in position expecting type "Boolean"."# + ) + } + + func testStringToStringList() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + query Query($stringVar: String) { + complicatedArgs { + stringListArgField(stringListArg: $stringVar) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 1, column: 13), + (line: 3, column: 39), + ], + message: #"Variable "$stringVar" of type "String" used in position expecting type "[String]"."# + ) + } + + func testBooleanToNonNullBooleanInDirective() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + query Query($boolVar: Boolean) { + dog @include(if: $boolVar) + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 1, column: 13), + (line: 2, column: 20), + ], + message: #"Variable "$boolVar" of type "Boolean" used in position expecting type "Boolean!"."# + ) + } + + func testStringToNonNullBooleanInDirective() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + query Query($stringVar: String) { + dog @include(if: $stringVar) + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 1, column: 13), + (line: 2, column: 20), + ], + message: #"Variable "$stringVar" of type "String" used in position expecting type "Boolean!"."# + ) + } + + func testStringListToStringListNonNull() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + query Query($stringListVar: [String]) { + complicatedArgs { + stringListNonNullArgField(stringListNonNullArg: $stringListVar) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 1, column: 13), + (line: 3, column: 53), + ], + message: #"Variable "$stringListVar" of type "[String]" used in position expecting type "[String!]"."# + ) + } + + func testOptionalVariableWithDefaultValue() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + query Query($intVar: Int = null) { + complicatedArgs { + nonNullIntArgField(nonNullIntArg: $intVar) + } + } + """ + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 1, column: 13), + (line: 3, column: 39), + ], + message: #"Variable "$intVar" of type "Int" used in position expecting type "Int!"."# + ) + } + + func testIntOptionalWithNonNullDefaultValue() throws { + try assertValid(""" + query Query($intVar: Int = 1) { + complicatedArgs { + nonNullIntArgField(nonNullIntArg: $intVar) + } + } + """) + } + + func testOptionalVariableWithDefaultValueAndNonNullField() throws { + try assertValid(""" + query Query($intVar: Int) { + complicatedArgs { + nonNullFieldWithDefault(nonNullIntArg: $intVar) + } + } + """) + } + + func testBooleanWithDefaultValueInDirective() throws { + try assertValid(""" + query Query($boolVar: Boolean = false) { + dog @include(if: $boolVar) + } + """) + } +} From 9922b2d25cb1d4fbaae50f5ba3755b9b126ad5c5 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 19 Nov 2023 11:56:44 -0700 Subject: [PATCH 25/34] fix: Adds variable directive support --- Sources/GraphQL/Language/AST.swift | 25 +++++++++++++++++++++++-- Sources/GraphQL/Language/Parser.swift | 3 ++- Sources/GraphQL/Language/Printer.swift | 3 +-- Sources/GraphQL/Language/Visitor.swift | 2 +- Sources/GraphQL/Type/Directives.swift | 3 ++- 5 files changed, 29 insertions(+), 7 deletions(-) diff --git a/Sources/GraphQL/Language/AST.swift b/Sources/GraphQL/Language/AST.swift index 3301e621..d2e2a3ac 100644 --- a/Sources/GraphQL/Language/AST.swift +++ b/Sources/GraphQL/Language/AST.swift @@ -401,7 +401,7 @@ public final class OperationDefinition { } return .array(variableDefinitions) case "directives": - guard !variableDefinitions.isEmpty else { + guard !directives.isEmpty else { return nil } return .array(directives) @@ -475,12 +475,20 @@ public final class VariableDefinition { public private(set) var variable: Variable public private(set) var type: Type public private(set) var defaultValue: Value? + public private(set) var directives: [Directive] - init(loc: Location? = nil, variable: Variable, type: Type, defaultValue: Value? = nil) { + init( + loc: Location? = nil, + variable: Variable, + type: Type, + defaultValue: Value? = nil, + directives: [Directive] = [] + ) { self.loc = loc self.variable = variable self.type = type self.defaultValue = defaultValue + self.directives = directives } public func get(key: String) -> NodeResult? { @@ -491,6 +499,11 @@ public final class VariableDefinition { return .node(type) case "defaultValue": return defaultValue.map { .node($0) } + case "directives": + guard !directives.isEmpty else { + return nil + } + return .array(directives) default: return nil } @@ -525,6 +538,14 @@ public final class VariableDefinition { return } self.defaultValue = defaultValue + case "directives": + guard + case let .array(values) = value, + let directives = values as? [Directive] + else { + return + } + self.directives = directives default: return } diff --git a/Sources/GraphQL/Language/Parser.swift b/Sources/GraphQL/Language/Parser.swift index 1520cbe1..6604c0dd 100644 --- a/Sources/GraphQL/Language/Parser.swift +++ b/Sources/GraphQL/Language/Parser.swift @@ -276,7 +276,8 @@ func parseVariableDefinition(lexer: Lexer) throws -> VariableDefinition { variable: parseVariable(lexer: lexer), type: (expect(lexer: lexer, kind: .colon), parseTypeReference(lexer: lexer)).1, defaultValue: skip(lexer: lexer, kind: .equals) ? - parseValueLiteral(lexer: lexer, isConst: true) : nil + parseValueLiteral(lexer: lexer, isConst: true) : nil, + directives: parseDirectives(lexer: lexer) ) } diff --git a/Sources/GraphQL/Language/Printer.swift b/Sources/GraphQL/Language/Printer.swift index bb35dc6b..4807a62e 100644 --- a/Sources/GraphQL/Language/Printer.swift +++ b/Sources/GraphQL/Language/Printer.swift @@ -48,8 +48,7 @@ extension OperationDefinition: Printable { extension VariableDefinition: Printable { var printed: String { variable + ": " + type.printed + wrap(" = ", defaultValue?.printed) -// + wrap(" ", join(directives, " ")) - // TODO: variable directives are currently not supported + + wrap(" ", join(directives, " ")) } } diff --git a/Sources/GraphQL/Language/Visitor.swift b/Sources/GraphQL/Language/Visitor.swift index 1f0bcafd..d5dd4a53 100644 --- a/Sources/GraphQL/Language/Visitor.swift +++ b/Sources/GraphQL/Language/Visitor.swift @@ -3,7 +3,7 @@ let QueryDocumentKeys: [Kind: [String]] = [ .document: ["definitions"], .operationDefinition: ["name", "variableDefinitions", "directives", "selectionSet"], - .variableDefinition: ["variable", "type", "defaultValue"], + .variableDefinition: ["variable", "type", "defaultValue", "directives"], .variable: ["name"], .selectionSet: ["selections"], .field: ["alias", "name", "arguments", "directives", "selectionSet"], diff --git a/Sources/GraphQL/Type/Directives.swift b/Sources/GraphQL/Type/Directives.swift index f9518acf..83facf55 100644 --- a/Sources/GraphQL/Type/Directives.swift +++ b/Sources/GraphQL/Type/Directives.swift @@ -7,6 +7,7 @@ public enum DirectiveLocation: String, Encodable { case fragmentDefinition = "FRAGMENT_DEFINITION" case fragmentSpread = "FRAGMENT_SPREAD" case inlineFragment = "INLINE_FRAGMENT" + case variableDefinition = "VARIABLE_DEFINITION" // Schema Definitions case schema = "SCHEMA" case scalar = "SCALAR" @@ -33,7 +34,7 @@ public struct GraphQLDirective: Encodable { public init( name: String, - description: String, + description: String = "", locations: [DirectiveLocation], args: GraphQLArgumentConfigMap = [:] ) throws { From f62de6838d3f94213993e83e30cf5be2844f19a5 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 19 Nov 2023 11:57:27 -0700 Subject: [PATCH 26/34] test: Adds onField directive to example schema --- .../ValidationTests/ExampleSchema.swift | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/Tests/GraphQLTests/ValidationTests/ExampleSchema.swift b/Tests/GraphQLTests/ValidationTests/ExampleSchema.swift index 803a4997..fa7a0e08 100644 --- a/Tests/GraphQLTests/ValidationTests/ExampleSchema.swift +++ b/Tests/GraphQLTests/ValidationTests/ExampleSchema.swift @@ -587,6 +587,11 @@ let ValidationExampleQueryRoot = try! GraphQLObjectType( ] ) +let ValidationFieldDirective = try! GraphQLDirective( + name: "onField", + locations: [.field] +) + let ValidationExampleSchema = try! GraphQLSchema( query: ValidationExampleQueryRoot, types: [ @@ -594,5 +599,12 @@ let ValidationExampleSchema = try! GraphQLSchema( ValidationExampleDog, ValidationExampleHuman, ValidationExampleAlien, - ] + ], + directives: { + var directives = specifiedDirectives + directives.append(contentsOf: [ + ValidationFieldDirective, + ]) + return directives + }() ) From 3fe1484628d6ee05d74684b1aca9332c051f12ef Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 19 Nov 2023 11:57:46 -0700 Subject: [PATCH 27/34] test: Adds schema injection to validation tests --- .../ValidationTests/ValidationTests.swift | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/Tests/GraphQLTests/ValidationTests/ValidationTests.swift b/Tests/GraphQLTests/ValidationTests/ValidationTests.swift index b8b7aff5..adb37eaf 100644 --- a/Tests/GraphQLTests/ValidationTests/ValidationTests.swift +++ b/Tests/GraphQLTests/ValidationTests/ValidationTests.swift @@ -6,16 +6,24 @@ class ValidationTestCase: XCTestCase { var rule: Rule! - private func validate(body request: String) throws -> [GraphQLError] { + private func validate( + body request: String, + schema: GraphQLSchema = ValidationExampleSchema + ) throws -> [GraphQLError] { return try GraphQL.validate( - schema: ValidationExampleSchema, + schema: schema, ast: parse(source: Source(body: request, name: "GraphQL request")), rules: [rule] ) } - func assertValid(_ query: String, file: StaticString = #file, line: UInt = #line) throws { - let errors = try validate(body: query) + func assertValid( + _ query: String, + schema: GraphQLSchema = ValidationExampleSchema, + file: StaticString = #file, + line: UInt = #line + ) throws { + let errors = try validate(body: query, schema: schema) XCTAssertEqual( errors.count, 0, @@ -28,10 +36,11 @@ class ValidationTestCase: XCTestCase { @discardableResult func assertInvalid( errorCount: Int, query: String, + schema: GraphQLSchema = ValidationExampleSchema, file: StaticString = #file, line: UInt = #line ) throws -> [GraphQLError] { - let errors = try validate(body: query) + let errors = try validate(body: query, schema: schema) XCTAssertEqual( errors.count, errorCount, From 958cbea6cc8b28751e832fde4f76f9f7aeec37a2 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 19 Nov 2023 11:57:56 -0700 Subject: [PATCH 28/34] feature: Adds KnownDirectivesRule --- .../Rules/KnownDirectivesRule.swift | 122 ++++++++ .../GraphQL/Validation/SpecifiedRules.swift | 2 +- .../KnownDirectivesRuleTests.swift | 273 ++++++++++++++++++ 3 files changed, 396 insertions(+), 1 deletion(-) create mode 100644 Sources/GraphQL/Validation/Rules/KnownDirectivesRule.swift create mode 100644 Tests/GraphQLTests/ValidationTests/KnownDirectivesRuleTests.swift diff --git a/Sources/GraphQL/Validation/Rules/KnownDirectivesRule.swift b/Sources/GraphQL/Validation/Rules/KnownDirectivesRule.swift new file mode 100644 index 00000000..5d893520 --- /dev/null +++ b/Sources/GraphQL/Validation/Rules/KnownDirectivesRule.swift @@ -0,0 +1,122 @@ + +/** + * Known directives + * + * A GraphQL document is only valid if all `@directives` are known by the + * schema and legally positioned. + * + * See https://spec.graphql.org/draft/#sec-Directives-Are-Defined + */ +func KnownDirectivesRule(context: ValidationContext) -> Visitor { + var locationsMap = [String: [String]]() + + let schema = context.schema + let definedDirectives = schema.directives + for directive in definedDirectives { + locationsMap[directive.name] = directive.locations.map { $0.rawValue } + } + + let astDefinitions = context.ast.definitions + for def in astDefinitions { + if let directive = def as? DirectiveDefinition { + locationsMap[directive.name.value] = directive.locations.map { $0.value } + } + } + + return Visitor( + enter: { node, _, _, _, ancestors in + if let node = node as? Directive { + let name = node.name.value + let locations = locationsMap[name] + + guard let locations = locations else { + context.report( + error: GraphQLError( + message: "Unknown directive \"@\(name)\".", + nodes: [node] + ) + ) + return .continue + } + + let candidateLocation = getDirectiveLocationForASTPath(ancestors) + if + let candidateLocation = candidateLocation, + !locations.contains(candidateLocation.rawValue) + { + context.report( + error: GraphQLError( + message: "Directive \"@\(name)\" may not be used on \(candidateLocation.rawValue).", + nodes: [node] + ) + ) + } + } + return .continue + } + ) +} + +func getDirectiveLocationForASTPath(_ ancestors: [NodeResult]) -> DirectiveLocation? { + guard let last = ancestors.last, case let .node(appliedTo) = last else { + return nil + } + + switch appliedTo { + case let appliedTo as OperationDefinition: + return getDirectiveLocationForOperation(appliedTo.operation) + case is Field: + return DirectiveLocation.field + case is FragmentSpread: + return DirectiveLocation.fragmentSpread + case is InlineFragment: + return DirectiveLocation.inlineFragment + case is FragmentDefinition: + return DirectiveLocation.fragmentDefinition + case is VariableDefinition: + return DirectiveLocation.variableDefinition + case is SchemaDefinition: + return DirectiveLocation.schema + case is ScalarTypeDefinition, is ScalarExtensionDefinition: + return DirectiveLocation.scalar + case is ObjectTypeDefinition: + return DirectiveLocation.object + case is FieldDefinition: + return DirectiveLocation.fieldDefinition + case is InterfaceTypeDefinition, is InterfaceExtensionDefinition: + return DirectiveLocation.interface + case is UnionTypeDefinition, is UnionExtensionDefinition: + return DirectiveLocation.union + case is EnumTypeDefinition, is EnumExtensionDefinition: + return DirectiveLocation.enum + case is EnumValueDefinition: + return DirectiveLocation.enumValue + case is InputObjectTypeDefinition, is InputObjectExtensionDefinition: + return DirectiveLocation.inputObject + case is InputValueDefinition: + guard ancestors.count >= 3 else { + return nil + } + let parentNode = ancestors[ancestors.count - 3] + guard case let .node(parentNode) = parentNode else { + return nil + } + return parentNode.kind == .inputObjectTypeDefinition + ? DirectiveLocation.inputFieldDefinition + : DirectiveLocation.argumentDefinition + // Not reachable, all possible types have been considered. + default: + return nil + } +} + +func getDirectiveLocationForOperation(_ operation: OperationType) -> DirectiveLocation { + switch operation { + case .query: + return DirectiveLocation.query + case .mutation: + return DirectiveLocation.mutation + case .subscription: + return DirectiveLocation.subscription + } +} diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index 7299503c..fb4f4e06 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -19,7 +19,7 @@ public let specifiedRules: [(ValidationContext) -> Visitor] = [ UniqueVariableNamesRule, NoUndefinedVariablesRule, NoUnusedVariablesRule, -// KnownDirectivesRule, + KnownDirectivesRule, // UniqueDirectivesPerLocationRule, // DeferStreamDirectiveOnRootFieldRule, // DeferStreamDirectiveOnValidOperationsRule, diff --git a/Tests/GraphQLTests/ValidationTests/KnownDirectivesRuleTests.swift b/Tests/GraphQLTests/ValidationTests/KnownDirectivesRuleTests.swift new file mode 100644 index 00000000..6304ba46 --- /dev/null +++ b/Tests/GraphQLTests/ValidationTests/KnownDirectivesRuleTests.swift @@ -0,0 +1,273 @@ +@testable import GraphQL +import XCTest + +class KnownDirectivesRuleTests: ValidationTestCase { + override func setUp() { + rule = KnownDirectivesRule + } + + func testWithNoDirectives() throws { + try assertValid( + """ + query Foo { + name + ...Frag + } + + fragment Frag on Dog { + name + } + """, + schema: schemaWithDirectives + ) + } + + func testWithStandardDirectives() throws { + try assertValid( + """ + { + human @skip(if: false) { + name + pets { + ... on Dog @include(if: true) { + name + } + } + } + } + """, + schema: schemaWithDirectives + ) + } + + func testWithUnknownDirective() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + { + human @unknown(directive: "value") { + name + } + } + """, + schema: schemaWithDirectives + ) + + try assertValidationError( + error: errors[0], + locations: [(line: 2, column: 11)], + message: "Unknown directive \"@unknown\"." + ) + } + + func testWithManyUnknownDirectives() throws { + let errors = try assertInvalid( + errorCount: 3, + query: + """ + { + __typename @unknown + human @unknown { + name + pets @unknown { + name + } + } + } + """, + schema: schemaWithDirectives + ) + + try assertValidationError( + error: errors[0], + locations: [(line: 2, column: 16)], + message: "Unknown directive \"@unknown\"." + ) + try assertValidationError( + error: errors[1], + locations: [(line: 3, column: 11)], + message: "Unknown directive \"@unknown\"." + ) + try assertValidationError( + error: errors[2], + locations: [(line: 5, column: 14)], + message: "Unknown directive \"@unknown\"." + ) + } + + func testWithWellPlacedDirectives() throws { + try assertValid( + """ + query ($var: Boolean @onVariableDefinition) @onQuery { + human @onField { + ...Frag @onFragmentSpread + ... @onInlineFragment { + name @onField + } + } + } + + mutation @onMutation { + someField @onField + } + + subscription @onSubscription { + someField @onField + } + + fragment Frag on Human @onFragmentDefinition { + name @onField + } + """, + schema: schemaWithDirectives + ) + } + + func testWithMisplacedDirectives() throws { + let errors = try assertInvalid( + errorCount: 12, + query: + """ + query ($var: Boolean @onQuery) @onMutation { + human @onQuery { + ...Frag @onQuery + ... @onQuery { + name @onQuery + } + } + } + + mutation @onQuery { + someField @onQuery + } + + subscription @onQuery { + someField @onQuery + } + + fragment Frag on Human @onQuery { + name @onQuery + } + """, + schema: schemaWithDirectives + ) + + try assertValidationError( + error: errors[0], + locations: [(line: 1, column: 22)], + message: "Directive \"@onQuery\" may not be used on VARIABLE_DEFINITION." + ) + try assertValidationError( + error: errors[1], + locations: [(line: 1, column: 32)], + message: "Directive \"@onMutation\" may not be used on QUERY." + ) + try assertValidationError( + error: errors[2], + locations: [(line: 2, column: 11)], + message: "Directive \"@onQuery\" may not be used on FIELD." + ) + try assertValidationError( + error: errors[3], + locations: [(line: 3, column: 17)], + message: "Directive \"@onQuery\" may not be used on FRAGMENT_SPREAD." + ) + try assertValidationError( + error: errors[4], + locations: [(line: 4, column: 13)], + message: "Directive \"@onQuery\" may not be used on INLINE_FRAGMENT." + ) + try assertValidationError( + error: errors[5], + locations: [(line: 5, column: 18)], + message: "Directive \"@onQuery\" may not be used on FIELD." + ) + try assertValidationError( + error: errors[6], + locations: [(line: 10, column: 10)], + message: "Directive \"@onQuery\" may not be used on MUTATION." + ) + try assertValidationError( + error: errors[7], + locations: [(line: 11, column: 15)], + message: "Directive \"@onQuery\" may not be used on FIELD." + ) + try assertValidationError( + error: errors[8], + locations: [(line: 14, column: 14)], + message: "Directive \"@onQuery\" may not be used on SUBSCRIPTION." + ) + try assertValidationError( + error: errors[9], + locations: [(line: 15, column: 15)], + message: "Directive \"@onQuery\" may not be used on FIELD." + ) + try assertValidationError( + error: errors[10], + locations: [(line: 18, column: 24)], + message: "Directive \"@onQuery\" may not be used on FRAGMENT_DEFINITION." + ) + try assertValidationError( + error: errors[11], + locations: [(line: 19, column: 10)], + message: "Directive \"@onQuery\" may not be used on FIELD." + ) + } + + let schemaWithDirectives = try! GraphQLSchema( + query: GraphQLObjectType( + name: "Query", + fields: [ + "dummy": GraphQLField(type: GraphQLString) { inputValue, _, _, _ -> String? in + print(type(of: inputValue)) + return nil + }, + ] + ), + directives: { + var directives = specifiedDirectives + directives.append(contentsOf: [ + try! GraphQLDirective(name: "onQuery", locations: [.query]), + try! GraphQLDirective(name: "onMutation", locations: [.mutation]), + try! GraphQLDirective(name: "onSubscription", locations: [.subscription]), + try! GraphQLDirective(name: "onField", locations: [.field]), + try! GraphQLDirective( + name: "onFragmentDefinition", + locations: [.fragmentDefinition] + ), + try! GraphQLDirective(name: "onFragmentSpread", locations: [.fragmentSpread]), + try! GraphQLDirective(name: "onInlineFragment", locations: [.inlineFragment]), + try! GraphQLDirective( + name: "onVariableDefinition", + locations: [.variableDefinition] + ), + ]) + return directives + }() + ) + + // TODO: Add SDL tests + +// let schemaWithSDLDirectives = try! GraphQLSchema( +// directives: { +// var directives = specifiedDirectives +// directives.append(contentsOf: [ +// try! GraphQLDirective(name: "onSchema", locations: [.schema]), +// try! GraphQLDirective(name: "onScalar", locations: [.scalar]), +// try! GraphQLDirective(name: "onObject", locations: [.object]), +// try! GraphQLDirective(name: "onFieldDefinition", locations: [.fieldDefinition]), +// try! GraphQLDirective(name: "onArgumentDefinition", locations: +// [.argumentDefinition]), +// try! GraphQLDirective(name: "onInterface", locations: [.interface]), +// try! GraphQLDirective(name: "onUnion", locations: [.union]), +// try! GraphQLDirective(name: "onEnum", locations: [.enum]), +// try! GraphQLDirective(name: "onEnumValue", locations: [.enumValue]), +// try! GraphQLDirective(name: "onInputObject", locations: [.inputObject]), +// try! GraphQLDirective(name: "onInputFieldDefinition", locations: +// [.inputFieldDefinition]), +// ]) +// return directives +// }() +// ) +} From 031d6c3348aa4ac62bd4fade2e100e2490719440 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 19 Nov 2023 13:05:06 -0700 Subject: [PATCH 29/34] feature: Adds ProvidedRequiredArgumentsRuleTests --- .../Rules/ProvidedNonNullArgumentsRule.swift | 44 --- .../Rules/ProvidedRequiredArgumentsRule.swift | 80 +++++ .../GraphQL/Validation/SpecifiedRules.swift | 3 +- .../ProvidedNonNullArgumentsTests.swift | 20 -- .../ProvidedRequiredArgumentsRuleTests.swift | 283 ++++++++++++++++++ 5 files changed, 364 insertions(+), 66 deletions(-) delete mode 100644 Sources/GraphQL/Validation/Rules/ProvidedNonNullArgumentsRule.swift create mode 100644 Sources/GraphQL/Validation/Rules/ProvidedRequiredArgumentsRule.swift delete mode 100644 Tests/GraphQLTests/ValidationTests/ProvidedNonNullArgumentsTests.swift create mode 100644 Tests/GraphQLTests/ValidationTests/ProvidedRequiredArgumentsRuleTests.swift diff --git a/Sources/GraphQL/Validation/Rules/ProvidedNonNullArgumentsRule.swift b/Sources/GraphQL/Validation/Rules/ProvidedNonNullArgumentsRule.swift deleted file mode 100644 index 7490adda..00000000 --- a/Sources/GraphQL/Validation/Rules/ProvidedNonNullArgumentsRule.swift +++ /dev/null @@ -1,44 +0,0 @@ -import Foundation - -func missingArgumentsMessage( - fieldName: String, - type: String, - missingArguments: [String] -) -> String { - let arguments = missingArguments.andList() - return "Field \"\(fieldName)\" on type \"\(type)\" is missing required arguments \(arguments)." -} - -func ProvidedNonNullArgumentsRule(context: ValidationContext) -> Visitor { - return Visitor( - leave: { node, _, _, _, _ in - if - let node = node as? Field, let field = context.fieldDef, - let type = context.parentType - { - let requiredArguments = Set( - field - .args - .filter { $0.type is GraphQLNonNull && $0.defaultValue == nil } - .map { $0.name } - ) - - let providedArguments = Set(node.arguments.map { $0.name.value }) - - let missingArguments = requiredArguments.subtracting(providedArguments) - if !missingArguments.isEmpty { - context.report(error: GraphQLError( - message: missingArgumentsMessage( - fieldName: field.name, - type: type.name, - missingArguments: Array(missingArguments) - ), - nodes: [node] - )) - } - } - - return .continue - } - ) -} diff --git a/Sources/GraphQL/Validation/Rules/ProvidedRequiredArgumentsRule.swift b/Sources/GraphQL/Validation/Rules/ProvidedRequiredArgumentsRule.swift new file mode 100644 index 00000000..85953b59 --- /dev/null +++ b/Sources/GraphQL/Validation/Rules/ProvidedRequiredArgumentsRule.swift @@ -0,0 +1,80 @@ +import Foundation + +/** + * Provided required arguments + * + * A field or directive is only valid if all required (non-null without a + * default value) field arguments have been provided. + */ +func ProvidedRequiredArgumentsRule(context: ValidationContext) -> Visitor { + var requiredArgsMap = [String: [String: String]]() + + let schema = context.schema + let definedDirectives = schema.directives + for directive in definedDirectives { + var requiredArgMap = [String: String]() + directive.args.filter { arg in + isRequiredArgument(arg) + }.forEach { arg in + requiredArgMap[arg.name] = "\(arg.type)" + } + requiredArgsMap[directive.name] = requiredArgMap + } + + let astDefinitions = context.ast.definitions + for def in astDefinitions { + if let directive = def as? DirectiveDefinition { + var requiredArgMap = [String: String]() + directive.arguments.filter { arg in + isRequiredArgumentNode(arg) + }.forEach { arg in + requiredArgMap[arg.name.value] = "\(arg.type)" + } + + requiredArgsMap[directive.name.value] = requiredArgMap + } + } + + return Visitor( + leave: { node, _, _, _, _ in + if let fieldNode = node as? Field { + guard let fieldDef = context.fieldDef else { + return .continue + } + + let providedArguments = Set(fieldNode.arguments.map { $0.name.value }) + + for argDef in fieldDef.args { + if !providedArguments.contains(argDef.name), isRequiredArgument(argDef) { + context.report(error: GraphQLError( + message: "Field \"\(fieldDef.name)\" argument \"\(argDef.name)\" of type \"\(argDef.type)\" is required, but it was not provided.", + nodes: [fieldNode] + )) + } + } + } + + if let directiveNode = node as? Directive { + let directiveName = directiveNode.name.value + + if let requiredArgs = requiredArgsMap[directiveName] { + let argNodes = directiveNode.arguments + let argNodeMap = Set(argNodes.map { $0.name.value }) + for (argName, argType) in requiredArgs { + if !argNodeMap.contains(argName) { + context.report(error: GraphQLError( + message: "Directive \"@\(directiveName)\" argument \"\(argName)\" of type \"\(argType)\" is required, but it was not provided.", + nodes: [directiveNode] + )) + } + } + } + } + return .continue + } + ) +} + +func isRequiredArgumentNode(_ arg: InputValueDefinition) -> Bool { + return arg.type.kind == .nonNullType && arg.defaultValue == nil +} diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index fb4f4e06..a1f060a3 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -27,8 +27,7 @@ public let specifiedRules: [(ValidationContext) -> Visitor] = [ KnownArgumentNamesRule, UniqueArgumentNamesRule, ValuesOfCorrectTypeRule, -// ProvidedRequiredArgumentsRule, - ProvidedNonNullArgumentsRule, + ProvidedRequiredArgumentsRule, VariablesInAllowedPositionRule, // OverlappingFieldsCanBeMergedRule, UniqueInputFieldNamesRule, diff --git a/Tests/GraphQLTests/ValidationTests/ProvidedNonNullArgumentsTests.swift b/Tests/GraphQLTests/ValidationTests/ProvidedNonNullArgumentsTests.swift deleted file mode 100644 index 9e0fb336..00000000 --- a/Tests/GraphQLTests/ValidationTests/ProvidedNonNullArgumentsTests.swift +++ /dev/null @@ -1,20 +0,0 @@ -@testable import GraphQL -import XCTest - -class ProvidedNonNullArgumentsTests: ValidationTestCase { - override func setUp() { - rule = ProvidedNonNullArgumentsRule - } - - func testValidWithObjectWithoutArguments() throws { - try assertValid( - "fragment objectFieldSelection on Dog { __typename name }" - ) - } - - func testValidWithCorrectArgumentNames() throws { - try assertValid( - "fragment objectFieldSelection on Dog { __typename doesKnowCommand(dogCommand: SIT) }" - ) - } -} diff --git a/Tests/GraphQLTests/ValidationTests/ProvidedRequiredArgumentsRuleTests.swift b/Tests/GraphQLTests/ValidationTests/ProvidedRequiredArgumentsRuleTests.swift new file mode 100644 index 00000000..835438d2 --- /dev/null +++ b/Tests/GraphQLTests/ValidationTests/ProvidedRequiredArgumentsRuleTests.swift @@ -0,0 +1,283 @@ +@testable import GraphQL +import XCTest + +class ProvidedRequiredArgumentsRuleTests: ValidationTestCase { + override func setUp() { + rule = ProvidedRequiredArgumentsRule + } + + func testIgnoresUnknownArguments() throws { + try assertValid( + """ + { + dog { + isHouseTrained(unknownArgument: true) + } + } + """ + ) + } + + // MARK: Valid non-nullable value + + func testArgOnOptionalArg() throws { + try assertValid( + """ + { + dog { + isHouseTrained(atOtherHomes: true) + } + } + """ + ) + } + + func testNoArgOnOptionalArg() throws { + try assertValid( + """ + { + dog { + isHouseTrained + } + } + """ + ) + } + + func testNoArgOnNonNullFieldWithDefault() throws { + try assertValid( + """ + { + complicatedArgs { + nonNullFieldWithDefault + } + } + """ + ) + } + + func testMultipleArgs() throws { + try assertValid( + """ + { + complicatedArgs { + multipleReqs(req1: 1, req2: 2) + } + } + """ + ) + } + + func testMultipleArgsInReverseOrder() throws { + try assertValid( + """ + { + complicatedArgs { + multipleReqs(req2: 2, req1: 1) + } + } + """ + ) + } + + func testNoArgsOnMultipleOptional() throws { + try assertValid( + """ + { + complicatedArgs { + multipleOpts + } + } + """ + ) + } + + func testOneArgOnMultipleOptional() throws { + try assertValid( + """ + { + complicatedArgs { + multipleOpts(opt1: 1) + } + } + """ + ) + } + + func testSecondArgOnMultipleOptional() throws { + try assertValid( + """ + { + complicatedArgs { + multipleOpts(opt2: 1) + } + } + """ + ) + } + + func testMultipleRequiredArgsOnMixedList() throws { + try assertValid( + """ + { + complicatedArgs { + multipleOptAndReq(req1: 3, req2: 4) + } + } + """ + ) + } + + func testMultipleRequiredAndOneOptionalArgOnMixedList() throws { + try assertValid( + """ + { + complicatedArgs { + multipleOptAndReq(req1: 3, req2: 4, opt1: 5) + } + } + """ + ) + } + + func testAllRequiredAndOptionalArgsOnMixedList() throws { + try assertValid( + """ + { + complicatedArgs { + multipleOptAndReq(req1: 3, req2: 4, opt1: 5, opt2: 6) + } + } + """ + ) + } + + // MARK: Invalid non-nullable value + + func testMissingOneNonNullableArgument() throws { + let errors = try assertInvalid( + errorCount: 1, + query: """ + { + complicatedArgs { + multipleReqs(req2: 2) + } + } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [ + (line: 3, column: 5), + ], + message: #"Field "multipleReqs" argument "req1" of type "Int!" is required, but it was not provided."# + ) + } + + func testMissingMultipleNonNullableArguments() throws { + let errors = try assertInvalid( + errorCount: 2, + query: """ + { + complicatedArgs { + multipleReqs + } + } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [ + (line: 3, column: 5), + ], + message: #"Field "multipleReqs" argument "req1" of type "Int!" is required, but it was not provided."# + ) + try assertValidationError( + error: errors[1], + locations: [ + (line: 3, column: 5), + ], + message: #"Field "multipleReqs" argument "req2" of type "Int!" is required, but it was not provided."# + ) + } + + func testIncorrectValueAndMissingArgument() throws { + let errors = try assertInvalid( + errorCount: 1, + query: """ + { + complicatedArgs { + multipleReqs(req1: "one") + } + } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [ + (line: 3, column: 5), + ], + message: #"Field "multipleReqs" argument "req2" of type "Int!" is required, but it was not provided."# + ) + } + + // MARK: Directive arguments + + func testIgnoresUnknonwnDirectives() throws { + try assertValid( + """ + { + dog @unknown + } + """ + ) + } + + func testWithDirectivesOfValidTypes() throws { + try assertValid( + """ + { + dog @include(if: true) { + name + } + human @skip(if: false) { + name + } + } + """ + ) + } + + func testWithDirectiveWithMissingTypes() throws { + let errors = try assertInvalid( + errorCount: 2, + query: """ + { + dog @include { + name @skip + } + } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [ + (line: 2, column: 7), + ], + message: #"Directive "@include" argument "if" of type "Boolean!" is required, but it was not provided."# + ) + try assertValidationError( + error: errors[1], + locations: [ + (line: 3, column: 10), + ], + message: #"Directive "@skip" argument "if" of type "Boolean!" is required, but it was not provided."# + ) + } + + // TODO: Add SDL tests +} From 31795b034166fca1f334b513a7244a1e60317a3d Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 19 Nov 2023 13:37:39 -0700 Subject: [PATCH 30/34] feature: ExecutableDefinitionsRule --- Sources/GraphQL/Language/AST.swift | 5 +- .../Rules/ExecutableDefinitionsRule.swift | 57 ++++++++++ .../GraphQL/Validation/SpecifiedRules.swift | 2 +- .../ExecutableDefinitionsRuleTests.swift | 102 ++++++++++++++++++ 4 files changed, 164 insertions(+), 2 deletions(-) create mode 100644 Sources/GraphQL/Validation/Rules/ExecutableDefinitionsRule.swift create mode 100644 Tests/GraphQLTests/ValidationTests/ExecutableDefinitionsRuleTests.swift diff --git a/Sources/GraphQL/Language/AST.swift b/Sources/GraphQL/Language/AST.swift index d2e2a3ac..698ac207 100644 --- a/Sources/GraphQL/Language/AST.swift +++ b/Sources/GraphQL/Language/AST.swift @@ -1769,7 +1769,10 @@ extension OperationTypeDefinition: Equatable { } } -public protocol TypeDefinition: TypeSystemDefinition {} +public protocol TypeDefinition: TypeSystemDefinition { + var name: Name { get } +} + extension ScalarTypeDefinition: TypeDefinition {} extension ObjectTypeDefinition: TypeDefinition {} extension InterfaceTypeDefinition: TypeDefinition {} diff --git a/Sources/GraphQL/Validation/Rules/ExecutableDefinitionsRule.swift b/Sources/GraphQL/Validation/Rules/ExecutableDefinitionsRule.swift new file mode 100644 index 00000000..d62fa16f --- /dev/null +++ b/Sources/GraphQL/Validation/Rules/ExecutableDefinitionsRule.swift @@ -0,0 +1,57 @@ + +/** + * Executable definitions + * + * A GraphQL document is only valid for execution if all definitions are either + * operation or fragment definitions. + * + * See https://spec.graphql.org/draft/#sec-Executable-Definitions + */ +func ExecutableDefinitionsRule(context: ValidationContext) -> Visitor { + let definitions = context.ast.definitions + let existingTypesMap = context.schema.typeMap + + var typeNames = Set() + for typeName in existingTypesMap.keys { + typeNames.insert(typeName) + } + for definition in definitions { + if + let type = definition as? TypeDefinition, + let nameResult = type.get(key: "name"), + case let .node(nameNode) = nameResult, + let name = nameNode as? Name + { + typeNames.insert(name.value) + } + } + + return Visitor( + enter: { node, _, _, _, _ in + if let node = node as? Document { + for definition in node.definitions { + if !isExecutable(definition) { + var defName = "schema" + if let definition = definition as? TypeDefinition { + defName = "\"\(definition.name.value)\"" + } else if let definition = definition as? TypeExtensionDefinition { + defName = "\"\(definition.definition.name.value)\"" + } + context.report( + error: GraphQLError( + message: "The \(defName) definition is not executable.", + nodes: [definition] + ) + ) + } + } + } + return .continue + } + ) +} + +func isExecutable(_ definition: Definition) -> Bool { + definition.kind == .operationDefinition || definition + .kind == .fragmentDefinition +} diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index a1f060a3..f12349d1 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -2,7 +2,7 @@ * This set includes all validation rules defined by the GraphQL spec. */ public let specifiedRules: [(ValidationContext) -> Visitor] = [ -// ExecutableDefinitionsRule, + ExecutableDefinitionsRule, UniqueOperationNamesRule, LoneAnonymousOperationRule, // SingleFieldSubscriptionsRule, diff --git a/Tests/GraphQLTests/ValidationTests/ExecutableDefinitionsRuleTests.swift b/Tests/GraphQLTests/ValidationTests/ExecutableDefinitionsRuleTests.swift new file mode 100644 index 00000000..d0113ede --- /dev/null +++ b/Tests/GraphQLTests/ValidationTests/ExecutableDefinitionsRuleTests.swift @@ -0,0 +1,102 @@ +@testable import GraphQL +import XCTest + +class ExecutableDefinitionsRuleTests: ValidationTestCase { + override func setUp() { + rule = ExecutableDefinitionsRule + } + + func testWithOnlyOperation() throws { + try assertValid( + """ + query Foo { + dog { + name + } + } + """ + ) + } + + func testWithOperationAndFragment() throws { + try assertValid( + """ + query Foo { + dog { + name + ...Frag + } + } + + fragment Frag on Dog { + name + } + """ + ) + } + + func testWithTypeDefinition() throws { + let errors = try assertInvalid( + errorCount: 2, + query: """ + query Foo { + dog { + name + } + } + + type Cow { + name: String + } + + extend type Dog { + color: String + } + """ + ) + + try assertValidationError( + error: errors[0], + locations: [(line: 7, column: 1)], + message: #"The "Cow" definition is not executable."# + ) + try assertValidationError( + error: errors[1], + locations: [(line: 11, column: 1)], + message: #"The "Dog" definition is not executable."# + ) + } + + func testWithSchemaDefinition() throws { + let errors = try assertInvalid( + errorCount: 3, + query: """ + schema { + query: Query + } + + type Query { + test: String + } + + extend schema @directive + """ + ) + + try assertValidationError( + error: errors[0], + locations: [(line: 1, column: 1)], + message: #"The schema definition is not executable."# + ) + try assertValidationError( + error: errors[1], + locations: [(line: 5, column: 1)], + message: #"The "Query" definition is not executable."# + ) + try assertValidationError( + error: errors[2], + locations: [(line: 9, column: 1)], + message: #"The schema definition is not executable."# + ) + } +} From 801026fa5b2ab33b03d6ec923018663436154593 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 19 Nov 2023 16:44:59 -0700 Subject: [PATCH 31/34] feature: Adds 'isRepeatable' field to Directives --- Sources/GraphQL/Type/Directives.swift | 5 ++++- Sources/GraphQL/Type/Introspection.swift | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Sources/GraphQL/Type/Directives.swift b/Sources/GraphQL/Type/Directives.swift index 83facf55..decc901a 100644 --- a/Sources/GraphQL/Type/Directives.swift +++ b/Sources/GraphQL/Type/Directives.swift @@ -31,18 +31,21 @@ public struct GraphQLDirective: Encodable { public let description: String public let locations: [DirectiveLocation] public let args: [GraphQLArgumentDefinition] + public let isRepeatable: Bool public init( name: String, description: String = "", locations: [DirectiveLocation], - args: GraphQLArgumentConfigMap = [:] + args: GraphQLArgumentConfigMap = [:], + isRepeatable: Bool = false ) throws { try assertValid(name: name) self.name = name self.description = description self.locations = locations self.args = try defineArgumentMap(args: args) + self.isRepeatable = isRepeatable } } diff --git a/Sources/GraphQL/Type/Introspection.swift b/Sources/GraphQL/Type/Introspection.swift index 974c17f7..1e0efd4b 100644 --- a/Sources/GraphQL/Type/Introspection.swift +++ b/Sources/GraphQL/Type/Introspection.swift @@ -82,6 +82,7 @@ let __Directive = try! GraphQLObjectType( fields: [ "name": GraphQLField(type: GraphQLNonNull(GraphQLString)), "description": GraphQLField(type: GraphQLString), + "isRepeatable": GraphQLField(type: GraphQLNonNull(GraphQLBoolean)), "locations": GraphQLField(type: GraphQLNonNull(GraphQLList(GraphQLNonNull(__DirectiveLocation)))), "args": GraphQLField( type: GraphQLNonNull(GraphQLList(GraphQLNonNull(__InputValue))), From 3705d06d150e6555b3842970f7e061a5ee91a1ae Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 19 Nov 2023 16:45:28 -0700 Subject: [PATCH 32/34] feature: Adds UniqueDirectivesPerLocationRule --- .../UniqueDirectivesPerLocationRule.swift | 73 +++++++ .../GraphQL/Validation/SpecifiedRules.swift | 2 +- ...UniqueDirectivesPerLocationRuleTests.swift | 196 ++++++++++++++++++ 3 files changed, 270 insertions(+), 1 deletion(-) create mode 100644 Sources/GraphQL/Validation/Rules/UniqueDirectivesPerLocationRule.swift create mode 100644 Tests/GraphQLTests/ValidationTests/UniqueDirectivesPerLocationRuleTests.swift diff --git a/Sources/GraphQL/Validation/Rules/UniqueDirectivesPerLocationRule.swift b/Sources/GraphQL/Validation/Rules/UniqueDirectivesPerLocationRule.swift new file mode 100644 index 00000000..331ebc00 --- /dev/null +++ b/Sources/GraphQL/Validation/Rules/UniqueDirectivesPerLocationRule.swift @@ -0,0 +1,73 @@ + +/** + * Unique directive names per location + * + * A GraphQL document is only valid if all non-repeatable directives at + * a given location are uniquely named. + * + * See https://spec.graphql.org/draft/#sec-Directives-Are-Unique-Per-Location + */ +func UniqueDirectivesPerLocationRule(context: ValidationContext) -> Visitor { + var uniqueDirectiveMap = [String: Bool]() + + let schema = context.schema + let definedDirectives = schema.directives + for directive in definedDirectives { + uniqueDirectiveMap[directive.name] = !directive.isRepeatable + } + + let astDefinitions = context.ast.definitions + for def in astDefinitions { + if let directive = def as? DirectiveDefinition { + uniqueDirectiveMap[directive.name.value] = !directive.repeatable + } + } + + let schemaDirectives = [String: Directive]() + var typeDirectivesMap = [String: [String: Directive]]() + + return Visitor( + enter: { node, _, _, _, _ in +// if let operation = node as? OperationDefinition { + // Many different AST nodes may contain directives. Rather than listing + // them all, just listen for entering any node, and check to see if it + // defines any directives. + if + let directiveNodeResult = node.get(key: "directives"), + case let .array(directiveNodes) = directiveNodeResult, + let directives = directiveNodes as? [Directive] + { + var seenDirectives = [String: Directive]() + if node.kind == .schemaDefinition || node.kind == .schemaExtensionDefinition { + seenDirectives = schemaDirectives + } else if let node = node as? TypeDefinition { + let typeName = node.name.value + seenDirectives = typeDirectivesMap[typeName] ?? [:] + typeDirectivesMap[typeName] = seenDirectives + } else if let node = node as? TypeExtensionDefinition { + let typeName = node.definition.name.value + seenDirectives = typeDirectivesMap[typeName] ?? [:] + typeDirectivesMap[typeName] = seenDirectives + } + + for directive in directives { + let directiveName = directive.name.value + + if uniqueDirectiveMap[directiveName] ?? false { + if let seenDirective = seenDirectives[directiveName] { + context.report( + error: GraphQLError( + message: "The directive \"@\(directiveName)\" can only be used once at this location.", + nodes: [seenDirective, directive] + ) + ) + } else { + seenDirectives[directiveName] = directive + } + } + } + } + return .continue + } + ) +} diff --git a/Sources/GraphQL/Validation/SpecifiedRules.swift b/Sources/GraphQL/Validation/SpecifiedRules.swift index f12349d1..e414c0a0 100644 --- a/Sources/GraphQL/Validation/SpecifiedRules.swift +++ b/Sources/GraphQL/Validation/SpecifiedRules.swift @@ -20,7 +20,7 @@ public let specifiedRules: [(ValidationContext) -> Visitor] = [ NoUndefinedVariablesRule, NoUnusedVariablesRule, KnownDirectivesRule, -// UniqueDirectivesPerLocationRule, + UniqueDirectivesPerLocationRule, // DeferStreamDirectiveOnRootFieldRule, // DeferStreamDirectiveOnValidOperationsRule, // DeferStreamDirectiveLabelRule, diff --git a/Tests/GraphQLTests/ValidationTests/UniqueDirectivesPerLocationRuleTests.swift b/Tests/GraphQLTests/ValidationTests/UniqueDirectivesPerLocationRuleTests.swift new file mode 100644 index 00000000..d511e24b --- /dev/null +++ b/Tests/GraphQLTests/ValidationTests/UniqueDirectivesPerLocationRuleTests.swift @@ -0,0 +1,196 @@ +@testable import GraphQL +import XCTest + +class UniqueDirectivesPerLocationRuleTests: ValidationTestCase { + override func setUp() { + rule = UniqueDirectivesPerLocationRule + } + + func testNoDirectives() throws { + try assertValid( + """ + fragment Test on Type { + field + } + """, + schema: schema + ) + } + + func testUniqueDirectivesInDifferentLocations() throws { + try assertValid( + """ + fragment Test on Type @directiveA { + field @directiveB + } + """, + schema: schema + ) + } + + func testUniqueDirectivesInSameLocation() throws { + try assertValid( + """ + fragment Test on Type @directiveA @directiveB { + field @directiveA @directiveB + } + """, + schema: schema + ) + } + + func testSameDirectivesInDifferentLocations() throws { + try assertValid( + """ + fragment Test on Type @directiveA { + field @directiveA + } + """, + schema: schema + ) + } + + func testSameDirectivesInSimilarLocations() throws { + try assertValid( + """ + fragment Test on Type { + field @directive + field @directive + } + """, + schema: schema + ) + } + + func testRepeatableDirectivesInSameLocation() throws { + try assertValid( + """ + fragment Test on Type @repeatable @repeatable { + field @repeatable @repeatable + } + """, + schema: schema + ) + } + + func testUnknownDirectivesMustBeIgnored() throws { + try assertValid( + """ + type Test @unknown @unknown { + field: String! @unknown @unknown + } + + extend type Test @unknown { + anotherField: String! + } + """, + schema: schema + ) + } + + func testDuplicateDirectivesInOneLocation() throws { + let errors = try assertInvalid( + errorCount: 1, + query: + """ + fragment Test on Type { + field @directive @directive + } + """, + schema: schema + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 2, column: 9), + (line: 2, column: 20), + ], + message: #"The directive "@directive" can only be used once at this location."# + ) + } + + func testManyDuplicateDirectivesInOneLocation() throws { + let errors = try assertInvalid( + errorCount: 2, + query: + """ + fragment Test on Type { + field @directive @directive @directive + } + """, + schema: schema + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 2, column: 9), + (line: 2, column: 20), + ], + message: #"The directive "@directive" can only be used once at this location."# + ) + try assertValidationError( + error: errors[1], + locations: [ + (line: 2, column: 9), + (line: 2, column: 31), + ], + message: #"The directive "@directive" can only be used once at this location."# + ) + } + + func testDifferentDuplicateDirectivesInOneLocation() throws { + let errors = try assertInvalid( + errorCount: 2, + query: + """ + fragment Test on Type { + field @directiveA @directiveB @directiveA @directiveB + } + """, + schema: schema + ) + try assertValidationError( + error: errors[0], + locations: [ + (line: 2, column: 9), + (line: 2, column: 33), + ], + message: #"The directive "@directiveA" can only be used once at this location."# + ) + try assertValidationError( + error: errors[1], + locations: [ + (line: 2, column: 21), + (line: 2, column: 45), + ], + message: #"The directive "@directiveB" can only be used once at this location."# + ) + } + + // TODO: Add SDL tests + + let schema = try! GraphQLSchema( + query: ValidationExampleQueryRoot, + types: [ + ValidationExampleCat, + ValidationExampleDog, + ValidationExampleHuman, + ValidationExampleAlien, + ], + directives: { + var directives = specifiedDirectives + directives.append(contentsOf: [ + ValidationFieldDirective, + try! GraphQLDirective(name: "directive", locations: [.field, .fragmentDefinition]), + try! GraphQLDirective(name: "directiveA", locations: [.field, .fragmentDefinition]), + try! GraphQLDirective(name: "directiveB", locations: [.field, .fragmentDefinition]), + try! GraphQLDirective( + name: "repeatable", + locations: [.field, .fragmentDefinition], + isRepeatable: true + ), + ]) + return directives + }() + ) +} From 699c6acf6384aa0d8887b2c67d2ad1d22ff19662 Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 19 Nov 2023 21:33:03 -0700 Subject: [PATCH 33/34] fix: Ensures correct visitor ordering --- Sources/GraphQL/Type/Definition.swift | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/Sources/GraphQL/Type/Definition.swift b/Sources/GraphQL/Type/Definition.swift index 2f20d884..0fe90d79 100644 --- a/Sources/GraphQL/Type/Definition.swift +++ b/Sources/GraphQL/Type/Definition.swift @@ -1,5 +1,6 @@ import Foundation import NIO +import OrderedCollections /** * These are all of the possible kinds of types. @@ -508,7 +509,7 @@ public struct GraphQLResolveInfo { public let variableValues: [String: Any] } -public typealias GraphQLFieldMap = [String: GraphQLField] +public typealias GraphQLFieldMap = OrderedDictionary public struct GraphQLField { public let type: GraphQLOutputType @@ -568,7 +569,7 @@ public struct GraphQLField { } } -public typealias GraphQLFieldDefinitionMap = [String: GraphQLFieldDefinition] +public typealias GraphQLFieldDefinitionMap = OrderedDictionary public final class GraphQLFieldDefinition { public let name: String @@ -654,7 +655,7 @@ extension GraphQLFieldDefinition: KeySubscriptable { } } -public typealias GraphQLArgumentConfigMap = [String: GraphQLArgument] +public typealias GraphQLArgumentConfigMap = OrderedDictionary public struct GraphQLArgument { public let type: GraphQLInputType @@ -1131,7 +1132,7 @@ func defineEnumValues( return definitions } -public typealias GraphQLEnumValueMap = [String: GraphQLEnumValue] +public typealias GraphQLEnumValueMap = OrderedDictionary public struct GraphQLEnumValue { public let value: Map @@ -1312,7 +1313,7 @@ public struct InputObjectField { } } -public typealias InputObjectFieldMap = [String: InputObjectField] +public typealias InputObjectFieldMap = OrderedDictionary public final class InputObjectFieldDefinition { public let name: String @@ -1383,7 +1384,10 @@ public func isRequiredInputField(_ field: InputObjectFieldDefinition) -> Bool { return field.type is GraphQLNonNull && field.defaultValue == nil } -public typealias InputObjectFieldDefinitionMap = [String: InputObjectFieldDefinition] +public typealias InputObjectFieldDefinitionMap = OrderedDictionary< + String, + InputObjectFieldDefinition +> /** * List Modifier From a4063a10941592c45c3dd92f3f0c5f9d70043aca Mon Sep 17 00:00:00 2001 From: Jay Herron Date: Sun, 19 Nov 2023 18:28:00 -0700 Subject: [PATCH 34/34] fix: Fixes unpredictable fragment path matching --- Sources/GraphQL/Validation/Validate.swift | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/Sources/GraphQL/Validation/Validate.swift b/Sources/GraphQL/Validation/Validate.swift index cd6e8232..3dfb4721 100644 --- a/Sources/GraphQL/Validation/Validate.swift +++ b/Sources/GraphQL/Validation/Validate.swift @@ -165,9 +165,11 @@ public final class ValidationContext { } public func getFragmentSpreads(node: SelectionSet) -> [FragmentSpread] { - if let spreads = fragmentSpreads[node] { - return spreads - } + // Uncommenting this creates unpredictably wrong fragment path matching. + // Failures can be seen in NoFragmentCyclesRuleTests.testNoSpreadingItselfDeeplyTwoPaths +// if let spreads = fragmentSpreads[node] { +// return spreads +// } var spreads = [FragmentSpread]() var setsToVisit: [SelectionSet] = [node] @@ -176,19 +178,18 @@ public final class ValidationContext { for selection in set.selections { if let selection = selection as? FragmentSpread { spreads.append(selection) - } - - if let selection = selection as? InlineFragment { + } else if let selection = selection as? InlineFragment { setsToVisit.append(selection.selectionSet) - } - - if let selection = selection as? Field, let selectionSet = selection.selectionSet { + } else if + let selection = selection as? Field, + let selectionSet = selection.selectionSet + { setsToVisit.append(selectionSet) } } } - fragmentSpreads[node] = spreads +// fragmentSpreads[node] = spreads return spreads }