Obliterate Logger class from existence (#9844)

* Remove all uses of Logger.logError() method

* Remove all uses of Logger.logWarning() method

* Remove all uses of Logger.logInformation() method

* Remove all uses of ILogger interface and then deleting the Logger class

Author: Kartik Raj

src/client/activation/jedi.ts

@@ -26,7 +26,8 @@ import {
 } from 'vscode';
 
 import { PYTHON } from '../common/constants';
-import { IConfigurationService, IDisposable, IExtensionContext, ILogger, Resource } from '../common/types';
+import { traceError } from '../common/logger';
+import { IConfigurationService, IDisposable, IExtensionContext, Resource } from '../common/types';
 import { IShebangCodeLensProvider, PythonInterpreter } from '../interpreter/contracts';
 import { IServiceContainer, IServiceManager } from '../ioc/types';
 import { JediFactory } from '../languageServices/jediProxyFactory';
@@ -93,7 +94,7 @@ export class JediExtensionActivator implements ILanguageServerActivator {
         }
 
         const testManagementService = this.serviceManager.get<ITestManagementService>(ITestManagementService);
-        testManagementService.activate(this.symbolProvider).catch(ex => this.serviceManager.get<ILogger>(ILogger).logError('Failed to activate Unit Tests', ex));
+        testManagementService.activate(this.symbolProvider).catch(ex => traceError('Failed to activate Unit Tests', ex));
     }
 
     public deactivate() {

src/client/application/diagnostics/applicationDiagnostics.ts

@@ -6,7 +6,8 @@
 import { inject, injectable, named } from 'inversify';
 import { DiagnosticSeverity } from 'vscode';
 import { isTestExecution, STANDARD_OUTPUT_CHANNEL } from '../../common/constants';
-import { ILogger, IOutputChannel, Resource } from '../../common/types';
+import { traceError, traceInfo, traceWarning } from '../../common/logger';
+import { IOutputChannel, Resource } from '../../common/types';
 import { IServiceContainer } from '../../ioc/types';
 import { IApplicationDiagnostics } from '../types';
 import { IDiagnostic, IDiagnosticsService, ISourceMapSupportService } from './types';
@@ -49,22 +50,21 @@ export class ApplicationDiagnostics implements IApplicationDiagnostics {
         );
     }
     private log(diagnostics: IDiagnostic[]): void {
-        const logger = this.serviceContainer.get<ILogger>(ILogger);
         diagnostics.forEach(item => {
             const message = `Diagnostic Code: ${item.code}, Message: ${item.message}`;
             switch (item.severity) {
                 case DiagnosticSeverity.Error: {
-                    logger.logError(message);
+                    traceError(message);
                     this.outputChannel.appendLine(message);
                     break;
                 }
                 case DiagnosticSeverity.Warning: {
-                    logger.logWarning(message);
+                    traceWarning(message);
                     this.outputChannel.appendLine(message);
                     break;
                 }
                 default: {
-                    logger.logInformation(message);
+                    traceInfo(message);
                 }
             }
         });

src/client/common/installer/productInstaller.ts

@@ -16,7 +16,7 @@ import { traceError } from '../logger';
 import { IPlatformService } from '../platform/types';
 import { IProcessServiceFactory, IPythonExecutionFactory } from '../process/types';
 import { ITerminalServiceFactory } from '../terminal/types';
-import { IConfigurationService, IInstaller, ILogger, InstallerResponse, IOutputChannel, IPersistentStateFactory, ModuleNamePurpose, Product, ProductType } from '../types';
+import { IConfigurationService, IInstaller, InstallerResponse, IOutputChannel, IPersistentStateFactory, ModuleNamePurpose, Product, ProductType } from '../types';
 import { isResource } from '../utils/misc';
 import { ProductNames } from './productNames';
 import { IInstallationChannelManager, InterpreterUri, IProductPathService, IProductService } from './types';
@@ -68,8 +68,7 @@ export abstract class BaseInstaller {
         }
 
         const moduleName = translateProductToModule(product, ModuleNamePurpose.install);
-        const logger = this.serviceContainer.get<ILogger>(ILogger);
-        await installer.installModule(moduleName, resource, cancel).catch(logger.logError.bind(logger, `Error in installing the module '${moduleName}'`));
+        await installer.installModule(moduleName, resource, cancel).catch(ex => traceError(`Error in installing the module '${moduleName}', ${ex}`));
 
         return this.isInstalled(product, resource).then(isInstalled => (isInstalled ? InstallerResponse.Installed : InstallerResponse.Ignore));
     }

