Skip to content

Commit

Permalink
Warn when using console logging methods (#1241)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
plemarquand authored Dec 10, 2024
1 parent feea97e commit b229e9c
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 10 deletions.
1 change: 1 addition & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
16 changes: 12 additions & 4 deletions src/TestExplorer/TestRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 4 additions & 2 deletions src/TestExplorer/TestXUnitParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import * as xml2js from "xml2js";
import { TestRunnerTestRunState } from "./TestRunner";
import { OutputChannel } from "vscode";

export interface TestResults {
tests: number;
Expand Down Expand Up @@ -48,14 +49,15 @@ export class TestXUnitParser {

async parse(
buffer: string,
runState: TestRunnerTestRunState
runState: TestRunnerTestRunState,
outputChannel: OutputChannel
): Promise<TestResults | undefined> {
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;
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/WorkspaceContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
9 changes: 7 additions & 2 deletions src/debugger/logTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}

Expand Down
6 changes: 4 additions & 2 deletions src/tasks/TaskManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>(resolve => {
this.taskStartObserver = () => {
Expand All @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions src/ui/SwiftOutputChannel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand All @@ -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);
}
}
Expand Down
1 change: 1 addition & 0 deletions test/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"rules": {
"no-console": "off",
"@typescript-eslint/no-explicit-any": "off"
}
}

0 comments on commit b229e9c

Please sign in to comment.