Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix get parent field #5

Merged
merged 5 commits into from
May 5, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions graphql/rules.lua
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,16 @@ end
function rules.fieldsDefinedOnType(node, context)
if context.objects[#context.objects] == false then
local parent = context.objects[#context.objects - 1]
if(parent.__type == 'List') then
parent = parent.ofType
end
error('Field "' .. node.name.value .. '" is not defined on type "' .. parent.name .. '"')
Copy link
Owner

Choose a reason for hiding this comment

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

I think Field "field" is not defined on type "[Parent]" here would be better (brackets to indicate List type).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see where you are going but i think the message is correct now since "field" is part of the "parent" definition and has nothing to do with the list. For example:
"is_complete" is a property of "task" type sounds good but
"is_complete" is a property of "[task]" type is a bit wired

end
end

function rules.argumentsDefinedOnType(node, context)
if node.arguments then
local parentField = context.objects[#context.objects - 1].fields[node.name.value]
local parentField = util.getParentField(context, node.name.value)
for _, argument in pairs(node.arguments) do
local name = argument.name.value
if not parentField.arguments[name] then
Expand Down Expand Up @@ -175,7 +178,7 @@ end

function rules.argumentsOfCorrectType(node, context)
if node.arguments then
local parentField = context.objects[#context.objects - 1].fields[node.name.value]
local parentField = util.getParentField(context, node.name.value)
for _, argument in pairs(node.arguments) do
local name = argument.name.value
local argumentType = parentField.arguments[name]
Expand All @@ -186,13 +189,7 @@ end

function rules.requiredArgumentsPresent(node, context)
local arguments = node.arguments or {}
local parentField
if context.objects[#context.objects - 1].__type == 'List' then
parentField = context.objects[#context.objects - 2].fields[node.name.value]
else
parentField = context.objects[#context.objects - 1].fields[node.name.value]
end

local parentField = util.getParentField(context, node.name.value)
for name, argument in pairs(parentField.arguments) do
if argument.__type == 'NonNull' then
local present = util.find(arguments, function(argument)
Expand Down Expand Up @@ -451,7 +448,7 @@ function rules.variableUsageAllowed(node, context)
if not arguments then return end

for field in pairs(arguments) do
local parentField = context.objects[#context.objects - 1].fields[field]
local parentField = util.getParentField(context, field)
for i = 1, #arguments[field] do
local argument = arguments[field][i]
if argument.value.kind == 'variable' then
Expand Down
12 changes: 11 additions & 1 deletion graphql/util.lua
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,16 @@ function util.bind1(func, x)
end
end

function util.getParentField(context, name, count)
count = count == nil and 1 or count
local obj = context.objects[#context.objects - count]
if obj.__type == 'List' then
return obj.ofType.fields[name]
else
return obj.fields[name]
end
end
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe we could default step_back to 1 if it's nil? Also would prefer a more succinct variable name of count. There's also an indentation problem. I can address all these post-merge if you want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'll fix that


function util.coerceValue(node, schemaType, variables)
variables = variables or {}

Expand Down Expand Up @@ -58,7 +68,7 @@ function util.coerceValue(node, schemaType, variables)
error('Unknown input object field "' .. field.name .. '"')
end

return util.coerceValue(schemaType.fields[field.name].kind, field.value, variables)
return util.coerceValue(field.value, schemaType.fields[field.name].kind, variables)
end)
end

Expand Down
9 changes: 2 additions & 7 deletions graphql/validate.lua
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
local path = (...):gsub('%.[^%.]+$', '')
local rules = require(path .. '.rules')
local util = require(path .. '.util')

local visitors = {
document = {
Expand Down Expand Up @@ -58,16 +59,10 @@ local visitors = {

field = {
enter = function(node, context)
local parentField
if context.objects[#context.objects].__type == 'List' then
parentField = context.objects[#context.objects - 1].fields[node.name.value]
else
parentField = context.objects[#context.objects].fields[node.name.value]
end
local parentField = util.getParentField(context, node.name.value, 0)

-- false is a special value indicating that the field was not present in the type definition.
local field = parentField and parentField.kind or false

table.insert(context.objects, field)
end,

Expand Down