diff --git a/packages/localize/src/tools/src/extract/duplicates.ts b/packages/localize/src/tools/src/extract/duplicates.ts new file mode 100644 index 0000000000..dfe1eb6f5e --- /dev/null +++ b/packages/localize/src/tools/src/extract/duplicates.ts @@ -0,0 +1,58 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +import {AbsoluteFsPath, FileSystem} from '@angular/compiler-cli/src/ngtsc/file_system'; +import {ɵMessageId, ɵParsedMessage} from '@angular/localize'; + +import {DiagnosticHandlingStrategy, Diagnostics} from '../diagnostics'; +import {serializeLocationPosition} from '../source_file_utils'; + +/** + * Check each of the given `messages` to find those that have the same id but different message + * text. Add diagnostics messages for each of these duplicate messages to the given `diagnostics` + * object (as necessary). + */ +export function checkDuplicateMessages( + fs: FileSystem, messages: ɵParsedMessage[], + duplicateMessageHandling: DiagnosticHandlingStrategy, basePath: AbsoluteFsPath): Diagnostics { + const diagnostics = new Diagnostics(); + if (duplicateMessageHandling === 'ignore') return diagnostics; + + const messageMap = new Map<ɵMessageId, ɵParsedMessage[]>(); + for (const message of messages) { + if (messageMap.has(message.id)) { + messageMap.get(message.id)!.push(message); + } else { + messageMap.set(message.id, [message]); + } + } + + for (const duplicates of messageMap.values()) { + if (duplicates.length <= 1) continue; + if (duplicates.every(message => message.text === duplicates[0].text)) continue; + + const diagnosticMessage = `Duplicate messages with id "${duplicates[0].id}":\n` + + duplicates.map(message => serializeMessage(fs, basePath, message)).join('\n'); + diagnostics.add(duplicateMessageHandling, diagnosticMessage); + } + + return diagnostics; +} + +/** + * Serialize the given `message` object into a string. + */ +function serializeMessage( + fs: FileSystem, basePath: AbsoluteFsPath, message: ɵParsedMessage): string { + if (message.location === undefined) { + return ` - "${message.text}"`; + } else { + const locationFile = fs.relative(basePath, message.location.file); + const locationPosition = serializeLocationPosition(message.location); + return ` - "${message.text}" : ${locationFile}:${locationPosition}`; + } +} diff --git a/packages/localize/src/tools/src/extract/main.ts b/packages/localize/src/tools/src/extract/main.ts index 517146945b..e0a41f0528 100644 --- a/packages/localize/src/tools/src/extract/main.ts +++ b/packages/localize/src/tools/src/extract/main.ts @@ -11,6 +11,10 @@ import {ConsoleLogger, Logger, LogLevel} from '@angular/compiler-cli/src/ngtsc/l import {ɵParsedMessage} from '@angular/localize'; import * as glob from 'glob'; import * as yargs from 'yargs'; + +import {DiagnosticHandlingStrategy, Diagnostics} from '../diagnostics'; + +import {checkDuplicateMessages} from './duplicates'; import {MessageExtractor} from './extraction'; import {TranslationSerializer} from './translation_files/translation_serializer'; import {SimpleJsonTranslationSerializer} from './translation_files/json_translation_serializer'; @@ -68,6 +72,12 @@ if (require.main === module) { describe: 'Whether to use the legacy id format for messages that were extracted from Angular templates.' }) + .option('d', { + alias: 'duplicateMessageHandling', + describe: 'How to handle messages with the same id but different text.', + choices: ['error', 'warning', 'ignore'], + default: 'warning', + }) .strict() .help() .parse(args); @@ -79,6 +89,7 @@ if (require.main === module) { const sourceFilePaths = glob.sync(options['source'], {cwd: rootPath, nodir: true}); const logLevel = options['loglevel'] as (keyof typeof LogLevel) | undefined; const logger = new ConsoleLogger(logLevel ? LogLevel[logLevel] : LogLevel.warn); + const duplicateMessageHandling: DiagnosticHandlingStrategy = options['d']; extractTranslations({ @@ -90,6 +101,7 @@ if (require.main === module) { logger, useSourceMaps: options['useSourceMaps'], useLegacyIds: options['useLegacyIds'], + duplicateMessageHandling, }); } @@ -129,6 +141,10 @@ export interface ExtractTranslationsOptions { * Whether to use the legacy id format for messages that were extracted from Angular templates */ useLegacyIds: boolean; + /** + * How to handle messages with the same id but not the same text. + */ + duplicateMessageHandling: DiagnosticHandlingStrategy; } export function extractTranslations({ @@ -139,22 +155,32 @@ export function extractTranslations({ outputPath: output, logger, useSourceMaps, - useLegacyIds + useLegacyIds, + duplicateMessageHandling, }: ExtractTranslationsOptions) { const fs = getFileSystem(); - const extractor = - new MessageExtractor(fs, logger, {basePath: fs.resolve(rootPath), useSourceMaps}); + const basePath = fs.resolve(rootPath); + const extractor = new MessageExtractor(fs, logger, {basePath, useSourceMaps}); const messages: ɵParsedMessage[] = []; for (const file of sourceFilePaths) { messages.push(...extractor.extractMessages(file)); } + const diagnostics = checkDuplicateMessages(fs, messages, duplicateMessageHandling, basePath); + if (diagnostics.hasErrors) { + throw new Error(diagnostics.formatDiagnostics('Failed to extract messages')); + } + const outputPath = fs.resolve(rootPath, output); const serializer = getSerializer(format, sourceLocale, fs.dirname(outputPath), useLegacyIds); const translationFile = serializer.serialize(messages); fs.ensureDir(fs.dirname(outputPath)); fs.writeFile(outputPath, translationFile); + + if (diagnostics.messages.length) { + logger.warn(diagnostics.formatDiagnostics('Messages extracted with warnings')); + } } export function getSerializer( @@ -175,4 +201,4 @@ export function getSerializer( return new SimpleJsonTranslationSerializer(sourceLocale); } throw new Error(`No translation serializer can handle the provided format: ${format}`); -} \ No newline at end of file +} diff --git a/packages/localize/src/tools/src/source_file_utils.ts b/packages/localize/src/tools/src/source_file_utils.ts index 0a5f58ba0b..ed20ac156a 100644 --- a/packages/localize/src/tools/src/source_file_utils.ts +++ b/packages/localize/src/tools/src/source_file_utils.ts @@ -365,6 +365,13 @@ export function getLocation(startPath: NodePath, endPath?: NodePath): ɵSourceLo }; } +export function serializeLocationPosition(location: ɵSourceLocation): string { + const endLineString = location.end !== undefined && location.end.line !== location.start.line ? + `,${location.end.line + 1}` : + ''; + return `${location.start.line + 1}${endLineString}`; +} + function getFileFromPath(path: NodePath|undefined): AbsoluteFsPath|null { const opts = path?.hub.file.opts; return opts?.filename ? diff --git a/packages/localize/src/tools/test/extract/integration/main_spec.ts b/packages/localize/src/tools/test/extract/integration/main_spec.ts index bb7d19c2ee..7e5fd37fc6 100644 --- a/packages/localize/src/tools/test/extract/integration/main_spec.ts +++ b/packages/localize/src/tools/test/extract/integration/main_spec.ts @@ -7,7 +7,6 @@ */ import {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system'; import {runInEachFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing'; -import {Logger} from '@angular/compiler-cli/src/ngtsc/logging'; import {MockLogger} from '@angular/compiler-cli/src/ngtsc/logging/testing'; import {loadTestDirectory} from '@angular/compiler-cli/test/helpers'; @@ -15,7 +14,7 @@ import {extractTranslations} from '../../../src/extract/main'; runInEachFileSystem(() => { let fs: FileSystem; - let logger: Logger; + let logger: MockLogger; let rootPath: AbsoluteFsPath; let outputPath: AbsoluteFsPath; let sourceFilePath: AbsoluteFsPath; @@ -40,12 +39,13 @@ runInEachFileSystem(() => { extractTranslations({ rootPath, sourceLocale: 'en', - sourceFilePaths: [], + sourceFilePaths: [textFile1, textFile2], format: 'json', outputPath, logger, useSourceMaps: false, useLegacyIds: false, + duplicateMessageHandling: 'ignore', }); expect(fs.readFile(outputPath)).toEqual([ `{`, @@ -65,6 +65,7 @@ runInEachFileSystem(() => { logger, useSourceMaps: false, useLegacyIds: false, + duplicateMessageHandling: 'ignore', }); expect(fs.readFile(outputPath)).toEqual([ `{`, @@ -87,6 +88,7 @@ runInEachFileSystem(() => { logger, useSourceMaps: false, useLegacyIds: false, + duplicateMessageHandling: 'ignore', }); expect(fs.readFile(outputPath)).toEqual([ ``, @@ -128,6 +130,7 @@ runInEachFileSystem(() => { logger, useSourceMaps: false, useLegacyIds: false, + duplicateMessageHandling: 'ignore', }); expect(fs.readFile(outputPath)).toEqual([ ``, @@ -164,6 +167,7 @@ runInEachFileSystem(() => { logger, useSourceMaps: false, useLegacyIds: false, + duplicateMessageHandling: 'ignore', }); expect(fs.readFile(outputPath)).toEqual([ ``, @@ -197,6 +201,7 @@ runInEachFileSystem(() => { logger, useSourceMaps: true, useLegacyIds: false, + duplicateMessageHandling: 'ignore', }); expect(fs.readFile(outputPath)).toEqual([ ``, @@ -224,5 +229,82 @@ runInEachFileSystem(() => { ].join('\n')); }); } + + describe('[duplicateMessageHandling]', () => { + it('should throw if set to "error"', () => { + expect(() => extractTranslations({ + rootPath, + sourceLocale: 'en-GB', + sourceFilePaths: [fs.resolve(rootPath, 'test_files/duplicate.js')], + format: 'json', + outputPath, + logger, + useSourceMaps: false, + useLegacyIds: false, + duplicateMessageHandling: 'error', + })) + .toThrowError( + `Failed to extract messages\n` + + `ERRORS:\n` + + ` - Duplicate messages with id "message-2":\n` + + ` - "message contents" : test_files/duplicate.js:6\n` + + ` - "different message contents" : test_files/duplicate.js:7`); + expect(fs.exists(outputPath)).toBe(false); + }); + + it('should log to the logger if set to "warning"', () => { + extractTranslations({ + rootPath, + sourceLocale: 'en-GB', + sourceFilePaths: [fs.resolve(rootPath, 'test_files/duplicate.js')], + format: 'json', + outputPath, + logger, + useSourceMaps: false, + useLegacyIds: false, + duplicateMessageHandling: 'warning', + }); + expect(logger.logs.warn).toEqual([ + ['Messages extracted with warnings\n' + + `WARNINGS:\n` + + ` - Duplicate messages with id "message-2":\n` + + ` - "message contents" : test_files/duplicate.js:6\n` + + ` - "different message contents" : test_files/duplicate.js:7`] + ]); + expect(fs.readFile(outputPath)).toEqual([ + `{`, + ` "locale": "en-GB",`, + ` "translations": {`, + ` "message-1": "message {$PH} contents",`, + ` "message-2": "different message contents"`, + ` }`, + `}`, + ].join('\n')); + }); + + it('should not log to the logger if set to "ignore"', () => { + extractTranslations({ + rootPath, + sourceLocale: 'en-GB', + sourceFilePaths: [fs.resolve(rootPath, 'test_files/duplicate.js')], + format: 'json', + outputPath, + logger, + useSourceMaps: false, + useLegacyIds: false, + duplicateMessageHandling: 'ignore', + }); + expect(logger.logs.warn).toEqual([]); + expect(fs.readFile(outputPath)).toEqual([ + `{`, + ` "locale": "en-GB",`, + ` "translations": {`, + ` "message-1": "message {$PH} contents",`, + ` "message-2": "different message contents"`, + ` }`, + `}`, + ].join('\n')); + }); + }); }); }); diff --git a/packages/localize/src/tools/test/extract/integration/test_files/duplicate.js b/packages/localize/src/tools/test/extract/integration/test_files/duplicate.js new file mode 100644 index 0000000000..b25f64566d --- /dev/null +++ b/packages/localize/src/tools/test/extract/integration/test_files/duplicate.js @@ -0,0 +1,7 @@ +// Different interpolation values will not affect message text +const a = $localize`:@@message-1:message ${10} contents`; +const b = $localize`:@@message-1:message ${20} contents`; + +// But different actual text content will +const c = $localize`:@@message-2:message contents`; +const d = $localize`:@@message-2:different message contents`; \ No newline at end of file