From 0a44c3f2dc081d5d97c5c5d8641013be11d370cc Mon Sep 17 00:00:00 2001 From: Maryia Lapata Date: Thu, 13 Sep 2018 14:51:08 +0300 Subject: [PATCH] [Tools] Throw FailError with code context on babel/parser exception (#22810) (#22992) * [Tools] Throw FailError with code context on babel/parser exception * Add description for createParserErrorMessage function --- src/dev/i18n/__snapshots__/utils.test.js.snap | 9 ++++ src/dev/i18n/extractors/code.js | 40 ++++++++++------- src/dev/i18n/extractors/html.js | 44 ++++++++++++++++--- src/dev/i18n/extractors/index.js | 2 - src/dev/i18n/extractors/pug.js | 20 ++++++++- src/dev/i18n/utils.js | 30 +++++++++++++ src/dev/i18n/utils.test.js | 30 ++++++++++++- 7 files changed, 148 insertions(+), 27 deletions(-) diff --git a/src/dev/i18n/__snapshots__/utils.test.js.snap b/src/dev/i18n/__snapshots__/utils.test.js.snap index 2a2f196d3f13..8f92d9b34efd 100644 --- a/src/dev/i18n/__snapshots__/utils.test.js.snap +++ b/src/dev/i18n/__snapshots__/utils.test.js.snap @@ -1,5 +1,14 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`i18n utils should create verbose parser error message 1`] = ` +"Unexpected token, expected \\",\\" (4:19): + const object = { + object: 'with', + semicolon: '->'; + }; +" +`; + exports[`i18n utils should not escape linebreaks 1`] = ` "Text with diff --git a/src/dev/i18n/extractors/code.js b/src/dev/i18n/extractors/code.js index e7477b17e275..9a986d53dc40 100644 --- a/src/dev/i18n/extractors/code.js +++ b/src/dev/i18n/extractors/code.js @@ -27,8 +27,9 @@ import { } from '@babel/types'; import { extractI18nCallMessages } from './i18n_call'; -import { isI18nTranslateFunction, traverseNodes } from '../utils'; +import { createParserErrorMessage, isI18nTranslateFunction, traverseNodes } from '../utils'; import { extractIntlMessages, extractFormattedMessages } from './react'; +import { createFailError } from '../../run'; /** * Detect Intl.formatMessage() function call (React). @@ -43,17 +44,11 @@ import { extractIntlMessages, extractFormattedMessages } from './react'; export function isIntlFormatMessageFunction(node) { return ( isCallExpression(node) && - ( - isIdentifier(node.callee, { name: 'formatMessage' }) || - ( - isMemberExpression(node.callee) && - ( - isIdentifier(node.callee.object, { name: 'intl' }) || - isIdentifier(node.callee.object.property, { name: 'intl' }) - ) && - isIdentifier(node.callee.property, { name: 'formatMessage' }) - ) - ) + (isIdentifier(node.callee, { name: 'formatMessage' }) || + (isMemberExpression(node.callee) && + (isIdentifier(node.callee.object, { name: 'intl' }) || + isIdentifier(node.callee.object.property, { name: 'intl' })) && + isIdentifier(node.callee.property, { name: 'formatMessage' }))) ); } @@ -67,12 +62,23 @@ export function isFormattedMessageElement(node) { } export function* extractCodeMessages(buffer) { - const content = parse(buffer.toString(), { - sourceType: 'module', - plugins: ['jsx', 'typescript', 'objectRestSpread', 'classProperties', 'asyncGenerators'], - }); + let ast; - for (const node of traverseNodes(content.program.body)) { + try { + ast = parse(buffer.toString(), { + sourceType: 'module', + plugins: ['jsx', 'typescript', 'objectRestSpread', 'classProperties', 'asyncGenerators'], + }); + } catch (error) { + if (error instanceof SyntaxError) { + const errorWithContext = createParserErrorMessage(buffer.toString(), error); + throw createFailError(errorWithContext); + } + + throw error; + } + + for (const node of traverseNodes(ast.program.body)) { if (isI18nTranslateFunction(node)) { yield extractI18nCallMessages(node); } else if (isIntlFormatMessageFunction(node)) { diff --git a/src/dev/i18n/extractors/html.js b/src/dev/i18n/extractors/html.js index b576acb31c6d..b52734cc8b8b 100644 --- a/src/dev/i18n/extractors/html.js +++ b/src/dev/i18n/extractors/html.js @@ -21,7 +21,13 @@ import { jsdom } from 'jsdom'; import { parse } from '@babel/parser'; import { isDirectiveLiteral, isObjectExpression, isStringLiteral } from '@babel/types'; -import { isPropertyWithKey, formatHTMLString, formatJSString, traverseNodes } from '../utils'; +import { + isPropertyWithKey, + formatHTMLString, + formatJSString, + traverseNodes, + createParserErrorMessage, +} from '../utils'; import { DEFAULT_MESSAGE_KEY, CONTEXT_KEY } from '../constants'; import { createFailError } from '../../run'; @@ -38,10 +44,23 @@ const I18N_FILTER_MARKER = '| i18n: '; * @returns {string} Default message */ function parseFilterObjectExpression(expression) { - // parse an object expression instead of block statement - const nodes = parse(`+${expression}`).program.body; + let ast; + + try { + // parse an object expression instead of block statement + ast = parse(`+${expression}`); + } catch (error) { + if (error instanceof SyntaxError) { + const errorWithContext = createParserErrorMessage(` ${expression}`, error); + throw createFailError( + `Couldn't parse angular expression with i18n filter:\n${errorWithContext}` + ); + } - for (const node of traverseNodes(nodes)) { + throw error; + } + + for (const node of traverseNodes(ast.program.body)) { if (!isObjectExpression(node)) { continue; } @@ -72,7 +91,22 @@ function parseFilterObjectExpression(expression) { } function parseIdExpression(expression) { - for (const node of traverseNodes(parse(expression).program.directives)) { + let ast; + + try { + ast = parse(expression); + } catch (error) { + if (error instanceof SyntaxError) { + const errorWithContext = createParserErrorMessage(expression, error); + throw createFailError( + `Couldn't parse angular expression with i18n filter:\n${errorWithContext}` + ); + } + + throw error; + } + + for (const node of traverseNodes(ast.program.directives)) { if (isDirectiveLiteral(node)) { return formatJSString(node.value); } diff --git a/src/dev/i18n/extractors/index.js b/src/dev/i18n/extractors/index.js index 7362eeb4e700..72e4378d3dfa 100644 --- a/src/dev/i18n/extractors/index.js +++ b/src/dev/i18n/extractors/index.js @@ -20,6 +20,4 @@ export { extractCodeMessages } from './code'; export { extractHandlebarsMessages } from './handlebars'; export { extractHtmlMessages } from './html'; -export { extractI18nCallMessages } from './i18n_call'; export { extractPugMessages } from './pug'; -export { extractFormattedMessages, extractIntlMessages } from './react'; diff --git a/src/dev/i18n/extractors/pug.js b/src/dev/i18n/extractors/pug.js index 59851d19e88a..61b4ba0ca866 100644 --- a/src/dev/i18n/extractors/pug.js +++ b/src/dev/i18n/extractors/pug.js @@ -20,7 +20,8 @@ import { parse } from '@babel/parser'; import { extractI18nCallMessages } from './i18n_call'; -import { isI18nTranslateFunction, traverseNodes } from '../utils'; +import { isI18nTranslateFunction, traverseNodes, createParserErrorMessage } from '../utils'; +import { createFailError } from '../../run'; /** * Matches `i18n(...)` in `#{i18n('id', { defaultMessage: 'Message text' })}` @@ -34,7 +35,22 @@ export function* extractPugMessages(buffer) { const expressions = buffer.toString().match(PUG_I18N_REGEX) || []; for (const expression of expressions) { - for (const node of traverseNodes(parse(expression).program.body)) { + let ast; + + try { + ast = parse(expression); + } catch (error) { + if (error instanceof SyntaxError) { + const errorWithContext = createParserErrorMessage(expression, error); + throw createFailError( + `Couldn't parse Pug expression with i18n(...) call:\n${errorWithContext}` + ); + } + + throw error; + } + + for (const node of traverseNodes(ast.program.body)) { if (isI18nTranslateFunction(node)) { yield extractI18nCallMessages(node); break; diff --git a/src/dev/i18n/utils.js b/src/dev/i18n/utils.js index 658c1cbe6717..04ab524d6c9b 100644 --- a/src/dev/i18n/utils.js +++ b/src/dev/i18n/utils.js @@ -27,6 +27,7 @@ import { import fs from 'fs'; import glob from 'glob'; import { promisify } from 'util'; +import chalk from 'chalk'; const ESCAPE_LINE_BREAK_REGEX = /(? ` @@ -44,6 +50,7 @@ describe('i18n utils', () => { test('should remove escaped linebreak', () => { expect(formatJSString('Test\\\n str\\\ning')).toEqual('Test string'); }); + test('should not escape linebreaks', () => { expect( formatJSString(`Text \n with @@ -51,6 +58,7 @@ describe('i18n utils', () => { `) ).toMatchSnapshot(); }); + test('should detect i18n translate function call', () => { let source = i18nTranslateSources[0]; let expressionStatementNode = [...traverseNodes(parse(source).program.body)].find(node => @@ -76,4 +84,24 @@ describe('i18n utils', () => { expect(isPropertyWithKey(objectExpresssionProperty, 'id')).toBe(true); expect(isPropertyWithKey(objectExpresssionProperty, 'not_id')).toBe(false); }); + + test('should create verbose parser error message', () => { + expect.assertions(1); + + const content = `function testFunction() { + const object = { + object: 'with', + semicolon: '->'; + }; + + return object; +} +`; + + try { + parse(content); + } catch (error) { + expect(createParserErrorMessage(content, error)).toMatchSnapshot(); + } + }); });