Merge branch 'master' of https://github.com/nveenjain/graphql-js into feature/addCommentInAST

Signed-off-by: Naveen Jain <[email protected]>

Author: nveenjain

.eslintrc.yml

@@ -433,7 +433,7 @@ overrides:
       flowtype/require-valid-file-annotation: off
 
       ##########################################################################
-      # `@typescript-eslint/eslint-plugin` rule list based on `v2.17.x`
+      # `@typescript-eslint/eslint-plugin` rule list based on `v2.21.x`
       ##########################################################################
 
       # Supported Rules

.flowconfig

@@ -40,4 +40,4 @@ suppress_comment=\\(.\\|\n\\)*\\$FlowFixMe\\($\\|[^(]\\|(\\(<VERSION>\\)?)\\)
 suppress_comment=\\(.\\|\n\\)*\\$DisableFlowOnNegativeTest
 
 [version]
-^0.118.0
+^0.119.0

package.json

@@ -44,23 +44,23 @@
   },
   "dependencies": {},
   "devDependencies": {
-    "@babel/core": "7.8.4",
+    "@babel/core": "7.8.6",
     "@babel/plugin-transform-flow-strip-types": "7.8.3",
-    "@babel/preset-env": "7.8.4",
-    "@babel/register": "7.8.3",
-    "@typescript-eslint/eslint-plugin": "2.19.2",
-    "@typescript-eslint/parser": "2.19.2",
-    "babel-eslint": "10.0.3",
+    "@babel/preset-env": "7.8.6",
+    "@babel/register": "7.8.6",
+    "@typescript-eslint/eslint-plugin": "2.21.0",
+    "@typescript-eslint/parser": "2.21.0",
+    "babel-eslint": "10.1.0",
     "chai": "4.2.0",
-    "cspell": "4.0.46",
-    "dtslint": "2.0.6",
+    "cspell": "4.0.55",
+    "dtslint": "3.2.0",
     "eslint": "6.8.0",
     "eslint-plugin-flowtype": "4.6.0",
     "eslint-plugin-import": "2.20.1",
-    "flow-bin": "0.118.0",
-    "mocha": "7.0.1",
+    "flow-bin": "0.119.1",
+    "mocha": "7.1.0",
     "nyc": "15.0.0",
     "prettier": "1.19.1",
-    "typescript": "^3.7.5"
+    "typescript": "^3.8.3"
   }
 }

src/README.md

@@ -17,7 +17,7 @@ Each sub directory within is a sub-module of graphql-js:
   fulfilling a GraphQL result.
 - [`graphql/execution`](execution/README.md): The Execution phase of fulfilling
   a GraphQL request.
-- [`graphql/error`](error/README.md): Creating and formating GraphQL errors.
+- [`graphql/error`](error/README.md): Creating and formatting GraphQL errors.
 - [`graphql/utilities`](utilities/README.md): Common useful computations upon
   the GraphQL language and type objects.
 - [`graphql/subscription`](subscription/README.md): Subscribe to data updates.

src/__tests__/graphql-test.js

