Commit Graph

33 Commits

Author SHA1 Message Date
Sam McCall dc8f3cf8b0 [clangd] Refactor JSON-over-stdin/stdout code into Transport abstraction. (re-land r344620)
Summary:
This paves the way for alternative transports (mac XPC, maybe messagepack?),
and also generally improves layering: testing ClangdLSPServer becomes less of
a pipe dream, we split up the JSONOutput monolith, etc.

This isn't a final state, much of what remains in JSONRPCDispatcher can go away,
handlers can call reply() on the transport directly, JSONOutput can be renamed
to StreamLogger and removed, etc. But this patch is sprawling already.

The main observable change (see tests) is that hitting EOF on input is now an
error: the client should send the 'exit' notification.
This is defensible: the protocol doesn't spell this case out. Reproducing the
current behavior for all combinations of shutdown/exit/EOF clutters interfaces.
We can iterate on this if desired.

Reviewers: jkorous, ioeric, hokein

Subscribers: mgorny, ilya-biryukov, MaskRay, arphaman, kadircet, cfe-commits

Differential Revision: https://reviews.llvm.org/D53286

llvm-svn: 344672
2018-10-17 07:32:05 +00:00
Krasimir Georgiev ae4e75fd6e Revert "[clangd] Refactor JSON-over-stdin/stdout code into Transport abstraction."
This reverts commit r344620.
Breaks upstream bots.

llvm-svn: 344637
2018-10-16 18:44:41 +00:00
Sam McCall 818ec5e965 [clangd] Refactor JSON-over-stdin/stdout code into Transport abstraction.
Summary:
This paves the way for alternative transports (mac XPC, maybe messagepack?),
and also generally improves layering: testing ClangdLSPServer becomes less of
a pipe dream, we split up the JSONOutput monolith, etc.

This isn't a final state, much of what remains in JSONRPCDispatcher can go away,
handlers can call reply() on the transport directly, JSONOutput can be renamed
to StreamLogger and removed, etc. But this patch is sprawling already.

The main observable change (see tests) is that hitting EOF on input is now an
error: the client should send the 'exit' notification.
This is defensible: the protocol doesn't spell this case out. Reproducing the
current behavior for all combinations of shutdown/exit/EOF clutters interfaces.
We can iterate on this if desired.

Reviewers: jkorous, ioeric, hokein

Subscribers: mgorny, ilya-biryukov, MaskRay, arphaman, kadircet, cfe-commits

Differential Revision: https://reviews.llvm.org/D53286

llvm-svn: 344620
2018-10-16 16:48:06 +00:00
Kadir Cetinkaya d4218dcd79 Fix lint tests for D50449
Reviewers: ilya-biryukov, hokein

Reviewed By: hokein

Subscribers: hokein, ioeric, jkorous, arphaman, cfe-commits

Differential Revision: https://reviews.llvm.org/D50635

