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

Include possible field, argument, type names when validation fails #355

Merged
merged 8 commits into from
Apr 26, 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
28 changes: 28 additions & 0 deletions src/jsutils/__tests__/suggestionList-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
* Copyright (c) 2015, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/

import { expect } from 'chai';
import { describe, it } from 'mocha';
import { suggestionList } from '../suggestionList';

describe('suggestionList', () => {

it('Returns results when input is empty', () => {
expect(suggestionList('', [ 'a' ])).to.deep.equal([ 'a' ]);
});

it('Returns empty array when there are no options', () => {
expect(suggestionList('input', [])).to.deep.equal([]);
});

it('Returns options sorted based on similarity', () => {
expect(suggestionList('abc', [ 'a', 'ab', 'abc' ]))
.to.deep.equal([ 'abc', 'ab' ]);
});
});
82 changes: 82 additions & 0 deletions src/jsutils/suggestionList.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/* @flow */
/**
* Copyright (c) 2015, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/

/**
* Given an invalid input string and a list of valid options, returns a filtered
* list of valid options sorted based on their similarity with the input.
*/
export function suggestionList(
input: string,
options: Array<string>
): Array<string> {
let i;
const d = {};
const oLength = options.length;
const inputThreshold = input.length / 2;
for (i = 0; i < oLength; i++) {
const distance = lexicalDistance(input, options[i]);
const threshold = Math.max(inputThreshold, options[i].length / 2, 1);
if (distance <= threshold) {
d[options[i]] = distance;
}
}
const result = Object.keys(d);
return result.sort((a , b) => d[a] - d[b]);
}

/**
* Computes the lexical distance between strings A and B.
*
* The "distance" between two strings is given by counting the minimum number
* of edits needed to transform string A into string B. An edit can be an
* insertion, deletion, or substitution of a single character, or a swap of two
* adjacent characters.
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth mentioning "Levenshtein" somewhere in here.

(Also, did you consider using Damerau–Levenshtein, which considers transpositions as well?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is copied pasta from https://github.com/graphql/codemirror-graphql/blob/master/src/utils/hintList.js#L40

I did not put too much though on which specific algorithm to use. I just found the one that will get us most of the way there. Improvements can always be made later when we understand the pitfalls of this one better. Looking at the wikipedia page for Damerau–Levenshtein, is "a transposition of two adjacent characters" different from "a swap of two adjacent characters" ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, no! I missed the word "swap" in there. It is exactly transposition. So this probably is Damerau–Levenshtein already.

*
* This distance can be useful for detecting typos in input or sorting
*
* @param {string} a
* @param {string} b
* @return {int} distance in number of edits
*/
function lexicalDistance(a, b) {
let i;
let j;
const d = [];
const aLength = a.length;
const bLength = b.length;

for (i = 0; i <= aLength; i++) {
d[i] = [ i ];
}

for (j = 1; j <= bLength; j++) {
d[0][j] = j;
}

for (i = 1; i <= aLength; i++) {
for (j = 1; j <= bLength; j++) {
const cost = a[i - 1] === b[j - 1] ? 0 : 1;

d[i][j] = Math.min(
d[i - 1][j] + 1,
d[i][j - 1] + 1,
d[i - 1][j - 1] + cost
);

if (i > 1 && j > 1 &&
a[i - 1] === b[j - 2] &&
a[i - 2] === b[j - 1]) {
d[i][j] = Math.min(d[i][j], d[i - 2][j - 2] + cost);
}
}
}

return d[aLength][bLength];
}
111 changes: 90 additions & 21 deletions src/validation/__tests__/FieldsOnCorrectType-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,21 @@ import {
} from '../rules/FieldsOnCorrectType';