@@ -93,6 +93,17 @@ describe('Execute: synchronously when possible', () => {
   });
 
   describe('graphqlSync', () => {
+    it('report errors raised during schema validation', () => {
+      const badSchema = new GraphQLSchema({});
+      const result = graphqlSync({
+        schema: badSchema,
+        source: '{ __typename }',
+      });
+      expect(result).to.deep.equal({
+        errors: [{ message: 'Query root type must be provided.' }],
+      });
+    });
+
     it('does not return a Promise for syntax errors', () => {
       const doc = 'fragment Example on Query { { { syncField }';
       const result = graphqlSync({

src/execution/__tests__/sync-test.js

@@ -128,8 +128,7 @@ describe('Lexer', () => {
       '{ kind: "Name", value: "foo", line: 1, column: 1 }',
     );
   });
-
-  it('skips whitespace and comments', () => {
+  it('skips whitespace and commas', () => {
     expect(
       lexOne(`
 
@@ -144,18 +143,6 @@ describe('Lexer', () => {
       value: 'foo',
     });
 
-    expect(
-      lexOne(`
-    #comment
-    foo#comment
-`),
-    ).to.contain({
-      kind: TokenKind.NAME,
-      start: 18,
-      end: 21,
-      value: 'foo',
-    });
-
     expect(lexOne(',,,foo,,,')).to.contain({
       kind: TokenKind.NAME,
       start: 3,
@@ -181,7 +168,6 @@ describe('Lexer', () => {
       4 |
     `);
   });
-
   it('updates line numbers in error for file context', () => {
     let caughtError;
     try {
@@ -218,6 +204,19 @@ describe('Lexer', () => {
         |     ^
     `);
   });
+  it('lexes comments', () => {
+    expect(
+      lexOne(
+        dedent`#this is a comment
+                a{}`,
+      ),
+    ).to.contain({
+      kind: TokenKind.COMMENT,
+      start: 0,
+      end: 18,
+      value: 'this is a comment',
+    });
+  });
 
   it('lexes strings', () => {
     expect(lexOne('""')).to.contain({
@@ -877,9 +876,6 @@ describe('Lexer', () => {
     let endToken;
     do {
       endToken = lexer.advance();
-      // Lexer advances over ignored comment tokens to make writing parsers
-      // easier, but will include them in the linked list result.
-      expect(endToken.kind).to.not.equal(TokenKind.COMMENT);
     } while (endToken.kind !== TokenKind.EOF);
 
     expect(startToken.prev).to.equal(null);

src/language/__tests__/lexer-test.js

@@ -147,37 +147,81 @@ describe('Parser', () => {
     );
   });
 
-  it('Add comments from type in AST', () => {
-    const ast = parse(`
+  it('Add single comments in AST', () => {
+    const ast = parse(dedent`
       #This comment has a \u0A0A multi-byte character.
       type alpha{ field(arg: string):string }
     `);
 
-    expect(ast).to.have.nested.property(
-      'comments[0].value',
-      'This comment has a \u0A0A multi-byte character.',
-    );
-    expect(ast.comments).to.have.length(1);
+    expect(toJSONDeep(ast.comments)).to.deep.equal([
+      {
+        kind: 'Comment',
+        loc: { start: 0, end: 43 },
+        value: 'This comment has a ਊ multi-byte character.',
+      },
+    ]);
+  });
+
+  it('Ignore comments that comes when we peek for a token in AST', () => {
+    const ast = parse(dedent`
+      type #This is a comment that gets ignored
+      alpha{ field(arg: string):string }
+    `);
+
+    expect(toJSONDeep(ast.comments)).to.deep.equal([
+      {
+        kind: 'Comment',
+        loc: {
+          end: 41,
+          start: 5,
+        },
+        value: 'This is a comment that gets ignored',
+      },
+    ]);
+  });
+
+  it('Add empty comments from in AST', () => {
+    const ast = parse(dedent`
+      #
+      type alpha{ field(arg: string):string }
+    `);
+
+    expect(toJSONDeep(ast.comments)).to.deep.equal([
+      {
+        kind: 'Comment',
+        loc: { start: 0, end: 1 },
+        value: '',
+      },
+    ]);
   });
 
   it('Add multiple comments in AST', () => {
-    const ast = parse(`
+    const ast = parse(dedent`
+      #This is top comment
       type alpha{
         #This comment is demo comment.
         field(arg: string):string 
         #This is another demo comment having # inside
       }
     `);
 
-    expect(ast).to.have.nested.property(
-      'comments[0].value',
-      'This comment is demo comment.',
-    );
-    expect(ast).to.have.nested.property(
-      'comments[1].value',
-      'This is another demo comment having # inside',
-    );
-    expect(ast.comments).to.have.length(2);
+    expect(toJSONDeep(ast.comments)).to.deep.equal([
+      {
+        kind: 'Comment',
+        loc: { start: 0, end: 20 },
+        value: 'This is top comment',
+      },
+      {
+        kind: 'Comment',
+        loc: { start: 35, end: 65 },
+        value: 'This comment is demo comment.',
+      },
+      {
+        kind: 'Comment',
+        loc: { start: 97, end: 142 },
+        value: 'This is another demo comment having # inside',
+      },
+    ]);
   });
 
   it('parses kitchen sink', () => {

src/language/__tests__/parser-test.js

@@ -4,6 +4,7 @@ import { expect } from 'chai';
 import { describe, it } from 'mocha';
 
 import invariant from '../../jsutils/invariant';
+import dedent from '../../jsutils/dedent';
 
 import { Kind } from '../kinds';
 import { parse } from '../parser';
@@ -115,6 +116,43 @@ describe('Visitor', () => {
     });
   });
 
+  it('allows visiting multiple comments including comments in definitions', () => {
+    const ast = parse(
+      dedent`
+      #This is a comment
+      { 
+        #a is a field
+        a 
+      }
+    `,
+      { noLocation: true },
+    );
+    const visited = [];
+
+    visit(ast, {
+      enter: {
+        Comment(node) {
+          visited.push(['enter', node.kind]);
+          visited.push(['value', node.value]);
+        },
+      },
+      leave: {
+        Comment(node) {
+          visited.push(['leave', node.kind]);
+        },
+      },
+    });
+
+    expect(visited).to.deep.equal([
+      ['enter', 'Comment'],
+      ['value', 'This is a comment'],
+      ['leave', 'Comment'],
+      ['enter', 'Comment'],
+      ['value', 'a is a field'],
+      ['leave', 'Comment'],
+    ]);
+  });
+
   it('allows visiting only specified nodes', () => {
     const ast = parse('{ a }', { noLocation: true });
     const visited = [];

src/language/__tests__/visitor-test.js

@@ -99,6 +99,7 @@ export function isNode(maybeNode: any): maybeNode is ASTNode;
 export type ASTNode =
   | NameNode
   | DocumentNode
+  | CommentNode
   | OperationDefinitionNode
   | VariableDefinitionNode
   | VariableNode
@@ -147,6 +148,7 @@ export type ASTNode =
 export interface ASTKindToNode {
   Name: NameNode;
   Document: DocumentNode;
+  Comment: CommentNode;
   OperationDefinition: OperationDefinitionNode;
   VariableDefinition: VariableDefinitionNode;
   Variable: VariableNode;
@@ -204,16 +206,13 @@ export interface DocumentNode {
   readonly kind: 'Document';
   readonly loc?: Location;
   readonly definitions: ReadonlyArray<DefinitionNode>;
-  readonly comments: ReadonlyArray<CommentNode>;
+  readonly comments?: ReadonlyArray<CommentNode>;
 }
 
 export interface CommentNode {
   readonly kind: 'Comment';
-  readonly start: number;
-  readonly end: number;
-  readonly column: number;
-  readonly line: number;
-  readonly value: string | void;
+  readonly loc?: Location;
+  readonly value: string;
 }
 
 export type DefinitionNode =

src/language/ast.d.ts

@@ -135,6 +135,7 @@ export function isNode(maybeNode: mixed): boolean %checks {
 export type ASTNode =
   | NameNode
   | DocumentNode
+  | CommentNode
   | OperationDefinitionNode
   | VariableDefinitionNode
   | VariableNode
@@ -183,6 +184,7 @@ export type ASTNode =
 export type ASTKindToNode = {|
   Name: NameNode,
   Document: DocumentNode,
+  Comment: CommentNode,
   OperationDefinition: OperationDefinitionNode,
   VariableDefinition: VariableDefinitionNode,
   Variable: VariableNode,
@@ -240,7 +242,7 @@ export type DocumentNode = {|
   +kind: 'Document',
   +loc?: Location,
   +definitions: $ReadOnlyArray<DefinitionNode>,
-  +comments: $ReadOnlyArray<CommentNode>,
+  +comments?: $ReadOnlyArray<CommentNode>,
 |};
 
 export type DefinitionNode =
@@ -250,11 +252,8 @@ export type DefinitionNode =
 
 export type CommentNode = {|
   +kind: 'Comment',
-  +start: number,
-  +end: number,
-  +column: number,
-  +line: number,
-  +value: string | void,
+  loc?: Location,
+  +value: string,
 |};
 
 export type ExecutableDefinitionNode =

src/language/ast.js

@@ -22,7 +22,6 @@ export class Lexer {
    */
   token: Token;
 
-  commentsList: Array<CommentNode>;
   /**
    * The (1-indexed) line containing the current token.
    */