Skip to content

Commit

Permalink
refactor(localize): remove deprecated canParse method from Translat…
Browse files Browse the repository at this point in the history
…ionParsers (#47275)

This change removed the deprecated `canParse` method from all the TranslationParsers.

BREAKING CHANGE:

- `canParse` method has been removed from all translation parsers in `@angular/localize/tools`. `analyze` should be used instead.
-  the `hint` parameter in the`parse` methods is now mandatory.

PR Close #47275
  • Loading branch information
alan-agius4 authored and AndrewKushnir committed Sep 6, 2022
1 parent 93d7d91 commit d36fd3d
Show file tree
Hide file tree
Showing 14 changed files with 1,764 additions and 1,928 deletions.
16 changes: 3 additions & 13 deletions goldens/public-api/localize/tools/index.md
Expand Up @@ -23,8 +23,6 @@ import { ɵSourceMessage } from '@angular/localize';
export class ArbTranslationParser implements TranslationParser<ArbJsonObject> {
// (undocumented)
analyze(_filePath: string, contents: string): ParseAnalysis<ArbJsonObject>;
// @deprecated (undocumented)
canParse(filePath: string, contents: string): ArbJsonObject | false;
// (undocumented)
parse(_filePath: string, contents: string, arb?: ArbJsonObject): ParsedTranslationBundle;
}
Expand Down Expand Up @@ -96,8 +94,6 @@ export class MessageExtractor {
export class SimpleJsonTranslationParser implements TranslationParser<SimpleJsonFile> {
// (undocumented)
analyze(filePath: string, contents: string): ParseAnalysis<SimpleJsonFile>;
// @deprecated (undocumented)
canParse(filePath: string, contents: string): SimpleJsonFile | false;
// (undocumented)
parse(_filePath: string, contents: string, json?: SimpleJsonFile): ParsedTranslationBundle;
}
Expand Down Expand Up @@ -131,10 +127,8 @@ export function unwrapSubstitutionsFromLocalizeCall(call: NodePath<t.CallExpress
export class Xliff1TranslationParser implements TranslationParser<XmlTranslationParserHint> {
// (undocumented)
analyze(filePath: string, contents: string): ParseAnalysis<XmlTranslationParserHint>;
// @deprecated (undocumented)
canParse(filePath: string, contents: string): XmlTranslationParserHint | false;
// (undocumented)
parse(filePath: string, contents: string, hint?: XmlTranslationParserHint): ParsedTranslationBundle;
parse(filePath: string, contents: string, hint: XmlTranslationParserHint): ParsedTranslationBundle;
}

// @public
Expand All @@ -148,10 +142,8 @@ export class Xliff1TranslationSerializer implements TranslationSerializer {
export class Xliff2TranslationParser implements TranslationParser<XmlTranslationParserHint> {
// (undocumented)
analyze(filePath: string, contents: string): ParseAnalysis<XmlTranslationParserHint>;
// @deprecated (undocumented)
canParse(filePath: string, contents: string): XmlTranslationParserHint | false;
// (undocumented)
parse(filePath: string, contents: string, hint?: XmlTranslationParserHint): ParsedTranslationBundle;
parse(filePath: string, contents: string, hint: XmlTranslationParserHint): ParsedTranslationBundle;
}

// @public
Expand All @@ -172,10 +164,8 @@ export class XmbTranslationSerializer implements TranslationSerializer {
export class XtbTranslationParser implements TranslationParser<XmlTranslationParserHint> {
// (undocumented)
analyze(filePath: string, contents: string): ParseAnalysis<XmlTranslationParserHint>;
// @deprecated (undocumented)
canParse(filePath: string, contents: string): XmlTranslationParserHint | false;
// (undocumented)
parse(filePath: string, contents: string, hint?: XmlTranslationParserHint): ParsedTranslationBundle;
parse(filePath: string, contents: string, hint: XmlTranslationParserHint): ParsedTranslationBundle;
}

// (No @packageDocumentation comment for this package)
Expand Down
Expand Up @@ -53,14 +53,6 @@ export interface ArbLocation {
* ```
*/
export class ArbTranslationParser implements TranslationParser<ArbJsonObject> {
/**
* @deprecated
*/
canParse(filePath: string, contents: string): ArbJsonObject|false {
const result = this.analyze(filePath, contents);
return result.canParse && result.hint;
}

analyze(_filePath: string, contents: string): ParseAnalysis<ArbJsonObject> {
const diagnostics = new Diagnostics();
if (!contents.includes('"@@locale"')) {
Expand Down
Expand Up @@ -34,14 +34,6 @@ interface SimpleJsonFile {
* @publicApi used by CLI
*/
export class SimpleJsonTranslationParser implements TranslationParser<SimpleJsonFile> {
/**
* @deprecated
*/
canParse(filePath: string, contents: string): SimpleJsonFile|false {
const result = this.analyze(filePath, contents);
return result.canParse && result.hint;
}

analyze(filePath: string, contents: string): ParseAnalysis<SimpleJsonFile> {
const diagnostics = new Diagnostics();
// For this to be parsable, the extension must be `.json` and the contents must include "locale"
Expand Down
Expand Up @@ -44,33 +44,21 @@ export interface ParsedTranslationBundle {
/**
* Implement this interface to provide a class that can parse the contents of a translation file.
*
* The `canParse()` method can return a hint that can be used by the `parse()` method to speed up
* parsing. This allows the parser to do significant work to determine if the file can be parsed
* The `analyze()` method can return a hint that can be used by the `parse()` method to speed
* up parsing. This allows the parser to do significant work to determine if the file can be parsed
* without duplicating the work when it comes to actually parsing the file.
*
* Example usage:
*
* ```
* const parser: TranslationParser = getParser();
* const result = parser.canParse(filePath, content);
* if (result) {
* return parser.parse(filePath, content, result);
* const analysis = parser.analyze(filePath, content);
* if (analysis.canParse) {
* return parser.parse(filePath, content, analysis.hint);
* }
* ```
*/
export interface TranslationParser<Hint = true> {
/**
* Can this parser parse the given file?
*
* @deprecated Use `analyze()` instead
*
* @param filePath The absolute path to the translation file.
* @param contents The contents of the translation file.
* @returns A hint, which can be used in doing the actual parsing, if the file can be parsed by
* this parser; false otherwise.
*/
canParse(filePath: string, contents: string): Hint|false;

/**
* Analyze the file to see if this parser can parse the given file.
*
Expand All @@ -89,22 +77,10 @@ export interface TranslationParser<Hint = true> {
* @param filePath The absolute path to the translation file.
* @param contents The contents of the translation file.
* @param hint A value that can be used by the parser to speed up parsing of the file. This will
* have been provided as the return result from calling `canParse()`.
* have been provided as the return result from calling `analyze()`.
* @returns The translation bundle parsed from the file.
* @throws No errors. If there was a problem with parsing the bundle will contain errors
* in the `diagnostics` property.
*/
parse(filePath: string, contents: string, hint: Hint): ParsedTranslationBundle;
/**
* Parses the given file, extracting the target locale and translations.
*
* @deprecated This overload is kept for backward compatibility. Going forward use the Hint
* returned from `canParse()` so that this method can avoid duplicating effort.
*
* @param filePath The absolute path to the translation file.
* @param contents The contents of the translation file.
* @returns The translation bundle parsed from the file.
* @throws An error if there was a problem parsing this file.
*/
parse(filePath: string, contents: string): ParsedTranslationBundle;
}
Expand Up @@ -59,7 +59,7 @@ function getInnerRange(element: Element): LexerRange {
}

/**
* This "hint" object is used to pass information from `canParse()` to `parse()` for
* This "hint" object is used to pass information from `analyze()` to `parse()` for
* `TranslationParser`s that expect XML contents.
*
* This saves the `parse()` method from having to re-parse the XML.
Expand Down
Expand Up @@ -24,25 +24,13 @@ import {addErrorsToBundle, addParseDiagnostic, addParseError, canParseXml, getAt
* @publicApi used by CLI
*/
export class Xliff1TranslationParser implements TranslationParser<XmlTranslationParserHint> {
/**
* @deprecated
*/
canParse(filePath: string, contents: string): XmlTranslationParserHint|false {
const result = this.analyze(filePath, contents);
return result.canParse && result.hint;
}

analyze(filePath: string, contents: string): ParseAnalysis<XmlTranslationParserHint> {
return canParseXml(filePath, contents, 'xliff', {version: '1.2'});
}

parse(filePath: string, contents: string, hint?: XmlTranslationParserHint):
parse(filePath: string, contents: string, hint: XmlTranslationParserHint):
ParsedTranslationBundle {
if (hint) {
return this.extractBundle(hint);
} else {
return this.extractBundleDeprecated(filePath, contents);
}
return this.extractBundle(hint);
}

private extractBundle({element, errors}: XmlTranslationParserHint): ParsedTranslationBundle {
Expand Down Expand Up @@ -89,28 +77,6 @@ export class Xliff1TranslationParser implements TranslationParser<XmlTranslation

return bundle;
}

private extractBundleDeprecated(filePath: string, contents: string) {
const hint = this.canParse(filePath, contents);
if (!hint) {
throw new Error(`Unable to parse "${filePath}" as XLIFF 1.2 format.`);
}
const bundle = this.extractBundle(hint);
if (bundle.diagnostics.hasErrors) {
const message =
bundle.diagnostics.formatDiagnostics(`Failed to parse "${filePath}" as XLIFF 1.2 format`);
throw new Error(message);
}
return bundle;
}
}

class XliffFileElementVisitor extends BaseVisitor {
override visitElement(fileElement: Element): any {
if (fileElement.name === 'file') {
return {fileElement, locale: getAttribute(fileElement, 'target-language')};
}
}
}

class XliffTranslationVisitor extends BaseVisitor {
Expand Down
Expand Up @@ -23,25 +23,13 @@ import {addErrorsToBundle, addParseDiagnostic, addParseError, canParseXml, getAt
* @publicApi used by CLI
*/
export class Xliff2TranslationParser implements TranslationParser<XmlTranslationParserHint> {
/**
* @deprecated
*/
canParse(filePath: string, contents: string): XmlTranslationParserHint|false {
const result = this.analyze(filePath, contents);
return result.canParse && result.hint;
}

analyze(filePath: string, contents: string): ParseAnalysis<XmlTranslationParserHint> {
return canParseXml(filePath, contents, 'xliff', {version: '2.0'});
}

parse(filePath: string, contents: string, hint?: XmlTranslationParserHint):
parse(filePath: string, contents: string, hint: XmlTranslationParserHint):
ParsedTranslationBundle {
if (hint) {
return this.extractBundle(hint);
} else {
return this.extractBundleDeprecated(filePath, contents);
}
return this.extractBundle(hint);
}

private extractBundle({element, errors}: XmlTranslationParserHint): ParsedTranslationBundle {
Expand All @@ -67,20 +55,6 @@ export class Xliff2TranslationParser implements TranslationParser<XmlTranslation
}
return bundle;
}

private extractBundleDeprecated(filePath: string, contents: string) {
const hint = this.canParse(filePath, contents);
if (!hint) {
throw new Error(`Unable to parse "${filePath}" as XLIFF 2.0 format.`);
}
const bundle = this.extractBundle(hint);
if (bundle.diagnostics.hasErrors) {
const message =
bundle.diagnostics.formatDiagnostics(`Failed to parse "${filePath}" as XLIFF 2.0 format`);
throw new Error(message);
}
return bundle;
}
}


Expand Down
Expand Up @@ -25,14 +25,6 @@ import {addErrorsToBundle, addParseDiagnostic, addParseError, canParseXml, getAt
* @publicApi used by CLI
*/
export class XtbTranslationParser implements TranslationParser<XmlTranslationParserHint> {
/**
* @deprecated
*/
canParse(filePath: string, contents: string): XmlTranslationParserHint|false {
const result = this.analyze(filePath, contents);
return result.canParse && result.hint;
}

analyze(filePath: string, contents: string): ParseAnalysis<XmlTranslationParserHint> {
const extension = extname(filePath);
if (extension !== '.xtb' && extension !== '.xmb') {
Expand All @@ -43,13 +35,9 @@ export class XtbTranslationParser implements TranslationParser<XmlTranslationPar
return canParseXml(filePath, contents, 'translationbundle', {});
}

parse(filePath: string, contents: string, hint?: XmlTranslationParserHint):
parse(filePath: string, contents: string, hint: XmlTranslationParserHint):
ParsedTranslationBundle {
if (hint) {
return this.extractBundle(hint);
} else {
return this.extractBundleDeprecated(filePath, contents);
}
return this.extractBundle(hint);
}

private extractBundle({element, errors}: XmlTranslationParserHint): ParsedTranslationBundle {
Expand All @@ -65,20 +53,6 @@ export class XtbTranslationParser implements TranslationParser<XmlTranslationPar
visitAll(bundleVisitor, element.children, bundle);
return bundle;
}

private extractBundleDeprecated(filePath: string, contents: string) {
const hint = this.canParse(filePath, contents);
if (!hint) {
throw new Error(`Unable to parse "${filePath}" as XMB/XTB format.`);
}
const bundle = this.extractBundle(hint);
if (bundle.diagnostics.hasErrors) {
const message =
bundle.diagnostics.formatDiagnostics(`Failed to parse "${filePath}" as XMB/XTB format`);
throw new Error(message);
}
return bundle;
}
}

class XtbVisitor extends BaseVisitor {
Expand Down
Expand Up @@ -41,7 +41,7 @@ runInEachFileSystem(() => {
jsonParser = new SimpleJsonTranslationParser();
});

it('should call `canParse()` and `parse()` for each file', () => {
it('should call `analyze()` and `parse()` for each file', () => {
const diagnostics = new Diagnostics();
const parser = new MockTranslationParser(alwaysCanParse, 'fr');
const loader = new TranslationLoader(fs, [parser], 'error', diagnostics);
Expand Down Expand Up @@ -219,11 +219,6 @@ runInEachFileSystem(() => {
private _canParse: (filePath: string) => boolean, private _locale?: string,
private _translations: Record<string, ɵParsedTranslation> = {}) {}

canParse(filePath: string, fileContents: string) {
const result = this.analyze(filePath, fileContents);
return result.canParse && result.hint;
}

analyze(filePath: string, fileContents: string): ParseAnalysis<true> {
const diagnostics = new Diagnostics();
diagnostics.warn('This is a mock failure warning.');
Expand Down
Expand Up @@ -6,18 +6,21 @@
* found in the LICENSE file at https://angular.io/license
*/
import {ɵmakeTemplateObject} from '@angular/localize';

import {ArbTranslationParser} from '../../../../src/translate/translation_files/translation_parsers/arb_translation_parser';

describe('SimpleArbTranslationParser', () => {
describe('canParse()', () => {
describe('analyze()', () => {
it('should return true if the file extension is `.json` and contains `@@locale` property',
() => {
const parser = new ArbTranslationParser();
expect(parser.canParse('/some/file.xlf', '')).toBe(false);
expect(parser.canParse('/some/file.json', 'xxx')).toBe(false);
expect(parser.canParse('/some/file.json', '{ "someKey": "someValue" }')).toBe(false);
expect(parser.canParse('/some/file.json', '{ "@@locale": "en", "someKey": "someValue" }'))
.toBeTruthy();
expect(parser.analyze('/some/file.xlf', '').canParse).toBeFalse();
expect(parser.analyze('/some/file.json', 'xxx').canParse).toBeFalse();
expect(parser.analyze('/some/file.json', '{ "someKey": "someValue" }').canParse)
.toBeFalse();
expect(parser.analyze('/some/file.json', '{ "@@locale": "en", "someKey": "someValue" }')
.canParse)
.toBeTrue();
});
});

Expand Down

0 comments on commit d36fd3d

Please sign in to comment.