From 885b7a5069ac00b9d204adbd0db266b1e17b19b3 Mon Sep 17 00:00:00 2001 From: "Phyks (Lucas Verney)" Date: Thu, 3 Aug 2017 20:12:03 +0200 Subject: [PATCH] Get code fragment with keys Update `extractCode` function to output the location of the extracted keys as well as the translation keys. This would close #31. --- src/extractFromCode.js | 13 +- src/extractFromCode.spec.js | 474 +++++++++++++++++++++++-- src/extractFromCodeFixtures/comment.js | 1 + src/extractFromFiles.js | 11 +- src/extractFromFiles.spec.js | 147 +++++++- src/findMissing.js | 4 +- src/findMissing.spec.js | 26 +- src/findUnused.js | 2 + src/findUnused.spec.js | 16 +- src/forbidDynamic.js | 4 +- src/forbidDynamic.spec.js | 18 +- src/mergeMessagesWithPO.js | 2 + 12 files changed, 659 insertions(+), 59 deletions(-) diff --git a/src/extractFromCode.js b/src/extractFromCode.js index 0b41108..4f43bba 100644 --- a/src/extractFromCode.js +++ b/src/extractFromCode.js @@ -1,6 +1,5 @@ import { parse } from 'babylon'; import traverse from 'babel-traverse'; -import { uniq } from './utils'; const noInformationTypes = [ 'CallExpression', @@ -64,7 +63,10 @@ export default function extractFromCode(code, options = {}) { ast.comments.forEach((comment) => { let match = commentRegExp.exec(comment.value); if (match) { - keys.push(match[1].trim()); + keys.push({ + key: match[1].trim(), + loc: comment.loc, + }); } // Check for ignored lines @@ -98,11 +100,14 @@ export default function extractFromCode(code, options = {}) { const key = getKey(node.arguments[0]); if (key) { - keys.push(key); + keys.push({ + key, + loc: node.loc, + }); } } }, }); - return uniq(keys); + return keys; } diff --git a/src/extractFromCode.spec.js b/src/extractFromCode.spec.js index 3b1ac38..145d3c5 100644 --- a/src/extractFromCode.spec.js +++ b/src/extractFromCode.spec.js @@ -15,11 +15,84 @@ describe('#extractFromCode()', () => { const keys = extractFromCode(getCode('es5.js')); assert.deepEqual([ - 'follow', - 'followed', - 'unfollowed', - 'unfollow', - 'following', + { + key: 'follow', + loc: { + end: { + column: 34, + line: 29, + }, + start: { + column: 20, + line: 29, + }, + }, + }, + { + key: 'follow', + loc: { + end: { + column: 35, + line: 30, + }, + start: { + column: 21, + line: 30, + }, + }, + }, + { + key: 'followed', + loc: { + end: { + column: 63, + line: 54, + }, + start: { + column: 47, + line: 54, + }, + }, + }, + { + key: 'unfollowed', + loc: { + end: { + column: 65, + line: 68, + }, + start: { + column: 47, + line: 68, + }, + }, + }, + { + key: 'unfollow', + loc: { + end: { + column: 63, + line: 145, + }, + start: { + column: 47, + line: 145, + }, + }, + }, + { + key: 'following', + loc: { + end: { + column: 83, + line: 145, + }, + start: { + column: 66, + line: 145, + }, + }, + }, ], keys, 'Should work with ES5 code.'); }); @@ -27,10 +100,58 @@ describe('#extractFromCode()', () => { const keys = extractFromCode(getCode('es6.js')); assert.deepEqual([ - 'reset', - 'revert', - 'sweep', - 'commit', + { + key: 'reset', + loc: { + end: { + column: 26, + line: 187, + }, + start: { + column: 13, + line: 187, + }, + }, + }, + { + key: 'revert', + loc: { + end: { + column: 27, + line: 190, + }, + start: { + column: 13, + line: 190, + }, + }, + }, + { + key: 'sweep', + loc: { + end: { + column: 26, + line: 197, + }, + start: { + column: 13, + line: 197, + }, + }, + }, + { + key: 'commit', + loc: { + end: { + column: 27, + line: 200, + }, + start: { + column: 13, + line: 200, + }, + }, + }, ], keys, 'Should work with ES6 code.'); }); @@ -40,7 +161,19 @@ describe('#extractFromCode()', () => { }); assert.deepEqual([ - 'this_is_a_custom_marker', + { + key: 'this_is_a_custom_marker', + loc: { + end: { + column: 29, + line: 5, + }, + start: { + column: 0, + line: 5, + }, + }, + }, ], keys, 'Should take into account the marker option.'); }); @@ -50,7 +183,19 @@ describe('#extractFromCode()', () => { }); assert.deepEqual([ - 'this_is_a_custom_marker', + { + key: 'this_is_a_custom_marker', + loc: { + end: { + column: 37, + line: 5, + }, + start: { + column: 0, + line: 5, + }, + }, + }, ], keys, 'Should take into account the marker option.'); }); @@ -58,7 +203,19 @@ describe('#extractFromCode()', () => { const keys = extractFromCode(getCode('many-args.js')); assert.deepEqual([ - 'hello_username', + { + key: 'hello_username', + loc: { + end: { + column: 2, + line: 11, + }, + start: { + column: 0, + line: 9, + }, + }, + }, ], keys, 'The second argument shoudn\'t have any impact.'); }); @@ -66,7 +223,32 @@ describe('#extractFromCode()', () => { const keys = extractFromCode(getCode('duplicated.js')); assert.deepEqual([ - 'key', + { + key: 'key', + loc: { + end: { + column: 11, + line: 5, + }, + start: { + column: 0, + line: 5, + }, + }, + }, + { + key: 'key', + loc: { + end: { + column: 11, + line: 6, + }, + start: { + column: 0, + line: 6, + }, + }, + }, ], keys, 'Should return only one key.'); }); @@ -74,7 +256,19 @@ describe('#extractFromCode()', () => { const keys = extractFromCode(getCode('template.js')); assert.deepEqual([ - 'key', + { + key: 'key', + loc: { + end: { + column: 11, + line: 5, + }, + start: { + column: 0, + line: 5, + }, + }, + }, ], keys, 'Should return only one key.'); }); @@ -82,7 +276,19 @@ describe('#extractFromCode()', () => { const keys = extractFromCode(getCode('function.js')); assert.deepEqual([ - '*', + { + key: '*', + loc: { + end: { + column: 21, + line: 7, + }, + start: { + column: 0, + line: 7, + }, + }, + }, ], keys, 'Should return one key.'); }); @@ -90,7 +296,19 @@ describe('#extractFromCode()', () => { const keys = extractFromCode(getCode('memberExpression.js')); assert.deepEqual([ - '*', + { + key: '*', + loc: { + end: { + column: 24, + line: 9, + }, + start: { + column: 0, + line: 9, + }, + }, + }, ], keys, 'Should return one key.'); }); @@ -98,7 +316,19 @@ describe('#extractFromCode()', () => { const keys = extractFromCode(getCode('dynamicImport.js')); assert.deepEqual([ - 'key', + { + key: 'key', + loc: { + end: { + column: 11, + line: 7, + }, + start: { + column: 0, + line: 7, + }, + }, + }, ], keys, 'Should return only one key.'); }); }); @@ -108,18 +338,119 @@ describe('#extractFromCode()', () => { it('should return the right key with a concat', () => { keys = extractFromCode(getCode('dynamicConcat.js')); + + assert.deepEqual([ + { + key: 'key.*', + loc: { + end: { + column: 18, + line: 8, + }, + start: { + column: 0, + line: 8, + }, + }, + }, + { + key: 'key.*.bar', + loc: { + end: { + column: 27, + line: 11, + }, + start: { + column: 0, + line: 11, + }, + }, + }, + { + key: '*.bar', + loc: { + end: { + column: 18, + line: 14, + }, + start: { + column: 0, + line: 14, + }, + }, + }, + { + key: '*', + loc: { + end: { + column: 9, + line: 17, + }, + start: { + column: 0, + line: 17, + }, + }, + }, + ], keys, 'Should return the right key.'); }); it('should return the right key with a template', () => { keys = extractFromCode(getCode('dynamicTemplate.js')); - }); - afterEach(() => { assert.deepEqual([ - 'key.*', - 'key.*.bar', - '*.bar', - '*', + { + key: 'key.*', + loc: { + end: { + column: 18, + line: 8, + }, + start: { + column: 0, + line: 8, + }, + }, + }, + { + key: 'key.*.bar', + loc: { + end: { + column: 22, + line: 11, + }, + start: { + column: 0, + line: 11, + }, + }, + }, + { + key: '*.bar', + loc: { + end: { + column: 18, + line: 14, + }, + start: { + column: 0, + line: 14, + }, + }, + }, + { + key: '*', + loc: { + end: { + column: 14, + line: 17, + }, + start: { + column: 0, + line: 17, + }, + }, + }, ], keys, 'Should return the right key.'); }); }); @@ -129,10 +460,71 @@ describe('#extractFromCode()', () => { const keys = extractFromCode(getCode('comment.js')); assert.deepEqual([ - 'foo.bar1', - 'foo.bar2', - 'foo spaced1', - 'foo spaced2', + { + key: 'foo.bar1', + loc: { + end: { + column: 27, + line: 3, + }, + start: { + column: 0, + line: 3, + }, + }, + }, + { + key: 'foo.bar2', + loc: { + end: { + column: 24, + line: 4, + }, + start: { + column: 0, + line: 4, + }, + }, + }, + { + key: 'foo.bar2', + loc: { + end: { + column: 27, + line: 5, + }, + start: { + column: 0, + line: 5, + }, + }, + }, + { + key: 'foo spaced1', + loc: { + end: { + column: 30, + line: 6, + }, + start: { + column: 0, + line: 6, + }, + }, + }, + { + key: 'foo spaced2', + loc: { + end: { + column: 27, + line: 7, + }, + start: { + column: 0, + line: 7, + }, + }, + }, ], keys, 'Should return the good keys.'); }); }); @@ -142,8 +534,32 @@ describe('#extractFromCode()', () => { const keys = extractFromCode(getCode('disable-comment.js')); assert.deepEqual([ - 'foo.bar1', - 'foo.bar3', + { + key: 'foo.bar1', + loc: { + end: { + column: 16, + line: 4, + }, + start: { + column: 0, + line: 4, + }, + }, + }, + { + key: 'foo.bar3', + loc: { + end: { + column: 16, + line: 11, + }, + start: { + column: 0, + line: 11, + }, + }, + }, ], keys, 'Should return the good keys.'); }); }); diff --git a/src/extractFromCodeFixtures/comment.js b/src/extractFromCodeFixtures/comment.js index 33e9683..7bc6a4e 100644 --- a/src/extractFromCodeFixtures/comment.js +++ b/src/extractFromCodeFixtures/comment.js @@ -2,5 +2,6 @@ /* i18n-extract foo.bar1 */ // i18n-extract foo.bar2 +/* i18n-extract foo.bar2 */ /* i18n-extract foo spaced1 */ // i18n-extract foo spaced2 diff --git a/src/extractFromFiles.js b/src/extractFromFiles.js index 8d69068..2d0649a 100644 --- a/src/extractFromFiles.js +++ b/src/extractFromFiles.js @@ -1,10 +1,9 @@ import glob from 'glob'; import fs from 'fs'; -import { uniq } from './utils'; import extractFromCode from './extractFromCode'; export default function extractFromFiles(filenames, options) { - let keys = []; + const keys = []; // filenames should be an array if (typeof filenames === 'string') { @@ -21,8 +20,12 @@ export default function extractFromFiles(filenames, options) { toScan.forEach((filename) => { const code = fs.readFileSync(filename, 'utf8'); - keys = keys.concat(extractFromCode(code, options)); + const extractedKeys = extractFromCode(code, options); + extractedKeys.forEach((keyObj) => { + keyObj.file = filename; + keys.push(keyObj); + }); }); - return uniq(keys); + return keys; } diff --git a/src/extractFromFiles.spec.js b/src/extractFromFiles.spec.js index 44b824b..415ef2f 100644 --- a/src/extractFromFiles.spec.js +++ b/src/extractFromFiles.spec.js @@ -8,9 +8,62 @@ describe('#extractFromFiles()', () => { const keys = extractFromFiles('src/extractFromFilesFixture/*View.js'); assert.deepEqual([ - 'key1', - 'key2', - 'key3', + { + key: 'key1', + file: 'src/extractFromFilesFixture/AccoutView.js', + loc: { + end: { + column: 12, + line: 5, + }, + start: { + column: 0, + line: 5, + }, + }, + }, + { + key: 'key2', + file: 'src/extractFromFilesFixture/AccoutView.js', + loc: { + end: { + column: 12, + line: 6, + }, + start: { + column: 0, + line: 6, + }, + }, + }, + { + key: 'key3', + file: 'src/extractFromFilesFixture/ExpenseView.js', + loc: { + end: { + column: 12, + line: 5, + }, + start: { + column: 0, + line: 5, + }, + }, + }, + { + key: 'key1', + file: 'src/extractFromFilesFixture/ExpenseView.js', + loc: { + end: { + column: 12, + line: 6, + }, + start: { + column: 0, + line: 6, + }, + }, + }, ], keys, 'Should find all the key without duplication'); }); @@ -21,10 +74,90 @@ describe('#extractFromFiles()', () => { ]); assert.deepEqual([ - 'key3', - 'key4', - 'key1', - 'key2', + { + key: 'key3', + file: 'src/extractFromFilesFixture/MemberView.jsx', + loc: { + end: { + column: 12, + line: 5, + }, + start: { + column: 0, + line: 5, + }, + }, + }, + { + key: 'key4', + file: 'src/extractFromFilesFixture/MemberView.jsx', + loc: { + end: { + column: 12, + line: 6, + }, + start: { + column: 0, + line: 6, + }, + }, + }, + { + key: 'key1', + file: 'src/extractFromFilesFixture/AccoutView.js', + loc: { + end: { + column: 12, + line: 5, + }, + start: { + column: 0, + line: 5, + }, + }, + }, + { + key: 'key2', + file: 'src/extractFromFilesFixture/AccoutView.js', + loc: { + end: { + column: 12, + line: 6, + }, + start: { + column: 0, + line: 6, + }, + }, + }, + { + key: 'key3', + file: 'src/extractFromFilesFixture/ExpenseView.js', + loc: { + end: { + column: 12, + line: 5, + }, + start: { + column: 0, + line: 5, + }, + }, + }, + { + key: 'key1', + file: 'src/extractFromFilesFixture/ExpenseView.js', + loc: { + end: { + column: 12, + line: 6, + }, + start: { + column: 0, + line: 6, + }, + }, + }, ], keys, 'Should work with an array as first parameter'); }); }); diff --git a/src/findMissing.js b/src/findMissing.js index 6788ed0..582782e 100644 --- a/src/findMissing.js +++ b/src/findMissing.js @@ -18,10 +18,10 @@ export default function findMissing(locale, keysUsed) { const reports = []; keysUsed.forEach((keyUsed) => { - if (isMissing(locale, keyUsed)) { + if (isMissing(locale, keyUsed.key)) { reports.push({ type: MISSING, - key: keyUsed, + ...keyUsed, }); } }); diff --git a/src/findMissing.spec.js b/src/findMissing.spec.js index 31911d4..b404f93 100644 --- a/src/findMissing.spec.js +++ b/src/findMissing.spec.js @@ -9,12 +9,17 @@ describe('#findMissing()', () => { const missing = findMissing({ key1: 'Key 1', key2: 'Key 2', - }, ['key1', 'key2', 'key3']); + }, [ + { key: 'key1', loc: null }, + { key: 'key2', loc: null }, + { key: 'key3', loc: null }, + ]); assert.deepEqual([ { type: 'MISSING', key: 'key3', + loc: null, }, ], missing, 'Should report one missing key.'); }); @@ -27,7 +32,12 @@ describe('#findMissing()', () => { 'foo.key2': 'Key 2', bar: 'Key 3', 'foo.key.bar': 'Key 4', - }, ['foo.*', '*.key1', '*', 'foo.*.bar']); + }, [ + { key: 'foo.*', loc: null }, + { key: '*.key1', loc: null }, + { key: '*', loc: null }, + { key: 'foo.*.bar', loc: null }, + ]); assert.deepEqual([], missing, 'Should report zero missing key.'); }); @@ -37,20 +47,27 @@ describe('#findMissing()', () => { 'bar.key1': 'Key 1', 'bar.key.foo': 'Key 1', foo: 'Key 2', - }, ['foo.*', '*.key2', 'bar.*.foo1']); + }, [ + { key: 'foo.*', loc: null }, + { key: '*.key2', loc: null }, + { key: 'bar.*.foo1', loc: null }, + ]); assert.deepEqual([ { key: 'foo.*', type: 'MISSING', + loc: null, }, { key: '*.key2', type: 'MISSING', + loc: null, }, { key: 'bar.*.foo1', type: 'MISSING', + loc: null, }, ], missing, 'Should report three missing key.'); }); @@ -58,12 +75,13 @@ describe('#findMissing()', () => { it('should do an exact match even with dynamic keys', () => { const missing = findMissing({ 'bar.key.foo': 'Key 1', - }, ['key.*']); + }, [{ key: 'key.*', loc: null }]); assert.deepEqual([ { key: 'key.*', type: 'MISSING', + loc: null, }, ], missing, 'Should report one missing key.'); }); diff --git a/src/findUnused.js b/src/findUnused.js index 58f1d0c..5a3ffc5 100644 --- a/src/findUnused.js +++ b/src/findUnused.js @@ -6,6 +6,8 @@ export default function findUnused(locale, keysUsed) { const keysUsedHash = {}; keysUsed.forEach((keyUsed) => { + keyUsed = keyUsed.key; + // Dynamic key if (keyUsed.includes('*')) { const regExp = new RegExp(`^${keyUsed.replace('*', '(.+)')}$`); diff --git a/src/findUnused.spec.js b/src/findUnused.spec.js index 407477a..de9fe88 100644 --- a/src/findUnused.spec.js +++ b/src/findUnused.spec.js @@ -9,7 +9,10 @@ describe('#findUnused()', () => { const unused = findUnused({ key1: 'Key 1', key2: 'Key 2', - }, ['key1', 'key2']); + }, [ + { key: 'key1', loc: null }, + { key: 'key2', loc: null }, + ]); assert.deepEqual([], unused, 'Should report zero unused key.'); }); @@ -19,7 +22,10 @@ describe('#findUnused()', () => { key1: 'Key 1', key2: 'Key 2', key3: 'Key 3', - }, ['key1', 'key2']); + }, [ + { key: 'key1', loc: null }, + { key: 'key2', loc: null }, + ]); assert.deepEqual([ { @@ -35,7 +41,7 @@ describe('#findUnused()', () => { const unused = findUnused({ 'foo.key1': 'Key 1', 'foo.key2': 'Key 2', - }, ['foo.*']); + }, [{ key: 'foo.*', loc: null }]); assert.deepEqual([], unused, 'Should report zero unused key.'); }); @@ -45,7 +51,7 @@ describe('#findUnused()', () => { 'foo.key1': 'Key 1', 'foo.key2': 'Key 2', key3: 'Key 3', - }, ['foo.*']); + }, [{ key: 'foo.*', loc: null }]); assert.deepEqual([ { @@ -58,7 +64,7 @@ describe('#findUnused()', () => { it('should do an exact match even with dynamic keys', () => { const missing = findUnused({ 'bar.key.foo': 'Key 1', - }, ['key.*']); + }, [{ key: 'key.*', loc: null }]); assert.deepEqual([ { diff --git a/src/forbidDynamic.js b/src/forbidDynamic.js index 9e9564f..73666d1 100644 --- a/src/forbidDynamic.js +++ b/src/forbidDynamic.js @@ -5,10 +5,10 @@ export default function forbidDynamic(locale, keysUsed) { keysUsed.forEach((keyUsed) => { // Dynamic key - if (keyUsed.includes('*')) { + if (keyUsed.key.includes('*')) { reports.push({ type: FORBID_DYNAMIC, - key: keyUsed, + ...keyUsed, }); } }); diff --git a/src/forbidDynamic.spec.js b/src/forbidDynamic.spec.js index 0848cd2..0c91261 100644 --- a/src/forbidDynamic.spec.js +++ b/src/forbidDynamic.spec.js @@ -6,22 +6,36 @@ import forbidDynamic from './forbidDynamic.js'; describe('#forbidDynamic()', () => { describe('simple case', () => { it('should report forbidden dynamic key', () => { - const missing = forbidDynamic({}, ['key1.*', '*.key2']); + const missing = forbidDynamic( + {}, + [ + { key: 'key1.*', loc: null }, + { key: '*.key2', loc: null }, + ], + ); assert.deepEqual([ { type: 'FORBID_DYNAMIC', key: 'key1.*', + loc: null, }, { type: 'FORBID_DYNAMIC', key: '*.key2', + loc: null, }, ], missing, 'Should report forbidden dynamic key.'); }); it('should no report static key', () => { - const missing = forbidDynamic({}, ['key1', 'key2']); + const missing = forbidDynamic( + {}, + [ + { key: 'key1', loc: null }, + { key: 'key2', loc: null }, + ], + ); assert.deepEqual([], missing, 'Should not report static key.'); }); diff --git a/src/mergeMessagesWithPO.js b/src/mergeMessagesWithPO.js index 4cedb35..a126920 100644 --- a/src/mergeMessagesWithPO.js +++ b/src/mergeMessagesWithPO.js @@ -12,6 +12,8 @@ export default function mergeMessagesWithPO(messages, poFileName, outputFileName let messagesReused = 0; messages.forEach((message) => { + message = message.key; + // The translation already exist if (poTransalations[message]) { messagesReused += 1;