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

Change devAsserts for checking source argument #2784

Merged
merged 1 commit into from
Sep 3, 2020
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
10 changes: 0 additions & 10 deletions src/language/__tests__/parser-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,6 @@ function expectSyntaxError(text: string) {
}

describe('Parser', () => {
it('asserts that a source to parse was provided', () => {
// $FlowExpectedError[incompatible-call]
expect(() => parse()).to.throw('Must provide Source. Received: undefined.');
});

it('asserts that an invalid source to parse was provided', () => {
// $FlowExpectedError[incompatible-call]
expect(() => parse({})).to.throw('Must provide Source. Received: {}.');
});

it('parse provides useful errors', () => {
let caughtError;
try {
Expand Down
14 changes: 14 additions & 0 deletions src/language/__tests__/source-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,20 @@ import { describe, it } from 'mocha';
import { Source } from '../source';

describe('Source', () => {
it('asserts that a body was provided', () => {
// $FlowExpectedError[incompatible-call]
expect(() => new Source()).to.throw(
'Body must be a string. Received: undefined.',
);
});

it('asserts that a valid body was provided', () => {
// $FlowExpectedError[incompatible-call]
expect(() => new Source({})).to.throw(
'Body must be a string. Received: {}.',
);
});

it('can be Object.toStringified', () => {
const source = new Source('');

Expand Down
11 changes: 2 additions & 9 deletions src/language/parser.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
import inspect from '../jsutils/inspect';
import devAssert from '../jsutils/devAssert';

import type { GraphQLError } from '../error/GraphQLError';
import { syntaxError } from '../error/syntaxError';

Expand Down Expand Up @@ -53,8 +50,8 @@ import type {
} from './ast';
import { Kind } from './kinds';
import { Location } from './ast';
import { Source } from './source';
import { TokenKind } from './tokenKind';
import { Source, isSource } from './source';
import { DirectiveLocation } from './directiveLocation';
import { Lexer, isPunctuatorTokenKind } from './lexer';

Expand Down Expand Up @@ -178,11 +175,7 @@ export class Parser {
_lexer: Lexer;

constructor(source: string | Source, options?: ParseOptions) {
const sourceObj = typeof source === 'string' ? new Source(source) : source;
devAssert(
sourceObj instanceof Source,
`Must provide Source. Received: ${inspect(sourceObj)}.`,
);
const sourceObj = isSource(source) ? source : new Source(source);

this._lexer = new Lexer(sourceObj);
this._options = options;
Expand Down
7 changes: 7 additions & 0 deletions src/language/source.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,10 @@ export class Source {
locationOffset: Location;
constructor(body: string, name?: string, locationOffset?: Location);
}

/**
* Test if the given value is a Source object.
*
* @internal
*/
export function isSource(source: any): source is Source;
19 changes: 19 additions & 0 deletions src/language/source.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { SYMBOL_TO_STRING_TAG } from '../polyfills/symbols';

import inspect from '../jsutils/inspect';
import devAssert from '../jsutils/devAssert';
import instanceOf from '../jsutils/instanceOf';

type Location = {|
line: number,
Expand All @@ -24,6 +26,11 @@ export class Source {
name: string = 'GraphQL request',
locationOffset: Location = { line: 1, column: 1 },
): void {
devAssert(
typeof body === 'string',
`Body must be a string. Received: ${inspect(body)}.`,
);

this.body = body;
this.name = name;
this.locationOffset = locationOffset;
Expand All @@ -42,3 +49,15 @@ export class Source {
return 'Source';
}
}

/**
* Test if the given value is a Source object.
*
* @internal
*/
declare function isSource(source: mixed): boolean %checks(source instanceof
Source);
// eslint-disable-next-line no-redeclare
export function isSource(source) {
return instanceOf(source, Source);
}
20 changes: 5 additions & 15 deletions src/utilities/__tests__/stripIgnoredCharacters-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import inspectStr from '../../__testUtils__/inspectStr';

import invariant from '../../jsutils/invariant';

import { Lexer } from '../../language/lexer';
import { parse } from '../../language/parser';
import { Source } from '../../language/source';
import { Lexer } from '../../language/lexer';

import { stripIgnoredCharacters } from '../stripIgnoredCharacters';

Expand Down Expand Up @@ -98,20 +98,6 @@ function expectStripped(docString: string) {
}

describe('stripIgnoredCharacters', () => {
it('asserts that a source was provided', () => {
// $FlowExpectedError[incompatible-call]
expect(() => stripIgnoredCharacters()).to.throw(
'Must provide string or Source. Received: undefined.',
);
});

it('asserts that a valid source was provided', () => {
// $FlowExpectedError[incompatible-call]
expect(() => stripIgnoredCharacters({})).to.throw(
'Must provide string or Source. Received: {}.',
);
});

it('strips ignored characters from GraphQL query document', () => {
const query = dedent`
query SomeQuery($foo: String!, $bar: String) {
Expand All @@ -130,6 +116,10 @@ describe('stripIgnoredCharacters', () => {
);
});

it('accepts Source object', () => {
expect(stripIgnoredCharacters(new Source('{ a }'))).to.equal('{a}');
});

it('strips ignored characters from GraphQL SDL document', () => {
const sdl = dedent`
"""
Expand Down
11 changes: 2 additions & 9 deletions src/utilities/stripIgnoredCharacters.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import inspect from '../jsutils/inspect';

import { Source } from '../language/source';
import { Source, isSource } from '../language/source';
import { TokenKind } from '../language/tokenKind';
import { Lexer, isPunctuatorTokenKind } from '../language/lexer';
import {
Expand Down Expand Up @@ -61,12 +59,7 @@ import {
* """Type description""" type Foo{"""Field description""" bar:String}
*/
export function stripIgnoredCharacters(source: string | Source): string {
const sourceObj = typeof source === 'string' ? new Source(source) : source;
if (!(sourceObj instanceof Source)) {
throw new TypeError(
`Must provide string or Source. Received: ${inspect(sourceObj)}.`,
);
}
const sourceObj = isSource(source) ? source : new Source(source);

const body = sourceObj.body;
const lexer = new Lexer(sourceObj);
Expand Down