Skip to content

Commit

Permalink
Improve the inspect() function. (#1380)
Browse files Browse the repository at this point in the history
As well as use it in more places, replace use of JSON.stringify, and ensure validation rule message formatters expect strings instead of types.
  • Loading branch information
leebyron committed Jun 11, 2018
1 parent faeea7f commit 37196f4
Show file tree
Hide file tree
Showing 17 changed files with 89 additions and 78 deletions.
2 changes: 1 addition & 1 deletion src/execution/__tests__/executor-test.js
Expand Up @@ -1054,7 +1054,7 @@ describe('Execute: Handles basic execution tasks', () => {
errors: [
{
message:
'Expected value of type "SpecialType" but got: [object Object].',
'Expected value of type "SpecialType" but got: {value: "bar"}.',
locations: [{ line: 1, column: 3 }],
path: ['specials', 1],
},
Expand Down
18 changes: 9 additions & 9 deletions src/execution/__tests__/variables-test.js
Expand Up @@ -371,7 +371,7 @@ describe('Execute: Handles inputs', () => {
{
message:
'Variable "$input" got invalid value ' +
'{"a":"foo","b":"bar","c":null}; ' +
'{a: "foo", b: "bar", c: null}; ' +
'Expected non-nullable type String! not to be null at value.c.',
locations: [{ line: 2, column: 16 }],
},
Expand Down Expand Up @@ -401,7 +401,7 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Variable "$input" got invalid value {"a":"foo","b":"bar"}; ' +
'Variable "$input" got invalid value {a: "foo", b: "bar"}; ' +
'Field value.c of required type String! was not provided.',
locations: [{ line: 2, column: 16 }],
},
Expand All @@ -421,13 +421,13 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Variable "$input" got invalid value {"na":{"a":"foo"}}; ' +
'Variable "$input" got invalid value {na: {a: "foo"}}; ' +
'Field value.na.c of required type String! was not provided.',
locations: [{ line: 2, column: 18 }],
},
{
message:
'Variable "$input" got invalid value {"na":{"a":"foo"}}; ' +
'Variable "$input" got invalid value {na: {a: "foo"}}; ' +
'Field value.nb of required type String! was not provided.',
locations: [{ line: 2, column: 18 }],
},
Expand All @@ -446,7 +446,7 @@ describe('Execute: Handles inputs', () => {
{
message:
'Variable "$input" got invalid value ' +
'{"a":"foo","b":"bar","c":"baz","extra":"dog"}; ' +
'{a: "foo", b: "bar", c: "baz", extra: "dog"}; ' +
'Field "extra" is not defined by type TestInputObject.',
locations: [{ line: 2, column: 16 }],
},
Expand Down Expand Up @@ -693,8 +693,8 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Variable "$value" got invalid value [1,2,3]; Expected type ' +
'String; String cannot represent an array value: [1,2,3]',
'Variable "$value" got invalid value [1, 2, 3]; Expected type ' +
'String; String cannot represent an array value: [1, 2, 3]',
locations: [{ line: 2, column: 16 }],
},
],
Expand Down Expand Up @@ -841,7 +841,7 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Variable "$input" got invalid value ["A",null,"B"]; ' +
'Variable "$input" got invalid value ["A", null, "B"]; ' +
'Expected non-nullable type String! not to be null at value[1].',
locations: [{ line: 2, column: 16 }],
},
Expand Down Expand Up @@ -891,7 +891,7 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Variable "$input" got invalid value ["A",null,"B"]; ' +
'Variable "$input" got invalid value ["A", null, "B"]; ' +
'Expected non-nullable type String! not to be null at value[1].',
locations: [{ line: 2, column: 16 }],
},
Expand Down
2 changes: 1 addition & 1 deletion src/execution/execute.js
Expand Up @@ -1046,7 +1046,7 @@ function ensureValidRuntimeType(
throw new GraphQLError(
`Abstract type ${returnType.name} must resolve to an Object type at ` +
`runtime for field ${info.parentType.name}.${info.fieldName} with ` +
`value "${inspect(result)}", received "${inspect(runtimeType)}". ` +
`value ${inspect(result)}, received "${inspect(runtimeType)}". ` +
`Either the ${returnType.name} type should provide a "resolveType" ` +
'function or each possible types should provide an ' +
'"isTypeOf" function.',
Expand Down
2 changes: 1 addition & 1 deletion src/execution/values.js
Expand Up @@ -99,7 +99,7 @@ export function getVariableValues(
coercionErrors.forEach(error => {
error.message =
`Variable "$${varName}" got invalid ` +
`value ${JSON.stringify(value)}; ${error.message}`;
`value ${inspect(value)}; ${error.message}`;
});
errors.push(...coercionErrors);
} else {
Expand Down
20 changes: 16 additions & 4 deletions src/jsutils/inspect.js
Expand Up @@ -7,9 +7,21 @@
* @flow strict
*/

/**
* Used to print values in error messages.
*/
export default function inspect(value: mixed): string {
if (Array.isArray(value)) {
return '[' + String(value) + ']';
}
return String(value);
return Array.isArray(value)
? '[' + value.map(inspect).join(', ') + ']'
: value && typeof value === 'object'
? typeof value.inspect === 'function'
? value.inspect()
: '{' +
Object.keys(value)
.map(k => `${k}: ${inspect(value[k])}`)
.join(', ') +
'}'
: typeof value === 'string'
? '"' + value + '"'
: String(value);
}
4 changes: 1 addition & 3 deletions src/language/__tests__/parser-test.js
Expand Up @@ -29,9 +29,7 @@ describe('Parser', () => {
});

it('asserts that a source to parse was provided', () => {
expect(() => parse({})).to.throw(
'Must provide Source. Received: [object Object]',
);
expect(() => parse({})).to.throw('Must provide Source. Received: {}');
});

it('parse provides useful errors', () => {
Expand Down
4 changes: 2 additions & 2 deletions src/subscription/__tests__/subscribe-test.js
Expand Up @@ -373,7 +373,7 @@ describe('Subscription Initialization Phase', () => {

await expectPromiseToThrow(
() => createSubscription(pubsub, invalidEmailSchema),
'Subscription field must return Async Iterable. Received: test',
'Subscription field must return Async Iterable. Received: "test"',
);
});

Expand Down Expand Up @@ -474,7 +474,7 @@ describe('Subscription Initialization Phase', () => {
{
message:
'Variable "$priority" got invalid value "meow"; Expected ' +
'type Int; Int cannot represent non-integer value: meow',
'type Int; Int cannot represent non-integer value: "meow"',
locations: [{ line: 2, column: 21 }],
},
],
Expand Down
7 changes: 4 additions & 3 deletions src/type/__tests__/definition-test.js
Expand Up @@ -23,6 +23,7 @@ import {
import { describe, it } from 'mocha';
import { expect } from 'chai';

import inspect from '../../jsutils/inspect';
import { isObjectType, isInputType, isOutputType } from '../definition';

const BlogImage = new GraphQLObjectType({
Expand Down Expand Up @@ -657,7 +658,7 @@ describe('Type System: Object fields must have valid resolve values', () => {
it('rejects an empty Object field resolver', () => {
expect(() => schemaWithObjectWithFieldResolver({})).to.throw(
'BadResolver.badField field resolver must be a function if provided, ' +
'but got: [object Object].',
'but got: {}.',
);
});

Expand Down Expand Up @@ -1139,7 +1140,7 @@ describe('Type System: List must accept only types', () => {
notTypes.forEach(type => {
it(`rejects a non-type as item type of list: ${type}`, () => {
expect(() => GraphQLList(type)).to.throw(
`Expected ${type} to be a GraphQL type.`,
`Expected ${inspect(type)} to be a GraphQL type.`,
);
});
});
Expand Down Expand Up @@ -1175,7 +1176,7 @@ describe('Type System: NonNull must only accept non-nullable types', () => {
notNullableTypes.forEach(type => {
it(`rejects a non-type as nullable type of non-null: ${type}`, () => {
expect(() => GraphQLNonNull(type)).to.throw(
`Expected ${type} to be a GraphQL nullable type.`,
`Expected ${inspect(type)} to be a GraphQL nullable type.`,
);
});
});
Expand Down
4 changes: 2 additions & 2 deletions src/type/__tests__/enumType-test.js
Expand Up @@ -205,7 +205,7 @@ describe('Type System: Enum Values', () => {
data: { colorEnum: null },
errors: [
{
message: 'Expected a value of type "Color" but received: GREEN',
message: 'Expected a value of type "Color" but received: "GREEN"',
locations: [{ line: 1, column: 3 }],
path: ['colorEnum'],
},
Expand Down Expand Up @@ -384,7 +384,7 @@ describe('Type System: Enum Values', () => {
errors: [
{
message:
'Expected a value of type "Complex" but received: [object Object]',
'Expected a value of type "Complex" but received: {someRandomValue: 123}',
locations: [{ line: 6, column: 9 }],
path: ['bad'],
},
Expand Down
6 changes: 3 additions & 3 deletions src/type/__tests__/serialization-test.js
Expand Up @@ -38,7 +38,7 @@ describe('Type System: Scalar coercion', () => {
'Int cannot represent non-integer value: -1.1',
);
expect(() => GraphQLInt.serialize('-1.1')).to.throw(
'Int cannot represent non-integer value: -1.1',
'Int cannot represent non-integer value: "-1.1"',
);
// Maybe a safe JavaScript int, but bigger than 2^32, so not
// representable as a GraphQL Int
Expand All @@ -56,7 +56,7 @@ describe('Type System: Scalar coercion', () => {
'Int cannot represent non 32-bit signed integer value: -1e+100',
);
expect(() => GraphQLInt.serialize('one')).to.throw(
'Int cannot represent non-integer value: one',
'Int cannot represent non-integer value: "one"',
);
// Doesn't represent number
expect(() => GraphQLInt.serialize('')).to.throw(
Expand Down Expand Up @@ -92,7 +92,7 @@ describe('Type System: Scalar coercion', () => {
'Float cannot represent non numeric value: Infinity',
);
expect(() => GraphQLFloat.serialize('one')).to.throw(
'Float cannot represent non numeric value: one',
'Float cannot represent non numeric value: "one"',
);
expect(() => GraphQLFloat.serialize('')).to.throw(
'Float cannot represent non numeric value: (empty string)',
Expand Down
2 changes: 1 addition & 1 deletion src/type/__tests__/validation-test.js
Expand Up @@ -383,7 +383,7 @@ describe('Type System: A Schema must have Object root types', () => {
});
expect(validateSchema(schema)).to.deep.equal([
{
message: 'Expected directive but got: somedirective.',
message: 'Expected directive but got: "somedirective".',
},
]);
});
Expand Down
18 changes: 9 additions & 9 deletions src/utilities/__tests__/coerceValue-test.js
Expand Up @@ -35,7 +35,7 @@ describe('coerceValue', () => {
it('returns error for array input as string', () => {
const result = coerceValue([1, 2, 3], scalar);
expectErrors(result).to.deep.equal([
`Expected type ${scalar}; String cannot represent an array value: [1,2,3]`,
`Expected type ${scalar}; String cannot represent an array value: [1, 2, 3]`,
]);
});
});
Expand Down Expand Up @@ -79,7 +79,7 @@ describe('coerceValue', () => {
it('returns a single error for float input as int', () => {
const result = coerceValue('1.5', GraphQLInt);
expectErrors(result).to.deep.equal([
'Expected type Int; Int cannot represent non-integer value: 1.5',
'Expected type Int; Int cannot represent non-integer value: "1.5"',
]);
});

Expand All @@ -100,14 +100,14 @@ describe('coerceValue', () => {
it('returns a single error for char input', () => {
const result = coerceValue('a', GraphQLInt);
expectErrors(result).to.deep.equal([
'Expected type Int; Int cannot represent non-integer value: a',
'Expected type Int; Int cannot represent non-integer value: "a"',
]);
});

it('returns a single error for char input', () => {
const result = coerceValue('meow', GraphQLInt);
expectErrors(result).to.deep.equal([
'Expected type Int; Int cannot represent non-integer value: meow',
'Expected type Int; Int cannot represent non-integer value: "meow"',
]);
});
});
Expand Down Expand Up @@ -157,14 +157,14 @@ describe('coerceValue', () => {
it('returns a single error for char input', () => {
const result = coerceValue('a', GraphQLFloat);
expectErrors(result).to.deep.equal([
'Expected type Float; Float cannot represent non numeric value: a',
'Expected type Float; Float cannot represent non numeric value: "a"',
]);
});

it('returns a single error for char input', () => {
const result = coerceValue('meow', GraphQLFloat);
expectErrors(result).to.deep.equal([
'Expected type Float; Float cannot represent non numeric value: meow',
'Expected type Float; Float cannot represent non numeric value: "meow"',
]);
});
});
Expand Down Expand Up @@ -226,15 +226,15 @@ describe('coerceValue', () => {
it('returns no error for an invalid field', () => {
const result = coerceValue({ foo: 'abc' }, TestInputObject);
expectErrors(result).to.deep.equal([
'Expected type Int at value.foo; Int cannot represent non-integer value: abc',
'Expected type Int at value.foo; Int cannot represent non-integer value: "abc"',
]);
});

it('returns multiple errors for multiple invalid fields', () => {
const result = coerceValue({ foo: 'abc', bar: 'def' }, TestInputObject);
expectErrors(result).to.deep.equal([
'Expected type Int at value.foo; Int cannot represent non-integer value: abc',
'Expected type Int at value.bar; Int cannot represent non-integer value: def',
'Expected type Int at value.foo; Int cannot represent non-integer value: "abc"',
'Expected type Int at value.bar; Int cannot represent non-integer value: "def"',
]);
});

Expand Down
12 changes: 4 additions & 8 deletions src/validation/rules/FragmentsOnCompositeTypes.js
Expand Up @@ -7,28 +7,24 @@
* @flow strict
*/

import inspect from '../../jsutils/inspect';
import type ValidationContext from '../ValidationContext';
import { GraphQLError } from '../../error';
import { print } from '../../language/printer';
import type { ASTVisitor } from '../../language/visitor';
import { isCompositeType } from '../../type/definition';
import type { GraphQLType } from '../../type/definition';
import { typeFromAST } from '../../utilities/typeFromAST';

export function inlineFragmentOnNonCompositeErrorMessage(
type: GraphQLType,
): string {
return `Fragment cannot condition on non composite type "${inspect(type)}".`;
export function inlineFragmentOnNonCompositeErrorMessage(type: string): string {
return `Fragment cannot condition on non composite type "${type}".`;
}

export function fragmentOnNonCompositeErrorMessage(
fragName: string,
type: GraphQLType,
type: string,
): string {
return (
`Fragment "${fragName}" cannot condition on non composite ` +
`type "${inspect(type)}".`
`type "${type}".`
);
}

Expand Down

0 comments on commit 37196f4

Please sign in to comment.