Add linter for naked console.errors

Summary:
`console.error(err)` are hard to identify in the codebase especially
as we often don't have reliable stack trace information.

I've already cleaned up a bunch of them manually by going after the most
high-firing ones; this should make it easier to identify the remaining ones.

Reviewed By: jknoxville

Differential Revision: D27913964

fbshipit-source-id: 0ff6624a0c083829846550b40954945d655b7cf6
This commit is contained in:
Pascal Hartig
2021-04-22 05:20:56 -07:00
committed by Facebook GitHub Bot
parent 87cdd21951
commit 3431206c0e
4 changed files with 121 additions and 0 deletions

View File

@@ -85,6 +85,7 @@ module.exports = {
'node/no-extraneous-require': [2, {allowModules: builtInModules}], 'node/no-extraneous-require': [2, {allowModules: builtInModules}],
'flipper/no-relative-imports-across-packages': [2], 'flipper/no-relative-imports-across-packages': [2],
'flipper/no-electron-remote-imports': [1], 'flipper/no-electron-remote-imports': [1],
'flipper/no-console-error-without-context': [1],
}, },
settings: { settings: {
'import/resolver': { 'import/resolver': {

View File

@@ -13,10 +13,14 @@ import noRelativeImportsAcrossPackages, {
import noElectronRemoteImports, { import noElectronRemoteImports, {
RULE_NAME as noElectronRemoteImportsRuleName, RULE_NAME as noElectronRemoteImportsRuleName,
} from './rules/noElectronRemoteImports'; } from './rules/noElectronRemoteImports';
import noConsoleErrorWithoutContext, {
RULE_NAME as noConsoleErrorWithoutContextRuleName,
} from './rules/noConsoleErrorWithoutContext';
module.exports = { module.exports = {
rules: { rules: {
[noRelativeImportsAcrossPackagesRuleName]: noRelativeImportsAcrossPackages, [noRelativeImportsAcrossPackagesRuleName]: noRelativeImportsAcrossPackages,
[noElectronRemoteImportsRuleName]: noElectronRemoteImports, [noElectronRemoteImportsRuleName]: noElectronRemoteImports,
[noConsoleErrorWithoutContextRuleName]: noConsoleErrorWithoutContext,
}, },
}; };

View File

@@ -0,0 +1,52 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @format
*/
import {TSESLint} from '@typescript-eslint/experimental-utils';
import rule, {RULE_NAME} from '../noConsoleErrorWithoutContext';
const tester = new TSESLint.RuleTester({
parser: require.resolve('@typescript-eslint/parser'),
parserOptions: {
sourceType: 'module',
ecmaVersion: 2020,
},
});
tester.run(RULE_NAME, rule, {
valid: [
{
code: `console.error("I've made a big mistake:", err);`,
filename: __filename,
},
{
code: `console.error("This should never happen.");`,
filename: __filename,
},
{
code: `console.error("Failed to open user settings: " + err);`,
filename: __filename,
},
{
code: `console.warn(e);`,
filename: __filename,
},
],
invalid: [
{
code: `console.error(err);`,
filename: __filename,
errors: [{messageId: 'noConsoleErrorWithoutContext'}],
},
{
code: `console.error(err, "Too late for context.");`,
filename: __filename,
errors: [{messageId: 'noConsoleErrorWithoutContext'}],
},
],
});

View File

@@ -0,0 +1,64 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @format
*/
import {TSESTree} from '@typescript-eslint/experimental-utils';
import {createESLintRule} from '../utils/createEslintRule';
type Options = [];
export type MessageIds = 'noConsoleErrorWithoutContext';
export const RULE_NAME = 'no-console-error-without-context';
export default createESLintRule<Options, MessageIds>({
name: RULE_NAME,
meta: {
type: 'problem',
docs: {
description: 'TBD',
category: 'Possible Errors',
recommended: 'warn',
},
schema: [],
messages: {
noConsoleErrorWithoutContext:
'"Naked" console.error calls are hard to identify without providing a context. Please add a static hint, e.g. `console.error("Reading user config failed", err);`',
},
},
defaultOptions: [],
create(context) {
return {
ExpressionStatement(node: TSESTree.ExpressionStatement) {
if (node.expression.type === 'CallExpression') {
const callee = node.expression.callee;
const isConsoleError =
callee.type === 'MemberExpression' &&
callee.object.type === 'Identifier' &&
callee.object.name === 'console' &&
callee.property.type === 'Identifier' &&
callee.property.name === 'error';
if (
isConsoleError &&
// If this is an Identifier it means the first argument is an object, e.g.
// console.error(err);
// Whereas a literal implies there's a static context given, e.g.
// console.error("My identifiable error: ", err);
// Expressions like concatenations are also fine here.
node.expression.arguments[0]?.type === 'Identifier'
) {
context.report({
node,
messageId: 'noConsoleErrorWithoutContext',
});
}
}
},
};
},
});