fix(completion): don't suggest <?php on > characer

closes #372

Author: felixfbecker

fixtures/completion/html_no_completion.php

@@ -0,0 +1 @@
+<

src/CompletionProvider.php

@@ -10,7 +10,9 @@
  Position,
  CompletionList,
  CompletionItem,
- CompletionItemKind
+ CompletionItemKind,
+ CompletionContext,
+ CompletionTriggerKind
 };
 use Microsoft\PhpParser;
 use Microsoft\PhpParser\Node;
@@ -122,9 +124,10 @@ public function __construct(DefinitionResolver $definitionResolver, ReadableInde
  *
  * @param PhpDocument $doc The opened document
  * @param Position $pos The cursor position
+ * @param CompletionContext $context The completion context
  * @return CompletionList
  */
- public function provideCompletion(PhpDocument $doc, Position $pos): CompletionList
+ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionContext $context = null): CompletionList
  {
  // This can be made much more performant if the tree follows specific invariants.
  $node = $doc->getNodeAtPosition($pos);
@@ -152,7 +155,21 @@ public function provideCompletion(PhpDocument $doc, Position $pos): CompletionLi
 
  // Inspect the type of expression under the cursor
 
- if ($node === null || $node instanceof Node\Statement\InlineHtml || $pos == new Position(0, 0)) {
+ $content = $doc->getContent();
+ $offset = $pos->toOffset($content);
+ if (
+ $node === null
+ || (
+ $node instanceof Node\Statement\InlineHtml
+ && (
+ $context === null
+ // Make sure to not suggest on the > trigger character in HTML
+ || $context->triggerKind === CompletionTriggerKind::INVOKED
+ || $context->triggerCharacter === '<'
+ )
+ )
+ || $pos == new Position(0, 0)
+ ) {
  // HTML, beginning of file
 
  // Inside HTML and at the beginning of the file, propose <?php

src/Protocol/CompletionContext.php

@@ -0,0 +1,30 @@
+<?php
+
+namespace LanguageServer\Protocol;
+
+/**
+ * Contains additional information about the context in which a completion request is triggered.
+ */
+class CompletionContext
+{
+ /**
+ * How the completion was triggered.
+ *
+ * @var int
+ */
+ public $triggerKind;
+
+ /**
+ * The trigger character (a single character) that has trigger code complete.
+ * Is null if `triggerKind !== CompletionTriggerKind::TRIGGER_CHARACTER`
+ *
+ * @var string|null
+ */
+ public $triggerCharacter;
+
+ public function __construct(int $triggerKind, string $triggerCharacter = null)
+ {
+ $this->triggerKind = $triggerKind;
+ $this->triggerCharacter = $triggerCharacter;
+ }
+}

src/Protocol/CompletionTriggerKind.php

@@ -0,0 +1,16 @@
+<?php
+
+namespace LanguageServer\Protocol;
+
+class CompletionTriggerKind
+{
+ /**
+ * Completion was triggered by invoking it manuall or using API.
+ */
+ const INVOKED = 1;
+
+ /**
+ * Completion was triggered by a trigger character.
+ */
+ const TRIGGER_CHARACTER = 2;
+}

src/Server/TextDocument.php

@@ -20,7 +20,8 @@
  SymbolLocationInformation,
  TextDocumentIdentifier,
  TextDocumentItem,
- VersionedTextDocumentIdentifier
+ VersionedTextDocumentIdentifier,
+ CompletionContext
 };
 use Microsoft\PhpParser;
 use Microsoft\PhpParser\Node;
@@ -335,13 +336,14 @@ public function hover(TextDocumentIdentifier $textDocument, Position $position):
  *
  * @param TextDocumentIdentifier The text document
  * @param Position $position The position
+ * @param CompletionContext|null $context The completion context
  * @return Promise <CompletionItem[]|CompletionList>
  */
- public function completion(TextDocumentIdentifier $textDocument, Position $position): Promise
+ public function completion(TextDocumentIdentifier $textDocument, Position $position, CompletionContext $context = null): Promise
  {
- return coroutine(function () use ($textDocument, $position) {
+ return coroutine(function () use ($textDocument, $position, $context) {
  $document = yield $this->documentLoader->getOrLoad($textDocument->uri);
- return $this->completionProvider->provideCompletion($document, $position);
+ return $this->completionProvider->provideCompletion($document, $position, $context);
  });
  }
 

tests/Index/IndexTest.php

@@ -0,0 +1,29 @@
+<?php
+
+namespace LanguageServer\Tests;
+
+use PHPUnit\Framework\TestCase;
+use LanguageServer\Index\Index;
+use LanguageServer\Definition;
+
+class IndexTest extends TestCase
+{
+ public function testGetSetMethodDefinition()
+ {
+ $index = new Index;
+ $index->setDefinition('SomeNamespace\SomeClass', new Definition);
+ $methodDefinition = new Definition;
+ $methodFqn = 'SomeNamespace\SomeClass->someMethod()';
+ $index->setDefinition($methodFqn, $methodDefinition);
+ $this->assertSame($methodDefinition, $index->getDefinition($methodFqn));
+ }
+
+ public function testGetSetClassDefinition()
+ {
+ $index = new Index;
+ $definition = new Definition;
+ $fqn = 'SomeNamespace\SomeClass';
+ $index->setDefinition($fqn, $definition);
+ $this->assertSame($definition, $index->getDefinition($fqn));
+ }
+}

tests/Server/TextDocument/CompletionTest.php

@@ -17,7 +17,9 @@
  Position,
  CompletionList,
  CompletionItem,
- CompletionItemKind
+ CompletionItemKind,
+ CompletionContext,
+ CompletionTriggerKind
 };
 use function LanguageServer\pathToUri;
 
@@ -464,6 +466,41 @@ public function testHtmlWithPrefix()
  ], true), $items);
  }
 
+ public function testHtmlPrefixShouldNotTriggerCompletion()
+ {
+ $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/html_no_completion.php');
+ $this->loader->open($completionUri, file_get_contents($completionUri));
+ $items = $this->textDocument->completion(
+ new TextDocumentIdentifier($completionUri),
+ new Position(0, 1),
+ new CompletionContext(CompletionTriggerKind::TRIGGER_CHARACTER, '>')
+ )->wait();
+ $this->assertEquals(new CompletionList([], true), $items);
+ }
+
+ public function testHtmlPrefixShouldTriggerCompletionIfManuallyInvoked()
+ {
+ $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/html_no_completion.php');
+ $this->loader->open($completionUri, file_get_contents($completionUri));
+ $items = $this->textDocument->completion(
+ new TextDocumentIdentifier($completionUri),
+ new Position(0, 1),
+ new CompletionContext(CompletionTriggerKind::INVOKED)
+ )->wait();
+ $this->assertEquals(new CompletionList([
+ new CompletionItem(
+ '<?php',
+ CompletionItemKind::KEYWORD,
+ null,
+ null,
+ null,
+ null,
+ null,
+ new TextEdit(new Range(new Position(0, 1), new Position(0, 1)), '?php')
+ )
+ ], true), $items);
+ }
+
  public function testNamespace()
  {
  $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/namespace.php');
@@ -605,7 +642,7 @@ public function testThisWithoutPrefix()
  )
  ], true), $items);
  }
- 
+
  public function testThisWithPrefix()
  {
  $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/this_with_prefix.php');