From 1f808c59e1c53cfc099fb3eeae5135733bb417c4 Mon Sep 17 00:00:00 2001 From: Kaloyan Raev Date: Tue, 11 Oct 2016 11:15:20 +0300 Subject: [PATCH 1/2] Fixes #59: Handle correctly negative endLine in PHP Parser errors (#62) * Fixes #59: Handle correctly negative endLine in PHP Parser errors * Clearer $startLine calculation * Add missing test file * Better calculation of endLine * Remove trailing spaces --- fixtures/namespace_not_first.php | 5 ++ src/PhpDocument.php | 9 ++-- tests/Server/TextDocument/ParseErrorsTest.php | 54 +++++++++++++------ 3 files changed, 48 insertions(+), 20 deletions(-) create mode 100644 fixtures/namespace_not_first.php diff --git a/fixtures/namespace_not_first.php b/fixtures/namespace_not_first.php new file mode 100644 index 0000000..8ef98e4 --- /dev/null +++ b/fixtures/namespace_not_first.php @@ -0,0 +1,5 @@ +range = new Range( - new Position($error->getStartLine() - 1, $error->hasColumnInfo() ? $error->getStartColumn($this->content) - 1 : 0), - new Position($error->getEndLine() - 1, $error->hasColumnInfo() ? $error->getEndColumn($this->content) : 0) - ); + $startLine = max($error->getStartLine() - 1, 0); + $startColumn = $error->hasColumnInfo() ? $error->getStartColumn($this->content) - 1 : 0; + $endLine = max($error->getEndLine() - 1, $startLine); + $endColumn = $error->hasColumnInfo() ? $error->getEndColumn($this->content) : 0; + $diagnostic->range = new Range(new Position($startLine, $startColumn), new Position($endLine, $endColumn)); $diagnostic->severity = DiagnosticSeverity::ERROR; $diagnostic->source = 'php'; // Do not include "on line ..." in the error message diff --git a/tests/Server/TextDocument/ParseErrorsTest.php b/tests/Server/TextDocument/ParseErrorsTest.php index 24d155c..37ce769 100644 --- a/tests/Server/TextDocument/ParseErrorsTest.php +++ b/tests/Server/TextDocument/ParseErrorsTest.php @@ -15,18 +15,12 @@ class ParseErrorsTest extends TestCase */ private $textDocument; + private $args; + public function setUp() { $client = new LanguageClient(new MockProtocolStream()); - $project = new Project($client); - $this->textDocument = new Server\TextDocument($project, $client); - } - - public function testParseErrorsArePublishedAsDiagnostics() - { - $args = null; - $client = new LanguageClient(new MockProtocolStream()); - $client->textDocument = new class($args) extends Client\TextDocument { + $client->textDocument = new class($this->args) extends Client\TextDocument { private $args; public function __construct(&$args) { @@ -38,18 +32,22 @@ class ParseErrorsTest extends TestCase $this->args = func_get_args(); } }; - $project = new Project($client); + $this->textDocument = new Server\TextDocument($project, $client); + } - $textDocument = new Server\TextDocument($project, $client); - - // Trigger parsing of source + private function openFile($file) { $textDocumentItem = new TextDocumentItem(); $textDocumentItem->uri = 'whatever'; $textDocumentItem->languageId = 'php'; $textDocumentItem->version = 1; - $textDocumentItem->text = file_get_contents(__DIR__ . '/../../../fixtures/invalid_file.php'); - $textDocument->didOpen($textDocumentItem); + $textDocumentItem->text = file_get_contents($file); + $this->textDocument->didOpen($textDocumentItem); + } + + public function testParseErrorsArePublishedAsDiagnostics() + { + $this->openFile(__DIR__ . '/../../../fixtures/invalid_file.php'); $this->assertEquals([ 'whatever', [[ @@ -68,6 +66,30 @@ class ParseErrorsTest extends TestCase 'source' => 'php', 'message' => "Syntax error, unexpected T_CLASS, expecting T_STRING" ]] - ], json_decode(json_encode($args), true)); + ], json_decode(json_encode($this->args), true)); + } + + public function testParseErrorsWithOnlyStartLine() + { + $this->openFile(__DIR__ . '/../../../fixtures/namespace_not_first.php'); + $this->assertEquals([ + 'whatever', + [[ + 'range' => [ + 'start' => [ + 'line' => 4, + 'character' => 0 + ], + 'end' => [ + 'line' => 4, + 'character' => 0 + ] + ], + 'severity' => DiagnosticSeverity::ERROR, + 'code' => null, + 'source' => 'php', + 'message' => "Namespace declaration statement has to be the very first statement in the script" + ]] + ], json_decode(json_encode($this->args), true)); } } From d41cde2039de8e34b041b2f64d4a65644ebf3c75 Mon Sep 17 00:00:00 2001 From: Kaloyan Raev Date: Tue, 11 Oct 2016 11:26:46 +0300 Subject: [PATCH 2/2] Return empty array instead of null for empty definitions result (#64) --- src/Server/TextDocument.php | 6 +++--- tests/Server/TextDocument/DefinitionTest.php | 12 ++++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/Server/TextDocument.php b/src/Server/TextDocument.php index 39a9699..f98ec57 100644 --- a/src/Server/TextDocument.php +++ b/src/Server/TextDocument.php @@ -96,18 +96,18 @@ class TextDocument * * @param TextDocumentIdentifier $textDocument The text document * @param Position $position The position inside the text document - * @return Location|Location[]|null + * @return Location|Location[] */ public function definition(TextDocumentIdentifier $textDocument, Position $position) { $document = $this->project->getDocument($textDocument->uri); $node = $document->getNodeAtPosition($position); if ($node === null) { - return null; + return []; } $def = $document->getDefinitionByNode($node); if ($def === null) { - return null; + return []; } return Location::fromNode($def); } diff --git a/tests/Server/TextDocument/DefinitionTest.php b/tests/Server/TextDocument/DefinitionTest.php index 948fb35..455a2b3 100644 --- a/tests/Server/TextDocument/DefinitionTest.php +++ b/tests/Server/TextDocument/DefinitionTest.php @@ -25,6 +25,18 @@ class DefinitionTest extends TestCase $project->getDocument('use')->updateContent(file_get_contents(__DIR__ . '/../../../fixtures/use.php')); } + public function testDefinitionFileBeginning() { + // |textDocument->definition(new TextDocumentIdentifier('references'), new Position(0, 0)); + $this->assertEquals([], json_decode(json_encode($result), true)); + } + + public function testDefinitionEmptyResult() { + // namespace keyword + $result = $this->textDocument->definition(new TextDocumentIdentifier('references'), new Position(2, 4)); + $this->assertEquals([], json_decode(json_encode($result), true)); + } + public function testDefinitionForClassLike() { // $obj = new TestClass();