honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

feat(localize): allow duplicate messages to be handled during extraction (#38082) 

Previously, the i18n message extractor just quietly ignored messages that
it extracted that had the same id. It can be helpful to identify these
to track down messages that have the same id but different message text.

Now the messages are checked for duplicate ids with different message text.
Any that are found can be reported based on the new `--duplicateMessageHandling`
command line option (or `duplicateMessageHandling` API options property).

* "ignore" - no action is taken
* "warning" - a diagnostic warning is written to the logger
* "error" - the extractor throws an error and exits

Fixes #38077

PR Close #38082
This commit is contained in:
Pete Bacon Darwin 2020-07-15 20:08:18 +01:00 committed by Andrew Kushnir
parent 56dd3e77ac
commit cf9a47ba53
5 changed files with 187 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

58
packages/localize/src/tools/src/extract/duplicates.ts Normal file
View File

@ -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}`;
}
}

34
packages/localize/src/tools/src/extract/main.ts
View File

@ -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}`);
}
}

7
packages/localize/src/tools/src/source_file_utils.ts
View File

@ -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 ?

88
packages/localize/src/tools/test/extract/integration/main_spec.ts
View File

@ -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([
`<?xml version="1.0" encoding="UTF-8" ?>`,
@ -128,6 +130,7 @@ runInEachFileSystem(() => {
logger,
useSourceMaps: false,
useLegacyIds: false,
duplicateMessageHandling: 'ignore',
});
expect(fs.readFile(outputPath)).toEqual([
`<?xml version="1.0" encoding="UTF-8" ?>`,
@ -164,6 +167,7 @@ runInEachFileSystem(() => {
logger,
useSourceMaps: false,
useLegacyIds: false,
duplicateMessageHandling: 'ignore',
});
expect(fs.readFile(outputPath)).toEqual([
`<?xml version="1.0" encoding="UTF-8" ?>`,
@ -197,6 +201,7 @@ runInEachFileSystem(() => {
logger,
useSourceMaps: true,
useLegacyIds: false,
duplicateMessageHandling: 'ignore',
});
expect(fs.readFile(outputPath)).toEqual([
`<?xml version="1.0" encoding="UTF-8" ?>`,
@ -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'));
});
});
});
});

7
packages/localize/src/tools/test/extract/integration/test_files/duplicate.js Normal file
View File

@ -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`;