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 f2070c8
Show file tree
Hide file tree
Showing 17 changed files with 100 additions and 87 deletions.
2 changes: 1 addition & 1 deletion src/execution/__tests__/executor-test.js
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
22 changes: 18 additions & 4 deletions src/jsutils/inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,23 @@
* @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 value && typeof value === 'object'
? typeof value.inspect === 'function'
? value.inspect()
: Array.isArray(value)
? '[' + value.map(inspect).join(', ') + ']'
: '{' +
Object.keys(value)
.map(k => `${k}: ${inspect(value[k])}`)
.join(', ') +
'}'
: typeof value === 'string'
? '"' + value + '"'
: typeof value === 'function'
? `[function ${value.name}]`
: String(value);
}
4 changes: 1 addition & 3 deletions src/language/__tests__/parser-test.js
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
20 changes: 10 additions & 10 deletions src/type/__tests__/validation-test.js
Original file line number Diff line number Diff line change
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 Expand Up @@ -861,10 +861,10 @@ describe('Type System: Object fields must have output types', () => {
const schema = schemaWithObjectFieldOfType(Number);
expect(validateSchema(schema)).to.deep.equal([
{
message: `The type of BadObject.badField must be Output Type but got: ${Number}.`,
message: `The type of BadObject.badField must be Output Type but got: [function Number].`,
},
{
message: `Expected GraphQL named type but got: ${Number}.`,
message: `Expected GraphQL named type but got: [function Number].`,
},
]);
});
Expand Down Expand Up @@ -1162,13 +1162,13 @@ describe('Type System: Interface fields must have output types', () => {
const schema = schemaWithInterfaceFieldOfType(Number);
expect(validateSchema(schema)).to.deep.equal([
{
message: `The type of BadInterface.badField must be Output Type but got: ${Number}.`,
message: `The type of BadInterface.badField must be Output Type but got: [function Number].`,
},
{
message: `Expected GraphQL named type but got: ${Number}.`,
message: `Expected GraphQL named type but got: [function Number].`,
},
{
message: `The type of BadImplementing.badField must be Output Type but got: ${Number}.`,
message: `The type of BadImplementing.badField must be Output Type but got: [function Number].`,
},
]);
});
Expand Down Expand Up @@ -1275,10 +1275,10 @@ describe('Type System: Field arguments must have input types', () => {
const schema = schemaWithArgOfType(Number);
expect(validateSchema(schema)).to.deep.equal([
{
message: `The type of BadObject.badField(badArg:) must be Input Type but got: ${Number}.`,
message: `The type of BadObject.badField(badArg:) must be Input Type but got: [function Number].`,
},
{
message: `Expected GraphQL named type but got: ${Number}.`,
message: `Expected GraphQL named type but got: [function Number].`,
},
]);
});
Expand Down Expand Up @@ -1359,10 +1359,10 @@ describe('Type System: Input Object fields must have input types', () => {
const schema = schemaWithInputFieldOfType(Number);
expect(validateSchema(schema)).to.deep.equal([
{
message: `The type of BadInputObject.badField must be Input Type but got: ${Number}.`,
message: `The type of BadInputObject.badField must be Input Type but got: [function Number].`,
},
{
message: `Expected GraphQL named type but got: ${Number}.`,
message: `Expected GraphQL named type but got: [function Number].`,
},
]);
});
Expand Down
18 changes: 9 additions & 9 deletions src/utilities/__tests__/coerceValue-test.js
Original file line number Diff line number Diff line change
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
Loading

0 comments on commit f2070c8

Please sign in to comment.