fix(decorations): no providers yields null, skip errors, add tests

Author: sqs

src/environment/providers/decoration.test.ts

@@ -0,0 +1,143 @@
+import * as assert from 'assert'
+import { of, throwError } from 'rxjs'
+import { TestScheduler } from 'rxjs/testing'
+import { Position } from 'vscode-languageserver-types'
+import { TextDocumentDecoration, TextDocumentDecorationParams } from '../../protocol'
+import { getDecorations, ProvideTextDocumentDecorationSignature } from './decoration'
+import { FIXTURE as COMMON_FIXTURE } from './textDocument.test'
+
+// const DELAY = 100 // msec
+
+const FIXTURE = {
+    ...COMMON_FIXTURE,
+    TextDocumentDecorationParams: { textDocument: { uri: 'file:///f' } } as TextDocumentDecorationParams,
+}
+
+const FIXTURE_RESULT: TextDocumentDecoration[] | null = [
+    {
+        range: { start: Position.create(1, 2), end: Position.create(3, 4) },
+        backgroundColor: 'red',
+    },
+]
+
+const scheduler = () => new TestScheduler((a, b) => assert.deepStrictEqual(a, b))
+
+describe('getDecorations', () => {
+    describe('0 providers', () => {
+        it('returns null', () =>
+            scheduler().run(({ cold, expectObservable }) =>
+                expectObservable(
+                    getDecorations(
+                        cold<ProvideTextDocumentDecorationSignature[]>('-a-|', { a: [] }),
+                        FIXTURE.TextDocumentDecorationParams
+                    )
+                ).toBe('-a-|', {
+                    a: null,
+                })
+            ))
+    })
+
+    describe('1 provider', () => {
+        it('returns null result from provider', () =>
+            scheduler().run(({ cold, expectObservable }) =>
+                expectObservable(
+                    getDecorations(
+                        cold<ProvideTextDocumentDecorationSignature[]>('-a-|', { a: [() => of(null)] }),
+                        FIXTURE.TextDocumentDecorationParams
+                    )
+                ).toBe('-a-|', {
+                    a: null,
+                })
+            ))
+
+        it('returns result from provider', () =>
+            scheduler().run(({ cold, expectObservable }) =>
+                expectObservable(
+                    getDecorations(
+                        cold<ProvideTextDocumentDecorationSignature[]>('-a-|', {
+                            a: [() => of(FIXTURE_RESULT)],
+                        }),
+                        FIXTURE.TextDocumentDecorationParams
+                    )
+                ).toBe('-a-|', {
+                    a: FIXTURE_RESULT,
+                })
+            ))
+    })
+
+    describe('2 providers', () => {
+        it('returns null result if both providers return null', () =>
+            scheduler().run(({ cold, expectObservable }) =>
+                expectObservable(
+                    getDecorations(
+                        cold<ProvideTextDocumentDecorationSignature[]>('-a-|', {
+                            a: [() => of(null), () => of(null)],
+                        }),
+                        FIXTURE.TextDocumentDecorationParams
+                    )
+                ).toBe('-a-|', {
+                    a: null,
+                })
+            ))
+
+        it('omits null result from 1 provider', () =>
+            scheduler().run(({ cold, expectObservable }) =>
+                expectObservable(
+                    getDecorations(
+                        cold<ProvideTextDocumentDecorationSignature[]>('-a-|', {
+                            a: [() => of(FIXTURE_RESULT), () => of(null)],
+                        }),
+                        FIXTURE.TextDocumentDecorationParams
+                    )
+                ).toBe('-a-|', {
+                    a: FIXTURE_RESULT,
+                })
+            ))
+
+        it('skips errors from providers', () =>
+            scheduler().run(({ cold, expectObservable }) =>
+                expectObservable(
+                    getDecorations(
+                        cold<ProvideTextDocumentDecorationSignature[]>('-a-|', {
+                            a: [() => of(FIXTURE_RESULT), () => throwError('error')],
+                        }),
+                        FIXTURE.TextDocumentDecorationParams
+                    )
+                ).toBe('-a-|', {
+                    a: FIXTURE_RESULT,
+                })
+            ))
+
+        it('merges results from providers', () =>
+            scheduler().run(({ cold, expectObservable }) =>
+                expectObservable(
+                    getDecorations(
+                        cold<ProvideTextDocumentDecorationSignature[]>('-a-|', {
+                            a: [() => of(FIXTURE_RESULT), () => of(FIXTURE_RESULT)],
+                        }),
+                        FIXTURE.TextDocumentDecorationParams
+                    )
+                ).toBe('-a-|', {
+                    a: [...FIXTURE_RESULT!, ...FIXTURE_RESULT!],
+                })
+            ))
+    })
+
+    describe('multiple emissions', () => {
+        it('returns stream of results', () =>
+            scheduler().run(({ cold, expectObservable }) =>
+                expectObservable(
+                    getDecorations(
+                        cold<ProvideTextDocumentDecorationSignature[]>('-a-b-|', {
+                            a: [() => of(FIXTURE_RESULT)],
+                            b: [() => of(null)],
+                        }),
+                        FIXTURE.TextDocumentDecorationParams
+                    )
+                ).toBe('-a-b-|', {
+                    a: FIXTURE_RESULT,
+                    b: null,
+                })
+            ))
+    })
+})

src/environment/providers/decoration.ts

@@ -15,22 +15,47 @@ export class TextDocumentDecorationProviderRegistry extends TextDocumentFeatureP
     ProvideTextDocumentDecorationSignature
 > {
     public getDecorations(params: TextDocumentDecorationParams): Observable<TextDocumentDecoration[] | null> {
-        return this.providers
-            .pipe(
-                switchMap(providers =>
-                    combineLatest(
-                        providers.map(provider =>
-                            provider(params).pipe(
-                                map(results => (results === null ? [] : compact(results))),
-                                catchError(error => {
-                                    console.error(error)
-                                    return [[]]
-                                })
-                            )
+        return getDecorations(this.providers, params)
+    }
+}
+
+/**
+ * Returns an observable that emits all decorations whenever any of the last-emitted set of providers emits
+ * decorations.
+ *
+ * Most callers should use TextDocumentDecorationProviderRegistry, which sources decorations from the set of
+ * registered providers.
+ */
+export function getDecorations(
+    providers: Observable<ProvideTextDocumentDecorationSignature[]>,
+    params: TextDocumentDecorationParams
+): Observable<TextDocumentDecoration[] | null> {
+    return providers
+        .pipe(
+            switchMap(providers => {
+                if (providers.length === 0) {
+                    return [null]
+                }
+                return combineLatest(
+                    providers.map(provider =>
+                        provider(params).pipe(
+                            map(results => (results === null ? [] : compact(results))),
+                            catchError(error => {
+                                console.error(error)
+                                return [null]
+                            })
                         )
                     )
                 )
-            )
-            .pipe(map(results => flatten(results)))
-    }
+            })
+        )
+        .pipe(
+            map(results => {
+                if (results === null) {
+                    return null
+                }
+                const merged = flatten(compact(results))
+                return merged.length === 0 ? null : merged
+            })
+        )
 }