Drop PlatformService as a dependency of FileSystem. (microsoft#9784)

Eric Snow · web-flow · commit f2f9c23787c0 · 2020-01-27T16:23:41.000-07:00
(for microsoft#8995) The dependency is unnecessary and adds complication to various places that FileSystem is used. Dropping it simplifies things going forward.
diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts
@@ -5,7 +5,7 @@
 import { createHash } from 'crypto';
 import * as fs from 'fs-extra';
 import * as glob from 'glob';
-import { inject, injectable } from 'inversify';
+import { injectable } from 'inversify';
 import { promisify } from 'util';
 import * as vscode from 'vscode';
 import { createDeferred } from '../utils/async';
@@ -16,7 +16,7 @@ import { TemporaryFileSystem } from './fs-temp';
 // prettier-ignore
 import {
     FileStat, FileType,
-    IFileSystem, IFileSystemPaths, IPlatformService, IRawFileSystem,
+    IFileSystem, IFileSystemPaths, IRawFileSystem,
     ReadStream, TemporaryFile, WriteStream
 } from './types';
 
@@ -282,14 +282,8 @@ export class FileSystem implements IFileSystem {
     private readonly paths: IFileSystemPaths;
     private readonly pathUtils: FileSystemPathUtils;
     private readonly tmp: TemporaryFileSystem;
-    // prettier-ignore
-    constructor(
-        @inject(IPlatformService) platformService: IPlatformService
-    ) {
-        // prettier-ignore
-        this.paths = FileSystemPaths.withDefaults(
-            platformService.isWindows
-        );
+    constructor() {
+        this.paths = FileSystemPaths.withDefaults();
         this.pathUtils = FileSystemPathUtils.withDefaults(this.paths);
         this.tmp = TemporaryFileSystem.withDefaults();
         this.raw = RawFileSystem.withDefaults(this.paths);
diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts
@@ -6,7 +6,6 @@
 import * as path from 'path';
 import { EXTENSION_ROOT_DIR } from '../../constants';
 import { FileSystem } from '../platform/fileSystem';
-import { PlatformService } from '../platform/platformService';
 
 // External callers of localize use these tables to retrieve localized values.
 export namespace Diagnostics {
@@ -688,7 +687,7 @@ function getString(key: string, defValue?: string) {
 }
 
 function load() {
-    const fs = new FileSystem(new PlatformService());
+    const fs = new FileSystem();
 
     // Figure out our current locale.
     loadedLocale = parseLocale();
diff --git a/src/client/sourceMapSupport.ts b/src/client/sourceMapSupport.ts
@@ -8,7 +8,6 @@ import { WorkspaceConfiguration } from 'vscode';
 import './common/extensions';
 import { traceError } from './common/logger';
 import { FileSystem } from './common/platform/fileSystem';
-import { PlatformService } from './common/platform/platformService';
 import { EXTENSION_ROOT_DIR } from './constants';
 
 type VSCode = typeof import('vscode');
@@ -59,7 +58,7 @@ export class SourceMapSupport {
         }
     }
     protected async rename(sourceFile: string, targetFile: string) {
-        const fs = new FileSystem(new PlatformService());
+        const fs = new FileSystem();
         if (await fs.fileExists(targetFile)) {
             return;
         }
diff --git a/src/test/common/crypto.unit.test.ts b/src/test/common/crypto.unit.test.ts
@@ -7,13 +7,12 @@ import { assert, expect } from 'chai';
 import * as path from 'path';
 import { CryptoUtils } from '../../client/common/crypto';
 import { FileSystem } from '../../client/common/platform/fileSystem';
-import { PlatformService } from '../../client/common/platform/platformService';
 import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../constants';
 
 // tslint:disable-next-line: max-func-body-length
 suite('Crypto Utils', async () => {
     let crypto: CryptoUtils;
-    const fs = new FileSystem(new PlatformService());
+    const fs = new FileSystem();
     const file = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'test', 'common', 'randomWords.txt');
     setup(() => {
         crypto = new CryptoUtils();
diff --git a/src/test/common/net/fileDownloader.unit.test.ts b/src/test/common/net/fileDownloader.unit.test.ts
@@ -93,7 +93,7 @@ suite('File Downloader', () => {
             httpClient = mock(HttpClient);
             appShell = mock(ApplicationShell);
             when(httpClient.downloadFile(anything())).thenCall(request);
-            fs = new FileSystem(new PlatformService());
+            fs = new FileSystem();
         });
         teardown(() => {
             rewiremock.disable();
diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts
@@ -8,7 +8,6 @@ import * as fs from 'fs-extra';
 import * as path from 'path';
 import { convertStat, FileSystem, RawFileSystem } from '../../../client/common/platform/fileSystem';
 import { FileSystemPaths, FileSystemPathUtils } from '../../../client/common/platform/fs-paths';
-import { PlatformService } from '../../../client/common/platform/platformService';
 import { FileType } from '../../../client/common/platform/types';
 import { sleep } from '../../../client/common/utils/async';
 // prettier-ignore
@@ -796,9 +795,7 @@ suite('FileSystem', () => {
     let fix: FSFixture;
     setup(async () => {
         // prettier-ignore
-        fileSystem = new FileSystem(
-            new PlatformService()
-        );
+        fileSystem = new FileSystem();
         fix = new FSFixture();
 
         await assertDoesNotExist(DOES_NOT_EXIST);
diff --git a/src/test/common/platform/filesystem.test.ts b/src/test/common/platform/filesystem.test.ts
@@ -10,7 +10,6 @@ import * as fsextra from 'fs-extra';
 import {
     convertStat, FileSystem, RawFileSystem
 } from '../../../client/common/platform/fileSystem';
-import { PlatformService } from '../../../client/common/platform/platformService';
 // prettier-ignore
 import {
     FileType, IFileSystem, IRawFileSystem
@@ -99,10 +98,7 @@ suite('FileSystem', () => {
     let filesystem: IFileSystem;
     let fix: FSFixture;
     setup(async () => {
-        // prettier-ignore
-        filesystem = new FileSystem(
-            new PlatformService()
-        );
+        filesystem = new FileSystem();
         fix = new FSFixture();
 
         await assertDoesNotExist(DOES_NOT_EXIST);
diff --git a/src/test/common/terminals/environmentActivationProviders/terminalActivation.testvirtualenvs.ts b/src/test/common/terminals/environmentActivationProviders/terminalActivation.testvirtualenvs.ts
@@ -8,7 +8,6 @@ import * as fs from 'fs-extra';
 import * as path from 'path';
 import * as vscode from 'vscode';
 import { FileSystem } from '../../../../client/common/platform/fileSystem';
-import { PlatformService } from '../../../../client/common/platform/platformService';
 import { PYTHON_VIRTUAL_ENVS_LOCATION } from '../../../ciConstants';
 import { PYTHON_PATH, restorePythonPathInWorkspaceRoot, setPythonPathInWorkspaceRoot, updateSetting, waitForCondition } from '../../../common';
 import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../../../constants';
@@ -20,7 +19,7 @@ suite('Activation of Environments in Terminal', () => {
     const file = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'testMultiRootWkspc', 'smokeTests', 'testExecInTerminal.py');
     let outputFile = '';
     let outputFileCounter = 0;
-    const fileSystem = new FileSystem(new PlatformService());
+    const fileSystem = new FileSystem();
     const outputFilesCreated: string[] = [];
     const envsLocation =
         PYTHON_VIRTUAL_ENVS_LOCATION !== undefined
diff --git a/src/test/common/variables/envVarsService.functional.test.ts b/src/test/common/variables/envVarsService.functional.test.ts
@@ -9,7 +9,6 @@ import { expect, use } from 'chai';
 import * as chaiAsPromised from 'chai-as-promised';
 import { FileSystem } from '../../../client/common/platform/fileSystem';
 import { PathUtils } from '../../../client/common/platform/pathUtils';
-import { PlatformService } from '../../../client/common/platform/platformService';
 import { IPathUtils } from '../../../client/common/types';
 import { OSType } from '../../../client/common/utils/platform';
 import { EnvironmentVariablesService } from '../../../client/common/variables/environment';
@@ -26,7 +25,7 @@ suite('Environment Variables Service', () => {
     let variablesService: IEnvironmentVariablesService;
     setup(() => {
         pathUtils = new PathUtils(getOSType() === OSType.Windows);
-        const fs = new FileSystem(new PlatformService());
+        const fs = new FileSystem();
         variablesService = new EnvironmentVariablesService(pathUtils, fs);
     });
 
diff --git a/src/test/common/variables/envVarsService.test.ts b/src/test/common/variables/envVarsService.test.ts
@@ -10,7 +10,6 @@ import * as chaiAsPromised from 'chai-as-promised';
 import * as path from 'path';
 import { FileSystem } from '../../../client/common/platform/fileSystem';
 import { PathUtils } from '../../../client/common/platform/pathUtils';
-import { PlatformService } from '../../../client/common/platform/platformService';
 import { IPathUtils } from '../../../client/common/types';
 import { OSType } from '../../../client/common/utils/platform';
 import { EnvironmentVariablesService } from '../../../client/common/variables/environment';
@@ -29,7 +28,7 @@ suite('Environment Variables Service', () => {
     let variablesService: IEnvironmentVariablesService;
     setup(() => {
         pathUtils = new PathUtils(getOSType() === OSType.Windows);
-        const fs = new FileSystem(new PlatformService());
+        const fs = new FileSystem();
         variablesService = new EnvironmentVariablesService(pathUtils, fs);
     });
 
diff --git a/src/test/common/webPanel/webPanel.unit.test.ts b/src/test/common/webPanel/webPanel.unit.test.ts
@@ -12,7 +12,6 @@ chai.use(chaiHttp);
 
 import { WebPanelServer } from '../../../client/common/application/webPanels/webPanelServer';
 import { FileSystem } from '../../../client/common/platform/fileSystem';
-import { PlatformService } from '../../../client/common/platform/platformService';
 import { EXTENSION_ROOT_DIR } from '../../../client/constants';
 
 // tslint:disable:no-any
@@ -24,7 +23,7 @@ suite('WebPanelServer', () => {
     const historyBundle = path.join(EXTENSION_ROOT_DIR, 'out', 'datascience-ui', 'history-react', 'index_bundle.js');
     setup(async () => {
         // So these are effectively functional tests rather than unit tests...
-        const fs = new FileSystem(new PlatformService());
+        const fs = new FileSystem();
         host = new WebPanelServer(await portfinder.getPortPromise(), token, fs);
         server = host.start();
     });
diff --git a/src/test/datascience/color.test.ts b/src/test/datascience/color.test.ts
@@ -10,7 +10,6 @@ import { IWorkspaceService } from '../../client/common/application/types';
 import { PythonSettings } from '../../client/common/configSettings';
 import { Logger } from '../../client/common/logger';
 import { FileSystem } from '../../client/common/platform/fileSystem';
-import { PlatformService } from '../../client/common/platform/platformService';
 import { CurrentProcess } from '../../client/common/process/currentProcess';
 import { IConfigurationService } from '../../client/common/types';
 import { CodeCssGenerator } from '../../client/datascience/codeCssGenerator';
@@ -34,7 +33,7 @@ suite('Theme colors', () => {
         extensions = new Extensions();
         currentProcess = new CurrentProcess();
         logger = new Logger();
-        const fs = new FileSystem(new PlatformService());
+        const fs = new FileSystem();
         themeFinder = new ThemeFinder(extensions, currentProcess, logger, fs);
 
         workspaceConfig = TypeMoq.Mock.ofType<WorkspaceConfiguration>();
@@ -158,7 +157,7 @@ suite('Theme colors', () => {
         mockThemeFinder.setup(m => m.isThemeDark(TypeMoq.It.isAnyString())).returns(() => Promise.resolve(false));
         mockThemeFinder.setup(m => m.findThemeRootJson(TypeMoq.It.isAnyString())).returns(() => Promise.resolve(undefined));
 
-        const fs = new FileSystem(new PlatformService());
+        const fs = new FileSystem();
         cssGenerator = new CodeCssGenerator(workspaceService.object, mockThemeFinder.object, configService.object, logger, fs);
 
         const colors = await cssGenerator.generateThemeCss(false, 'Kimbie Dark');
diff --git a/src/test/interpreters/condaService.unit.test.ts b/src/test/interpreters/condaService.unit.test.ts
@@ -8,7 +8,7 @@ import * as TypeMoq from 'typemoq';
 import { Disposable, EventEmitter } from 'vscode';
 
 import { IWorkspaceService } from '../../client/common/application/types';
-import { FileSystem } from '../../client/common/platform/fileSystem';
+import { FileSystemPaths, FileSystemPathUtils } from '../../client/common/platform/fs-paths';
 import { IFileSystem, IPlatformService } from '../../client/common/platform/types';
 import { IProcessService, IProcessServiceFactory } from '../../client/common/process/types';
 import { ITerminalActivationCommandProvider } from '../../client/common/terminal/types';
@@ -91,7 +91,11 @@ suite('Interpreters Conda Service', () => {
         fileSystem
             .setup(fs => fs.arePathsSame(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
             .returns((p1, p2) => {
-                return new FileSystem(platformService.object).arePathsSame(p1, p2);
+                // prettier-ignore
+                const utils = FileSystemPathUtils.withDefaults(
+                    FileSystemPaths.withDefaults(platformService.object.isWindows)
+                );
+                return utils.arePathsSame(p1, p2);
             });
 
         condaService = new CondaService(
diff --git a/src/test/linters/lint.functional.test.ts b/src/test/linters/lint.functional.test.ts
@@ -248,7 +248,7 @@ class TestFixture extends BaseTestFixture {
         serviceContainer.setup(s => s.get(TypeMoq.It.isValue(IProcessLogger), TypeMoq.It.isAny())).returns(() => processLogger.object);
 
         const platformService = new PlatformService();
-        const filesystem = new FileSystem(platformService);
+        const filesystem = new FileSystem();
 
         super(
             platformService,
diff --git a/src/test/serviceRegistry.ts b/src/test/serviceRegistry.ts
@@ -2,7 +2,7 @@
 // Licensed under the MIT License.
 
 import * as fsextra from 'fs-extra';
-import { Container, inject } from 'inversify';
+import { Container } from 'inversify';
 import { anything, instance, mock, when } from 'ts-mockito';
 import * as TypeMoq from 'typemoq';
 import { Disposable, Memento, OutputChannel, Uri } from 'vscode';
@@ -95,8 +95,8 @@ class FakeVSCodeFileSystemAPI {
     }
 }
 class LegacyFileSystem extends FileSystem {
-    constructor(@inject(IPlatformService) platformService: IPlatformService) {
-        super(platformService);
+    constructor() {
+        super();
         const vscfs = new FakeVSCodeFileSystemAPI();
         this.raw = RawFileSystem.withDefaults(undefined, vscfs);
     }
diff --git a/src/test/sourceMapSupport.test.ts b/src/test/sourceMapSupport.test.ts
@@ -9,7 +9,6 @@ import { expect } from 'chai';
 import * as fs from 'fs';
 import { ConfigurationTarget, Disposable } from 'vscode';
 import { FileSystem } from '../client/common/platform/fileSystem';
-import { PlatformService } from '../client/common/platform/platformService';
 import { Diagnostics } from '../client/common/utils/localize';
 import { SourceMapSupport } from '../client/sourceMapSupport';
 import { noop } from './core';
@@ -62,7 +61,7 @@ suite('Source Map Support', () => {
         });
     });
     test('When disabling source maps, the map file is renamed and vice versa', async () => {
-        const fileSystem = new FileSystem(new PlatformService());
+        const fileSystem = new FileSystem();
         const jsFile = await fileSystem.createTemporaryFile('.js');
         disposables.push(jsFile);
         const mapFile = `${jsFile.filePath}.map`;
diff --git a/src/test/testing/display/picker.functional.test.ts b/src/test/testing/display/picker.functional.test.ts
@@ -9,7 +9,6 @@ import { ApplicationShell } from '../../../client/common/application/application
 import { CommandManager } from '../../../client/common/application/commandManager';
 import { IApplicationShell, ICommandManager } from '../../../client/common/application/types';
 import { FileSystem } from '../../../client/common/platform/fileSystem';
-import { PlatformService } from '../../../client/common/platform/platformService';
 import { IFileSystem } from '../../../client/common/platform/types';
 import { ServiceContainer } from '../../../client/ioc/container';
 import { IServiceContainer } from '../../../client/ioc/types';
@@ -75,7 +74,7 @@ suite('Testing - TestDisplay', () => {
 
         setup(() => {
             tests = createEmptyResults();
-            when(mockedServiceContainer.get<IFileSystem>(IFileSystem)).thenReturn(new FileSystem(new PlatformService()));
+            when(mockedServiceContainer.get<IFileSystem>(IFileSystem)).thenReturn(new FileSystem());
             when(mockedTestCollectionStorage.getTests(wkspace)).thenReturn(tests);
             when(mockedAppShell.showQuickPick(anything(), anything())).thenResolve();
         });
diff --git a/src/test/testing/pytest/pytest.testMessageService.test.ts b/src/test/testing/pytest/pytest.testMessageService.test.ts
@@ -13,7 +13,6 @@ import { IWorkspaceService } from '../../../client/common/application/types';
 import { EXTENSION_ROOT_DIR } from '../../../client/common/constants';
 import { ProductNames } from '../../../client/common/installer/productNames';
 import { FileSystem } from '../../../client/common/platform/fileSystem';
-import { PlatformService } from '../../../client/common/platform/platformService';
 import { Product } from '../../../client/common/types';
 import { ICondaService, IInterpreterService } from '../../../client/interpreter/contracts';
 import { InterpreterService } from '../../../client/interpreter/interpreterService';
@@ -97,8 +96,7 @@ async function getExpectedLocationStackFromTestDetails(testDetails: ITestDetails
 
 suite('Unit Tests - PyTest - TestMessageService', () => {
     let ioc: UnitTestIocContainer;
-    const platformService = new PlatformService();
-    const filesystem = new FileSystem(platformService);
+    const filesystem = new FileSystem();
     const configTarget = IS_MULTI_ROOT_TEST ? vscode.ConfigurationTarget.WorkspaceFolder : vscode.ConfigurationTarget.Workspace;
     suiteSetup(async () => {
         await initialize();

Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,6 @@ import { WorkspaceConfiguration } from 'vscode';`
`8`	`8`	`import './common/extensions';`
`9`	`9`	`import { traceError } from './common/logger';`
`10`	`10`	`import { FileSystem } from './common/platform/fileSystem';`
`11`		`-import { PlatformService } from './common/platform/platformService';`
`12`	`11`	`import { EXTENSION_ROOT_DIR } from './constants';`
`13`	`12`
`14`	`13`	`type VSCode = typeof import('vscode');`
`@@ -59,7 +58,7 @@ export class SourceMapSupport {`
`59`	`58`	`}`
`60`	`59`	`}`
`61`	`60`	`protected async rename(sourceFile: string, targetFile: string) {`
`62`		`- const fs = new FileSystem(new PlatformService());`
	`61`	`+ const fs = new FileSystem();`
`63`	`62`	`if (await fs.fileExists(targetFile)) {`
`64`	`63`	`return;`
`65`	`64`	`}`