Merge pull request #1031 from ckeditor/i/3846

Fix (translations): Align the number of plural forms to plural forms defined by a language in the `synchronizeTranslations()` function.

packages/ckeditor5-dev-translations/lib/utils/createmissingpackagetranslations.js

@@ -7,10 +7,10 @@ import upath from 'upath';
 import fs from 'fs-extra';
 import PO from 'pofile';
 import { fileURLToPath } from 'url';
-import { getNPlurals, getFormula } from 'plural-forms';
 import getLanguages from './getlanguages.js';
 import { TRANSLATION_FILES_PATH } from './constants.js';
 import cleanTranslationFileContent from './cleantranslationfilecontent.js';
+import getHeaders from './getheaders.js';
 
 const __filename = fileURLToPath( import.meta.url );
 const __dirname = upath.dirname( __filename );
@@ -33,12 +33,7 @@ export default function createMissingPackageTranslations( { packagePath, skipLic
 		}
 
 		const translations = PO.parse( translationsTemplate );
-
-		translations.headers.Language = localeCode;
-		translations.headers[ 'Plural-Forms' ] = [
-			`nplurals=${ getNPlurals( languageCode ) };`,
-			`plural=${ getFormula( languageCode ) };`
-		].join( ' ' );
+		translations.headers = getHeaders( languageCode, localeCode );
 
 		fs.outputFileSync( translationFilePath, cleanTranslationFileContent( translations ).toString(), 'utf-8' );
 	}

packages/ckeditor5-dev-translations/lib/utils/getheaders.js

@@ -0,0 +1,22 @@
+/**
+ * @license Copyright (c) 2003-2024, CKSource Holding sp. z o.o. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+import { getNPlurals, getFormula } from 'plural-forms';
+
+/**
+ * @param {string} languageCode
+ * @param {string} localeCode
+ * @returns {object}
+ */
+export default function getHeaders( languageCode, localeCode ) {
+	return {
+		Language: localeCode,
+		'Plural-Forms': [
+			`nplurals=${ getNPlurals( languageCode ) };`,
+			`plural=${ getFormula( languageCode ) };`
+		].join( ' ' ),
+		'Content-Type': 'text/plain; charset=UTF-8'
+	};
+}

packages/ckeditor5-dev-translations/lib/utils/synchronizetranslationsbasedoncontext.js

@@ -10,6 +10,8 @@ import { glob } from 'glob';
 import createMissingPackageTranslations from './createmissingpackagetranslations.js';
 import { TRANSLATION_FILES_PATH } from './constants.js';
 import cleanTranslationFileContent from './cleantranslationfilecontent.js';
