From b229e9cbfea92323e5aee7c76b7a354c3e4f867d Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Tue, 10 Dec 2024 08:37:31 -0500 Subject: [PATCH] Warn when using `console` logging methods (#1241) Add an ESLint warning when using methods like console.log in the extension code. It is preferable to use the `SwiftOutputChannel` so that these logs are captured by the extension diagnostics mechanism, and so that they are all grouped together in the same place when debugging. Sources under the tests directory do not have this restriction. --- .eslintrc.json | 1 + src/TestExplorer/TestRunner.ts | 16 ++++++++++++---- src/TestExplorer/TestXUnitParser.ts | 6 ++++-- src/WorkspaceContext.ts | 2 ++ src/debugger/logTracker.ts | 9 +++++++-- src/tasks/TaskManager.ts | 6 ++++-- src/ui/SwiftOutputChannel.ts | 2 ++ test/.eslintrc.json | 1 + 8 files changed, 33 insertions(+), 10 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index 81a495120..a1f488343 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -12,6 +12,7 @@ "no-throw-literal": "warn", // TODO "@typescript-eslint/semi" rule moved to https://eslint.style "semi": "error", + "no-console": "warn", // Mostly fails tests, ex. expect(...).to.be.true returns a Chai.Assertion "@typescript-eslint/no-unused-expressions": "off", "@typescript-eslint/no-non-null-assertion": "off", diff --git a/src/TestExplorer/TestRunner.ts b/src/TestExplorer/TestRunner.ts index bf4cd80a1..7753f2322 100644 --- a/src/TestExplorer/TestRunner.ts +++ b/src/TestExplorer/TestRunner.ts @@ -735,7 +735,11 @@ export class TestRunner { const xUnitParser = new TestXUnitParser( this.folderContext.workspaceContext.toolchain.hasMultiLineParallelTestOutput ); - const results = await xUnitParser.parse(buffer, runState); + const results = await xUnitParser.parse( + buffer, + runState, + this.workspaceContext.outputChannel + ); if (results) { this.testRun.appendOutput( `\r\nExecuted ${results.tests} tests, with ${results.failures} failures and ${results.errors} errors.\r\n` @@ -865,9 +869,13 @@ export class TestRunner { ); const outputHandler = this.testOutputHandler(config.testType, runState); - LoggingDebugAdapterTracker.setDebugSessionCallback(session, output => { - outputHandler(output); - }); + LoggingDebugAdapterTracker.setDebugSessionCallback( + session, + this.workspaceContext.outputChannel, + output => { + outputHandler(output); + } + ); const cancellation = this.testRun.token.onCancellationRequested(() => { this.workspaceContext.outputChannel.logDiagnostic( diff --git a/src/TestExplorer/TestXUnitParser.ts b/src/TestExplorer/TestXUnitParser.ts index 26c76fb72..8ef064d92 100644 --- a/src/TestExplorer/TestXUnitParser.ts +++ b/src/TestExplorer/TestXUnitParser.ts @@ -14,6 +14,7 @@ import * as xml2js from "xml2js"; import { TestRunnerTestRunState } from "./TestRunner"; +import { OutputChannel } from "vscode"; export interface TestResults { tests: number; @@ -48,14 +49,15 @@ export class TestXUnitParser { async parse( buffer: string, - runState: TestRunnerTestRunState + runState: TestRunnerTestRunState, + outputChannel: OutputChannel ): Promise { const xml = await xml2js.parseStringPromise(buffer); try { return await this.parseXUnit(xml, runState); } catch (error) { // ignore error - console.log(error); + outputChannel.appendLine(`Error parsing xUnit output: ${error}`); return undefined; } } diff --git a/src/WorkspaceContext.ts b/src/WorkspaceContext.ts index cba4a7c47..664f18fc1 100644 --- a/src/WorkspaceContext.ts +++ b/src/WorkspaceContext.ts @@ -253,6 +253,7 @@ export class WorkspaceContext implements vscode.Disposable { // add event listener for when a workspace folder is added/removed const onWorkspaceChange = vscode.workspace.onDidChangeWorkspaceFolders(event => { if (this === undefined) { + // eslint-disable-next-line no-console console.log("Trying to run onDidChangeWorkspaceFolders on deleted context"); return; } @@ -261,6 +262,7 @@ export class WorkspaceContext implements vscode.Disposable { // add event listener for when the active edited text document changes const onDidChangeActiveWindow = vscode.window.onDidChangeActiveTextEditor(async editor => { if (this === undefined) { + // eslint-disable-next-line no-console console.log("Trying to run onDidChangeWorkspaceFolders on deleted context"); return; } diff --git a/src/debugger/logTracker.ts b/src/debugger/logTracker.ts index 4696a7df7..b5dde65e1 100644 --- a/src/debugger/logTracker.ts +++ b/src/debugger/logTracker.ts @@ -15,6 +15,7 @@ import * as vscode from "vscode"; import { DebugAdapter } from "./debugAdapter"; import { Version } from "../utilities/version"; +import { SwiftOutputChannel } from "../ui/SwiftOutputChannel"; /** * Factory class for building LoggingDebugAdapterTracker @@ -82,12 +83,16 @@ export class LoggingDebugAdapterTracker implements vscode.DebugAdapterTracker { LoggingDebugAdapterTracker.debugSessionIdMap[id] = this; } - static setDebugSessionCallback(session: vscode.DebugSession, cb: (log: string) => void) { + static setDebugSessionCallback( + session: vscode.DebugSession, + outputChannel: SwiftOutputChannel, + cb: (log: string) => void + ) { const loggingDebugAdapter = this.debugSessionIdMap[session.id]; if (loggingDebugAdapter) { loggingDebugAdapter.cb = cb; } else { - console.error("Could not find debug adapter for session:", session.id); + outputChannel.appendLine("Could not find debug adapter for session: " + session.id); } } diff --git a/src/tasks/TaskManager.ts b/src/tasks/TaskManager.ts index 980f53093..dfa1e3434 100644 --- a/src/tasks/TaskManager.ts +++ b/src/tasks/TaskManager.ts @@ -104,7 +104,9 @@ export class TaskManager implements vscode.Disposable { }); // setup startingTaskPromise to be resolved one task has started if (this.startingTaskPromise !== undefined) { - console.warn("TaskManager: Starting promise should be undefined if we reach here."); + this.workspaceContext.outputChannel.appendLine( + "TaskManager: Starting promise should be undefined if we reach here." + ); } this.startingTaskPromise = new Promise(resolve => { this.taskStartObserver = () => { @@ -122,7 +124,7 @@ export class TaskManager implements vscode.Disposable { }); }, error => { - console.log(error); + this.workspaceContext.outputChannel.appendLine(`Error executing task: ${error}`); disposable.dispose(); this.startingTaskPromise = undefined; reject(error); diff --git a/src/ui/SwiftOutputChannel.ts b/src/ui/SwiftOutputChannel.ts index 04291b0ba..4dfe482ab 100644 --- a/src/ui/SwiftOutputChannel.ts +++ b/src/ui/SwiftOutputChannel.ts @@ -39,6 +39,7 @@ export class SwiftOutputChannel implements vscode.OutputChannel { this.logStore.append(value); if (this.logToConsole) { + // eslint-disable-next-line no-console console.log(value); } } @@ -48,6 +49,7 @@ export class SwiftOutputChannel implements vscode.OutputChannel { this.logStore.appendLine(value); if (this.logToConsole) { + // eslint-disable-next-line no-console console.log(value); } } diff --git a/test/.eslintrc.json b/test/.eslintrc.json index 8b8e4250e..ac8f7c1d5 100644 --- a/test/.eslintrc.json +++ b/test/.eslintrc.json @@ -1,5 +1,6 @@ { "rules": { + "no-console": "off", "@typescript-eslint/no-explicit-any": "off" } }