[core] Fix #6377: Handle URI#getAllLocations for paths with no parent

- Adds `Path#hasDir` - Adds `location.path.hasDir` check in while loop of `URI#allLocations` - Adds some tests Fixes #6377 Signed-off-by: Cornelius A. Ludmann <cornelius.ludmann@typefox.io>
eclipse-theia · Oct 14, 2019 · b59987e · b59987e
1 parent b98c5fb
commit b59987e
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 5 deletions.
diff --git a/packages/core/src/common/path.spec.ts b/packages/core/src/common/path.spec.ts
@@ -25,6 +25,7 @@ describe('Path', () => {
         assert.deepEqual(path.isAbsolute, true);
         assert.deepEqual(path.root!.toString(), '/');
         assert.deepEqual(path.dir.toString(), '/foo/bar');
+        assert.deepEqual(path.hasDir, true);
         assert.deepEqual(path.base, 'file.txt');
         assert.deepEqual(path.name, 'file');
         assert.deepEqual(path.ext, '.txt');
@@ -36,6 +37,7 @@ describe('Path', () => {
         assert.deepEqual(path.isAbsolute, false);
         assert.deepEqual(path.root, undefined);
         assert.deepEqual(path.dir.toString(), 'foo/bar');
+        assert.deepEqual(path.hasDir, true);
         assert.deepEqual(path.base, 'file.txt');
         assert.deepEqual(path.name, 'file');
         assert.deepEqual(path.ext, '.txt');
@@ -47,6 +49,7 @@ describe('Path', () => {
         assert.deepEqual(path.isAbsolute, true);
         assert.deepEqual(path.root!.toString(), '/');
         assert.deepEqual(path.dir.toString(), '/');
+        assert.deepEqual(path.hasDir, true);
         assert.deepEqual(path.base, 'foo');
         assert.deepEqual(path.name, 'foo');
         assert.deepEqual(path.ext, '');
@@ -58,6 +61,7 @@ describe('Path', () => {
         assert.deepEqual(path.isAbsolute, false);
         assert.deepEqual(path.root, undefined);
         assert.deepEqual(path.dir.toString(), 'foo');
+        assert.deepEqual(path.hasDir, false);
         assert.deepEqual(path.base, 'foo');
         assert.deepEqual(path.name, 'foo');
         assert.deepEqual(path.ext, '');
@@ -69,6 +73,7 @@ describe('Path', () => {
         assert.deepEqual(path.isAbsolute, true);
         assert.deepEqual(path.root!.toString(), '/');
         assert.deepEqual(path.dir.toString(), '/');
+        assert.deepEqual(path.hasDir, false);
         assert.deepEqual(path.base, '');
         assert.deepEqual(path.name, '');
         assert.deepEqual(path.ext, '');
@@ -80,6 +85,7 @@ describe('Path', () => {
         assert.deepEqual(path.isAbsolute, true);
         assert.deepEqual(path.root!.toString(), '/c:');
         assert.deepEqual(path.dir.toString(), '/c:/foo/bar');
+        assert.deepEqual(path.hasDir, true);
         assert.deepEqual(path.base, 'file.txt');
         assert.deepEqual(path.name, 'file');
         assert.deepEqual(path.ext, '.txt');
@@ -91,6 +97,7 @@ describe('Path', () => {
         assert.deepEqual(path.isAbsolute, true);
         assert.deepEqual(path.root!.toString(), '/c:');
         assert.deepEqual(path.dir.toString(), '/c:');
+        assert.deepEqual(path.hasDir, true);
         assert.deepEqual(path.base, 'foo');
         assert.deepEqual(path.name, 'foo');
         assert.deepEqual(path.ext, '');
@@ -102,6 +109,7 @@ describe('Path', () => {
         assert.deepEqual(path.isAbsolute, true);
         assert.deepEqual(path.root!.toString(), '/c:');
         assert.deepEqual(path.dir.toString(), '/c:');
+        assert.deepEqual(path.hasDir, true);
         assert.deepEqual(path.base, '');
         assert.deepEqual(path.name, '');
         assert.deepEqual(path.ext, '');
@@ -113,6 +121,7 @@ describe('Path', () => {
         assert.deepEqual(path.isAbsolute, true);
         assert.deepEqual(path.root!.toString(), '/c:');
         assert.deepEqual(path.dir.toString(), '/c:');
+        assert.deepEqual(path.hasDir, false);
         assert.deepEqual(path.base, 'c:');
         assert.deepEqual(path.name, 'c:');
         assert.deepEqual(path.ext, '');

diff --git a/packages/core/src/common/path.ts b/packages/core/src/common/path.ts
@@ -108,21 +108,31 @@ export class Path {
         return new Path(this.raw.substr(0, index)).root;
     }
 
+    /**
+     * Returns the parent directory if it exists (`hasDir === true`) or `this` otherwise.
+     */
     get dir(): Path {
         if (this._dir === undefined) {
             this._dir = this.computeDir();
         }
         return this._dir;
     }
 
+    /**
+     * Returns `true` if this has a parent directory, `false` otherwise.
+     *
+     * _This implementation returns `true` if and only if this is not the root dir and
+     * there is a path separator in the raw path._
+     */
+    get hasDir(): boolean {
+        return !this.isRoot && this.raw.lastIndexOf(Path.separator) !== -1;
+    }
+
     protected computeDir(): Path {
-        if (this.isRoot) {
+        if (!this.hasDir) {
             return this;
         }
         const lastIndex = this.raw.lastIndexOf(Path.separator);
-        if (lastIndex === -1) {
-            return this;
-        }
         if (this.isAbsolute) {
             const firstIndex = this.raw.indexOf(Path.separator);
             if (firstIndex === lastIndex) {

diff --git a/packages/core/src/common/uri.spec.ts b/packages/core/src/common/uri.spec.ts
@@ -21,6 +21,42 @@ const expect = chai.expect;
 
 describe('uri', () => {
 
+    describe('#getAllLocations', () => {
+
+        it('of /foo/bar/file.txt', () => {
+            expect(new URI('/foo/bar/file.txt').allLocations.map(x => x.toString()))
+                .deep.equals([
+                    new URI('/foo/bar/file.txt').toString(),
+                    new URI('/foo/bar').toString(),
+                    new URI('/foo').toString(),
+                    new URI('/').toString()
+                ]);
+        });
+
+        it('of foo', () => {
+            expect(new URI('foo').allLocations.map(x => x.toString()))
+                .deep.equals([
+                    new URI('foo').toString(),
+                    new URI('/').toString()
+                ]);
+        });
+
+        it('of foo:bar.txt', () => {
+            expect(new URI().withScheme('foo').withPath('bar.txt').allLocations.map(x => x.toString()))
+                .deep.equals([
+                    'foo:bar.txt'
+                ]);
+        });
+
+        it('of foo:bar/foobar.txt', () => {
+            expect(new URI().withScheme('foo').withPath('bar/foobar.txt').allLocations.map(x => x.toString()))
+                .deep.equals([
+                    new URI().withScheme('foo').withPath('bar/foobar.txt').toString(),
+                    new URI().withScheme('foo').withPath('bar').toString()
+                ]);
+        });
+    });
+
     describe('#getParent', () => {
 
         it('of file:///foo/bar.txt', () => {

diff --git a/packages/core/src/common/uri.ts b/packages/core/src/common/uri.ts
@@ -51,7 +51,7 @@ export default class URI {
     get allLocations(): URI[] {
         const locations = [];
         let location: URI = this;
-        while (!location.path.isRoot) {
+        while (!location.path.isRoot && location.path.hasDir) {
             locations.push(location);
             location = location.parent;
         }