+import getHeaders from './getheaders.js';
+import getLanguages from './getlanguages.js';
 
 /**
  * @param {object} options
@@ -18,6 +20,8 @@ import cleanTranslationFileContent from './cleantranslationfilecontent.js';
  * @param {boolean} options.skipLicenseHeader Whether to skip adding the license header to newly created translation files.
  */
 export default function synchronizeTranslationsBasedOnContext( { packageContexts, sourceMessages, skipLicenseHeader } ) {
+	const languages = getLanguages();
+
 	// For each package:
 	for ( const { packagePath, contextContent } of packageContexts ) {
 		// (1) Skip packages that do not contain language context.
@@ -43,15 +47,21 @@ export default function synchronizeTranslationsBasedOnContext( { packageContexts
 			const translationFile = fs.readFileSync( translationFilePath, 'utf-8' );
 			const translations = PO.parse( translationFile );
 
-			// (4.1) Remove unused translations.
+			// (4.1) Update file headers.
+			const { languageCode, localeCode } = languages.find( language => language.localeCode === translations.headers.Language );
+
+			translations.headers = getHeaders( languageCode, localeCode );
+
+			const numberOfPluralForms = parseInt( PO.parsePluralForms( translations.headers[ 'Plural-Forms' ] ).nplurals );
+
+			// (4.2) Remove unused translations.
 			translations.items = translations.items.filter( item => contextContent[ item.msgid ] );
 
-			// (4.2) Add missing translations.
+			// (4.3) Add missing translations.
 			translations.items.push(
 				...sourceMessagesForPackage
 					.filter( message => !translations.items.find( item => item.msgid === message.id ) )
 					.map( message => {
-						const numberOfPluralForms = PO.parsePluralForms( translations.headers[ 'Plural-Forms' ] ).nplurals;
 						const item = new PO.Item( { nplurals: numberOfPluralForms } );
 
 						item.msgctxt = contextContent[ message.id ];
@@ -67,9 +77,19 @@ export default function synchronizeTranslationsBasedOnContext( { packageContexts
 					} )
 			);
 
+			// (4.4) Align the number of plural forms to plural forms defined by a language.
+			translations.items = translations.items.map( item => {
+				if ( item.msgid_plural ) {
+					item.msgstr = [ ...Array( numberOfPluralForms ) ]
+						.map( ( value, index ) => item.msgstr[ index ] || '' );
+				}
+
+				return item;
+			} );
+
 			const translationFileUpdated = cleanTranslationFileContent( translations ).toString();
 
-			// (4.3) Save translation file only if it has been updated.
+			// (4.5) Save translation file only if it has been updated.
 			if ( translationFile === translationFileUpdated ) {
 				continue;
 			}

packages/ckeditor5-dev-translations/tests/utils/createmissingpackagetranslations.js

@@ -6,16 +6,16 @@
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 import fs from 'fs-extra';
 import PO from 'pofile';
-import { getNPlurals, getFormula } from 'plural-forms';
 import cleanTranslationFileContent from '../../lib/utils/cleantranslationfilecontent.js';
 import getLanguages from '../../lib/utils/getlanguages.js';
+import getHeaders from '../../lib/utils/getheaders.js';
 import createMissingPackageTranslations from '../../lib/utils/createmissingpackagetranslations.js';
 
 vi.mock( 'fs-extra' );
 vi.mock( 'pofile' );
-vi.mock( 'plural-forms' );
 vi.mock( '../../lib/utils/cleantranslationfilecontent.js' );
 vi.mock( '../../lib/utils/getlanguages.js' );
+vi.mock( '../../lib/utils/getheaders.js' );
 
 describe( 'createMissingPackageTranslations()', () => {
 	let translations, defaultOptions;
@@ -32,14 +32,19 @@ describe( 'createMissingPackageTranslations()', () => {
 
 		vi.mocked( PO.parse ).mockReturnValue( translations );
 
-		vi.mocked( getNPlurals ).mockReturnValue( 4 );
-		vi.mocked( getFormula ).mockReturnValue( 'example plural formula' );
-
 		vi.mocked( getLanguages ).mockReturnValue( [
 			{ localeCode: 'en', languageCode: 'en', languageFileName: 'en' },
 			{ localeCode: 'zh_TW', languageCode: 'zh', languageFileName: 'zh-tw' }
 		] );
 
+		vi.mocked( getHeaders ).mockImplementation( ( languageCode, localeCode ) => {
+			return {
+				Language: localeCode,
+				'Plural-Forms': 'nplurals=4; plural=example plural formula;',
+				'Content-Type': 'text/plain; charset=UTF-8'
+			};
+		} );
+
 		vi.mocked( cleanTranslationFileContent ).mockReturnValue( {
 			toString: () => 'Clean PO file content.'
 		} );
@@ -81,11 +86,11 @@ describe( 'createMissingPackageTranslations()', () => {
 			'utf-8'
 		);
 
-		expect( getNPlurals ).toHaveBeenCalledWith( 'zh' );
-		expect( getFormula ).toHaveBeenCalledWith( 'zh' );
+		expect( getHeaders ).toHaveBeenCalledWith( 'zh', 'zh_TW' );
 
 		expect( translations.headers.Language ).toEqual( 'zh_TW' );
 		expect( translations.headers[ 'Plural-Forms' ] ).toEqual( 'nplurals=4; plural=example plural formula;' );
+		expect( translations.headers[ 'Content-Type' ] ).toEqual( 'text/plain; charset=UTF-8' );
 	} );
 
 	it( 'should not read the template if `skipLicenseHeader` flag is set', () => {
@@ -94,12 +99,6 @@ describe( 'createMissingPackageTranslations()', () => {
 		createMissingPackageTranslations( defaultOptions );
 
 		expect( fs.readFileSync ).not.toHaveBeenCalled();
-
-		expect( getNPlurals ).toHaveBeenCalledWith( 'zh' );
-		expect( getFormula ).toHaveBeenCalledWith( 'zh' );
-
-		expect( translations.headers.Language ).toEqual( 'zh_TW' );
-		expect( translations.headers[ 'Plural-Forms' ] ).toEqual( 'nplurals=4; plural=example plural formula;' );
 	} );
 
 	it( 'should save missing translation files on filesystem after cleaning the content', () => {

packages/ckeditor5-dev-translations/tests/utils/getheaders.js

@@ -0,0 +1,48 @@
+/**
+ * @license Copyright (c) 2003-2024, CKSource Holding sp. z o.o. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+import { describe, expect, it, beforeEach, vi } from 'vitest';
+import { getNPlurals, getFormula } from 'plural-forms';
+import getHeaders from '../../lib/utils/getheaders.js';
+
+vi.mock( 'plural-forms' );
+
+describe( 'getHeaders()', () => {
+	beforeEach( () => {
+		vi.mocked( getNPlurals ).mockReturnValue( 4 );
+		vi.mocked( getFormula ).mockReturnValue( 'example plural formula' );
+	} );
+
+	it( 'should be a function', () => {
+		expect( getHeaders ).toBeInstanceOf( Function );
+	} );
+
+	it( 'should return "Language" header', () => {
+		const headers = getHeaders( 'en', 'en_GB' );
+
+		expect( headers ).toEqual( expect.objectContaining( {
+			Language: 'en_GB'
+		} ) );
+	} );
+
+	it( 'should return "Plural-Forms" header', () => {
+		const headers = getHeaders( 'en', 'en_GB' );
+
+		expect( headers ).toEqual( expect.objectContaining( {
+			'Plural-Forms': 'nplurals=4; plural=example plural formula;'
+		} ) );
+
+		expect( getNPlurals ).toHaveBeenCalledWith( 'en' );
+		expect( getFormula ).toHaveBeenCalledWith( 'en' );
+	} );
+
+	it( 'should return "Content-Type" header', () => {
+		const headers = getHeaders( 'en', 'en_GB' );
+
+		expect( headers ).toEqual( expect.objectContaining( {
+			'Content-Type': 'text/plain; charset=UTF-8'
+		} ) );
+	} );
+} );

packages/ckeditor5-dev-translations/tests/utils/synchronizetranslationsbasedoncontext.js

@@ -9,13 +9,17 @@ import PO from 'pofile';
 import { glob } from 'glob';
 import cleanTranslationFileContent from '../../lib/utils/cleantranslationfilecontent.js';
 import createMissingPackageTranslations from '../../lib/utils/createmissingpackagetranslations.js';
+import getLanguages from '../../lib/utils/getlanguages.js';
+import getHeaders from '../../lib/utils/getheaders.js';
 import synchronizeTranslationsBasedOnContext from '../../lib/utils/synchronizetranslationsbasedoncontext.js';
 
 vi.mock( 'fs-extra' );
 vi.mock( 'pofile' );
 vi.mock( 'glob' );
 vi.mock( '../../lib/utils/createmissingpackagetranslations.js' );
 vi.mock( '../../lib/utils/cleantranslationfilecontent.js' );
+vi.mock( '../../lib/utils/getlanguages.js' );
+vi.mock( '../../lib/utils/getheaders.js' );
 
 describe( 'synchronizeTranslationsBasedOnContext()', () => {
 	let defaultOptions, translations, stubs;
@@ -46,7 +50,9 @@ describe( 'synchronizeTranslationsBasedOnContext()', () => {
 		};
 
 		translations = {
-			headers: {},
+			headers: {
+				Language: 'en'
+			},
 			items: [
 				{ msgid: 'id1' },
 				{ msgid: 'id2' }
@@ -58,6 +64,19 @@ describe( 'synchronizeTranslationsBasedOnContext()', () => {
 			poItemConstructor: vi.fn()
 		};
 
+		vi.mocked( getLanguages ).mockReturnValue( [
+			{ localeCode: 'en', languageCode: 'en', languageFileName: 'en' },
+			{ localeCode: 'zh_TW', languageCode: 'zh', languageFileName: 'zh-tw' }
+		] );
+
+		vi.mocked( getHeaders ).mockImplementation( ( languageCode, localeCode ) => {
+			return {
+				Language: localeCode,
+				'Plural-Forms': 'nplurals=4; plural=example plural formula;',
+				'Content-Type': 'text/plain; charset=UTF-8'
+			};
+		} );
+
 		vi.mocked( PO.parse ).mockReturnValue( translations );
 
 		vi.mocked( PO.parsePluralForms ).mockReturnValue( { nplurals: 4 } );
@@ -131,14 +150,24 @@ describe( 'synchronizeTranslationsBasedOnContext()', () => {
 
 	it( 'should parse each translation file', () => {
 		vi.mocked( glob.sync ).mockImplementation( pattern => {
-			return [ 'en', 'pl' ].map( language => pattern.replace( '*', language ) );
+			return [ 'en', 'zh-tw' ].map( language => pattern.replace( '*', language ) );
 		} );
 
 		synchronizeTranslationsBasedOnContext( defaultOptions );
 
 		expect( fs.readFileSync ).toHaveBeenCalledTimes( 2 );
 		expect( fs.readFileSync ).toHaveBeenCalledWith( 'packages/ckeditor5-foo/lang/translations/en.po', 'utf-8' );
-		expect( fs.readFileSync ).toHaveBeenCalledWith( 'packages/ckeditor5-foo/lang/translations/pl.po', 'utf-8' );
+		expect( fs.readFileSync ).toHaveBeenCalledWith( 'packages/ckeditor5-foo/lang/translations/zh-tw.po', 'utf-8' );
+	} );
+
+	it( 'should update file header', () => {
+		synchronizeTranslationsBasedOnContext( defaultOptions );
+
+		expect( translations.headers ).toEqual( {
+			Language: 'en',
+			'Plural-Forms': 'nplurals=4; plural=example plural formula;',
+			'Content-Type': 'text/plain; charset=UTF-8'
+		} );
 	} );
 
 	it( 'should remove unused translations', () => {
@@ -176,6 +205,76 @@ describe( 'synchronizeTranslationsBasedOnContext()', () => {
 		] );
 	} );
 
+	it( 'should remove existing plural forms if a source file contains more than a language defines', () => {
+		translations.items = [
+			{
+				msgid: 'id1',
+				msgctxt: 'Context for example message 1',
+				msgid_plural: '',
+				msgstr: [ '' ]
+			},
+			{
+				msgid: 'id2',
+				msgctxt: 'Context for example message 2',
+				msgid_plural: 'Example message 2 - plural form',
+				msgstr: [ '1', '2', '3', '4', '5', '6' ]
+			}
+		];
+
+		synchronizeTranslationsBasedOnContext( defaultOptions );
+
+		expect( translations.items ).toEqual( [
+			// `id1` is not updated as it does not offer plural forms.
+			{
+				msgid: 'id1',
+				msgctxt: 'Context for example message 1',
+				msgid_plural: '',
+				msgstr: [ '' ]
+			},
+			{
+				msgid: 'id2',
+				msgctxt: 'Context for example message 2',
+				msgid_plural: 'Example message 2 - plural form',
+				msgstr: [ '1', '2', '3', '4' ]
+			}
+		] );
+	} );
+
+	it( 'should add empty plural forms if a source file contains less than a language defines', () => {
+		translations.items = [
+			{
+				msgid: 'id1',
+				msgctxt: 'Context for example message 1',
+				msgid_plural: '',
+				msgstr: [ '' ]
+			},
+			{
+				msgid: 'id2',
+				msgctxt: 'Context for example message 2',
+				msgid_plural: 'Example message 2 - plural form',
+				msgstr: [ '1', '2' ]
+			}
+		];
+
+		synchronizeTranslationsBasedOnContext( defaultOptions );
+
+		expect( translations.items ).toEqual( [
+			// `id1` is not updated as it does not offer plural forms.
+			{
+				msgid: 'id1',
+				msgctxt: 'Context for example message 1',
+				msgid_plural: '',
+				msgstr: [ '' ]
+			},
+			{
+				msgid: 'id2',
+				msgctxt: 'Context for example message 2',
+				msgid_plural: 'Example message 2 - plural form',
+				msgstr: [ '1', '2', '', '' ]
+			}
+		] );
+	} );
+
 	it( 'should save updated translation files on filesystem after cleaning the content', () => {
 		synchronizeTranslationsBasedOnContext( defaultOptions );