Skip to content

Commit

Permalink
fix: Fixes NoUnusedVariablesRule
Browse files Browse the repository at this point in the history
  • Loading branch information
NeedleInAJayStack committed Nov 6, 2023
1 parent aa447f1 commit 93bdc5a
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 36 deletions.
21 changes: 20 additions & 1 deletion Sources/GraphQL/Language/AST.swift
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,6 @@ public func == (lhs: Definition, rhs: Definition) -> Bool {
public enum OperationType: String {
case query
case mutation
// Note: subscription is an experimental non-spec addition.
case subscription
}

Expand Down Expand Up @@ -949,6 +948,15 @@ public final class ObjectValue {
self.loc = loc
self.fields = fields
}

public func get(key: String) -> NodeResult? {
switch key {
case "fields":
return .array(fields)
default:
return nil
}
}
}

extension ObjectValue: Equatable {
Expand All @@ -968,6 +976,17 @@ public final class ObjectField {
self.name = name
self.value = value
}

public func get(key: String) -> NodeResult? {
switch key {
case "name":
return .node(name)
case "value":
return .node(value)
default:
return nil
}
}
}

extension ObjectField: Equatable {
Expand Down
2 changes: 1 addition & 1 deletion Sources/GraphQL/Language/Kinds.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
public enum Kind {
public enum Kind: CaseIterable {
case name
case document
case operationDefinition
Expand Down
12 changes: 6 additions & 6 deletions Sources/GraphQL/Language/Visitor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,8 @@ final class Stack {
* If a prior visitor edits a node, no following visitors will see that node.
*/
func visitInParallel(visitors: [Visitor]) -> Visitor {
var skipping = [Node?](repeating: nil, count: visitors.count)

var skipping = [VisitResult?](repeating: nil, count: visitors.count)
return Visitor(
enter: { node, key, parent, path, ancestors in
for i in 0 ..< visitors.count {
Expand All @@ -271,9 +271,9 @@ func visitInParallel(visitors: [Visitor]) -> Visitor {
)

if case .skip = result {
skipping[i] = node
skipping[i] = .node(node)
} else if case .break = result {
// skipping[i] = BREAK
skipping[i] = .break
} else if case .node = result {
return result
}
Expand All @@ -294,11 +294,11 @@ func visitInParallel(visitors: [Visitor]) -> Visitor {
)

if case .break = result {
// skipping[i] = BREAK
skipping[i] = .break
} else if case .node = result {
return result
}
} // else if skipping[i] == node {
} //else if case let .node(skippedNode) = skipping[i], skippedNode == node {
// skipping[i] = nil
// }
}
Expand Down
30 changes: 7 additions & 23 deletions Sources/GraphQL/Validation/Rules/NoUnusedVariablesRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,38 +5,23 @@
* are used, either directly or within a spread fragment.
*/
func NoUnusedVariablesRule(context: ValidationContext) -> Visitor {
var variableDefs: [VariableDefinition] = []

return Visitor(
enter: { node, _, _, _, _ in
if node is OperationDefinition {
variableDefs = []
return .continue
}

if let def = node as? VariableDefinition {
variableDefs.append(def)
return .continue
}

return .continue
},
leave: { node, _, _, _, _ -> VisitResult in
guard let operation = node as? OperationDefinition else {
return .continue
}

var variableNameUsed: [String: Bool] = [:]

let usages = context.getRecursiveVariableUsages(operation: operation)

for usage in usages {
variableNameUsed[usage.node.name.value] = true
}

for variableDef in variableDefs {
let variableNameUsed = Set(usages.map { usage in
usage.node.name.value
})

for variableDef in operation.variableDefinitions {
let variableName = variableDef.variable.name.value

if variableNameUsed[variableName] != true {
if !variableNameUsed.contains(variableName) {
context.report(
error: GraphQLError(
message: operation.name.map {
Expand All @@ -47,7 +32,6 @@ func NoUnusedVariablesRule(context: ValidationContext) -> Visitor {
)
}
}

return .continue
}
)
Expand Down
9 changes: 4 additions & 5 deletions Sources/GraphQL/Validation/Validate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -240,11 +240,10 @@ public final class ValidationContext {
}

if let variable = node as? Variable {
usages
.append(VariableUsage(
node: variable,
type: typeInfo.inputType
))
usages.append(VariableUsage(
node: variable,
type: typeInfo.inputType
))
}

return .continue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,4 +256,14 @@ class NoUnusedVariablesRuleTests: ValidationTestCase {
message: #"Variable "$a" is never used in operation "Bar"."#
)
}

func testVariableUsedInsideObject() throws {
try assertValid(
"""
query Foo($a: String) {
field(object: { a: $a })
}
"""
)
}
}

0 comments on commit 93bdc5a

Please sign in to comment.