Merge pull request #27 from radiovisual/20-no-extra-whitespace

Add `no-extra-whitespace` rule. Closes #20

README.md

@@ -33,7 +33,7 @@ For each project where you want to run the keeli, you will need to have a file n
 	 **/
 	sourceFile: "en.json",
 	/**
-	 * An object describing the locale and its assosiated translation file name.
+	 * An object describing the locale and its associated translation file name.
 	 * Example: {"de": "de.json", "fr": "fr.json" }
 	 *
 	 **/
@@ -102,6 +102,8 @@ Each rule can have a default setting of `error`, `warn` or `off`, these defaults
 | no-invalid-variables     | `error` |
 | no-html-messages         | `error` |
 | no-missing-keys          | `error` |
+| no-malformed-keys        | `error` |
+| no-extra-whitespace      | `error` |
 
 # Overriding Rules
 

fixtures/i18n/de.json

@@ -10,5 +10,7 @@
 	"declares-a-malfomed-variable-in-de": "[DE] malformed {variable",
 	"html-message": "[DE] Hi, <b>{firstName}</b>",
 	"missing-key-in-fr": "[DE] this key should not be found in fr.json",
-	"unexpected-key": "[DE] this key is not defined in the source file en.json"
+	"unexpected-key": "[DE] this key is not defined in the source file en.json",
+	"extra-whitespace-padding": "  [DE] this key has extra whitespace padding  ",
+	"extra-whitespace-internally-de": "[DE] this key has    extra internal whitespace    padding in de"
 }

fixtures/i18n/en.json

@@ -9,5 +9,7 @@
 	"declares-a-single-missing-variable-in-fr": "[EN] should be two {one} {two}",
 	"declares-a-malfomed-variable-in-de": "[EN] malformed {variable}",
 	"html-message": "Hi, <b>{firstName}</b>",
-	"missing-key-in-fr": "[EN] this key should not be found in fr.json"
+	"missing-key-in-fr": "[EN] this key should not be found in fr.json",
+	"extra-whitespace-padding": "  [EN] this key has extra whitespace padding  ",
+	"extra-whitespace-internally-de": "[EN] this key has extra internal whitespace padding in de"
 }

fixtures/i18n/fr.json

@@ -9,5 +9,7 @@
 	"declares-a-single-missing-variable-in-fr": "[FR] should be two {one}",
 	"declares-a-malfomed-variable-in-de": "[FR] malformed {variable}",
 	"html-message": "[FR] Hi, <b>{firstName}</b>",
-	"unexpected-key": "[FR] this key is not defined in the source file en.json"
+	"unexpected-key": "[FR] this key is not defined in the source file en.json",
+	"extra-whitespace-padding": "  [FR] this key has extra whitespace padding  ",
+	"extra-whitespace-internally-de": "[FR] this key has extra internal whitespace padding in de"
 }

keeli.config.json

