From 6d7f9beb998e467ec7284862785592d02a6ac92d Mon Sep 17 00:00:00 2001 From: Stefan Kolb Date: Wed, 22 Feb 2017 14:18:51 +0100 Subject: [PATCH 1/4] Test for #1985 --- .../l10n/LocalizationConsistencyTest.java | 20 +++++ .../jabref/logic/l10n/LocalizationParser.java | 84 ++++++++++++++----- 2 files changed, 82 insertions(+), 22 deletions(-) diff --git a/src/test/java/org/jabref/logic/l10n/LocalizationConsistencyTest.java b/src/test/java/org/jabref/logic/l10n/LocalizationConsistencyTest.java index 8f066201e03..8b5cec0b403 100644 --- a/src/test/java/org/jabref/logic/l10n/LocalizationConsistencyTest.java +++ b/src/test/java/org/jabref/logic/l10n/LocalizationConsistencyTest.java @@ -16,6 +16,7 @@ import org.junit.Test; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; public class LocalizationConsistencyTest { @@ -168,4 +169,23 @@ public void findObsoleteMenuLocalizationKeys() throws IOException { Collections.emptySet(), obsoleteKeys); } + @Test + public void localizationMustOnlyUseConcatenatedStrings() throws IOException { + // Must start or end with " + // Localization.lang("test"), Localization.lang("test" + var), Localization.lang(var + "test") + // TODO: Localization.lang(var1 + "test" + var2) not covered + // Localization.lang("Problem downloading from %1", address) + Set keys = LocalizationParser.findLocalizationParametersStringsInJavaFiles(LocalizationBundleForTest.LANG); + for(LocalizationEntry e : keys) { + assertTrue("Illegal localization parameter found. Must include a String with potential concatenation or replacement parameters. Illegal parameter: Localization.lang(" + e.getKey(), + e.getKey().startsWith("\"") || e.getKey().endsWith("\"")); + } + + keys = LocalizationParser.findLocalizationParametersStringsInJavaFiles(LocalizationBundleForTest.MENU); + for(LocalizationEntry e : keys) { + assertTrue("Illegal localization parameter found. Must include a String with potential concatenation or replacement parameters. Illegal parameter: Localization.lang(" + e.getKey(), + e.getKey().startsWith("\"") || e.getKey().endsWith("\"")); + } + } + } diff --git a/src/test/java/org/jabref/logic/l10n/LocalizationParser.java b/src/test/java/org/jabref/logic/l10n/LocalizationParser.java index 428fe8c129c..09f5cc91675 100644 --- a/src/test/java/org/jabref/logic/l10n/LocalizationParser.java +++ b/src/test/java/org/jabref/logic/l10n/LocalizationParser.java @@ -75,6 +75,14 @@ private static Set findLocalizationEntriesInFiles(Localizatio } } + public static Set findLocalizationParametersStringsInJavaFiles(LocalizationBundleForTest type) + throws IOException { + return Files.walk(Paths.get("src/main")) + .filter(LocalizationParser::isJavaFile) + .flatMap(path -> getLocalizationParametersInJavaFile(path, type).stream()) + .collect(Collectors.toSet()); + } + private static Set findLocalizationEntriesInJavaFiles(LocalizationBundleForTest type) throws IOException { return Files.walk(Paths.get("src/main")) @@ -141,6 +149,26 @@ private static List getLanguageKeysInJavaFile(Path path, Loca return result; } + private static List getLocalizationParametersInJavaFile(Path path, LocalizationBundleForTest type) { + List result = new LinkedList<>(); + + try { + List lines = Files.readAllLines(path, StandardCharsets.UTF_8); + String content = String.join("\n", lines); + + List keys = JavaLocalizationEntryParser.getLocalizationParameter(content, type); + + for (String key : keys) { + result.add(new LocalizationEntry(path, key, type)); + } + + } catch (IOException ignore) { + ignore.printStackTrace(); + } + + return result; + } + /** * Loads the fxml file and returns all used language resources. */ @@ -192,31 +220,13 @@ static class JavaLocalizationEntryParser { private static final Pattern QUOTATION_SYMBOL = Pattern.compile("QUOTATIONPLACEHOLDER"); public static List getLanguageKeysInString(String content, LocalizationBundleForTest type) { + List parameters = getLocalizationParameter(content, type); + List result = new LinkedList<>(); - Matcher matcher; - if (type == LocalizationBundleForTest.LANG) { - matcher = LOCALIZATION_START_PATTERN.matcher(content); - } else { - matcher = LOCALIZATION_MENU_START_PATTERN.matcher(content); - } - while (matcher.find()) { - // find contents between the brackets, covering multi-line strings as well - int index = matcher.end(); - int brackets = 1; - StringBuilder buffer = new StringBuilder(); - while (brackets != 0) { - char c = content.charAt(index); - if (c == '(') { - brackets++; - } else if (c == ')') { - brackets--; - } - buffer.append(c); - index++; - } + for (String param : parameters) { - String parsedContentsOfLangMethod = ESCAPED_QUOTATION_SYMBOL.matcher(buffer.toString()).replaceAll("QUOTATIONPLACEHOLDER"); + String parsedContentsOfLangMethod = ESCAPED_QUOTATION_SYMBOL.matcher(param).replaceAll("QUOTATIONPLACEHOLDER"); // only retain what is within quotation StringBuilder b = new StringBuilder(); @@ -259,6 +269,36 @@ public static List getLanguageKeysInString(String content, LocalizationB return result; } + public static List getLocalizationParameter(String content, LocalizationBundleForTest type) { + List result = new LinkedList<>(); + + Matcher matcher; + if (type == LocalizationBundleForTest.LANG) { + matcher = LOCALIZATION_START_PATTERN.matcher(content); + } else { + matcher = LOCALIZATION_MENU_START_PATTERN.matcher(content); + } + while (matcher.find()) { + // find contents between the brackets, covering multi-line strings as well + int index = matcher.end(); + int brackets = 1; + StringBuilder buffer = new StringBuilder(); + while (brackets != 0) { + char c = content.charAt(index); + if (c == '(') { + brackets++; + } else if (c == ')') { + brackets--; + } + buffer.append(c); + index++; + } + // trim newlines and whitespace + result.add(buffer.toString().trim()); + } + + return result; + } } } From d99115ef86deb4c0f8131250e01bc4b32f4ecf2c Mon Sep 17 00:00:00 2001 From: Stefan Kolb Date: Wed, 22 Feb 2017 14:33:28 +0100 Subject: [PATCH 2/4] Remove closing brackets --- src/test/java/org/jabref/logic/l10n/LocalizationParser.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/jabref/logic/l10n/LocalizationParser.java b/src/test/java/org/jabref/logic/l10n/LocalizationParser.java index 09f5cc91675..a02bee4d0eb 100644 --- a/src/test/java/org/jabref/logic/l10n/LocalizationParser.java +++ b/src/test/java/org/jabref/logic/l10n/LocalizationParser.java @@ -290,7 +290,10 @@ public static List getLocalizationParameter(String content, Localization } else if (c == ')') { brackets--; } - buffer.append(c); + // skip closing brackets + if (brackets != 0) { + buffer.append(c); + } index++; } // trim newlines and whitespace From 09576fbce78673bb0fa6b7b82727bf945f6c409d Mon Sep 17 00:00:00 2001 From: Stefan Kolb Date: Wed, 22 Feb 2017 14:33:34 +0100 Subject: [PATCH 3/4] Add tests --- .../logic/l10n/LocalizationParserTest.java | 43 +++++++++++++------ 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/src/test/java/org/jabref/logic/l10n/LocalizationParserTest.java b/src/test/java/org/jabref/logic/l10n/LocalizationParserTest.java index 54b6e6bdc27..dd781a9599b 100644 --- a/src/test/java/org/jabref/logic/l10n/LocalizationParserTest.java +++ b/src/test/java/org/jabref/logic/l10n/LocalizationParserTest.java @@ -11,27 +11,44 @@ public class LocalizationParserTest { @Test - public void testParsingCode() { - assertLocalizationParsing("Localization.lang(\"one per line\")", "one_per_line"); - assertLocalizationParsing("Localization.lang(\n \"Copy \\\\cite{BibTeX key}\")", "Copy_\\cite{BibTeX_key}"); - assertLocalizationParsing("Localization.lang(\"two per line\") Localization.lang(\"two per line\")", Arrays.asList("two_per_line", "two_per_line")); - assertLocalizationParsing("Localization.lang(\"multi \" + \n\"line\")", "multi_line"); - assertLocalizationParsing("Localization.lang(\"one per line with var\", var)", "one_per_line_with_var"); - assertLocalizationParsing("Localization.lang(\"Search %0\", \"Springer\")", "Search_%0"); - assertLocalizationParsing("Localization.lang(\"Reset preferences (key1,key2,... or 'all')\")", "Reset_preferences_(key1,key2,..._or_'all')"); - assertLocalizationParsing("Localization.lang(\"Multiple entries selected. Do you want to change the type of all these to '%0'?\")", + public void testKeyParsingCode() { + assertLocalizationKeyParsing("Localization.lang(\"one per line\")", "one_per_line"); + assertLocalizationKeyParsing("Localization.lang(\n \"Copy \\\\cite{BibTeX key}\")", "Copy_\\cite{BibTeX_key}"); + assertLocalizationKeyParsing("Localization.lang(\"two per line\") Localization.lang(\"two per line\")", Arrays.asList("two_per_line", "two_per_line")); + assertLocalizationKeyParsing("Localization.lang(\"multi \" + \n\"line\")", "multi_line"); + assertLocalizationKeyParsing("Localization.lang(\"one per line with var\", var)", "one_per_line_with_var"); + assertLocalizationKeyParsing("Localization.lang(\"Search %0\", \"Springer\")", "Search_%0"); + assertLocalizationKeyParsing("Localization.lang(\"Reset preferences (key1,key2,... or 'all')\")", "Reset_preferences_(key1,key2,..._or_'all')"); + assertLocalizationKeyParsing("Localization.lang(\"Multiple entries selected. Do you want to change the type of all these to '%0'?\")", "Multiple_entries_selected._Do_you_want_to_change_the_type_of_all_these_to_'%0'?"); - assertLocalizationParsing("Localization.lang(\"Run fetcher, e.g. \\\"--fetch=Medline:cancer\\\"\");", + assertLocalizationKeyParsing("Localization.lang(\"Run fetcher, e.g. \\\"--fetch=Medline:cancer\\\"\");", "Run_fetcher,_e.g._\"--fetch\\=Medline\\:cancer\""); } - private void assertLocalizationParsing(String code, String expectedLanguageKeys) { - assertLocalizationParsing(code, Collections.singletonList(expectedLanguageKeys)); + @Test + public void testParameterParsingCode() { + assertLocalizationParameterParsing("Localization.lang(\"one per line\")", "\"one per line\""); + assertLocalizationParameterParsing("Localization.lang(\"one per line\" + var)", "\"one per line\" + var"); + assertLocalizationParameterParsing("Localization.lang(var + \"one per line\")", "var + \"one per line\""); + assertLocalizationParameterParsing("Localization.lang(\"Search %0\", \"Springer\")", "\"Search %0\", \"Springer\""); + } + + private void assertLocalizationKeyParsing(String code, String expectedLanguageKeys) { + assertLocalizationKeyParsing(code, Collections.singletonList(expectedLanguageKeys)); } - private void assertLocalizationParsing(String code, List expectedLanguageKeys) { + private void assertLocalizationKeyParsing(String code, List expectedLanguageKeys) { List languageKeysInString = LocalizationParser.JavaLocalizationEntryParser.getLanguageKeysInString(code, LocalizationBundleForTest.LANG); assertEquals(expectedLanguageKeys, languageKeysInString); } + private void assertLocalizationParameterParsing(String code, List expectedParameter) { + List languageKeysInString = LocalizationParser.JavaLocalizationEntryParser.getLocalizationParameter(code, LocalizationBundleForTest.LANG); + assertEquals(expectedParameter, languageKeysInString); + } + + private void assertLocalizationParameterParsing(String code, String expectedParameter) { + assertLocalizationParameterParsing(code, Collections.singletonList(expectedParameter)); + } + } From 0ab0cb577cc5a1d64373b15b760dc4405a72443e Mon Sep 17 00:00:00 2001 From: Stefan Kolb Date: Wed, 22 Feb 2017 14:38:40 +0100 Subject: [PATCH 4/4] Renaming --- .../java/org/jabref/logic/l10n/LocalizationConsistencyTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/jabref/logic/l10n/LocalizationConsistencyTest.java b/src/test/java/org/jabref/logic/l10n/LocalizationConsistencyTest.java index 8b5cec0b403..52bdb1e755b 100644 --- a/src/test/java/org/jabref/logic/l10n/LocalizationConsistencyTest.java +++ b/src/test/java/org/jabref/logic/l10n/LocalizationConsistencyTest.java @@ -170,7 +170,7 @@ public void findObsoleteMenuLocalizationKeys() throws IOException { } @Test - public void localizationMustOnlyUseConcatenatedStrings() throws IOException { + public void localizationParameterMustIncludeAString() throws IOException { // Must start or end with " // Localization.lang("test"), Localization.lang("test" + var), Localization.lang(var + "test") // TODO: Localization.lang(var1 + "test" + var2) not covered