Fixed eslint issues and enhanced Disposables test

packages/core/src/common/disposable.spec.ts

@@ -14,8 +14,11 @@
 // SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
 // *****************************************************************************
 
-import { expect } from 'chai';
+import { expect, spy, use } from 'chai';
 import { DisposableCollection, Disposable } from './disposable';
+import * as spies from 'chai-spies';
+
+use(spies);
 
 describe('Disposables', () => {
     it('Is safe to use Disposable.NULL', () => {
@@ -26,4 +29,72 @@ describe('Disposables', () => {
         expect(collectionA.disposed, 'A should be disposed after being disposed.').to.be.true;
         expect(collectionB.disposed, 'B should not be disposed because A was disposed.').to.be.false;
     });
+
+    it('Collection is auto-pruned when an element is disposed', () => {
+        const onDispose = spy(() => { });
+        const elementDispose = () => { };
+
+        const collection = new DisposableCollection();
+        collection.onDispose(onDispose);
+
+        // DisposableCollection doesn't provide direct access to its array,
+        // so we have to defeat TypeScript to properly test pruning.
+        function collectionSize(): number {
+            /* eslint-disable  @typescript-eslint/no-explicit-any */
+            return (<any>collection).disposables.length;
+        }
+        const disposable1 = Disposable.create(elementDispose);
+        collection.push(disposable1);
+        expect(collectionSize()).equal(1);
+
+        const disposable2 = Disposable.create(elementDispose);
+        collection.push(disposable2);
+        expect(collectionSize()).equal(2);
+
+        disposable1.dispose();
+        expect(collectionSize()).equal(1);
+        expect(onDispose).to.have.not.been.called();
+        expect(collection.disposed).is.false;
+
+        // Test that calling dispose on an already disposed element doesn't
+        // alter the collection state
+        disposable1.dispose();
+        expect(collectionSize()).equal(1);
+        expect(onDispose).to.have.not.been.called();
+        expect(collection.disposed).is.false;
+
+        disposable2.dispose();
+        expect(collectionSize()).equal(0);
+        expect(collection.disposed).is.true;
+        expect(onDispose).to.have.been.called.once;
+    });
+
+    it('onDispose is only called once on actual disposal of elements', () => {
+        const onDispose = spy(() => { });
+        const elementDispose = spy(() => { });
+
+        const collection = new DisposableCollection();
+        collection.onDispose(onDispose);
+
+        // if the collection is empty 'onDispose' is not called
+        collection.dispose();
+        expect(onDispose).to.not.have.been.called();
+
+        // 'onDispose' is called because we actually dispose an element
+        collection.push(Disposable.create(elementDispose));
+        collection.dispose();
+        expect(elementDispose).to.have.been.called.once;
+        expect(onDispose).to.have.been.called.once;
+
+        // if the collection is empty 'onDispose' is not called and no further element is disposed
+        collection.dispose();
+        expect(elementDispose).to.have.been.called.once;
+        expect(onDispose).to.have.been.called.once;
+
+        // 'onDispose' is not called again even if we actually dispose an element
+        collection.push(Disposable.create(elementDispose));
+        collection.dispose();
+        expect(elementDispose).to.have.been.called.twice;
+        expect(onDispose).to.have.been.called.once;
+    });
 });

packages/core/src/common/disposable.ts

@@ -56,7 +56,7 @@ Object.defineProperty(Disposable, 'NULL', {
  * - the collection is auto-pruned when an element it contains is disposed by
  * any code that has a reference to it
  * - you can register to be notified when all elements in the collection have
- * been disposed*
+ * been disposed[1]
  * - you can conveniently dispose all elements by calling dipose()
  * on the collection
  *
@@ -71,10 +71,10 @@ Object.defineProperty(Disposable, 'NULL', {
  * });
  * ```
  *
- * [*] The collection will notify only once. It will continue to function in so
+ * [1] The collection will notify only once. It will continue to function in so
  * far as accepting new Disposables and pruning them when they are disposed, but
  * such activity will never result in another notification.
-  */
+ */
 export class DisposableCollection implements Disposable {
 
     protected readonly disposables: Disposable[] = [];

packages/core/src/common/reference.ts

@@ -202,7 +202,7 @@ export class ReferenceCollection<K, V extends Disposable> extends AbstractRefere
  * Synchronous implementation of AbstractReferenceCollection that requires
  * the client to provide a value factory, used to service the acquire()
  * function.
-*/
+ */
 export class SyncReferenceCollection<K, V extends Disposable> extends AbstractReferenceCollection<K, V> {
 
     constructor(protected readonly factory: (key: K) => V) {