@@ -9,6 +9,7 @@
 	"rules": {
 		"no-untranslated-messages": "error",
 		"no-empty-messages": "error",
+		"no-extra-whitespace": "error",
 		"no-html-messages": "error",
 		"no-missing-keys": "error",
 		"no-invalid-variables": "error",

src/rules/index.ts

@@ -6,6 +6,7 @@ import { noInvalidConfiguration } from "./no-invalid-configuration/index.js";
 import { noMissingKeys } from "./no-missing-keys/index.js";
 import { noInvalidSeverity } from "./no-invalid-severity/index.js";
 import { noMalformedKeys } from "./no-malformed-keys/index.js";
+import { noExtraWhitespace } from "./no-extra-whitespace/index.js";
 
 export {
 	noInvalidConfiguration,
@@ -16,4 +17,5 @@ export {
 	noHtmlMessages,
 	noMissingKeys,
 	noMalformedKeys,
+	noExtraWhitespace,
 };

src/rules/no-extra-whitespace/README.md

@@ -0,0 +1,80 @@
+# no-extra-whitespace
+
+## Rule Details
+
+**Default Severity**: `error`  
+**Rule Type**: `validation`
+
+Messages in your translation files should not contain any whitespace before or after the message, or more than 1 contiguous whitespace character inside the message.
+
+## Why?
+
+Whitespace is sometimes deliberately added to the messages in your application to help solve some styling or layout issues. This is not recommended since it becomes hard to maintain consistent whitespace across all your translated strings and is an easy source of layout-related bugs in your application. It is safer and cleaner to let your application make layout decisions, and treat extraneous whitespace in your translated strings as unintended/accidental.
+
+## Examples
+
+Assuming `en.json` is where your default language is (the source file):
+
+❌ Example of **incorrect** setup for this rule (leading and trailing whitespace found):
+
+```js
+en.json: { 'hello': '   Hi, <b>{firstName}</b>  ' }
+fr.json: { 'hello': '   Salut, <b>{firstName}</b>  ' }
+de.json: { 'hello': '   Hallo, <b>{firstName}</b>  ' }
+```
+
+❌ Example of **incorrect** setup for this rule (too much internal whitespace found):
+
+```js
+en.json: { 'hello': 'Hi,       <b>{firstName}</b>  ' }
+fr.json: { 'hello': 'Salut,    <b>{firstName}</b>  ' }
+de.json: { 'hello': 'Hallo,    <b>{firstName}</b>  ' }
+```
+
+✅ Examples of a **correct** setup for this rule (no whitespace padding is found):
+
+```js
+en.json: { 'hello': 'Hi, {firstName}' }
+fr.json: { 'hello': 'Salut, {firstName}' }
+de.json: { 'hello': 'Hallo, {firstName}' }
+```
+
+## Example Configuration
+
+Simple configuration where you just supply the severity level of `error` | `warn` | `off`:
+
+```json
+{
+	"rules": {
+		"no-extra-whitespace": "error"
+	}
+}
+```
+
+## Advanced configuration options
+
+This rule supports some advanced configuration.
+
+Note that when you use the advanced configuration option you need to set the severity level using the `severity` property, otherwise the rule's default severity will apply.
+
+### ignoreKeys
+
+To disable the check for this rule for specific keys, you can pass in the name of the keys where you don't want this rule to run in the `ignoreKeys` array.
+
+```json
+{
+	"rules": {
+		"no-extra-whitespace": {
+			"severity": "error",
+			"ignoreKeys": ["foo", "bar"]
+		}
+	}
+}
+```
+
+> [!IMPORTANT]
+> Be careful when using the `ignoreKeys` array: ignoring keys means means potentially ignoring real problems that can affect the UI/UX and reliability of your application.
+
+## Version
+
+This rule was introduced in keeli v1.0.0.

src/rules/no-extra-whitespace/index.ts

@@ -0,0 +1 @@
+export { noExtraWhitespace } from "./no-extra-whitespace";

src/rules/no-extra-whitespace/no-extra-whitespace.spec.ts

@@ -0,0 +1,172 @@
+import { createMockProblemReporter } from "../../utils/test-helpers.ts";
+import {
+	Config,
+	RuleContext,
+	RuleSeverity,
+	TranslationFiles,
+} from "../../types.js";
+import { noExtraWhitespace } from "./no-extra-whitespace.ts";
+import { getExtraWhitespaceFoundInMessageProblem } from "./problems.ts";
+
+const ruleMeta = noExtraWhitespace.meta;
+const rule = noExtraWhitespace;
+
+const defaultLocale = "en";
+
+const baseConfig: Config = {
+	defaultLocale,
+	sourceFile: "en.json",
+	translationFiles: { fr: "fr.json" },
+	pathToTranslatedFiles: "i18n",
+	rules: {
+		"no-extra-whitespace": "error",
+	},
+	dryRun: false,
+	enabled: true,
+};
+
+describe.each([["error"], ["warning"]])(`${rule.meta.name}`, (severityStr) => {
+	const severity = severityStr as unknown as RuleSeverity;
+
+	const context: RuleContext = {
+		severity,
+		ignoreKeys: [],
+	};
+
+	it(`should report html in messages with ${severity}`, () => {
+		const problemStore = createMockProblemReporter();
+
+		const translationFiles: TranslationFiles = {
+			en: {
+				greeting: "   [EN] hello   ",
+				farewell: "[EN]       goodbye",
+			},
+			fr: {
+				greeting: "   [FR] hello   ",
+				farewell: "[FR]       goodbye",
+			},
+		};
+
+		rule.run(translationFiles, baseConfig, problemStore, context);
+
+		const expected1 = getExtraWhitespaceFoundInMessageProblem({
+			key: "greeting",
+			locale: "en",
+			severity,
+			ruleMeta,
+			isIgnored: false,
+		});
+
+		const expected2 = getExtraWhitespaceFoundInMessageProblem({
+			key: "farewell",
+			locale: "en",
+			severity,
+			ruleMeta,
+			isIgnored: false,
+		});
+
+		const expected3 = getExtraWhitespaceFoundInMessageProblem({
+			key: "greeting",
+			locale: "fr",
+			severity,
+			ruleMeta,
+			isIgnored: false,
+		});
+
+		const expected4 = getExtraWhitespaceFoundInMessageProblem({
+			key: "farewell",
+			locale: "fr",
+			severity,
+			ruleMeta,
+			isIgnored: false,
+		});
+
+		expect(problemStore.report).toHaveBeenCalledTimes(4);
+		expect(problemStore.report).toHaveBeenCalledWith(expected1);
+		expect(problemStore.report).toHaveBeenCalledWith(expected2);
+		expect(problemStore.report).toHaveBeenCalledWith(expected3);
+		expect(problemStore.report).toHaveBeenCalledWith(expected4);
+	});
+
+	it(`should ignore keys in ignoreKeys with severity ${severity}`, () => {
+		const problemStore = createMockProblemReporter();
+
+		const translationFiles: TranslationFiles = {
+			en: {
+				greeting: "   [EN] hello   ",
+				farewell: "[EN]       goodbye",
+			},
+			fr: {
+				greeting: "   [FR] hello   ",
+				farewell: "[FR]       goodbye",
+			},
+		};
+
+		const ignored1 = getExtraWhitespaceFoundInMessageProblem({
+			key: "greeting",
+			locale: "en",
+			severity,
+			ruleMeta,
+			isIgnored: true,
+		});
+
+		const ignored2 = getExtraWhitespaceFoundInMessageProblem({
+			key: "farewell",
+			locale: "en",
+			severity,
+			ruleMeta,
+			isIgnored: true,
+		});
+
+		const ignored3 = getExtraWhitespaceFoundInMessageProblem({
+			key: "greeting",
+			locale: "fr",
+			severity,
+			ruleMeta,
+			isIgnored: true,
+		});
+
+		const ignored4 = getExtraWhitespaceFoundInMessageProblem({
+			key: "farewell",
+			locale: "fr",
+			severity,
+			ruleMeta,
+			isIgnored: true,
+		});
+
+		const ignoreKeysContext = {
+			...context,
+			ignoreKeys: ["farewell", "greeting"],
+		};
+
+		rule.run(translationFiles, baseConfig, problemStore, ignoreKeysContext);
+
+		expect(problemStore.report).toHaveBeenCalledWith(ignored1);
+		expect(problemStore.report).toHaveBeenCalledWith(ignored2);
+		expect(problemStore.report).toHaveBeenCalledWith(ignored3);
+		expect(problemStore.report).toHaveBeenCalledWith(ignored4);
+	});
+});
+
+describe(`${rule.meta.name}: off`, () => {
+	const problemStore = createMockProblemReporter();
+
+	const context: RuleContext = {
+		severity: "off",
+		ignoreKeys: [],
+	};
+
+	const translationFiles: TranslationFiles = {
+		en: {
+			greeting: "   [EN] hello   ",
+			farewell: "[EN]       goodbye",
+		},
+		fr: {
+			greeting: "   [FR] hello   ",
+			farewell: "[FR]       goodbye",
+		},
+	};
+
+	rule.run(translationFiles, baseConfig, problemStore, context);
+	expect(problemStore.report).not.toHaveBeenCalled();
+});

src/rules/no-extra-whitespace/no-extra-whitespace.ts

@@ -0,0 +1,61 @@
+import { SEVERITY_LEVEL } from "../../constants.ts";
+import {
+	Config,
+	Rule,
+	RuleContext,
+	RuleMeta,
+	TranslationFiles,
+} from "../../types.ts";
+import {
+	stringHasExtraneousWhitespace,
+	stringHasWhitespacePadding,
+} from "../../utils/string-helpers.ts";
+
+import { getExtraWhitespaceFoundInMessageProblem } from "./problems.ts";
+
+const ruleMeta: RuleMeta = {
+	name: "no-extra-whitespace",
+	description: `Whitespace should not appear before or after a message, or have more than 1 contiguous whitespace character internally.`,
+	url: "TBD",
+	type: "validation",
+	defaultSeverity: "error",
+};
+
+const noExtraWhitespace: Rule = {
+	meta: ruleMeta,
+	run: (
+		translationFiles: TranslationFiles,
+		config: Config,
+		problemStore,
+		context: RuleContext
+	) => {
+		const { severity, ignoreKeys } = context;
+
+		if (severity === SEVERITY_LEVEL.off) {
+			return;
+		}
+
+		for (let [locale, data] of Object.entries(translationFiles)) {
+			for (let [key, message] of Object.entries(data)) {
+				const isIgnored = ignoreKeys.includes(key);
+
+				if (
+					stringHasExtraneousWhitespace(message) ||
+					stringHasWhitespacePadding(message)
+				) {
+					problemStore.report(
+						getExtraWhitespaceFoundInMessageProblem({
+							severity,
+							locale,
+							ruleMeta,
+							key,
+							isIgnored,
+						})
+					);
+				}
+			}
+		}
+	},
+};
+
+export { noExtraWhitespace };