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

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/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');