diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index f9446020..fcbf6d6c 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -2,4 +2,10 @@ // DO NOT EDIT THIS CODE BY HAND // YOU CAN REGENERATE IT USING yarn generate:configs -export = { extends: ['./configs/base'], rules: { '@sourcegraph/sourcegraph/check-help-links': 'error' } } +export = { + extends: ['./configs/base'], + rules: { + '@sourcegraph/sourcegraph/check-help-links': 'error', + '@sourcegraph/sourcegraph/no-unexplained-console-error': 'error', + }, +} diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index ea44cd20..34627f81 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -1,7 +1,9 @@ // This file is used by `scripts/generate-configs.ts` for rules extraction. import { checkHelpLinks } from './check-help-links' +import { noUnexplainedConsoleError } from './no-unexplained-console-error' // eslint-disable-next-line import/no-default-export export default { 'check-help-links': checkHelpLinks, + 'no-unexplained-console-error': noUnexplainedConsoleError, } diff --git a/packages/eslint-plugin/src/rules/no-unexplained-console-error/__tests__/no-unexplained-console-error.test.ts b/packages/eslint-plugin/src/rules/no-unexplained-console-error/__tests__/no-unexplained-console-error.test.ts new file mode 100644 index 00000000..08af1986 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-unexplained-console-error/__tests__/no-unexplained-console-error.test.ts @@ -0,0 +1,39 @@ +import { getFixturesRootDir, RuleTester } from '../../../testing/RuleTester' +import { noUnexplainedConsoleError } from '../no-unexplained-console-error' + +const ruleTester = new RuleTester({ + parserOptions: { + tsconfigRootDir: getFixturesRootDir(), + project: './tsconfig.json', + }, + parser: '@typescript-eslint/parser', +}) + +ruleTester.run('no-unexplained-console-error', noUnexplainedConsoleError, { + valid: [ + { + code: ` + try {} + catch (error) { + // This comment explains why we need to do this + console.error(error) + } + `, + }, + ], + invalid: [ + { code: 'console.error(err)' }, + { + code: ` + try {} catch (err) { + console.error(err) + } + `, + }, + ].map(test => { + return { + ...test, + errors: [{ messageId: 'noUnexplainedConsoleError' }], + } + }), +}) diff --git a/packages/eslint-plugin/src/rules/no-unexplained-console-error/index.ts b/packages/eslint-plugin/src/rules/no-unexplained-console-error/index.ts new file mode 100644 index 00000000..12e8c43a --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-unexplained-console-error/index.ts @@ -0,0 +1 @@ +export * from './no-unexplained-console-error' diff --git a/packages/eslint-plugin/src/rules/no-unexplained-console-error/no-unexplained-console-error.md b/packages/eslint-plugin/src/rules/no-unexplained-console-error/no-unexplained-console-error.md new file mode 100644 index 00000000..04ce862e --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-unexplained-console-error/no-unexplained-console-error.md @@ -0,0 +1,44 @@ +# Ban unexplained `console.error` calls + +## Rule details + +This rule bans logging errors directly to the console using `console.error()` calls, unless supported with a comment explaining why it's required. Otherwise, it's recommended to pass the error through `Sentry.captureException()`. + +### Bad code + +```ts +try { + // do something +} catch (error) { + console.error(error) +} +``` + +### Okay code + +```ts +try { + // do something +} catch (error) { + // We need to log this to the browser console because XYZ + console.error(error) +} +``` + +### (Bonus) Awesome code + +```ts +try { + // do something +} catch (error) { + Sentry.captureException(error) +} +``` + +## How to Use + +```jsonc +{ + "@sourcegraph/sourcegraph/no-unexplained-console-error": "error" +} +``` diff --git a/packages/eslint-plugin/src/rules/no-unexplained-console-error/no-unexplained-console-error.ts b/packages/eslint-plugin/src/rules/no-unexplained-console-error/no-unexplained-console-error.ts new file mode 100644 index 00000000..52cc2f82 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-unexplained-console-error/no-unexplained-console-error.ts @@ -0,0 +1,45 @@ +import { createRule } from '../../utils' + +export const messages = { + noUnexplainedConsoleError: + 'Directly logging through `console.error()` is discouraged. If required, please add a comment explaining why so, otherwise consider passing the error through `Sentry.captureException()` instead.', +} + +export const noUnexplainedConsoleError = createRule<[], keyof typeof messages>({ + name: 'no-unexplained-console-error', + meta: { + docs: { + description: + "Bans usage of the console.error() calls, unless supported by a comment explain why it's required.", + recommended: 'error', + }, + messages, + schema: [], + type: 'problem', + }, + defaultOptions: [], + create(context) { + const sourceCode = context.getSourceCode() + + return { + CallExpression(node) { + const { callee } = node + if ( + callee.type === 'MemberExpression' && + callee.object.type === 'Identifier' && + callee.object.name === 'console' && + callee.property.type === 'Identifier' && + callee.property.name === 'error' + ) { + const comments = sourceCode.getCommentsBefore(node) + if (comments.length === 0) { + context.report({ + node, + messageId: 'noUnexplainedConsoleError', + }) + } + } + }, + } + }, +})