function undefinedField(field, type, suggestions, line, column) {
function undefinedField(
field,
type,
suggestedTypes,
suggestedFields,
line,
column
) {
return {
message: undefinedFieldMessage(field, type, suggestions),
message: undefinedFieldMessage(
field,
type,
suggestedTypes,
suggestedFields
),
locations: [ { line, column } ],
};
}
Expand Down Expand Up @@ -85,8 +97,16 @@ describe('Validate: Fields on correct type', () => {
}
}
}`,
[ undefinedField('unknown_pet_field', 'Pet', [], 3, 9),
undefinedField('unknown_cat_field', 'Cat', [], 5, 13) ]
[ undefinedField('unknown_pet_field', 'Pet', [], [], 3, 9),
undefinedField(
'unknown_cat_field',
'Cat',
[],
[],
5,
13
)
]
);
});

Expand All @@ -95,7 +115,15 @@ describe('Validate: Fields on correct type', () => {
fragment fieldNotDefined on Dog {
meowVolume
}`,
[ undefinedField('meowVolume', 'Dog', [], 3, 9) ]
[ undefinedField(
'meowVolume',
'Dog',
[],
[ 'barkVolume' ],
3,
9
)
]
);
});

Expand All @@ -106,7 +134,15 @@ describe('Validate: Fields on correct type', () => {
deeper_unknown_field
}
}`,
[ undefinedField('unknown_field', 'Dog', [], 3, 9) ]
[ undefinedField(
'unknown_field',
'Dog',
[],
[],
3,
9
)
]
);
});

Expand All @@ -117,7 +153,7 @@ describe('Validate: Fields on correct type', () => {
unknown_field
}
}`,
[ undefinedField('unknown_field', 'Pet', [], 4, 11) ]
[ undefinedField('unknown_field', 'Pet', [], [], 4, 11) ]
);
});

Expand All @@ -128,7 +164,15 @@ describe('Validate: Fields on correct type', () => {
meowVolume
}
}`,
[ undefinedField('meowVolume', 'Dog', [], 4, 11) ]
[ undefinedField(
'meowVolume',
'Dog',
[],
[ 'barkVolume' ],
4,
11
)
]
);
});

Expand All @@ -137,7 +181,15 @@ describe('Validate: Fields on correct type', () => {
fragment aliasedFieldTargetNotDefined on Dog {
volume : mooVolume
}`,
[ undefinedField('mooVolume', 'Dog', [], 3, 9) ]
[ undefinedField(
'mooVolume',
'Dog',
[],
[ 'barkVolume' ],
3,
9
)
]
);
});

Expand All @@ -146,7 +198,15 @@ describe('Validate: Fields on correct type', () => {
fragment aliasedLyingFieldTargetNotDefined on Dog {
barkVolume : kawVolume
}`,
[ undefinedField('kawVolume', 'Dog', [], 3, 9) ]
[ undefinedField(
'kawVolume',
'Dog',
[],
[ 'barkVolume' ],
3,
9
)
]
);
});

Expand All @@ -155,7 +215,7 @@ describe('Validate: Fields on correct type', () => {
fragment notDefinedOnInterface on Pet {
tailLength
}`,
[ undefinedField('tailLength', 'Pet', [], 3, 9) ]
[ undefinedField('tailLength', 'Pet', [], [], 3, 9) ]
);
});

Expand All @@ -164,7 +224,7 @@ describe('Validate: Fields on correct type', () => {
fragment definedOnImplementorsButNotInterface on Pet {
nickname
}`,
[ undefinedField('nickname', 'Pet', [ 'Cat', 'Dog' ], 3, 9) ]
[ undefinedField('nickname', 'Pet', [ 'Cat', 'Dog' ], [ 'name' ], 3, 9) ]
);
});

Expand All @@ -181,7 +241,7 @@ describe('Validate: Fields on correct type', () => {
fragment directFieldSelectionOnUnion on CatOrDog {
directField
}`,
[ undefinedField('directField', 'CatOrDog', [], 3, 9) ]
[ undefinedField('directField', 'CatOrDog', [], [], 3, 9) ]
);
});