llvm-svn: 339572
2018-08-13 14:32:19 +00:00
Eric Liu 8f3678dbab [clangd] UI for completion items that would trigger include insertion.
Summary:
For completion items that would trigger include insertions (i.e. index symbols
that are not #included yet), add a visual indicator "+" before the completion
label. The inserted headers will appear in the completion detail.

Open to suggestions for better visual indicators; "+" was picked because it
seems cleaner than a few other candidates I've tried (*, #, @ ...).

The displayed header would be like a/b/c.h (without quote) or <vector> for system
headers. I didn't add quotation or "#include" because they can take up limited
space and do not provide additional information after users know what the
headers are. I think a header alone should be obvious for users to infer that
this is an include header..

To align indentation, also prepend ' ' to labels of candidates that would not
trigger include insertions (only for completions where index results are
possible).

Vim:
{F6357587}

vscode:
{F6357589}
{F6357591}

Reviewers: sammccall, ilya-biryukov, hokein

Reviewed By: sammccall

Subscribers: MaskRay, jkorous, cfe-commits

Differential Revision: https://reviews.llvm.org/D48163

llvm-svn: 334828
2018-06-15 13:34:18 +00:00
Sam McCall 4caa85129f [clangd] Code completion: drop explicit injected names/operators, ignore Sema priority
Summary:
Now we have most of Sema's code completion signals incorporated in Quality,
which will allow us to give consistent ranking to sema/index results.

Therefore we can/should stop using Sema priority as an explicit signal.
This fixes some issues like namespaces always having a terrible score.

The most important missing signals are:
 - Really dumb/rarely useful completions like:
    SomeStruct().^SomeStruct
    SomeStruct().^operator=
    SomeStruct().~SomeStruct()
   We already filter out destructors, this patch adds injected names and
   operators to that list.
 - type matching the expression context.
   Ilya has a plan to add this in a way that's compatible with indexes
   (design doc should be shared real soon now!)

Reviewers: ioeric

Subscribers: ilya-biryukov, MaskRay, jkorous, cfe-commits

Differential Revision: https://reviews.llvm.org/D47871

llvm-svn: 334192
2018-06-07 12:49:17 +00:00
Sam McCall 5ed599eafa [clangd] Support simpler JSON-RPC stream parsing for lit tests.
Summary:
Instead of content-length, we delimit messages with ---.
This also removes the need for (most) dos-formatted test files.

Reviewers: ioeric

Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits

Differential Revision: https://reviews.llvm.org/D42919

llvm-svn: 324333
2018-02-06 10:47:30 +00:00
Eric Liu 5740ff5bed [clangd] Add a test URI scheme for lit tests to unbreak platform-specific URI failures.
Summary:
This should also fix the current windows buildbot breakage
(http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/9838/steps/ninja%20check%201/logs/stdio)

Reviewers: sammccall

Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits

Differential Revision: https://reviews.llvm.org/D42735

llvm-svn: 323885
2018-01-31 16:26:27 +00:00
Sam McCall 800d4371f3 [clangd] Add unit tests for signature help. SigHelp/CodeComplete lit tests are smoke only.
llvm-svn: 321065
2017-12-19 10:29:27 +00:00
Sam McCall b8d548a3de [clangd] Make completion scores use 0-1 floats internally.
Summary:
This scale is much easier to mix with other signals, such as fuzzy match strength.
Mostly NFC, but it does reorder some low-priority items that get folded together at a score of 0 (see completion-qualifiers.test).
Removed the exact sortText from the testcases, because it's the ranking that we want to test.

Reviewers: hokein

Subscribers: ilya-biryukov, cfe-commits

Differential Revision: https://reviews.llvm.org/D40089

llvm-svn: 318927
2017-11-23 17:09:04 +00:00
Ilya Biryukov e9eb7f0cb8 [clangd] Use in-memory preambles in clangd.
Reviewers: klimek, bkramer, sammccall

Reviewed By: sammccall

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D39843

llvm-svn: 318412
2017-11-16 16:25:18 +00:00
Sam McCall afbe849d77 [clangd] Loosen more brittle tests
llvm-svn: 318339
2017-11-15 21:50:53 +00:00
Sam McCall a40371bcb6 [clangd] Support returning a limited number of completion results.
Summary:
All results are scored, we only process CodeCompletionStrings for the winners.
We now return CompletionList rather than CompletionItem[] (both are valid).
sortText is now based on CodeCompletionResult::orderedName (mostly the same).

This is the first clangd-only completion option, so plumbing changed.
It requires a small clangd patch (exposing CodeCompletionResult::orderedName).

(This can't usefully be enabled yet: we don't support server-side filtering)

Reviewers: ilya-biryukov

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D39852

llvm-svn: 318287
2017-11-15 09:16:29 +00:00
Sam McCall 28ef7d662f [clangd] loosen tests for flag-dependence revealed by r317670
llvm-svn: 317673
2017-11-08 09:15:01 +00:00
Sam McCall c78ccbdefb [clangd] Sort completion results.
Summary:
This is (probably) not required by LSP, but at least one buggy client wants it.
It also simplifies some tests - changed a few completion tests to use -pretty.

Reviewers: hokein, malaperle

Subscribers: ilya-biryukov, cfe-commits

Differential Revision: https://reviews.llvm.org/D39738

llvm-svn: 317670
2017-11-08 07:44:12 +00:00
Sam McCall dd0566bb2c Adds a json::Expr type to represent intermediate JSON expressions.
Summary:
This form can be created with a nice clang-format-friendly literal syntax,
and gets escaping right. It knows how to call unparse() on our Protocol types.
All the places where we pass around JSON internally now use this type.

Object properties are sorted (stored as std::map) and so serialization is
canonicalized, with optional prettyprinting (triggered by a -pretty flag).
This makes the lit tests much nicer to read and somewhat nicer to debug.
(Unfortunately the completion tests use CHECK-DAG, which only has
line-granularity, so pretty-printing is disabled there. In future we
could make completion ordering deterministic, or switch to unittests).

Compared to the current approach, it has some efficiencies like avoiding copies
of string literals used as object keys, but is probably slower overall.
I think the code/test quality benefits are worth it.

This patch doesn't attempt to do anything about JSON *parsing*.
It takes direction from the proposal in this doc[1], but is limited in scope
and visibility, for now.
I am of half a mind just to use Expr as the target of a parser, and maybe do a
little string deduplication, but not bother with clever memory allocation.
That would be simple, and fast enough for clangd...
[1] https://docs.google.com/document/d/1OEF9IauWwNuSigZzvvbjc1cVS1uGHRyGTXaoy3DjqM4/edit

+cc d0k so he can tell me not to use std::map.

Reviewers: ioeric, malaperle

Subscribers: bkramer, ilya-biryukov, mgorny, klimek

Differential Revision: https://reviews.llvm.org/D39435

llvm-svn: 317486
2017-11-06 15:40:30 +00:00
Ilya Biryukov 0d9b8a3ee8 [clangd] Handle exit notification (proper shutdown)
Summary:
This changes the onShutdown handler to do essentially nothing (for now), and
instead exits the runloop when we receive the exit notification from the client.

Some clients may wait on the reply from the shutdown request before sending an
exit notification. If we exit the runloop already in the shutdown request, a
client might block forever.

This also gives us the opportunity to do any global cleanups and/or
serializations of PCH preambles to disk, but I've left that out for now.

See the LSP protocol documentation for details.

Reviewers: malaperle, krasimir, bkramer, sammccall, ilya-biryukov

Reviewed By: malaperle, sammccall, ilya-biryukov

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D38939

llvm-svn: 316564
2017-10-25 08:45:41 +00:00
Ilya Biryukov fee6e4aba8 [clangd] Updated gold for completion tests after cfe changes.
llvm-svn: 314020
2017-09-22 19:07:45 +00:00
Ilya Biryukov 77f61badf4 [clangd] Put inacessible items to the end of completion list.
Reviewers: bkramer, krasimir

Reviewed By: krasimir

Subscribers: klimek, cfe-commits

Differential Revision: https://reviews.llvm.org/D38077

llvm-svn: 313759
2017-09-20 15:09:14 +00:00
Ilya Biryukov b33c15741b [clangd] Add support for snippet completions
Enhances CompletionItemsCollector in such a way that snippet
completions can be presented to the client. Enable snippet completion
items by specifying -enable-snippets while invoking the clangd
executable.

See: https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#completion-request
See: https://github.com/Microsoft/vscode/blob/master/src/vs/editor/contrib/snippet/browser/snippet.md

Patch by Raoul Wols.

llvm-svn: 313029
2017-09-12 13:57:14 +00:00
Ilya Biryukov de46274d7b [clangd] Don't reverse priorities of completion items.
Summary: Current algorithm incorrectly provides completion results in a reverse order.

Reviewers: krasimir, bkramer

Reviewed By: krasimir

Subscribers: cfe-commits, klimek

Differential Revision: https://reviews.llvm.org/D35950

llvm-svn: 309295
2017-07-27 17:43:07 +00:00
Krasimir Georgiev a1de3c91e5 [clangd] Add priority to completion item sort text
Summary:
This patch adds the priority of a completion item to the sort text of the
returned LSP result.

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: cfe-commits

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D34137

llvm-svn: 305454
2017-06-15 09:11:57 +00:00
Krasimir Georgiev e1cac9171c [clangd] extend completion test
llvm-svn: 304983
2017-06-08 16:02:27 +00:00
Krasimir Georgiev 5579eb0a7a [clangd] Separate authority less uris from completion tests
llvm-svn: 304981
2017-06-08 15:21:55 +00:00
Krasimir Georgiev e6035a51f7 [clangd] Add parameter and return type information to completion results
Summary:
This patch adds information about the parameters and return types of completion
candidates.
Previously, for the following code:
```
struct S {
  int func(int a, double b) const;
};
```
the completer would only return the label of the candidate `func`.
Now it will also return the return type `int` and will format the label for the
candidate as `func(int a, double b) const`.

Reviewers: bkramer, ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: cfe-commits

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D34033

llvm-svn: 304980
2017-06-08 15:11:51 +00:00
Benjamin Kramer d3326a04e0 [Clangd] Failed to decode params using 1.x-compatible request message
textDocument/completion sends a TextDocumentPositionParams message in the 2.x
and 3.x. But in 1.x it was instead a TextDocumentPosition with inlined
parameters. This means that the "uri" field is at the top level and not in
textDocument. Because of this, some clients that maintain compability with 1.x
have both uri and textDocument.uri. Clangd, however, early returns in the
presence of anything but 'textDocument' or 'position' which prevents a client
compatible with both 3.x and 1.x to work correctly. If Clangd was a bit more
permissive (no early return), clients implementing all the versions of the
protocol would work.

Patch by Marc-Andre Laperle!

Differential Revision: https://reviews.llvm.org/D32238

llvm-svn: 300991
2017-04-21 15:51:23 +00:00
Benjamin Kramer 8c3ba63d07 [Clangd] Support Authority-less URIs
Clangd strips URIs by removing the file:// part but some clients can send file:
which is also valid according to RFC 3896. For example, if a client sends
file:///home/user, it gets converted to /home/user but if a client sends
file:/home/user, it is left untouched and problems arise.

Patch by Marc-Andre Laperle!

Differential Revision: https://reviews.llvm.org/D32234

llvm-svn: 300990
2017-04-21 15:51:18 +00:00
Krasimir Georgiev 4714f7cba9 [clangd] Implement item kind for completion results
Summary: The patch implements the conversion method from CXCursorKind to clangd::CompletionItemKind.
Contributed by stanionascu!

Reviewers: cfe-commits, bkramer, krasimir

Reviewed By: krasimir

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D31853

llvm-svn: 299935
2017-04-11 13:27:15 +00:00
Reid Kleckner 08126372e3 Revert "XFAIL clangd tests on Windows"
This reverts r299849, apparently these tests only fail on my machine.

llvm-svn: 299850
2017-04-10 16:55:48 +00:00
Reid Kleckner 7e1548d6db XFAIL clangd tests on Windows
They all assert. Filed as PR32596.

llvm-svn: 299849
2017-04-10 16:50:55 +00:00
Krasimir Georgiev beaff10d39 [clangd] Fix completion test to not depend on the standard library
llvm-svn: 299440
2017-04-04 15:08:42 +00:00
Krasimir Georgiev d7f1512897 [clangd] Remove private vector fields from completion test.
llvm-svn: 299426
2017-04-04 10:42:22 +00:00
Krasimir Georgiev 6d2131a04c [clangd] Add code completion support
Summary: Adds code completion support to clangd.

Reviewers: bkramer, malaperle-ericsson

Reviewed By: bkramer, malaperle-ericsson

Subscribers: stanionascu, malaperle-ericsson, cfe-commits

Differential Revision: https://reviews.llvm.org/D31328

llvm-svn: 299421
2017-04-04 09:46:39 +00:00