From d617373a764f1e856c5dfe5f385404ae4d46f54b Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Fri, 11 Oct 2019 07:48:41 +0100 Subject: [PATCH] refactor(ivy): i18n - change `unwrapMessagePartsFromLocalizeCall` to accept a `NodePath` (#33097) In Babel `NodePath` objects have more useful information available than simple AST nodes. But they are more difficult to create, especially for testing. This commit prepares the way for parsing more complex code downlevelling scenarios. PR Close #33097 --- .../source_files/es5_translate_plugin.ts | 2 +- .../source_files/source_file_utils.ts | 46 ++++++++++++------- .../source_files/es5_translate_plugin_spec.ts | 18 +++++++- .../source_files/source_file_utils_spec.ts | 29 +++++++++--- 4 files changed, 69 insertions(+), 26 deletions(-) diff --git a/packages/localize/src/tools/src/translate/source_files/es5_translate_plugin.ts b/packages/localize/src/tools/src/translate/source_files/es5_translate_plugin.ts index d0fc51d038..bcbfd51021 100644 --- a/packages/localize/src/tools/src/translate/source_files/es5_translate_plugin.ts +++ b/packages/localize/src/tools/src/translate/source_files/es5_translate_plugin.ts @@ -21,7 +21,7 @@ export function makeEs5TranslatePlugin( try { const calleePath = callPath.get('callee'); if (isNamedIdentifier(calleePath, localizeName) && isGlobalIdentifier(calleePath)) { - const messageParts = unwrapMessagePartsFromLocalizeCall(callPath.node); + const messageParts = unwrapMessagePartsFromLocalizeCall(callPath); const expressions = unwrapSubstitutionsFromLocalizeCall(callPath.node); const translated = translate(diagnostics, translations, messageParts, expressions, missingTranslation); diff --git a/packages/localize/src/tools/src/translate/source_files/source_file_utils.ts b/packages/localize/src/tools/src/translate/source_files/source_file_utils.ts index 48fcf35dee..d00a6454c0 100644 --- a/packages/localize/src/tools/src/translate/source_files/source_file_utils.ts +++ b/packages/localize/src/tools/src/translate/source_files/source_file_utils.ts @@ -51,41 +51,53 @@ export function buildLocalizeReplacement( * * @param call The AST node of the call to process. */ -export function unwrapMessagePartsFromLocalizeCall(call: t.CallExpression): TemplateStringsArray { - let cooked = call.arguments[0]; - if (!t.isExpression(cooked)) { - throw new BabelParseError(call, 'Unexpected argument to `$localize`: ' + cooked); +export function unwrapMessagePartsFromLocalizeCall(call: NodePath): + TemplateStringsArray { + let cooked = call.get('arguments')[0]; + + if (cooked === undefined) { + throw new BabelParseError(call.node, '`$localize` called without any arguments.'); + } + if (!cooked.isExpression()) { + throw new BabelParseError( + cooked.node, 'Unexpected argument to `$localize` (expected an array).'); } // If there is no call to `__makeTemplateObject(...)`, then `raw` must be the same as `cooked`. let raw = cooked; // Check for cached call of the form `x || x = __makeTemplateObject(...)` - if (t.isLogicalExpression(cooked) && cooked.operator === '||' && t.isIdentifier(cooked.left) && - t.isExpression(cooked.right)) { - if (t.isAssignmentExpression(cooked.right)) { - cooked = cooked.right.right; + if (cooked.isLogicalExpression() && cooked.node.operator === '||' && + cooked.get('left').isIdentifier()) { + const right = cooked.get('right'); + if (right.isAssignmentExpression()) { + cooked = right.get('right'); + if (!cooked.isExpression()) { + throw new BabelParseError( + cooked.node, 'Unexpected "makeTemplateObject()" function (expected an expression).'); + } } } // Check for `__makeTemplateObject(cooked, raw)` call - if (t.isCallExpression(cooked)) { - raw = cooked.arguments[1] as t.Expression; - if (!t.isExpression(raw)) { + if (cooked.isCallExpression()) { + const arg2 = cooked.get('arguments')[1]; + if (!arg2.isExpression()) { throw new BabelParseError( - raw, + arg2.node, 'Unexpected `raw` argument to the "makeTemplateObject()" function (expected an expression).'); } - cooked = cooked.arguments[0]; - if (!t.isExpression(cooked)) { + raw = arg2; + cooked = cooked.get('arguments')[0]; + if (!cooked.isExpression()) { throw new BabelParseError( - cooked, + cooked.node, 'Unexpected `cooked` argument to the "makeTemplateObject()" function (expected an expression).'); } } - const cookedStrings = unwrapStringLiteralArray(cooked); - const rawStrings = unwrapStringLiteralArray(raw); + const cookedStrings = unwrapStringLiteralArray(cooked.node); + const rawStrings = unwrapStringLiteralArray(raw.node); return ɵmakeTemplateObject(cookedStrings, rawStrings); } diff --git a/packages/localize/src/tools/test/translate/source_files/es5_translate_plugin_spec.ts b/packages/localize/src/tools/test/translate/source_files/es5_translate_plugin_spec.ts index bc263d614d..ed4e470cad 100644 --- a/packages/localize/src/tools/test/translate/source_files/es5_translate_plugin_spec.ts +++ b/packages/localize/src/tools/test/translate/source_files/es5_translate_plugin_spec.ts @@ -97,12 +97,28 @@ describe('makeEs5Plugin', () => { expect(diagnostics.hasErrors).toBe(true); expect(diagnostics.messages[0]).toEqual({ type: 'error', - message: '/app/dist/test.js: Unexpected argument to `$localize`: undefined\n' + + message: '/app/dist/test.js: `$localize` called without any arguments.\n' + '> 1 | $localize()\n' + ' | ^', }); }); + it('should add diagnostic error with code-frame information if the arguments to `$localize` are invalid', + () => { + const diagnostics = new Diagnostics(); + const input = '$localize(...x)'; + transformSync( + input, + {plugins: [makeEs5TranslatePlugin(diagnostics, {})], filename: '/app/dist/test.js'}); + expect(diagnostics.hasErrors).toBe(true); + expect(diagnostics.messages[0]).toEqual({ + type: 'error', + message: '/app/dist/test.js: Unexpected argument to `$localize` (expected an array).\n' + + '> 1 | $localize(...x)\n' + + ' | ^', + }); + }); + it('should add diagnostic error with code-frame information if the first argument to `$localize` is not an array', () => { const diagnostics = new Diagnostics(); diff --git a/packages/localize/src/tools/test/translate/source_files/source_file_utils_spec.ts b/packages/localize/src/tools/test/translate/source_files/source_file_utils_spec.ts index 317c6519ca..4dba4b5b66 100644 --- a/packages/localize/src/tools/test/translate/source_files/source_file_utils_spec.ts +++ b/packages/localize/src/tools/test/translate/source_files/source_file_utils_spec.ts @@ -9,7 +9,7 @@ import {ɵmakeTemplateObject} from '@angular/localize'; import {NodePath, transformSync} from '@babel/core'; import generate from '@babel/generator'; import template from '@babel/template'; -import {Expression, Identifier, TaggedTemplateExpression, ExpressionStatement, FunctionDeclaration, CallExpression, isParenthesizedExpression, numericLiteral, binaryExpression, NumericLiteral} from '@babel/types'; +import {Expression, Identifier, TaggedTemplateExpression, ExpressionStatement, FunctionDeclaration, CallExpression, isParenthesizedExpression, numericLiteral, binaryExpression, NumericLiteral, traverse} from '@babel/types'; import {isGlobalIdentifier, isNamedIdentifier, isStringLiteralArray, isArrayOfExpressions, unwrapStringLiteralArray, unwrapMessagePartsFromLocalizeCall, wrapInParensIfNecessary, buildLocalizeReplacement, unwrapSubstitutionsFromLocalizeCall, unwrapMessagePartsFromTemplateLiteral} from '../../../src/translate/source_files/source_file_utils'; describe('utils', () => { @@ -71,17 +71,14 @@ describe('utils', () => { describe('unwrapMessagePartsFromLocalizeCall', () => { it('should return an array of string literals from a direct call to a tag function', () => { - const ast = template.ast `$localize(['a', 'b\\t', 'c'], 1, 2)` as ExpressionStatement; - const call = ast.expression as CallExpression; + const call = getFirstCallExpression(`$localize(['a', 'b\\t', 'c'], 1, 2)`); const parts = unwrapMessagePartsFromLocalizeCall(call); expect(parts).toEqual(['a', 'b\t', 'c']); }); it('should return an array of string literals from a downleveled tagged template', () => { - const ast = template.ast - `$localize(__makeTemplateObject(['a', 'b\\t', 'c'], ['a', 'b\\\\t', 'c']), 1, 2)` as - ExpressionStatement; - const call = ast.expression as CallExpression; + let call = getFirstCallExpression( + `$localize(__makeTemplateObject(['a', 'b\\t', 'c'], ['a', 'b\\\\t', 'c']), 1, 2)`); const parts = unwrapMessagePartsFromLocalizeCall(call); expect(parts).toEqual(['a', 'b\t', 'c']); expect(parts.raw).toEqual(['a', 'b\\t', 'c']); @@ -182,3 +179,21 @@ function collectExpressionsPlugin() { const visitor = {Expression: (path: NodePath) => { expressions.push(path); }}; return {expressions, plugin: {visitor}}; } + +function getFirstCallExpression(code: string): NodePath { + let callPath: NodePath|undefined = undefined; + transformSync(code, { + plugins: [{ + visitor: { + CallExpression(path) { + callPath = path; + path.stop(); + } + } + }] + }); + if (callPath === undefined) { + throw new Error('CallExpression not found in code:' + code); + } + return callPath; +}