Skip to content

Commit 34d8097

Browse files
author
Kartik Raj
authored
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
1 parent 1a1f811 commit 34d8097

53 files changed

Lines changed: 118 additions & 303 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/client/activation/jedi.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ import {
2626
} from 'vscode';
2727

2828
import { PYTHON } from '../common/constants';
29-
import { IConfigurationService, IDisposable, IExtensionContext, ILogger, Resource } from '../common/types';
29+
import { traceError } from '../common/logger';
30+
import { IConfigurationService, IDisposable, IExtensionContext, Resource } from '../common/types';
3031
import { IShebangCodeLensProvider, PythonInterpreter } from '../interpreter/contracts';
3132
import { IServiceContainer, IServiceManager } from '../ioc/types';
3233
import { JediFactory } from '../languageServices/jediProxyFactory';
@@ -93,7 +94,7 @@ export class JediExtensionActivator implements ILanguageServerActivator {
9394
}
9495

9596
const testManagementService = this.serviceManager.get<ITestManagementService>(ITestManagementService);
96-
testManagementService.activate(this.symbolProvider).catch(ex => this.serviceManager.get<ILogger>(ILogger).logError('Failed to activate Unit Tests', ex));
97+
testManagementService.activate(this.symbolProvider).catch(ex => traceError('Failed to activate Unit Tests', ex));
9798
}
9899

99100
public deactivate() {

‎src/client/application/diagnostics/applicationDiagnostics.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
import { inject, injectable, named } from 'inversify';
77
import { DiagnosticSeverity } from 'vscode';
88
import { isTestExecution, STANDARD_OUTPUT_CHANNEL } from '../../common/constants';
9-
import { ILogger, IOutputChannel, Resource } from '../../common/types';
9+
import { traceError, traceInfo, traceWarning } from '../../common/logger';
10+
import { IOutputChannel, Resource } from '../../common/types';
1011
import { IServiceContainer } from '../../ioc/types';
1112
import { IApplicationDiagnostics } from '../types';
1213
import { IDiagnostic, IDiagnosticsService, ISourceMapSupportService } from './types';
@@ -49,22 +50,21 @@ export class ApplicationDiagnostics implements IApplicationDiagnostics {
4950
);
5051
}
5152
private log(diagnostics: IDiagnostic[]): void {
52-
const logger = this.serviceContainer.get<ILogger>(ILogger);
5353
diagnostics.forEach(item => {
5454
const message = `Diagnostic Code: ${item.code}, Message: ${item.message}`;
5555
switch (item.severity) {
5656
case DiagnosticSeverity.Error: {
57-
logger.logError(message);
57+
traceError(message);
5858
this.outputChannel.appendLine(message);
5959
break;
6060
}
6161
case DiagnosticSeverity.Warning: {
62-
logger.logWarning(message);
62+
traceWarning(message);
6363
this.outputChannel.appendLine(message);
6464
break;
6565
}
6666
default: {
67-
logger.logInformation(message);
67+
traceInfo(message);
6868
}
6969
}
7070
});

‎src/client/common/installer/productInstaller.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { traceError } from '../logger';
1616
import { IPlatformService } from '../platform/types';
1717
import { IProcessServiceFactory, IPythonExecutionFactory } from '../process/types';
1818
import { ITerminalServiceFactory } from '../terminal/types';
19-
import { IConfigurationService, IInstaller, ILogger, InstallerResponse, IOutputChannel, IPersistentStateFactory, ModuleNamePurpose, Product, ProductType } from '../types';
19+
import { IConfigurationService, IInstaller, InstallerResponse, IOutputChannel, IPersistentStateFactory, ModuleNamePurpose, Product, ProductType } from '../types';
2020
import { isResource } from '../utils/misc';
2121
import { ProductNames } from './productNames';
2222
import { IInstallationChannelManager, InterpreterUri, IProductPathService, IProductService } from './types';
@@ -68,8 +68,7 @@ export abstract class BaseInstaller {
6868
}
6969

7070
const moduleName = translateProductToModule(product, ModuleNamePurpose.install);
71-
const logger = this.serviceContainer.get<ILogger>(ILogger);
72-
await installer.installModule(moduleName, resource, cancel).catch(logger.logError.bind(logger, `Error in installing the module '${moduleName}'`));
71+
await installer.installModule(moduleName, resource, cancel).catch(ex => traceError(`Error in installing the module '${moduleName}', ${ex}`));
7372

7473
return this.isInstalled(product, resource).then(isInstalled => (isInstalled ? InstallerResponse.Installed : InstallerResponse.Ignore));
7574
}

