Implement pathExists using fs-extra

Author: Kartik Raj

src/client/common/platform/fileSystem.ts

@@ -88,6 +88,7 @@ interface IRawFSExtra {
     readFileSync(path: string, encoding: string): string;
     createReadStream(filename: string): ReadStream;
     createWriteStream(filename: string): WriteStream;
+    pathExists(filename: string): Promise<boolean>;
 }
 
 interface IRawPath {
@@ -125,6 +126,10 @@ export class RawFileSystem implements IRawFileSystem {
         );
     }
 
+    public async pathExists(filename: string): Promise<boolean> {
+        return this.fsExtra.pathExists(filename);
+    }
+
     public async stat(filename: string): Promise<FileStat> {
         // Note that, prior to the November release of VS Code,
         // stat.ctime was always 0.
@@ -355,6 +360,10 @@ export class FileSystemUtils implements IFileSystemUtils {
         // matches; otherwise a mismatch results in a "false" value
         fileType?: FileType,
     ): Promise<boolean> {
+        if (fileType === undefined) {
+            // Do not need to run stat if not asking for file type.
+            return this.raw.pathExists(filename);
+        }
         let stat: FileStat;
         try {
             // Note that we are using stat() rather than lstat().  This
@@ -368,9 +377,6 @@ export class FileSystemUtils implements IFileSystemUtils {
             return false;
         }
 
-        if (fileType === undefined) {
-            return true;
-        }
         if (fileType === FileType.Unknown) {
             // FileType.Unknown == 0, hence do not use bitwise operations.
             return stat.type === FileType.Unknown;
@@ -563,6 +569,10 @@ export class FileSystem implements IFileSystem {
         return this.utils.fileExists(filename);
     }
 
+    public pathExists(filename: string): Promise<boolean> {
+        return this.utils.pathExists(filename);
+    }
+
     public fileExistsSync(filename: string): boolean {
         return this.utils.fileExistsSync(filename);
     }

src/client/common/platform/types.ts

@@ -95,6 +95,7 @@ export type WriteStream = fs.WriteStream;
 
 // The low-level filesystem operations on which the extension depends.
 export interface IRawFileSystem {
+    pathExists(filename: string): Promise<boolean>;
     // Get information about a file (resolve symlinks).
     stat(filename: string): Promise<FileStat>;
     // Get information about a file (do not resolve synlinks).
@@ -216,6 +217,7 @@ export interface IFileSystem {
     createWriteStream(path: string): fs.WriteStream;
 
     // utils
+    pathExists(path: string): Promise<boolean>;
     fileExists(path: string): Promise<boolean>;
     fileExistsSync(path: string): boolean;
     directoryExists(path: string): Promise<boolean>;

src/client/common/variables/environment.ts

@@ -5,6 +5,7 @@ import { inject, injectable } from 'inversify';
 import * as path from 'path';
 import { sendTelemetryEvent } from '../../telemetry';
 import { EventName } from '../../telemetry/constants';
+import { traceError } from '../logger';
 import { IFileSystem } from '../platform/types';
 import { IPathUtils } from '../types';
 import { EnvironmentVariables, IEnvironmentVariablesService } from './types';
@@ -22,7 +23,14 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService
         filePath?: string,
         baseVars?: EnvironmentVariables,
     ): Promise<EnvironmentVariables | undefined> {
-        if (!filePath || !(await this.fs.fileExists(filePath))) {
+        if (!filePath || !(await this.fs.pathExists(filePath))) {
+            return;
+        }
+        const contents = await this.fs.readFile(filePath).catch((ex) => {
+            traceError('Custom .env is likely not pointing to a valid file', ex);
+            return undefined;
+        });
+        if (!contents) {
             return;
         }
         return parseEnvFile(await this.fs.readFile(filePath), baseVars);

src/test/common/platform/filesystem.unit.test.ts

@@ -56,6 +56,7 @@ interface IRawFS extends IPaths {
     writeFile(uri: vscode.Uri, content: Uint8Array): Thenable<void>;
 
     // "fs-extra"
+    pathExists(filename: string): Promise<boolean>;
     lstat(filename: string): Promise<fs.Stats>;
     chmod(filePath: string, mode: string | number): Promise<void>;
     appendFile(filename: string, data: {}): Promise<void>;
@@ -926,51 +927,26 @@ suite('FileSystemUtils', () => {
     suite('pathExists', () => {
         test('exists (without type)', async () => {
             const filename = 'x/y/z/spam.py';
-            const stat = createMockStat();
-            deps.setup((d) => d.stat(filename)) // The "file" exists.
-                .returns(() => Promise.resolve(stat.object));
+            deps.setup((d) => d.pathExists(filename)) // The "file" exists.
+                .returns(() => Promise.resolve(true));
 
             const exists = await utils.pathExists(filename);
 
             expect(exists).to.equal(true);
             verifyAll();
         });
 
-        test('does not exist', async () => {
+        test('does not exist (without type)', async () => {
             const filename = 'x/y/z/spam.py';
-            const err = vscode.FileSystemError.FileNotFound(filename);
-            deps.setup((d) => d.stat(filename)) // The file does not exist.
-                .throws(err);
+            deps.setup((d) => d.pathExists(filename)) // The "file" exists.
+                .returns(() => Promise.resolve(false));
 
             const exists = await utils.pathExists(filename);
 
             expect(exists).to.equal(false);
             verifyAll();
         });
 
-        test('ignores errors from stat()', async () => {
-            const filename = 'x/y/z/spam.py';
-            deps.setup((d) => d.stat(filename)) // It's broken.
-                .returns(() => Promise.reject(new Error('oops!')));
-
-            const exists = await utils.pathExists(filename);
-
-            expect(exists).to.equal(false);
-            verifyAll();
-        });
-
-        test('matches (type: undefined)', async () => {
-            const filename = 'x/y/z/spam.py';
-            const stat = createMockStat();
-            deps.setup((d) => d.stat(filename)) // The "file" exists.
-                .returns(() => Promise.resolve(stat.object));
-
-            const exists = await utils.pathExists(filename);
-
-            expect(exists).to.equal(true);
-            verifyAll();
-        });
-
         test('matches (type: file)', async () => {
             const filename = 'x/y/z/spam.py';
             const stat = createMockStat();
@@ -1231,8 +1207,8 @@ suite('FileSystemUtils', () => {
             const err = vscode.FileSystemError.FileNotFound(dirname);
             deps.setup((d) => d.listdir(dirname)) // The "file" does not exist.
                 .returns(() => Promise.reject(err));
-            deps.setup((d) => d.stat(dirname)) // The "file" does not exist.
-                .returns(() => Promise.reject(err));
+            deps.setup((d) => d.pathExists(dirname)) // The "file" does not exist.
+                .returns(() => Promise.resolve(false));
 
             const entries = await utils.listdir(dirname);
 
@@ -1245,9 +1221,7 @@ suite('FileSystemUtils', () => {
             const err = vscode.FileSystemError.FileNotADirectory(dirname);
             deps.setup((d) => d.listdir(dirname)) // Fail (async) with not-a-directory.
                 .returns(() => Promise.reject(err));
-            const stat = createMockStat();
-            deps.setup((d) => d.stat(dirname)) // The "file" exists.
-                .returns(() => Promise.resolve(stat.object));
+            deps.setup((d) => d.pathExists(dirname)).returns(() => Promise.resolve(true)); // The "file" exists.
 
             const promise = utils.listdir(dirname);
 
@@ -1260,29 +1234,13 @@ suite('FileSystemUtils', () => {
             const err = new Error('oops!');
             deps.setup((d) => d.listdir(dirname)) // Fail (async) with an arbitrary error.
                 .returns(() => Promise.reject(err));
-            deps.setup((d) => d.stat(dirname)) // Fail with file-not-found.
-                .throws(vscode.FileSystemError.FileNotFound(dirname));
+            deps.setup((d) => d.pathExists(dirname)).returns(() => Promise.resolve(false));
 
             const entries = await utils.listdir(dirname);
 
             expect(entries).to.deep.equal([]);
             verifyAll();
         });
-
-        test('fails if the raw call fails', async () => {
-            const dirname = 'x/y/z/spam';
-            const err = new Error('oops!');
-            deps.setup((d) => d.listdir(dirname)) // Fail with an arbirary error.
-                .throws(err);
-            const stat = createMockStat();
-            deps.setup((d) => d.stat(dirname)) // The "file" exists.
-                .returns(() => Promise.resolve(stat.object));
-
-            const promise = utils.listdir(dirname);
-
-            await expect(promise).to.eventually.be.rejected;
-            verifyAll();
-        });
     });
 
     suite('getSubDirectories', () => {

src/test/common/variables/envVarsService.unit.test.ts

@@ -38,7 +38,7 @@ suite('Environment Variables Service', () => {
         fs.verifyAll();
     }
     function setFile(fileName: string, text: string) {
-        fs.setup((f) => f.fileExists(fileName)) // Handle the specific file.
+        fs.setup((f) => f.pathExists(fileName)) // Handle the specific file.
             .returns(() => Promise.resolve(true)); // The file exists.
         fs.setup((f) => f.readFile(fileName)) // Handle the specific file.
             .returns(() => Promise.resolve(text)); // Pretend to read from the file.
@@ -53,7 +53,7 @@ suite('Environment Variables Service', () => {
         });
 
         test('Custom variables should be undefined with non-existent files', async () => {
-            fs.setup((f) => f.fileExists(filename)) // Handle the specific file.
+            fs.setup((f) => f.pathExists(filename)) // Handle the specific file.
                 .returns(() => Promise.resolve(false)); // The file is missing.
 
             const vars = await variablesService.parseFile(filename);
@@ -64,7 +64,7 @@ suite('Environment Variables Service', () => {
 
         test('Custom variables should be undefined when folder name is passed instead of a file name', async () => {
             const dirname = 'x/y/z';
-            fs.setup((f) => f.fileExists(dirname)) // Handle the specific "file".
+            fs.setup((f) => f.pathExists(dirname)) // Handle the specific "file".
                 .returns(() => Promise.resolve(false)); // It isn't a "regular" file.
 
             const vars = await variablesService.parseFile(dirname);