src/client/common/logger.ts

@@ -1,12 +1,11 @@
 // tslint:disable:no-console no-any
-import { injectable } from 'inversify';
 import * as path from 'path';
 import * as util from 'util';
 import { createLogger, format, transports } from 'winston';
 import { EXTENSION_ROOT_DIR } from '../constants';
 import { sendTelemetryEvent } from '../telemetry';
 import { isTestExecution } from './constants';
-import { ILogger, LogLevel } from './types';
+import { LogLevel } from './types';
 import { StopWatch } from './utils/stopWatch';
 
 // tslint:disable-next-line: no-var-requires no-require-imports
@@ -183,39 +182,6 @@ function initializeFileLogger() {
     fileLogger.add(logFileSink);
 }
 
-const enableLogging = !isTestExecution() || process.env.VSC_PYTHON_FORCE_LOGGING || process.env.VSC_PYTHON_LOG_FILE;
-
-@injectable()
-export class Logger implements ILogger {
-    // tslint:disable-next-line:no-any
-    public static error(...args: any[]) {
-        if (enableLogging) {
-            log(LogLevel.Error, ...args);
-        }
-    }
-    // tslint:disable-next-line:no-any
-    public static warn(...args: any[]) {
-        if (enableLogging) {
-            log(LogLevel.Warning, ...args);
-        }
-    }
-    // tslint:disable-next-line:no-any
-    public static verbose(...args: any[]) {
-        if (enableLogging) {
-            log(LogLevel.Information, ...args);
-        }
-    }
-    public logError(...args: any[]) {
-        traceError(...args);
-    }
-    public logWarning(...args: any[]) {
-        traceWarning(...args);
-    }
-    public logInformation(...args: any[]) {
-        traceVerbose(...args);
-    }
-}
-
 /**
  * What do we want to log.
  * @export

src/client/common/serviceRegistry.ts

@@ -39,7 +39,6 @@ import { InsidersExtensionPrompt } from './insidersBuild/insidersExtensionPrompt
 import { InsidersExtensionService } from './insidersBuild/insidersExtensionService';
 import { ExtensionChannel, IExtensionChannelRule, IExtensionChannelService, IInsiderExtensionPrompt } from './insidersBuild/types';
 import { ProductInstaller } from './installer/productInstaller';
-import { Logger } from './logger';
 import { BrowserService } from './net/browser';
 import { FileDownloader } from './net/fileDownloader';
 import { HttpClient } from './net/httpClient';
@@ -84,7 +83,6 @@ import {
     IExtensions,
     IFeatureDeprecationManager,
     IInstaller,
-    ILogger,
     IPathUtils,
     IPersistentStateFactory,
     IRandom,
@@ -99,7 +97,6 @@ export function registerTypes(serviceManager: IServiceManager) {
     serviceManager.addSingleton<IExtensions>(IExtensions, Extensions);
     serviceManager.addSingleton<IRandom>(IRandom, Random);
     serviceManager.addSingleton<IPersistentStateFactory>(IPersistentStateFactory, PersistentStateFactory);
-    serviceManager.addSingleton<ILogger>(ILogger, Logger);
     serviceManager.addSingleton<ITerminalServiceFactory>(ITerminalServiceFactory, TerminalServiceFactory);
     serviceManager.addSingleton<IPathUtils>(IPathUtils, PathUtils);
     serviceManager.addSingleton<IApplicationShell>(IApplicationShell, ApplicationShell);

src/client/common/types.ts

@@ -71,17 +71,6 @@ export enum LogLevel {
     Warning = 'Warning'
 }
 
-export const ILogger = Symbol('ILogger');
-
-export interface ILogger {
-    // tslint:disable-next-line: no-any
-    logError(...args: any[]): void;
-    // tslint:disable-next-line: no-any
-    logWarning(...args: any[]): void;
-    // tslint:disable-next-line: no-any
-    logInformation(...args: any[]): void;
-}
-
 export enum InstallerResponse {
     Installed,
     Disabled,

src/client/datascience/codeCssGenerator.ts

@@ -8,8 +8,9 @@ import * as monacoEditor from 'monaco-editor/esm/vs/editor/editor.api';
 import * as path from 'path';
 
 import { IWorkspaceService } from '../common/application/types';
+import { traceError, traceInfo, traceWarning } from '../common/logger';
 import { IFileSystem } from '../common/platform/types';
-import { IConfigurationService, ILogger } from '../common/types';
+import { IConfigurationService } from '../common/types';
 import { DefaultTheme } from './constants';
 import { ICodeCssGenerator, IThemeFinder } from './types';
 
@@ -98,7 +99,6 @@ export class CodeCssGenerator implements ICodeCssGenerator {
         @inject(IWorkspaceService) private workspaceService: IWorkspaceService,
         @inject(IThemeFinder) private themeFinder: IThemeFinder,
         @inject(IConfigurationService) private configService: IConfigurationService,
-        @inject(ILogger) private logger: ILogger,
         @inject(IFileSystem) private fs: IFileSystem
     ) {}
 
@@ -123,13 +123,13 @@ export class CodeCssGenerator implements ICodeCssGenerator {
 
             // Then we have to find where the theme resources are loaded from
             if (theme) {
-                this.logger.logInformation('Searching for token colors ...');
+                traceInfo('Searching for token colors ...');
                 const tokenColors = await this.findTokenColors(theme);
                 const baseColors = await this.findBaseColors(theme);
 
                 // The tokens object then contains the necessary data to generate our css
                 if (tokenColors && fontFamily && fontSize) {
-                    this.logger.logInformation('Using colors to generate CSS ...');
+                    traceInfo('Using colors to generate CSS ...');
                     result = applier({ tokenColors, baseColors, fontFamily, fontSize, isDark: isDarkUpdated, defaultStyle: ignoreTheme ? LightTheme : undefined });
                 } else if (tokenColors === null && fontFamily && fontSize) {
                     // No colors found. See if we can figure out what type of theme we have
@@ -139,7 +139,7 @@ export class CodeCssGenerator implements ICodeCssGenerator {
             }
         } catch (err) {
             // On error don't fail, just log
-            this.logger.logError(err);
+            traceError(err);
         }
 
         return result;
@@ -379,12 +379,12 @@ export class CodeCssGenerator implements ICodeCssGenerator {
 
     private findTokenColors = async (theme: string): Promise<JSONArray | null> => {
         try {
-            this.logger.logInformation('Attempting search for colors ...');
+            traceInfo('Attempting search for colors ...');
             const themeRoot = await this.themeFinder.findThemeRootJson(theme);
 
             // Use the first result if we have one
             if (themeRoot) {
-                this.logger.logInformation(`Loading colors from ${themeRoot} ...`);
+                traceInfo(`Loading colors from ${themeRoot} ...`);
 
                 // This should be the path to the file. Load it as a json object
                 const contents = await this.fs.readFile(themeRoot);
@@ -415,15 +415,15 @@ export class CodeCssGenerator implements ICodeCssGenerator {
                     // Then the path entry should contain a relative path to the json file with
                     // the tokens in it
                     const themeFile = path.join(path.dirname(themeRoot), found.path);
-                    this.logger.logInformation(`Reading colors from ${themeFile}`);
+                    traceInfo(`Reading colors from ${themeFile}`);
                     return await this.readTokenColors(themeFile);
                 }
             } else {
-                this.logger.logWarning(`Color theme ${theme} not found. Using default colors.`);
+                traceWarning(`Color theme ${theme} not found. Using default colors.`);
             }
         } catch (err) {
             // Swallow any exceptions with searching or parsing
-            this.logger.logError(err);
+            traceError(err);
         }
 
         // Force the colors to the defaults
@@ -432,12 +432,12 @@ export class CodeCssGenerator implements ICodeCssGenerator {
 
     private findBaseColors = async (theme: string): Promise<JSONObject | null> => {
         try {
-            this.logger.logInformation('Attempting search for colors ...');
+            traceInfo('Attempting search for colors ...');
             const themeRoot = await this.themeFinder.findThemeRootJson(theme);
 
             // Use the first result if we have one
             if (themeRoot) {
-                this.logger.logInformation(`Loading base colors from ${themeRoot} ...`);
+                traceInfo(`Loading base colors from ${themeRoot} ...`);
 
                 // This should be the path to the file. Load it as a json object
                 const contents = await this.fs.readFile(themeRoot);
@@ -465,15 +465,15 @@ export class CodeCssGenerator implements ICodeCssGenerator {
                     // Then the path entry should contain a relative path to the json file with
                     // the tokens in it
                     const themeFile = path.join(path.dirname(themeRoot), found.path);
-                    this.logger.logInformation(`Reading base colors from ${themeFile}`);
+                    traceInfo(`Reading base colors from ${themeFile}`);
                     return await this.readBaseColors(themeFile);
                 }
             } else {
-                this.logger.logWarning(`Color theme ${theme} not found. Using default colors.`);
+                traceWarning(`Color theme ${theme} not found. Using default colors.`);
             }
         } catch (err) {
             // Swallow any exceptions with searching or parsing
-            this.logger.logError(err);
+            traceError(err);
         }
 
         // Force the colors to the defaults

src/client/datascience/interactive-window/interactiveWindowCommandListener.ts

@@ -11,8 +11,9 @@ import { CancellationToken, CancellationTokenSource } from 'vscode-jsonrpc';
 import { IApplicationShell, ICommandManager, IDocumentManager } from '../../common/application/types';
 import { CancellationError } from '../../common/cancellation';
 import { PYTHON_LANGUAGE } from '../../common/constants';
+import { traceError, traceInfo } from '../../common/logger';
 import { IFileSystem } from '../../common/platform/types';
-import { IConfigurationService, IDisposableRegistry, ILogger } from '../../common/types';
+import { IConfigurationService, IDisposableRegistry } from '../../common/types';
 import * as localize from '../../common/utils/localize';
 import { captureTelemetry } from '../../telemetry';
 import { CommandSource } from '../../testing/common/constants';
@@ -42,7 +43,6 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList
         @inject(IDocumentManager) private documentManager: IDocumentManager,
         @inject(IApplicationShell) private applicationShell: IApplicationShell,
         @inject(IFileSystem) private fileSystem: IFileSystem,
-        @inject(ILogger) private logger: ILogger,
         @inject(IConfigurationService) private configuration: IConfigurationService,
         @inject(IStatusProvider) private statusProvider: IStatusProvider,
         @inject(INotebookImporter) private jupyterImporter: INotebookImporter,
@@ -122,14 +122,14 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList
         } catch (err) {
             if (!(err instanceof CancellationError)) {
                 if (err.message) {
-                    this.logger.logError(err.message);
+                    traceError(err.message);
                     this.applicationShell.showErrorMessage(err.message);
                 } else {
-                    this.logger.logError(err.toString());
+                    traceError(err.toString());
                     this.applicationShell.showErrorMessage(err.toString());
                 }
             } else {
-                this.logger.logInformation('Canceled');
+                traceInfo('Canceled');
             }
         }
         return result;