Skip to content

Commit b229e9c

Browse files
authored
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.
1 parent feea97e commit b229e9c

File tree

8 files changed

+33
-10
lines changed

8 files changed

+33
-10
lines changed

.eslintrc.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
"no-throw-literal": "warn",
1313
// TODO "@typescript-eslint/semi" rule moved to https://eslint.style
1414
"semi": "error",
15+
"no-console": "warn",
1516
// Mostly fails tests, ex. expect(...).to.be.true returns a Chai.Assertion
1617
"@typescript-eslint/no-unused-expressions": "off",
1718
"@typescript-eslint/no-non-null-assertion": "off",

src/TestExplorer/TestRunner.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -735,7 +735,11 @@ export class TestRunner {
735735
const xUnitParser = new TestXUnitParser(
736736
this.folderContext.workspaceContext.toolchain.hasMultiLineParallelTestOutput
737737
);
738-
const results = await xUnitParser.parse(buffer, runState);
738+
const results = await xUnitParser.parse(
739+
buffer,
740+
runState,
741+
this.workspaceContext.outputChannel
742+
);
739743
if (results) {
740744
this.testRun.appendOutput(
741745
`\r\nExecuted ${results.tests} tests, with ${results.failures} failures and ${results.errors} errors.\r\n`
@@ -865,9 +869,13 @@ export class TestRunner {
865869
);
866870

867871
const outputHandler = this.testOutputHandler(config.testType, runState);
868-
LoggingDebugAdapterTracker.setDebugSessionCallback(session, output => {
869-
outputHandler(output);
870-
});
872+
LoggingDebugAdapterTracker.setDebugSessionCallback(
873+
session,
874+
this.workspaceContext.outputChannel,
875+
output => {
876+
outputHandler(output);
877+
}
878+
);
871879

872880
const cancellation = this.testRun.token.onCancellationRequested(() => {
873881
this.workspaceContext.outputChannel.logDiagnostic(

src/TestExplorer/TestXUnitParser.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import * as xml2js from "xml2js";
1616
import { TestRunnerTestRunState } from "./TestRunner";
17+
import { OutputChannel } from "vscode";
1718

1819
export interface TestResults {
1920
tests: number;
@@ -48,14 +49,15 @@ export class TestXUnitParser {
4849

4950
async parse(
5051
buffer: string,
51-
runState: TestRunnerTestRunState
52+
runState: TestRunnerTestRunState,
53+
outputChannel: OutputChannel
5254
): Promise<TestResults | undefined> {
5355
const xml = await xml2js.parseStringPromise(buffer);
5456
try {
5557
return await this.parseXUnit(xml, runState);
5658
} catch (error) {
5759
// ignore error
58-
console.log(error);
60+
outputChannel.appendLine(`Error parsing xUnit output: ${error}`);
5961
return undefined;
6062
}
6163
}

src/WorkspaceContext.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ export class WorkspaceContext implements vscode.Disposable {
253253
// add event listener for when a workspace folder is added/removed
254254
const onWorkspaceChange = vscode.workspace.onDidChangeWorkspaceFolders(event => {
255255
if (this === undefined) {
256+
// eslint-disable-next-line no-console
256257
console.log("Trying to run onDidChangeWorkspaceFolders on deleted context");
257258
return;
258259
}
@@ -261,6 +262,7 @@ export class WorkspaceContext implements vscode.Disposable {
261262
// add event listener for when the active edited text document changes
262263
const onDidChangeActiveWindow = vscode.window.onDidChangeActiveTextEditor(async editor => {
263264
if (this === undefined) {
265+
// eslint-disable-next-line no-console
264266
console.log("Trying to run onDidChangeWorkspaceFolders on deleted context");
265267
return;
266268
}

src/debugger/logTracker.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import * as vscode from "vscode";
1616
import { DebugAdapter } from "./debugAdapter";
1717
import { Version } from "../utilities/version";
18+
import { SwiftOutputChannel } from "../ui/SwiftOutputChannel";
1819

1920
/**
2021
* Factory class for building LoggingDebugAdapterTracker
@@ -82,12 +83,16 @@ export class LoggingDebugAdapterTracker implements vscode.DebugAdapterTracker {
8283
LoggingDebugAdapterTracker.debugSessionIdMap[id] = this;
8384
}
8485

85-
static setDebugSessionCallback(session: vscode.DebugSession, cb: (log: string) => void) {
86+
static setDebugSessionCallback(
87+
session: vscode.DebugSession,
88+
outputChannel: SwiftOutputChannel,
89+
cb: (log: string) => void
90+
) {
8691
const loggingDebugAdapter = this.debugSessionIdMap[session.id];
8792
if (loggingDebugAdapter) {
8893
loggingDebugAdapter.cb = cb;
8994
} else {
90-
console.error("Could not find debug adapter for session:", session.id);
95+
outputChannel.appendLine("Could not find debug adapter for session: " + session.id);
9196
}
9297
}
9398

src/tasks/TaskManager.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,9 @@ export class TaskManager implements vscode.Disposable {
104104
});
105105
// setup startingTaskPromise to be resolved one task has started
106106
if (this.startingTaskPromise !== undefined) {
107-
console.warn("TaskManager: Starting promise should be undefined if we reach here.");
107+
this.workspaceContext.outputChannel.appendLine(
108+
"TaskManager: Starting promise should be undefined if we reach here."
109+
);
108110
}
109111
this.startingTaskPromise = new Promise<void>(resolve => {
110112
this.taskStartObserver = () => {
@@ -122,7 +124,7 @@ export class TaskManager implements vscode.Disposable {
122124
});
123125
},
124126
error => {
125-
console.log(error);
127+
this.workspaceContext.outputChannel.appendLine(`Error executing task: ${error}`);
126128
disposable.dispose();
127129
this.startingTaskPromise = undefined;
128130
reject(error);

src/ui/SwiftOutputChannel.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export class SwiftOutputChannel implements vscode.OutputChannel {
3939
this.logStore.append(value);
4040

4141
if (this.logToConsole) {
42+
// eslint-disable-next-line no-console
4243
console.log(value);
4344
}
4445
}
@@ -48,6 +49,7 @@ export class SwiftOutputChannel implements vscode.OutputChannel {
4849
this.logStore.appendLine(value);
4950

5051
if (this.logToConsole) {
52+
// eslint-disable-next-line no-console
5153
console.log(value);
5254
}
5355
}

test/.eslintrc.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
{
22
"rules": {
3+
"no-console": "off",
34
"@typescript-eslint/no-explicit-any": "off"
45
}
56
}

0 commit comments

Comments
 (0)