From cf276340a4249fb426a6c4fa940fb0e5fcbd455c Mon Sep 17 00:00:00 2001
From: Daniel Tschinder <daniel.tschinder@researchgate.net>
Date: Tue, 13 Feb 2018 10:37:00 +0100
Subject: [PATCH] Fix printError/locations for multiple nodes.

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.
ref: graphql/graphql-js#1131
---
 src/Error/Error.php             | 12 ++++++-
 src/Error/FormattedError.php    | 56 +++++++++++++++---------------
 tests/{ => Error}/ErrorTest.php |  2 +-
 tests/Error/PrintErrorTest.php  | 61 +++++++++++++++++++++++++++++++++
 4 files changed, 102 insertions(+), 29 deletions(-)
 rename tests/{ => Error}/ErrorTest.php (99%)
 create mode 100644 tests/Error/PrintErrorTest.php

diff --git a/src/Error/Error.php b/src/Error/Error.php
index a96a4c7..3694592 100644
--- a/src/Error/Error.php
+++ b/src/Error/Error.php
@@ -53,7 +53,10 @@ class Error extends \Exception implements \JsonSerializable, ClientAware
     public $nodes;
 
     /**
-     * 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.
      *
      * @var Source|null
      */
@@ -250,11 +253,18 @@ class Error extends \Exception implements \JsonSerializable, ClientAware
         if (null === $this->locations) {
             $positions = $this->getPositions();
             $source = $this->getSource();
+            $nodes = $this->nodes;
 
             if ($positions && $source) {
                 $this->locations = array_map(function ($pos) use ($source) {
                     return $source->getLocation($pos);
                 }, $positions);
+            } else if ($nodes) {
+                $this->locations = array_filter(array_map(function ($node) {
+                    if ($node->loc) {
+                        return $node->loc->source->getLocation($node->loc->start);
+                    }
+                }, $nodes));
             } else {
                 $this->locations = [];
             }
diff --git a/src/Error/FormattedError.php b/src/Error/FormattedError.php
index 16ed5e7..82bb0cb 100644
--- a/src/Error/FormattedError.php
+++ b/src/Error/FormattedError.php
@@ -1,6 +1,7 @@
 <?php
 namespace GraphQL\Error;
 
+use GraphQL\Language\AST\Node;
 use GraphQL\Language\Source;
 use GraphQL\Language\SourceLocation;
 use GraphQL\Type\Definition\Type;
@@ -37,18 +38,27 @@ class FormattedError
      */
     public static function printError(Error $error)
     {
-        $source = $error->getSource();
-        $locations = $error->getLocations();
-
-        $message = $error->getMessage();
-
-        foreach($locations as $location) {
-            $message .= $source
-                ? self::highlightSourceAtLocation($source, $location)
-                : " ({$location->line}:{$location->column})";
+        $printedLocations = [];
+        if ($error->nodes) {
+            /** @var Node $node */
+            foreach($error->nodes as $node) {
+                if ($node->loc) {
+                    $printedLocations[] = self::highlightSourceAtLocation(
+                        $node->loc->source,
+                        $node->loc->source->getLocation($node->loc->start)
+                    );
+                }
+            }
+        } else if ($error->getSource() && $error->getLocations()) {
+            $source = $error->getSource();
+            foreach($error->getLocations() as $location) {
+                $printedLocations[] = self::highlightSourceAtLocation($source, $location);
+            }
         }
 
-        return $message;
+        return !$printedLocations
+            ? $error->getMessage()
+            : join("\n\n", array_merge([$error->getMessage()], $printedLocations)) . "\n";
     }
 
     /**
@@ -74,23 +84,15 @@ class FormattedError
 
         $lines[0] = self::whitespace($source->locationOffset->column - 1) . $lines[0];
 
-        return (
-            "\n\n{$source->name} ($contextLine:$contextColumn)\n" .
-            ($line >= 2
-                ? (self::lpad($padLen, $prevLineNum) . ': ' . $lines[$line - 2] . "\n")
-                : ''
-            ) .
-            self::lpad($padLen, $lineNum) .
-            ': ' .
-            $lines[$line - 1] .
-            "\n" .
-            self::whitespace(2 + $padLen + $contextColumn - 1) .
-            "^\n" .
-            ($line < count($lines)
-                ? (self::lpad($padLen, $nextLineNum) . ': ' . $lines[$line] . "\n")
-                : ''
-            )
-        );
+        $outputLines = [
+            "{$source->name} ($contextLine:$contextColumn)",
+            $line >= 2 ? (self::lpad($padLen, $prevLineNum) . ': ' . $lines[$line - 2]) : null,
+            self::lpad($padLen, $lineNum) . ': ' . $lines[$line - 1],
+            self::whitespace(2 + $padLen + $contextColumn - 1) . '^',
+            $line < count($lines)? self::lpad($padLen, $nextLineNum) . ': ' . $lines[$line] : null
+        ];
+
+        return join("\n", array_filter($outputLines));
     }
 
     /**
diff --git a/tests/ErrorTest.php b/tests/Error/ErrorTest.php
similarity index 99%
rename from tests/ErrorTest.php
rename to tests/Error/ErrorTest.php
index fc52609..3843835 100644
--- a/tests/ErrorTest.php
+++ b/tests/Error/ErrorTest.php
@@ -1,5 +1,5 @@
 <?php
-namespace GraphQL\Tests;
+namespace GraphQL\Tests\Error;
 
 use GraphQL\Error\Error;
 use GraphQL\Language\Parser;
diff --git a/tests/Error/PrintErrorTest.php b/tests/Error/PrintErrorTest.php
new file mode 100644
index 0000000..aa4affc
--- /dev/null
+++ b/tests/Error/PrintErrorTest.php
@@ -0,0 +1,61 @@
+<?php
+namespace GraphQL\Tests\Error;
+
+use GraphQL\Error\Error;
+use GraphQL\Error\FormattedError;
+use GraphQL\Language\Parser;
+use GraphQL\Language\Source;
+
+class PrintErrorTest extends \PHPUnit_Framework_TestCase
+{
+    // Describe printError
+
+    /**
+     * @it prints an error with nodes from different sources
+     */
+    public function testPrintsAnErrorWithNodesFromDifferentSources()
+    {
+        $sourceA = Parser::parse(new Source('type Foo {
+  field: String
+}',
+        'SourceA'
+        ));
+
+        $fieldTypeA = $sourceA->definitions[0]->fields[0]->type;
+
+        $sourceB = Parser::parse(new Source('type Foo {
+  field: Int
+}',
+            'SourceB'
+        ));
+
+        $fieldTypeB = $sourceB->definitions[0]->fields[0]->type;
+
+
+        $error = new Error(
+            'Example error with two nodes',
+            [
+                $fieldTypeA,
+                $fieldTypeB,
+            ]
+        );
+
+        $this->assertEquals(
+            '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: }
+',
+            FormattedError::printError($error)
+        );
+    }
+}