‎src/client/common/logger.ts‎

Lines changed: 1 addition & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
// tslint:disable:no-console no-any
2-
import { injectable } from 'inversify';
32
import * as path from 'path';
43
import * as util from 'util';
54
import { createLogger, format, transports } from 'winston';
65
import { EXTENSION_ROOT_DIR } from '../constants';
76
import { sendTelemetryEvent } from '../telemetry';
87
import { isTestExecution } from './constants';
9-
import { ILogger, LogLevel } from './types';
8+
import { LogLevel } from './types';
109
import { StopWatch } from './utils/stopWatch';
1110

1211
// tslint:disable-next-line: no-var-requires no-require-imports
@@ -183,39 +182,6 @@ function initializeFileLogger() {
183182
fileLogger.add(logFileSink);
184183
}
185184

186-
const enableLogging = !isTestExecution() || process.env.VSC_PYTHON_FORCE_LOGGING || process.env.VSC_PYTHON_LOG_FILE;
187-
188-
@injectable()
189-
export class Logger implements ILogger {
190-
// tslint:disable-next-line:no-any
191-
public static error(...args: any[]) {
192-
if (enableLogging) {
193-
log(LogLevel.Error, ...args);
194-
}
195-
}
196-
// tslint:disable-next-line:no-any
197-
public static warn(...args: any[]) {
198-
if (enableLogging) {
199-
log(LogLevel.Warning, ...args);
200-
}
201-
}
202-
// tslint:disable-next-line:no-any
203-
public static verbose(...args: any[]) {
204-
if (enableLogging) {
205-
log(LogLevel.Information, ...args);
206-
}
207-
}
208-
public logError(...args: any[]) {
209-
traceError(...args);
210-
}
211-
public logWarning(...args: any[]) {
212-
traceWarning(...args);
213-
}
214-
public logInformation(...args: any[]) {
215-
traceVerbose(...args);
216-
}
217-
}
218-
219185
/**
220186
* What do we want to log.
221187
* @export

‎src/client/common/serviceRegistry.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ import { InsidersExtensionPrompt } from './insidersBuild/insidersExtensionPrompt
3939
import { InsidersExtensionService } from './insidersBuild/insidersExtensionService';
4040
import { ExtensionChannel, IExtensionChannelRule, IExtensionChannelService, IInsiderExtensionPrompt } from './insidersBuild/types';
4141
import { ProductInstaller } from './installer/productInstaller';
42-
import { Logger } from './logger';
4342
import { BrowserService } from './net/browser';
4443
import { FileDownloader } from './net/fileDownloader';
4544
import { HttpClient } from './net/httpClient';
@@ -84,7 +83,6 @@ import {
8483
IExtensions,
8584
IFeatureDeprecationManager,
8685
IInstaller,
87-
ILogger,
8886
IPathUtils,
8987
IPersistentStateFactory,
9088
IRandom,
@@ -99,7 +97,6 @@ export function registerTypes(serviceManager: IServiceManager) {
9997
serviceManager.addSingleton<IExtensions>(IExtensions, Extensions);
10098
serviceManager.addSingleton<IRandom>(IRandom, Random);
10199
serviceManager.addSingleton<IPersistentStateFactory>(IPersistentStateFactory, PersistentStateFactory);
102-
serviceManager.addSingleton<ILogger>(ILogger, Logger);
103100
serviceManager.addSingleton<ITerminalServiceFactory>(ITerminalServiceFactory, TerminalServiceFactory);
104101
serviceManager.addSingleton<IPathUtils>(IPathUtils, PathUtils);
105102
serviceManager.addSingleton<IApplicationShell>(IApplicationShell, ApplicationShell);

‎src/client/common/types.ts‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -71,17 +71,6 @@ export enum LogLevel {
7171
Warning = 'Warning'
7272
}
7373

74-
export const ILogger = Symbol('ILogger');
75-
76-
export interface ILogger {
77-
// tslint:disable-next-line: no-any
78-
logError(...args: any[]): void;
79-
// tslint:disable-next-line: no-any
80-
logWarning(...args: any[]): void;
81-
// tslint:disable-next-line: no-any
82-
logInformation(...args: any[]): void;
83-
}
84-
8574
export enum InstallerResponse {
8675
Installed,
8776
Disabled,

‎src/client/datascience/codeCssGenerator.ts‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@ import * as monacoEditor from 'monaco-editor/esm/vs/editor/editor.api';
88
import * as path from 'path';
99

1010
import { IWorkspaceService } from '../common/application/types';
11+
import { traceError, traceInfo, traceWarning } from '../common/logger';
1112
import { IFileSystem } from '../common/platform/types';
12-
import { IConfigurationService, ILogger } from '../common/types';
13+
import { IConfigurationService } from '../common/types';
1314
import { DefaultTheme } from './constants';
1415
import { ICodeCssGenerator, IThemeFinder } from './types';
1516

@@ -98,7 +99,6 @@ export class CodeCssGenerator implements ICodeCssGenerator {
9899
@inject(IWorkspaceService) private workspaceService: IWorkspaceService,
99100
@inject(IThemeFinder) private themeFinder: IThemeFinder,
100101
@inject(IConfigurationService) private configService: IConfigurationService,
101-
@inject(ILogger) private logger: ILogger,
102102
@inject(IFileSystem) private fs: IFileSystem
103103
) {}
104104

@@ -123,13 +123,13 @@ export class CodeCssGenerator implements ICodeCssGenerator {
123123

124124
// Then we have to find where the theme resources are loaded from
125125
if (theme) {
126-
this.logger.logInformation('Searching for token colors ...');
126+
traceInfo('Searching for token colors ...');
127127
const tokenColors = await this.findTokenColors(theme);
128128
const baseColors = await this.findBaseColors(theme);
129129

130130
// The tokens object then contains the necessary data to generate our css
131131
if (tokenColors && fontFamily && fontSize) {
132-
this.logger.logInformation('Using colors to generate CSS ...');
132+
traceInfo('Using colors to generate CSS ...');
133133
result = applier({ tokenColors, baseColors, fontFamily, fontSize, isDark: isDarkUpdated, defaultStyle: ignoreTheme ? LightTheme : undefined });
134134
} else if (tokenColors === null && fontFamily && fontSize) {
135135
// No colors found. See if we can figure out what type of theme we have
@@ -139,7 +139,7 @@ export class CodeCssGenerator implements ICodeCssGenerator {
139139
}
140140
} catch (err) {
141141
// On error don't fail, just log
142-
this.logger.logError(err);
142+
traceError(err);
143143
}
144144

145145
return result;
@@ -379,12 +379,12 @@ export class CodeCssGenerator implements ICodeCssGenerator {
379379

380380
private findTokenColors = async (theme: string): Promise<JSONArray | null> => {
381381
try {
382-
this.logger.logInformation('Attempting search for colors ...');
382+
traceInfo('Attempting search for colors ...');
383383
const themeRoot = await this.themeFinder.findThemeRootJson(theme);
384384

385385
// Use the first result if we have one
386386
if (themeRoot) {
387-
this.logger.logInformation(`Loading colors from ${themeRoot} ...`);
387+
traceInfo(`Loading colors from ${themeRoot} ...`);
388388

389389
// This should be the path to the file. Load it as a json object
390390
const contents = await this.fs.readFile(themeRoot);
@@ -415,15 +415,15 @@ export class CodeCssGenerator implements ICodeCssGenerator {
415415
// Then the path entry should contain a relative path to the json file with
416416
// the tokens in it
417417
const themeFile = path.join(path.dirname(themeRoot), found.path);
418-
this.logger.logInformation(`Reading colors from ${themeFile}`);
418+
traceInfo(`Reading colors from ${themeFile}`);
419419
return await this.readTokenColors(themeFile);
420420
}
421421
} else {
422-
this.logger.logWarning(`Color theme ${theme} not found. Using default colors.`);
422+
traceWarning(`Color theme ${theme} not found. Using default colors.`);
423423
}
424424
} catch (err) {
425425
// Swallow any exceptions with searching or parsing
426-
this.logger.logError(err);
426+
traceError(err);
427427
}
428428

429429
// Force the colors to the defaults
@@ -432,12 +432,12 @@ export class CodeCssGenerator implements ICodeCssGenerator {
432432

433433
private findBaseColors = async (theme: string): Promise<JSONObject | null> => {
434434
try {
435-
this.logger.logInformation('Attempting search for colors ...');
435+
traceInfo('Attempting search for colors ...');
436436
const themeRoot = await this.themeFinder.findThemeRootJson(theme);
437437

438438
// Use the first result if we have one
439439
if (themeRoot) {
440-
this.logger.logInformation(`Loading base colors from ${themeRoot} ...`);
440+
traceInfo(`Loading base colors from ${themeRoot} ...`);
441441

442442
// This should be the path to the file. Load it as a json object
443443
const contents = await this.fs.readFile(themeRoot);
@@ -465,15 +465,15 @@ export class CodeCssGenerator implements ICodeCssGenerator {
465465
// Then the path entry should contain a relative path to the json file with
466466
// the tokens in it
467467
const themeFile = path.join(path.dirname(themeRoot), found.path);
468-
this.logger.logInformation(`Reading base colors from ${themeFile}`);
468+
traceInfo(`Reading base colors from ${themeFile}`);
469469
return await this.readBaseColors(themeFile);
470470
}
471471
} else {
472-
this.logger.logWarning(`Color theme ${theme} not found. Using default colors.`);
472+
traceWarning(`Color theme ${theme} not found. Using default colors.`);
473473
}
474474
} catch (err) {
475475
// Swallow any exceptions with searching or parsing
476-
this.logger.logError(err);
476+
traceError(err);
477477
}
478478

479479
// Force the colors to the defaults

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ import { CancellationToken, CancellationTokenSource } from 'vscode-jsonrpc';
1111
import { IApplicationShell, ICommandManager, IDocumentManager } from '../../common/application/types';
1212
import { CancellationError } from '../../common/cancellation';
1313
import { PYTHON_LANGUAGE } from '../../common/constants';
14+
import { traceError, traceInfo } from '../../common/logger';
1415
import { IFileSystem } from '../../common/platform/types';
15-
import { IConfigurationService, IDisposableRegistry, ILogger } from '../../common/types';
16+
import { IConfigurationService, IDisposableRegistry } from '../../common/types';
1617
import * as localize from '../../common/utils/localize';
1718
import { captureTelemetry } from '../../telemetry';
1819
import { CommandSource } from '../../testing/common/constants';
@@ -42,7 +43,6 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList
4243
@inject(IDocumentManager) private documentManager: IDocumentManager,
4344
@inject(IApplicationShell) private applicationShell: IApplicationShell,
4445
@inject(IFileSystem) private fileSystem: IFileSystem,
45-
@inject(ILogger) private logger: ILogger,
4646
@inject(IConfigurationService) private configuration: IConfigurationService,
4747
@inject(IStatusProvider) private statusProvider: IStatusProvider,
4848
@inject(INotebookImporter) private jupyterImporter: INotebookImporter,
@@ -122,14 +122,14 @@ export class InteractiveWindowCommandListener implements IDataScienceCommandList
122122
} catch (err) {
123123
if (!(err instanceof CancellationError)) {
124124
if (err.message) {
125-
this.logger.logError(err.message);
125+
traceError(err.message);
126126
this.applicationShell.showErrorMessage(err.message);
127127
} else {
128-
this.logger.logError(err.toString());
128+
traceError(err.toString());
129129
this.applicationShell.showErrorMessage(err.toString());
130130
}
131131
} else {
132-
this.logger.logInformation('Canceled');
132+
traceInfo('Canceled');
133133
}
134134
}
135135
return result;

0 commit comments

Comments
 (0)