Fix printError/locations for multiple nodes. (#1131)

If a GraphQLError represents multiple nodes across files (could happen for validation across multiple parsed files) then the reported locations and printError output can be incorrect for the second node. This ensures locations are derived from nodes whenever possible to get correct location and amends comment documentation.

Author: leebyron

src/error/GraphQLError.js

@@ -65,7 +65,10 @@ declare class GraphQLError extends Error {
   +nodes: $ReadOnlyArray<ASTNode> | void;
 
   /**
-   * The source GraphQL document corresponding to this error.
+   * The source GraphQL document for the first location of this error.
+   *
+   * Note that if this Error represents more than one node, the source may not
+   * represent nodes after the first node.
    */
   +source: Source | void;
 
@@ -121,9 +124,15 @@ export function GraphQLError( // eslint-disable-line no-redeclare
   }
 
   let _locations;
-  const _source2 = _source; // seems here Flow need a const to resolve type.
-  if (_source2 && _positions) {
-    _locations = _positions.map(pos => getLocation(_source2, pos));
+  if (positions && source) {
+    _locations = positions.map(pos => getLocation(source, pos));
+  } else if (_nodes) {
+    _locations = _nodes.reduce((list, node) => {
+      if (node.loc) {
+        list.push(getLocation(node.loc.source, node.loc.start));
+      }
+      return list;
+    }, []);
   }
 
   Object.defineProperties(this, {

src/error/__tests__/printError-test.js

@@ -0,0 +1,61 @@
+/**
+ * Copyright (c) 2015-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import { expect } from 'chai';
+import { describe, it } from 'mocha';
+
+import { GraphQLError } from '../GraphQLError';
+import { printError } from '../printError';
+import { parse, Source } from '../../language';
+import dedent from '../../jsutils/dedent';
+
+describe('printError', () => {
+  it('prints an error with nodes from different sources', () => {
+    const sourceA = parse(
+      new Source(
+        dedent`
+        type Foo {
+          field: String
+        }`,
+        'SourceA',
+      ),
+    );
+    const fieldTypeA = sourceA.definitions[0].fields[0].type;
+
+    const sourceB = parse(
+      new Source(
+        dedent`
+        type Foo {
+          field: Int
+        }`,
+        'SourceB',
+      ),
+    );
+    const fieldTypeB = sourceB.definitions[0].fields[0].type;
+
+    const error = new GraphQLError('Example error with two nodes', [
+      fieldTypeA,
+      fieldTypeB,
+    ]);
+
+    expect(printError(error)).to.equal(dedent`
+      Example error with two nodes
+
+      SourceA (2:10)
+      1: type Foo {
+      2:   field: String
+                  ^
+      3: }
+
+      SourceB (2:10)
+      1: type Foo {
+      2:   field: Int
+                  ^
+      3: }
+    `);
+  });
+});

src/error/printError.js

@@ -7,6 +7,7 @@
  * @flow
  */
 
+import { getLocation } from '../language/location';
 import type { SourceLocation } from '../language/location';
 import type { Source } from '../language/source';
 import type { GraphQLError } from './GraphQLError';
@@ -16,15 +17,27 @@ import type { GraphQLError } from './GraphQLError';
  * about the error's position in the source.
  */
 export function printError(error: GraphQLError): string {
-  const source = error.source;
-  const locations = error.locations || [];
-  const printedLocations = locations.map(
-    location =>
-      source
-        ? highlightSourceAtLocation(source, location)
-        : ` (${location.line}:${location.column})`,
-  );
-  return error.message + printedLocations.join('');
+  const printedLocations = [];
+  if (error.nodes) {
+    error.nodes.forEach(node => {
+      if (node.loc) {
+        printedLocations.push(
+          highlightSourceAtLocation(
+            node.loc.source,
+            getLocation(node.loc.source, node.loc.start),
+          ),
+        );
+      }
+    });
+  } else if (error.source && error.locations) {
+    const source = error.source;
+    error.locations.forEach(location => {
+      printedLocations.push(highlightSourceAtLocation(source, location));
+    });
+  }
+  return printedLocations.length === 0
+    ? error.message
+    : [error.message, ...printedLocations].join('\n\n') + '\n';
 }
 
 /**
@@ -46,21 +59,14 @@ function highlightSourceAtLocation(
   const padLen = nextLineNum.length;
   const lines = source.body.split(/\r\n|[\n\r]/g);
   lines[0] = whitespace(source.locationOffset.column - 1) + lines[0];
-  return (
-    `\n\n${source.name} (${contextLine}:${contextColumn})\n` +
-    (line >= 2
-      ? lpad(padLen, prevLineNum) + ': ' + lines[line - 2] + '\n'
-      : '') +
-    lpad(padLen, lineNum) +
-    ': ' +
-    lines[line - 1] +
-    '\n' +
-    whitespace(2 + padLen + contextColumn - 1) +
-    '^\n' +
-    (line < lines.length
-      ? lpad(padLen, nextLineNum) + ': ' + lines[line] + '\n'
-      : '')
-  );
+  const outputLines = [
+    `${source.name} (${contextLine}:${contextColumn})`,
+    line >= 2 && lpad(padLen, prevLineNum) + ': ' + lines[line - 2],
+    lpad(padLen, lineNum) + ': ' + lines[line - 1],
+    whitespace(2 + padLen + contextColumn - 1) + '^',
+    line < lines.length && lpad(padLen, nextLineNum) + ': ' + lines[line],
+  ];
+  return outputLines.filter(Boolean).join('\n');
 }
 
 function getColumnOffset(source: Source, location: SourceLocation): number {