Add LSIF support

[chrismwendt]

Resolves https://github.com/sourcegraph/sourcegraph/issues/5008

This adds LSIF support to the TypeScript extension. Here's the behavior:

• When codeIntel.lsif is true: use LSIF but fall back to basic-code-intel when LSIF data doesn't exist
• Otherwise when typescript.serverUrl is not set: only use basic-code-intel
• Otherwise typescript.serverUrl must be set, so use the language server

Due to the amount of time it would take to implement and considering that we're migrating away from language servers (see the roadmap), this isn't quite the same as the other language extensions, which fall back in this order: 1. LSIF 2. language server 3. basic-code-intel

[felixfbecker]

I think this should be implemented properly. Here's why I think so:

• We don't have any urgency on this. Nothing is blocked on this.
• We implemented it for Go. Why not for TypeScript? This creates inconsistency in our product.
• If we don't do it that way, OpenTracing is not supported.
• With this current approach, in LSIF mode, there is a TypeScript codeintel toggle button, that throws when clicking it.
• I don't see how it is significantly harder to implement than the current approach. We need to check if one source returns a result, if not use the server if defined, else use basic. What is difficult about that logic? After all, we did implement it for Go, and the effort was not too big there. It would probably have been even easier if basic code intel had been integrated properly, but this is a prime opportunity to fix that and #141, which has been open since March, in one go (instead of piling up even more debt on top of it).
  Is there any reason why it's more difficult than this? Am I missing something?

+    const isLSIFAvailable = mkIsLSIFAvailable()
+    const lsif = initLSIF()
+    const basicCodeIntel = initBasicCodeIntel()
+
     ctx.subscriptions.add(registerFeedbackButton({ languageID: 'typescript', sourcegraph, isPrecise: true }))
 
     const tracer: Tracer = config.value['lightstep.token']
@@ -545,6 +550,14 @@ export async function activate(ctx: sourcegraph.ExtensionContext): Promise<void>
                 if (canGenerateTraceUrl(span)) {
                     logger.log('Hover trace', span.generateTraceURL())
                 }
+
+                if (await isLSIFAvailable(textDocument, position)) {
+                    const lsifResult = await lsif.hover(textDocument, position)
+                    yield lsifResult && lsifResult.value
+                } else if (!config.value['typescript.serverUrl']) {
+                    yield await basicCodeIntel.hover(textDocument, position)
+                } else {
                     const textDocumentUri = new URL(textDocument.uri)
                     const serverRootUri = resolveServerRootUri(textDocumentUri, serverSgEndpoint)
                     const serverTextDocumentUri = toServerTextDocumentUri(textDocumentUri, serverSgEndpoint)
@@ -565,6 +578,7 @@ export async function activate(ctx: sourcegraph.ExtensionContext): Promise<void>
                         yield convertHover(hoverResult)
                         await delayPromise(HOVER_DEF_POLL_INTERVAL)
                     }
+                }
             })

[chrismwendt]

@felixfbecker Ah, thanks for showing that approach, that works nicely ✨ Ready for another review

[chrismwendt]

Thanks for approving with comments 🙇 If there's anything unsatisfactory remaining, feel free to ping me about it.

[felixfbecker]

(not important, maybe keep in mind for future code) Since this is only a factory function now, I'd recommend the create prefix instead of init. That's the conventional prefix used e.g. in the Node stdlib. With init, I personally wouldn't be sure if it might do side effects like register providers in the Sourcegraph API, while create communicates it's only creating an object but not causes side effects (assuming that is the case here)