mirror of
https://github.com/retailcrm/graphql-php.git
synced 2024-11-25 14:26:08 +03:00
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
This commit is contained in:
parent
06c6c4bd97
commit
cf276340a4
@ -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 = [];
|
||||
}
|
||||
|
@ -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));
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -1,5 +1,5 @@
|
||||
<?php
|
||||
namespace GraphQL\Tests;
|
||||
namespace GraphQL\Tests\Error;
|
||||
|
||||
use GraphQL\Error\Error;
|
||||
use GraphQL\Language\Parser;
|
61
tests/Error/PrintErrorTest.php
Normal file
61
tests/Error/PrintErrorTest.php
Normal file
@ -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)
|
||||
);
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue
Block a user