From c0f6f0394d55627b105bd098c892e9291e2e31b2 Mon Sep 17 00:00:00 2001 From: Brian Ng Date: Wed, 5 Aug 2020 15:28:35 -0500 Subject: [PATCH] Support ConditionalExpressions in dry-error-messages rule (#11917) * Support ConditionalExpressions in dry-error-messages rule * tests --- .../src/rules/dry-error-messages.js | 27 +++++++++-- .../test/rules/dry-error-messages.js | 46 +++++++++++++++++++ packages/babel-parser/src/parser/lval.js | 4 -- packages/babel-parser/src/parser/statement.js | 2 - packages/babel-parser/src/tokenizer/index.js | 2 - 5 files changed, 70 insertions(+), 11 deletions(-) diff --git a/eslint/babel-eslint-plugin-development-internal/src/rules/dry-error-messages.js b/eslint/babel-eslint-plugin-development-internal/src/rules/dry-error-messages.js index 2994d933912a..d6e2226e2e76 100644 --- a/eslint/babel-eslint-plugin-development-internal/src/rules/dry-error-messages.js +++ b/eslint/babel-eslint-plugin-development-internal/src/rules/dry-error-messages.js @@ -41,6 +41,25 @@ function findIdNode(node) { return null; } +function findIdNodes(node) { + if (node.type === "ConditionalExpression") { + const consequent = findIdNode(node.consequent); + const alternate = findIdNode(node.alternate); + + if (consequent && alternate) { + return [consequent, alternate]; + } + } + + const idNode = findIdNode(node); + + if (idNode) { + return [idNode]; + } + + return null; +} + function findReference(node, scope) { let currentScope = scope; @@ -128,11 +147,13 @@ export default { node, ) { const [, errorMsgNode] = node.arguments; - const nodeToCheck = findIdNode(errorMsgNode); + const nodesToCheck = findIdNodes(errorMsgNode); if ( - nodeToCheck && - referencesImportedBinding(nodeToCheck, getScope(), importedBindings) + Array.isArray(nodesToCheck) && + nodesToCheck.every(node => + referencesImportedBinding(node, getScope(), importedBindings), + ) ) { return; } diff --git a/eslint/babel-eslint-plugin-development-internal/test/rules/dry-error-messages.js b/eslint/babel-eslint-plugin-development-internal/test/rules/dry-error-messages.js index 5f8d41479119..7f5ecd284b82 100644 --- a/eslint/babel-eslint-plugin-development-internal/test/rules/dry-error-messages.js +++ b/eslint/babel-eslint-plugin-development-internal/test/rules/dry-error-messages.js @@ -261,6 +261,14 @@ ruleTester.run("dry-error-messages", rule, { code: "this.raise(loc);", options: [{ errorModule: ERRORS_MODULE }], }, + + // Support ternary as second argument + { + filename: FILENAME, + code: + "import Errors, { NotErrors } from 'errorsModule'; this.raise(loc, a ? Errors.someErrorMessage : Errors.someOtherErrorMessage);", + options: [{ errorModule: ERRORS_MODULE }], + }, ], invalid: [ { @@ -691,5 +699,43 @@ ruleTester.run("dry-error-messages", rule, { }, ], }, + + // Should error if either part of a ternary isn't from error module + { + filename: FILENAME, + code: + "import Errors, { NotErrors } from 'errorsModule'; this.raise(loc, a ? Errors.someErrorMessage : 'hello');", + options: [{ errorModule: ERRORS_MODULE }], + errors: [ + { + messageId: "mustBeImported", + data: { errorModule: ERRORS_MODULE }, + }, + ], + }, + { + filename: FILENAME, + code: + "import Errors, { NotErrors } from 'errorsModule'; this.raise(loc, a ? 'hello' : Errors.someErrorMessage);", + options: [{ errorModule: ERRORS_MODULE }], + errors: [ + { + messageId: "mustBeImported", + data: { errorModule: ERRORS_MODULE }, + }, + ], + }, + { + filename: FILENAME, + code: + "import Errors, { NotErrors } from 'errorsModule'; this.raise(loc, a ? 'hello' : 'world');", + options: [{ errorModule: ERRORS_MODULE }], + errors: [ + { + messageId: "mustBeImported", + data: { errorModule: ERRORS_MODULE }, + }, + ], + }, ], }); diff --git a/packages/babel-parser/src/parser/lval.js b/packages/babel-parser/src/parser/lval.js index e9b5e67589f2..d286259fa24f 100644 --- a/packages/babel-parser/src/parser/lval.js +++ b/packages/babel-parser/src/parser/lval.js @@ -366,7 +366,6 @@ export default class LValParser extends NodeUtils { ? isStrictBindReservedWord(expr.name, this.inModule) : isStrictBindOnlyReservedWord(expr.name)) ) { - /* eslint-disable @babel/development-internal/dry-error-messages */ this.raise( expr.start, bindingType === BIND_NONE @@ -374,7 +373,6 @@ export default class LValParser extends NodeUtils { : Errors.StrictEvalArgumentsBinding, expr.name, ); - /* eslint-enable @babel/development-internal/dry-error-messages */ } if (checkClashes) { @@ -471,7 +469,6 @@ export default class LValParser extends NodeUtils { break; default: { - /* eslint-disable @babel/development-internal/dry-error-messages */ this.raise( expr.start, bindingType === BIND_NONE @@ -479,7 +476,6 @@ export default class LValParser extends NodeUtils { : Errors.InvalidLhsBinding, contextDescription, ); - /* eslint-enable @babel/development-internal/dry-error-messages */ } } } diff --git a/packages/babel-parser/src/parser/statement.js b/packages/babel-parser/src/parser/statement.js index 8f584240d85f..732214851535 100644 --- a/packages/babel-parser/src/parser/statement.js +++ b/packages/babel-parser/src/parser/statement.js @@ -2042,7 +2042,6 @@ export default class StatementParser extends ExpressionParser { name: string, ): void { if (this.state.exportedIdentifiers.indexOf(name) > -1) { - /* eslint-disable @babel/development-internal/dry-error-messages */ this.raise( node.start, name === "default" @@ -2050,7 +2049,6 @@ export default class StatementParser extends ExpressionParser { : Errors.DuplicateExport, name, ); - /* eslint-enable @babel/development-internal/dry-error-messages */ } this.state.exportedIdentifiers.push(name); } diff --git a/packages/babel-parser/src/tokenizer/index.js b/packages/babel-parser/src/tokenizer/index.js index fca85af541e8..d8104c4f9b45 100644 --- a/packages/babel-parser/src/tokenizer/index.js +++ b/packages/babel-parser/src/tokenizer/index.js @@ -420,14 +420,12 @@ export default class Tokenizer extends ParserErrors { // misleading this.expectPlugin("recordAndTuple"); if (this.getPluginOption("recordAndTuple", "syntaxType") !== "hash") { - /* eslint-disable @babel/development-internal/dry-error-messages */ throw this.raise( this.state.pos, next === charCodes.leftCurlyBrace ? Errors.RecordExpressionHashIncorrectStartSyntaxType : Errors.TupleExpressionHashIncorrectStartSyntaxType, ); - /* eslint-enable @babel/development-internal/dry-error-messages */ } if (next === charCodes.leftCurlyBrace) {