lint: disallow console.log and similar

.eslintrc.js

@@ -159,6 +159,7 @@ module.exports = {
         'aws-toolkits/no-banned-usages': 'error',
         'aws-toolkits/no-incorrect-once-usage': 'error',
         'aws-toolkits/no-string-exec-for-child-process': 'error',
+        'aws-toolkits/no-console-log': 'error',
 
         'no-restricted-imports': [
             'error',

CONTRIBUTING.md

@@ -477,6 +477,8 @@ The package.json 'devDependencies' includes `eslint-plugin-aws-toolkits`. This i
 3. Register your rule in `plugins/eslint-plugin-aws-toolkits/index.ts`.
 4. Enable your rule in `.eslintrc`.
 
+Writing lint rules can be tricky if you are unfamiliar with the process. Use an AST viewer such as https://astexplorer.net/
+
 ### AWS SDK generator
 
 When the AWS SDK does not (yet) support a service but you have an API

packages/core/src/lambda/vue/configEditor/samInvokeFrontend.ts

@@ -3,6 +3,9 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+// Disable because it is a front-end file.
+/* eslint-disable aws-toolkits/no-console-log */
+
 import { defineComponent } from 'vue'
 import { AwsSamDebuggerConfiguration } from '../../../shared/sam/debugger/awsSamDebugConfiguration'
 import { AwsSamDebuggerConfigurationLoose, SamInvokeWebview } from './samInvokeBackend'

packages/core/src/login/webview/vue/backend.ts

@@ -33,6 +33,7 @@ import { AuthEnabledFeatures, AuthError, AuthFlowState, AuthUiClick, TelemetryMe
 import { AuthUtil } from '../../../codewhisperer/util/authUtil'
 import { DevSettings } from '../../../shared/settings'
 import { AuthSSOServer } from '../../../auth/sso/server'
+import { getLogger } from '../../../shared/logger/logger'
 
 export abstract class CommonAuthWebview extends VueWebview {
     private metricMetadata: TelemetryMetadata = {}
@@ -75,7 +76,8 @@ export abstract class CommonAuthWebview extends VueWebview {
                 await setupFunc()
                 return
             } catch (e) {
-                console.log(e)
+                getLogger().error('ssoSetup encountered an error: %s', e)
+
                 if (e instanceof ToolkitError && e.code === 'NotOnboarded') {
                     /**
                      * Connection is fine, they just skipped onboarding so not an actual error.

packages/core/src/shared/clients/codecatalystClient.ts

@@ -11,7 +11,7 @@ const localize = nls.loadMessageBundle()
 
 import * as AWS from 'aws-sdk'
 import * as logger from '../logger/logger'
-import { PerfLog } from '../logger/logger'
+import { PerfLog } from '../logger/perfLogger'
 import { ServiceConfigurationOptions } from 'aws-sdk/lib/service'
 import { CancellationError, Timeout, waitTimeout, waitUntil } from '../utilities/timeoutUtils'
 import { isUserCancelledError } from '../../shared/errors'

packages/core/src/shared/extensionGlobals.ts

@@ -18,6 +18,7 @@ import { TelemetryService } from './telemetry/telemetryService'
 import { UriHandler } from './vscode/uriHandler'
 import { GlobalState } from './globalState'
 import { setContext } from './vscode/setContext'
+import { getLogger } from './logger/logger'
 
 type Clock = Pick<
     typeof globalThis,
@@ -47,7 +48,7 @@ function copyClock(): Clock {
 
     const browserAlternatives = getBrowserAlternatives()
     if (Object.keys(browserAlternatives).length > 0) {
-        console.log('globals: Using browser alternatives for clock functions')
+        getLogger().info('globals: Using browser alternatives for clock functions')
         Object.assign(clock, browserAlternatives)
     } else {
         // In node.js context

packages/core/src/shared/fs/templateRegistry.ts

@@ -16,7 +16,7 @@ import { getLogger } from '../logger'
 import globals from '../extensionGlobals'
 import { Timeout } from '../utilities/timeoutUtils'
 import { localize } from '../utilities/vsCodeUtils'
-import { PerfLog } from '../logger/logger'
+import { PerfLog } from '../logger/perfLogger'
 import { showMessageWithCancel } from '../utilities/messages'
 
 export class CloudFormationTemplateRegistry extends WatchedFiles<CloudFormation.Template> {

packages/core/src/shared/languageServer/utils/runner.ts

@@ -8,6 +8,9 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+// Disable because this is a language server.
+/* eslint-disable aws-toolkits/no-console-log */
+
 import { CancellationToken, ErrorCodes, ResponseError } from 'vscode-languageserver'
 
 export function formatError(message: string, err: any): string {

packages/core/src/shared/logger/logger.ts

@@ -156,23 +156,28 @@ export class ConsoleLogger implements Logger {
         }
     }
     public debug(message: string | Error, ...meta: any[]): number {
+        // eslint-disable-next-line aws-toolkits/no-console-log
         console.debug(message, ...meta)
         return 0
     }
     public verbose(message: string | Error, ...meta: any[]): number {
+        // eslint-disable-next-line aws-toolkits/no-console-log
         console.debug(message, ...meta)
         return 0
     }
     public info(message: string | Error, ...meta: any[]): number {
+        // eslint-disable-next-line aws-toolkits/no-console-log
         console.info(message, ...meta)
         return 0
     }
     public warn(message: string | Error, ...meta: any[]): number {
+        // eslint-disable-next-line aws-toolkits/no-console-log
         console.warn(message, ...meta)
         return 0
     }
     /** Note: In nodejs this prints to `stderr` (see {@link Console.error}). */
     public error(message: string | Error, ...meta: any[]): number {
+        // eslint-disable-next-line aws-toolkits/no-console-log
         console.error(message, ...meta)
         return 0
     }
@@ -193,26 +198,3 @@ export function getNullLogger(type?: 'channel' | 'debugConsole' | 'main'): Logge
 export function setLogger(logger: Logger | undefined, type?: 'channel' | 'debugConsole' | 'main') {
     toolkitLoggers[type ?? 'main'] = logger
 }
-
-export class PerfLog {
-    private readonly log
-    public readonly start
-
-    public constructor(public readonly topic: string) {
-        const log = getLogger()
-        this.log = log
-        this.start = performance.now()
-    }
-
-    public elapsed(): number {
-        return performance.now() - this.start
-    }
-
-    public done(): void {
-        if (!this.log.logLevelEnabled('verbose')) {
-            return
-        }
-        const elapsed = this.elapsed()
-        this.log.verbose('%s took %dms', this.topic, elapsed.toFixed(1))
-    }
-}

packages/core/src/shared/logger/perfLogger.ts

@@ -0,0 +1,29 @@
+/*!
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { getLogger } from './logger'
+
+export class PerfLog {
+    private readonly log
+    public readonly start
+
+    public constructor(public readonly topic: string) {
+        const log = getLogger()
+        this.log = log
+        this.start = performance.now()
+    }
+
+    public elapsed(): number {
+        return performance.now() - this.start
+    }
+
+    public done(): void {
+        if (!this.log.logLevelEnabled('verbose')) {
+            return
+        }
+        const elapsed = this.elapsed()
+        this.log.verbose('%s took %dms', this.topic, elapsed.toFixed(1))
+    }
+}

packages/core/src/shared/sam/activation.ts

@@ -26,7 +26,8 @@ import { SamTemplateCodeLensProvider } from '../codelens/samTemplateCodeLensProv
 import * as jsLensProvider from '../codelens/typescriptCodeLensProvider'
 import { ExtContext, VSCODE_EXTENSION_ID } from '../extensions'
 import { getIdeProperties, getIdeType, IDE, isCloud9 } from '../extensionUtilities'
-import { PerfLog, getLogger } from '../logger/logger'
+import { getLogger } from '../logger/logger'
+import { PerfLog } from '../logger/perfLogger'
 import { NoopWatcher } from '../fs/watchedFiles'
 import { detectSamCli } from './cli/samCliDetection'
 import { CodelensRootRegistry } from '../fs/codelensRootRegistry'

packages/core/src/shared/sam/cli/samCliLocator.ts

@@ -9,7 +9,7 @@ import * as filesystemUtilities from '../../filesystemUtilities'
 import { getLogger, Logger } from '../../logger'
 import { SamCliInfoInvocation } from './samCliInfo'
 import { DefaultSamCliValidator, SamCliValidatorContext, SamCliVersionValidation } from './samCliValidator'
-import { PerfLog } from '../../logger/logger'
+import { PerfLog } from '../../logger/perfLogger'
 import { tryRun } from '../../utilities/pathFind'
 
 export class SamCliLocationProvider {

packages/core/src/shared/sam/sync.ts

@@ -686,6 +686,8 @@ class ProcessTerminal implements vscode.Pseudoterminal {
     public constructor(private readonly process: ChildProcess) {
         // Used in integration tests
         if (isAutomation()) {
+            // Disable because it is a test.
+            // eslint-disable-next-line aws-toolkits/no-console-log
             this.onDidWrite(text => console.log(text.trim()))
         }
     }

packages/core/src/shared/telemetry/telemetryService.ts

@@ -245,7 +245,7 @@ export class DefaultTelemetryService {
                 return DefaultTelemetryPublisher.fromIdentityId(clientId, identity)
             }
         } catch (err) {
-            console.error(`Got ${err} while initializing telemetry publisher`)
+            getLogger().error(`Got ${err} while initializing telemetry publisher`)
         }
     }
 

packages/core/src/ssmDocument/activation.ts

@@ -17,7 +17,7 @@ import { DocumentItemNodeWriteable } from './explorer/documentItemNodeWriteable'
 import { updateDocumentVersion } from './commands/updateDocumentVersion'
 import { Commands } from '../shared/vscode/commands2'
 import * as constants from '../shared/constants'
-import { PerfLog } from '../shared/logger/logger'
+import { PerfLog } from '../shared/logger/perfLogger'
 
 // Activate SSM Document related functionality for the extension.
 export async function activate(

packages/core/src/stepFunctions/activation.ts

@@ -21,7 +21,7 @@ import { AslVisualizationCDKManager } from './commands/visualizeStateMachine/asl
 import { renderCdkStateMachineGraph } from './commands/visualizeStateMachine/renderStateMachineGraphCDK'
 import { ToolkitError } from '../shared/errors'
 import { telemetry } from '../shared/telemetry/telemetry'
-import { PerfLog } from '../shared/logger/logger'
+import { PerfLog } from '../shared/logger/perfLogger'
 import { ASLLanguageClient } from './asl/client'
 
 /**

packages/core/src/stepFunctions/asl/aslServer.ts

@@ -8,6 +8,9 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  */
 
+// Disable because this is a language server.
+/* eslint-disable aws-toolkits/no-console-log */
+
 import {
     ClientCapabilities as aslClientCapabilities,
     DocumentLanguageSettings,

packages/core/src/threatComposer/handlers/onFileChangedHandler.ts

@@ -6,6 +6,7 @@
 import { Command, FileChangedMessage, MessageType, WebviewContext } from '../types'
 import vscode from 'vscode'
 import fs from '../../shared/fs/fs'
+import { getLogger } from '../../shared/logger'
 
 /**
  * Function to call when the text document has been modified
@@ -36,7 +37,7 @@ export async function onFileChanged(context: WebviewContext) {
             // There are no unsaved local changes in the file, so the file change is
             // triggered due to an external change in the file. This must be propagated
             // to the webview, so that it can be updated.
-            console.log('DocumentChanged')
+            getLogger().debug('DocumentChanged')
             await broadcastFileChange(fileName, filePath, fileContents, context.panel)
             context.fileStates[filePath] = { fileContents: fileContents }
             context.autoSaveFileState[filePath] = { fileContents: fileContents }
@@ -46,7 +47,7 @@ export async function onFileChanged(context: WebviewContext) {
             // change trigger is ignored. When the user decides to save the local
             // changes, they can decide to overwrite the file.
             void vscode.window.showWarningMessage(`${fileName}  has been modified externally.`)
-            console.warn('Document Changed externally before local changes are saved')
+            getLogger().warn('Document Changed externally before local changes are saved')
         }
     }
 }

packages/core/src/threatComposer/threatComposerEditor.ts

@@ -12,6 +12,7 @@ import { telemetry } from '../shared/telemetry/telemetry'
 import { onFileChanged } from './handlers/onFileChangedHandler'
 import { onThemeChanged } from './handlers/onThemeChangedHandler'
 import { sendThreatComposerOpenCancelled } from './handlers/webviewTelemetryHandler'
+import { getLogger } from '../shared/logger'
 
 const localize = nls.loadMessageBundle()
 
@@ -130,7 +131,7 @@ export class ThreatComposerEditor {
             },
             (progress, token) => {
                 token.onCancellationRequested(async () => {
-                    console.log('User canceled opening in TC operation')
+                    getLogger().debug('User canceled opening in TC operation')
                     await cancelOpenInThreatComposer('User canceled opening in THreatComposer operation')
                 })
 

packages/core/src/webviews/client.ts

@@ -187,6 +187,8 @@ export class WebviewClientFactory {
                 },
                 get: (_, prop) => {
                     if (typeof prop !== 'string') {
+                        // Disable because we cannot log to extension in a webview.
+                        // eslint-disable-next-line aws-toolkits/no-console-log
                         console.warn(`Tried to index webview client with non-string property: ${String(prop)}`)
                         return
                     }

packages/core/src/webviews/mixins/saveData.ts

@@ -36,6 +36,8 @@ const saveData: Vue.ComponentOptionsMixin = {
         this.$options._unid = unid
 
         if (_unids.has(unid)) {
+            // Disable because we cannot log to extension in a webview.
+            // eslint-disable-next-line aws-toolkits/no-console-log
             console.warn(`Component "${unid}" already exists. State-saving functionality will be disabled.`)
             return
         }

plugins/eslint-plugin-aws-toolkits/index.ts

@@ -8,13 +8,15 @@ import NoBannedUsages from './lib/rules/no-banned-usages'
 import NoIncorrectOnceUsage from './lib/rules/no-incorrect-once-usage'
 import NoOnlyInTests from './lib/rules/no-only-in-tests'
 import NoStringExecForChildProcess from './lib/rules/no-string-exec-for-child-process'
+import NoConsoleLog from './lib/rules/no-console-log'
 
 const rules = {
     'no-await-on-vscode-msg': NoAwaitOnVscodeMsg,
     'no-banned-usages': NoBannedUsages,
     'no-incorrect-once-usage': NoIncorrectOnceUsage,
     'no-only-in-tests': NoOnlyInTests,
     'no-string-exec-for-child-process': NoStringExecForChildProcess,
+    'no-console-log': NoConsoleLog,
 }
 
 export { rules }

plugins/eslint-plugin-aws-toolkits/lib/rules/no-console-log.ts

@@ -0,0 +1,61 @@
+/*!
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/**
+ * Enforce usage of our getLogger() function instead of using node's built in console.log() and similar.
+ * An eslint rule already exists for this (https://eslint.org/docs/latest/rules/no-console), but this
+ * rule is trivial to implement and we can add our own error message, so here it is.
+ */
+
+import { ESLintUtils, TSESTree } from '@typescript-eslint/utils'
+import { AST_NODE_TYPES } from '@typescript-eslint/types'
+import { Rule } from 'eslint'
+
+export const errMsg =
+    'do not use console.log, console.warn, or console.err, or similar; instead, use `getLogger().info`, `getLogger().warn`, etc; disable this rule for files that cannot import getLogger()'
+
+const disallowFunctions = new Set(['info', 'debug', 'error', 'warn', 'log'])
+
+export default ESLintUtils.RuleCreator.withoutDocs({
+    meta: {
+        docs: {
+            description: 'disallow use of console.log() to encourage use of getLogger()',
+            recommended: 'recommended',
+        },
+        messages: {
+            errMsg,
+        },
+        type: 'problem',
+        fixable: 'code',
+        schema: [],
+    },
+    defaultOptions: [],
+    create(context) {
+        return {
+            MemberExpression(node: TSESTree.MemberExpression) {
+                // Exception for anything that isn't extension code (in a src/ folder) (e.g. build scripts, tests)
+                if (context.physicalFilename.includes('test') || !context.physicalFilename.includes('src')) {
+                    return
+                }
+
+                // Validate this node
+                if (
+                    node.object.type !== AST_NODE_TYPES.Identifier ||
+                    node.object.name !== 'console' ||
+                    node.property.type !== AST_NODE_TYPES.Identifier
+                ) {
+                    return
+                }
+
+                if (disallowFunctions.has(node.property.name)) {
+                    return context.report({
+                        node,
+                        messageId: 'errMsg',
+                    })
+                }
+            },
+        }
+    },
+}) as unknown as Rule.RuleModule