file-search: fix possible infinite recursion

The commit fixes the possible infinite recursion when comparing two
identical quick-open-items, namely with equal `label` and `uri`.

The code to compare items compares them first by:
- `label`.
- `uri` if the labels are equal.

The issue is that it attempts to compare `uri` recursively if it is
equal.

Signed-off-by: vince-fugnitto <[email protected]>

examples/api-tests/src/file-search.spec.js

@@ -0,0 +1,60 @@
+/********************************************************************************
+ * Copyright (C) 2021 Ericsson and others.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the Eclipse
+ * Public License v. 2.0 are satisfied: GNU General Public License, version 2
+ * with the GNU Classpath Exception which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ ********************************************************************************/
+
+// @ts-check
+describe('file-search', function () {
+
+    const { assert } = chai;
+
+    const Uri = require('@theia/core/lib/common/uri');
+    const { QuickOpenItem } = require('@theia/core/lib/browser');
+    const { QuickFileOpenService } = require('@theia/file-search/lib/browser/quick-file-open');
+
+    /** @type {import('inversify').Container} */
+    const container = window['theia'].container;
+    const quickFileOpenService = container.get(QuickFileOpenService);
+
+    describe('quick-file-open', () => {
+
+        describe('#compareItems', () => {
+
+            it('should compare two quick-open-items by `label`', () => {
+                const a = new QuickOpenItem({ label: 'a', uri: new Uri.default('a') });
+                const b = new QuickOpenItem({ label: 'a', uri: new Uri.default('b') });
+
+                assert.equal(quickFileOpenService['compareItems'](a, b), 1, 'a should be before b');
+                assert.equal(quickFileOpenService['compareItems'](b, a), -1, 'a should be before b');
+                assert.equal(quickFileOpenService['compareItems'](a, a), 0, 'items should be equal');
+
+                assert.equal(quickFileOpenService['compareItems'](a, b, 'getLabel'), 1, 'a should be before b');
+                assert.equal(quickFileOpenService['compareItems'](b, a, 'getLabel'), -1, 'a should be before b');
+                assert.equal(quickFileOpenService['compareItems'](a, a, 'getLabel'), 0, 'items should be equal');
+            });
+
+            it('should compare two quick-open-items by `uri`', () => {
+                const a = new QuickOpenItem({ label: 'a', uri: new Uri.default('a') });
+                const b = new QuickOpenItem({ label: 'a', uri: new Uri.default('b') });
+
+                assert.equal(quickFileOpenService['compareItems'](a, b, 'getUri'), 1, 'a should be before b');
+                assert.equal(quickFileOpenService['compareItems'](b, a, 'getUri'), -1, 'a should be before b');
+                assert.equal(quickFileOpenService['compareItems'](a, a, 'getUri'), 0, 'items should be equal');
+            });
+
+        });
+
+    });
+
+});

packages/file-search/src/browser/quick-file-open.ts

@@ -345,9 +345,11 @@ export class QuickFileOpenService implements QuickOpenModel, QuickOpenHandler {
                 // Fallback to the alphabetical order.
                 const comparison = itemB.localeCompare(itemA);
 
-                // If the alphabetical comparison is equal, call `compareItems` recursively using the `URI` member instead.
+                // Compare results by `uri` if necessary.
                 if (comparison === 0) {
-                    return this.compareItems(a, b, 'getUri');
+                    return member === 'getUri'
+                        ? 0 // Avoid infinite recursion if we have already compared by `uri`.
+                        : this.compareItems(a, b, 'getUri');
                 }
 
                 return itemB.localeCompare(itemA);