From be426b8a2f7c7d95f5d193adb0f5cf20fedc4693 Mon Sep 17 00:00:00 2001 From: Shane Exterkamp Date: Mon, 22 Jul 2019 13:55:58 -0700 Subject: [PATCH] reorg example logic to directIcu replacement. --- .../scripts/i18n/collect-strings.js | 35 +++++++------- .../test/scripts/i18n/collect-strings-test.js | 47 +++++++++---------- 2 files changed, 37 insertions(+), 45 deletions(-) diff --git a/lighthouse-core/scripts/i18n/collect-strings.js b/lighthouse-core/scripts/i18n/collect-strings.js index 5c90dc714a90..b0178c6af087 100644 --- a/lighthouse-core/scripts/i18n/collect-strings.js +++ b/lighthouse-core/scripts/i18n/collect-strings.js @@ -32,7 +32,7 @@ const ignoredPathComponents = [ // @ts-ignore - @types/esprima lacks all of these function computeDescription(ast, property, value, startRange) { const endRange = property.range[0]; - const findIcu = /\{(\w+)\}/g; + for (const comment of ast.comments || []) { if (comment.range[0] < startRange) continue; if (comment.range[0] > endRange) continue; @@ -52,20 +52,9 @@ function computeDescription(ast, property, value, startRange) { if (tagName === 'description') { description = message; } else if (tagName === 'example') { - // Make sure the ICU var exists in string - if (!value.includes(placeholder)) { - throw Error(`Example missing ICU replacement in message "${message}"`); - } examples[placeholder.substring(1, placeholder.length - 1)] = message; } } - // Make sure all ICU vars have examples - while ((matches = findIcu.exec(value)) !== null) { - const varName = matches[1]; - if (!examples[varName]) { - throw Error(`Variable '${varName}' is missing example comment in message "${value}"`); - } - } // Make sure description is not empty if (description.length === 0) throw Error(`Empty @description for message "${value}"`); @@ -74,12 +63,6 @@ function computeDescription(ast, property, value, startRange) { const description = comment.value.replace('*', '').trim(); - // Make sure all ICU vars have examples - if (value.match(findIcu)) { - throw Error(`Variable '${value.match(/.*\{(\w+)\}.*/)[1]}' ` + - `is missing example comment in message "${value}"`); - } - // Make sure description is not empty if (description.length === 0) throw Error(`Empty description for message "${value}"`); @@ -291,8 +274,22 @@ function _processPlaceholderCustomFormattedIcu(icu) { function _processPlaceholderDirectIcu(icu, examples) { let tempMessage = icu.message; let idx = 0; + const findIcu = /\{(\w+)\}/g; + + let matches; + // Make sure all ICU vars have examples + while ((matches = findIcu.exec(tempMessage)) !== null) { + const varName = matches[1]; + if (!examples[varName]) { + throw Error(`Variable '${varName}' is missing example comment in message "${tempMessage}"`); + } + } + for (const [key, value] of Object.entries(examples)) { - if (!icu.message.includes(`{${key}}`)) continue; + // Make sure all examples have ICU vars + if (!icu.message.includes(`{${key}}`)) { + throw Error(`Example '${key}' provided, but has not corresponding ICU replacement in message "${icu.message}"`); + } const eName = `ICU_${idx++}`; tempMessage = tempMessage.replace(`{${key}}`, `$${eName}$`); diff --git a/lighthouse-core/test/scripts/i18n/collect-strings-test.js b/lighthouse-core/test/scripts/i18n/collect-strings-test.js index 136da9b16f5f..b66247001e77 100644 --- a/lighthouse-core/test/scripts/i18n/collect-strings-test.js +++ b/lighthouse-core/test/scripts/i18n/collect-strings-test.js @@ -128,25 +128,28 @@ describe('Compute Description', () => { expect(res.examples['variable']).toBe('Variable example.'); }); - it('errors when example given without variable', () => { + it('collects complex description with multiple examples', () => { const justUIStrings = `const UIStrings = { /** * @description Tagged description for Hello World. * @example {variable} Variable example. + * @example {variable2} Variable2 example. */ - message: 'Hello World', + message: 'Hello World {variable} {variable2}', };`; const ast = esprima.parse(justUIStrings, {comment: true, range: true}); const stmt = ast.body[0]; const prop = stmt.declarations[0].init.properties[0]; - expect(() => collect.computeDescription(ast, prop, 'Hello World', 0)) - .toThrow(/Example missing ICU replacement in message "Variable example."/); + const res = collect.computeDescription(ast, prop, 'Hello World {variable} {variable2}', 0); + expect(res.description).toBe('Tagged description for Hello World.'); + expect(res.examples['variable']).toBe('Variable example.'); + expect(res.examples['variable2']).toBe('Variable2 example.'); }); - it('errors when variable has no example', () => { + it('does not throw when no example for ICU', () => { const justUIStrings = `const UIStrings = { /** @@ -159,23 +162,9 @@ describe('Compute Description', () => { const stmt = ast.body[0]; const prop = stmt.declarations[0].init.properties[0]; - expect(() => collect.computeDescription(ast, prop, 'Hello World {variable}', 0)).toThrow( - /Variable 'variable' is missing example comment in message "Hello World {variable}"/); - }); - - it('errors when variable has no example, with basic description', () => { - const justUIStrings = - `const UIStrings = { - /** Tagged description for Hello World. */ - message: 'Hello World {variable}', - };`; - - const ast = esprima.parse(justUIStrings, {comment: true, range: true}); - - const stmt = ast.body[0]; - const prop = stmt.declarations[0].init.properties[0]; - expect(() => collect.computeDescription(ast, prop, 'Hello World {variable}', 0)).toThrow( - /Variable 'variable' is missing example comment in message "Hello World {variable}"/); + const res = collect.computeDescription(ast, prop, 'Hello World {variable}', 0); + expect(res.description).toBe('Tagged description for Hello World.'); + expect(res.examples).toEqual({}); }); }); @@ -353,11 +342,17 @@ describe('Convert Message to Placeholder', () => { }); }); - it('ignores direct ICU with no examples', () => { + it('errors when example given without variable', () => { + const message = 'Hello name.'; + expect(() => collect.convertMessageToPlaceholders(message, {name: 'Mary'})) + // eslint-disable-next-line max-len + .toThrow(/Example 'name' provided, but has not corresponding ICU replacement in message "Hello name."/); + }); + + it('errors when direct ICU has no examples', () => { const message = 'Hello {name}.'; - const res = collect.convertMessageToPlaceholders(message, undefined); - expect(res.message).toBe(message); - expect(res.placeholders).toEqual({}); + expect(() => collect.convertMessageToPlaceholders(message, undefined)).toThrow( + /Variable 'name' is missing example comment in message "Hello {name}."/); }); });