Expand All @@ -195,6 +255,7 @@ describe('Validate: Fields on correct type', () => {
'name',
'CatOrDog',
[ 'Being', 'Pet', 'Canine', 'Cat', 'Dog' ],
[],
3,
9
)
Expand All @@ -218,25 +279,33 @@ describe('Validate: Fields on correct type', () => {
describe('Fields on correct type error message', () => {
it('Works with no suggestions', () => {
expect(
undefinedFieldMessage('T', 'f', [])
).to.equal('Cannot query field "T" on type "f".');
undefinedFieldMessage('f', 'T', [], [])
).to.equal('Cannot query field "f" on type "T".');
});

it('Works with no small numbers of suggestions', () => {
expect(
undefinedFieldMessage('T', 'f', [ 'A', 'B' ])
).to.equal('Cannot query field "T" on type "f". ' +
undefinedFieldMessage('f', 'T', [ 'A', 'B' ], [ 'z', 'y' ])
).to.equal('Cannot query field "f" on type "T". ' +
'However, this field exists on "A", "B". ' +
'Perhaps you meant to use an inline fragment?');
'Perhaps you meant to use an inline fragment? ' +
'Did you mean to query "z", "y"?');
});

it('Works with lots of suggestions', () => {
expect(
undefinedFieldMessage('T', 'f', [ 'A', 'B', 'C', 'D', 'E', 'F' ])
).to.equal('Cannot query field "T" on type "f". ' +
undefinedFieldMessage(
'f',
'T',
[ 'A', 'B', 'C', 'D', 'E', 'F' ],
[ 'z', 'y', 'x', 'w', 'v', 'u' ]
)
).to.equal('Cannot query field "f" on type "T". ' +
'However, this field exists on "A", "B", "C", "D", "E", ' +
'and 1 other types. ' +
'Perhaps you meant to use an inline fragment?');
'Perhaps you meant to use an inline fragment? ' +
'Did you mean to query "z", "y", "x", "w", "v", or 1 other field?'
);
});
});
});
Expand Down
26 changes: 16 additions & 10 deletions src/validation/__tests__/KnownArgumentNames-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,22 @@ import {
} from '../rules/KnownArgumentNames';


function unknownArg(argName, fieldName, typeName, line, column) {
function unknownArg(argName, fieldName, typeName, suggestedArgs, line, column) {
return {
message: unknownArgMessage(argName, fieldName, typeName),
message: unknownArgMessage(argName, fieldName, typeName, suggestedArgs),
locations: [ { line, column } ],
};
}

function unknownDirectiveArg(argName, directiveName, line, column) {
function unknownDirectiveArg(
argName,
directiveName,
suggestedArgs,
line,
column
) {
return {
message: unknownDirectiveArgMessage(argName, directiveName),
message: unknownDirectiveArgMessage(argName, directiveName, suggestedArgs),
locations: [ { line, column } ],
};
}
Expand Down Expand Up @@ -103,7 +109,7 @@ describe('Validate: Known argument names', () => {
dog @skip(unless: true)
}
`, [
unknownDirectiveArg('unless', 'skip', 3, 19),
unknownDirectiveArg('unless', 'skip', [], 3, 19),
]);
});

Expand All @@ -113,7 +119,7 @@ describe('Validate: Known argument names', () => {
doesKnowCommand(unknown: true)
}
`, [
unknownArg('unknown', 'doesKnowCommand', 'Dog', 3, 25),
unknownArg('unknown', 'doesKnowCommand', 'Dog', [], 3, 25),
]);
});

Expand All @@ -123,8 +129,8 @@ describe('Validate: Known argument names', () => {
doesKnowCommand(whoknows: 1, dogCommand: SIT, unknown: true)
}
`, [
unknownArg('whoknows', 'doesKnowCommand', 'Dog', 3, 25),
unknownArg('unknown', 'doesKnowCommand', 'Dog', 3, 55),
unknownArg('whoknows', 'doesKnowCommand', 'Dog', [], 3, 25),
unknownArg('unknown', 'doesKnowCommand', 'Dog', [], 3, 55),
]);
});

Expand All @@ -143,8 +149,8 @@ describe('Validate: Known argument names', () => {
}
}
`, [
unknownArg('unknown', 'doesKnowCommand', 'Dog', 4, 27),
unknownArg('unknown', 'doesKnowCommand', 'Dog', 9, 31),
unknownArg('unknown', 'doesKnowCommand', 'Dog', [], 4, 27),
unknownArg('unknown', 'doesKnowCommand', 'Dog', [], 9, 31),
]);
});

Expand Down
Loading