From 0bfa53b58d7ce95771d91cfbe43269c671540bda Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Fri, 19 Jul 2024 21:50:54 +0100 Subject: [PATCH] Teach CheckTranslationReference about translations in Lua scripts Using the glory of regex, we can scrape any Lua script files that a map includes and locate calls to the UserInterface.Translate method. We can then treat them in the same way as C# fields marked with a TranslationReferenceAttribute. This allows the lint check to validate the translation invoked in the .lua script has a matching entry in the translation .ftl files, with all the required arguments (if any). We can also locate any calls to AddPrimaryObjective or AddSecondaryObjective defined by the utils.lua script, which also accept translation keys. The are a couple of restrictions: - When linting the map, we don't check for keys in the ftl file that are unused. This is because the linter doesn't load all the keys when checking maps. - In order to validate translation arguments with the regex, we require the Lua script to pass the table of arguments inline at the callsite. If it does not, we raise a warning so the user can adjust the code. --- .../Lint/CheckTranslationReference.cs | 160 +++++++++++++----- mods/d2k/maps/atreides-01a/atreides01a.lua | 4 +- mods/d2k/maps/atreides-01b/atreides01b.lua | 4 +- mods/d2k/maps/atreides-03a/atreides03a.lua | 4 +- mods/d2k/maps/atreides-03b/atreides03b.lua | 4 +- mods/d2k/maps/atreides-05/atreides05.lua | 4 +- mods/d2k/maps/harkonnen-01a/harkonnen01a.lua | 4 +- mods/d2k/maps/harkonnen-01b/harkonnen01b.lua | 4 +- mods/d2k/maps/ordos-01a/ordos01a.lua | 4 +- mods/d2k/maps/ordos-01b/ordos01b.lua | 4 +- mods/d2k/maps/ordos-05/ordos05.lua | 4 +- mods/d2k/maps/ordos-06a/ordos06a.lua | 10 +- mods/ra/maps/exodus/exodus.lua | 6 +- .../evacuation.lua | 4 +- 14 files changed, 150 insertions(+), 70 deletions(-) diff --git a/OpenRA.Mods.Common/Lint/CheckTranslationReference.cs b/OpenRA.Mods.Common/Lint/CheckTranslationReference.cs index d1738d9fd0..14196bc260 100644 --- a/OpenRA.Mods.Common/Lint/CheckTranslationReference.cs +++ b/OpenRA.Mods.Common/Lint/CheckTranslationReference.cs @@ -17,7 +17,10 @@ using System.Reflection; using System.Text.RegularExpressions; using Linguini.Syntax.Ast; using Linguini.Syntax.Parser; +using OpenRA.Mods.Common.Scripting; +using OpenRA.Mods.Common.Scripting.Global; using OpenRA.Mods.Common.Traits; +using OpenRA.Scripting; using OpenRA.Traits; using OpenRA.Widgets; @@ -32,7 +35,7 @@ namespace OpenRA.Mods.Common.Lint if (map.TranslationDefinitions == null) return; - var usedKeys = GetUsedTranslationKeysInRuleset(map.Rules); + var usedKeys = GetUsedTranslationKeysInMap(map, emitWarning); foreach (var context in usedKeys.EmptyKeyContexts) emitWarning($"Empty key in map translation files required by {context}"); @@ -41,6 +44,8 @@ namespace OpenRA.Mods.Common.Lint foreach (var language in GetTranslationLanguages(modData)) { + CheckKeys(modData.Manifest.Translations.Concat(mapTranslations), map.Open, usedKeys, language, false, emitError, emitWarning); + var modTranslation = new Translation(language, modData.Manifest.Translations, modData.DefaultFileSystem, _ => { }); var mapTranslation = new Translation(language, mapTranslations, map, error => emitError(error.Message)); @@ -72,10 +77,19 @@ namespace OpenRA.Mods.Common.Lint Console.WriteLine($"Testing translation: {language}"); var translation = new Translation(language, modData.Manifest.Translations, modData.DefaultFileSystem, error => emitError(error.Message)); CheckModWidgets(modData, usedKeys, testedFields); - } - // With the fully populated keys, check keys and variables are not missing and not unused across all language files. - CheckModTranslationFiles(modData, usedKeys, emitError, emitWarning); + // With the fully populated keys, check keys and variables are not missing and not unused across all language files. + var keyWithAttrs = CheckKeys(modData.Manifest.Translations, modData.DefaultFileSystem.Open, usedKeys, language, true, emitError, emitWarning); + + foreach (var group in usedKeys.KeysWithContext) + { + if (keyWithAttrs.Contains(group.Key)) + continue; + + foreach (var context in group) + emitWarning($"Missing key `{group.Key}` in `{language}` language in mod translation files required by {context}"); + } + } // Check if we couldn't test any fields. const BindingFlags Binding = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static; @@ -119,6 +133,81 @@ namespace OpenRA.Mods.Common.Lint return usedKeys; } + static TranslationKeys GetUsedTranslationKeysInMap(Map map, Action emitWarning) + { + var usedKeys = GetUsedTranslationKeysInRuleset(map.Rules); + + var luaScriptInfo = map.Rules.Actors[SystemActors.World].TraitInfoOrDefault(); + if (luaScriptInfo != null) + { + // Matches expressions such as: + // UserInterface.Translate("translation-key") + // UserInterface.Translate("translation-key\"with-escape") + // UserInterface.Translate("translation-key", { ["attribute"] = foo }) + // UserInterface.Translate("translation-key", { ["attribute\"-with-escape"] = foo }) + // UserInterface.Translate("translation-key", { ["attribute1"] = foo, ["attribute2"] = bar }) + // UserInterface.Translate("translation-key", tableVariable) + // Extracts groups for the 'key' and each 'attr'. + // If the table isn't inline like in the last example, extracts it as 'variable'. + const string UserInterfaceTranslatePattern = + @"UserInterface\s*\.\s*Translate\s*\(" + // UserInterface.Translate( + @"\s*""(?(?:[^""\\]|\\.)+?)""\s*" + // "translation-key" + @"(,\s*({\s*\[\s*""(?(?:[^""\\]|\\.)*?)""\s*\]\s*=\s*.*?" + // { ["attribute1"] = foo + @"(\s*,\s*\[\s*""(?(?:[^""\\]|\\.)*?)""\s*\]\s*=\s*.*?)*\s*}\s*)" + // , ["attribute2"] = bar } + "|\\s*,\\s*(?.*?))?" + // tableVariable + @"\)"; // ) + var translateRegex = new Regex(UserInterfaceTranslatePattern); + + // The script in mods/common/scripts/utils.lua defines some helpers which accept a translation key + // Matches expressions such as: + // AddPrimaryObjective(Player, "translation-key") + // AddSecondaryObjective(Player, "translation-key") + // AddPrimaryObjective(Player, "translation-key\"with-escape") + // Extracts groups for the 'key'. + const string AddObjectivePattern = + @"(AddPrimaryObjective|AddSecondaryObjective)\s*\(" + // AddPrimaryObjective( + @".*?\s*,\s*""(?(?:[^""\\]|\\.)+?)""\s*" + // Player, "translation-key" + @"\)"; // ) + var objectiveRegex = new Regex(AddObjectivePattern); + + foreach (var script in luaScriptInfo.Scripts) + { + if (!map.TryOpen(script, out var scriptStream)) + continue; + + using (scriptStream) + { + var scriptText = scriptStream.ReadAllText(); + IEnumerable matches = translateRegex.Matches(scriptText); + if (luaScriptInfo.Scripts.Contains("utils.lua")) + matches = matches.Concat(objectiveRegex.Matches(scriptText)); + var scriptTranslations = matches.Select(m => + { + var key = m.Groups["key"].Value.Replace(@"\""", @""""); + var attrs = m.Groups["attr"].Captures.Select(c => c.Value.Replace(@"\""", @"""")).ToArray(); + var variable = m.Groups["variable"].Value; + var line = scriptText.Take(m.Index).Count(x => x == '\n') + 1; + return (Key: key, Attrs: attrs, Variable: variable, Line: line); + }).ToArray(); + foreach (var (key, attrs, variable, line) in scriptTranslations) + { + var context = $"Script {script}:{line}"; + usedKeys.Add(key, new TranslationReferenceAttribute(attrs), context); + + if (variable != "") + { + var userInterface = typeof(UserInterfaceGlobal).GetCustomAttribute().Name; + const string Translate = nameof(UserInterfaceGlobal.Translate); + emitWarning($"{context} calls {userInterface}.{Translate} with key `{key}` and translate args passed as `{variable}`. Inline the args at the callsite for lint analysis."); + } + } + } + } + } + + return usedKeys; + } + static (TranslationKeys UsedKeys, List TestedFields) GetUsedTranslationKeysInMod(ModData modData) { var usedKeys = GetUsedTranslationKeysInRuleset(modData.DefaultRules); @@ -233,54 +322,45 @@ namespace OpenRA.Mods.Common.Lint CheckChrome(n, translationReferencesByWidgetField, usedKeys); } - static void CheckModTranslationFiles(ModData modData, TranslationKeys usedKeys, Action emitError, Action emitWarning) + static HashSet CheckKeys(IEnumerable translationFiles, Func openFile, TranslationKeys usedKeys, string language, bool checkUnusedKeys, Action emitError, Action emitWarning) { - foreach (var language in GetTranslationLanguages(modData)) + var keyWithAttrs = new HashSet(); + foreach (var file in translationFiles) { - var keyWithAttrs = new HashSet(); - foreach (var file in modData.Manifest.Translations) + if (!file.EndsWith($"{language}.ftl", StringComparison.Ordinal)) + continue; + + var stream = openFile(file); + using (var reader = new StreamReader(stream)) { - if (!file.EndsWith($"{language}.ftl", StringComparison.Ordinal)) - continue; + var parser = new LinguiniParser(reader); + var result = parser.Parse(); - var stream = modData.DefaultFileSystem.Open(file); - using (var reader = new StreamReader(stream)) + foreach (var entry in result.Entries) { - var parser = new LinguiniParser(reader); - var result = parser.Parse(); + if (entry is not AstMessage message) + continue; - foreach (var entry in result.Entries) + IEnumerable<(Pattern Node, string AttributeName)> nodeAndAttributeNames; + if (message.Attributes.Count == 0) + nodeAndAttributeNames = new[] { (message.Value, (string)null) }; + else + nodeAndAttributeNames = message.Attributes.Select(a => (a.Value, a.Id.Name.ToString())); + + var key = message.GetId(); + foreach (var (node, attributeName) in nodeAndAttributeNames) { - if (entry is not AstMessage message) - continue; - - IEnumerable<(Pattern Node, string AttributeName)> nodeAndAttributeNames; - if (message.Attributes.Count == 0) - nodeAndAttributeNames = new[] { (message.Value, (string)null) }; - else - nodeAndAttributeNames = message.Attributes.Select(a => (a.Value, a.Id.Name.ToString())); - - var key = message.GetId(); - foreach (var (node, attributeName) in nodeAndAttributeNames) - { - keyWithAttrs.Add(attributeName == null ? key : $"{key}.{attributeName}"); + keyWithAttrs.Add(attributeName == null ? key : $"{key}.{attributeName}"); + if (checkUnusedKeys) CheckUnusedKey(key, attributeName, file, usedKeys, emitWarning); - CheckVariables(node, key, attributeName, file, usedKeys, emitError, emitWarning); - } + CheckVariables(node, key, attributeName, file, usedKeys, emitError, emitWarning); } } } - - foreach (var group in usedKeys.KeysWithContext) - { - if (keyWithAttrs.Contains(group.Key)) - continue; - - foreach (var context in group) - emitWarning($"Missing key `{group.Key}` in `{language}` language in mod translation files required by {context}"); - } } + return keyWithAttrs; + static void CheckUnusedKey(string key, string attribute, string file, TranslationKeys usedKeys, Action emitWarning) { var isAttribute = !string.IsNullOrEmpty(attribute); @@ -355,7 +435,7 @@ namespace OpenRA.Mods.Common.Lint return; } - if (translationReference.RequiredVariableNames != null) + if (translationReference.RequiredVariableNames != null && translationReference.RequiredVariableNames.Length > 0) { var rv = requiredVariablesByKey.GetOrAdd(key, _ => new HashSet()); rv.UnionWith(translationReference.RequiredVariableNames); diff --git a/mods/d2k/maps/atreides-01a/atreides01a.lua b/mods/d2k/maps/atreides-01a/atreides01a.lua index 6d633115ff..f8ceae20e5 100644 --- a/mods/d2k/maps/atreides-01a/atreides01a.lua +++ b/mods/d2k/maps/atreides-01a/atreides01a.lua @@ -87,8 +87,8 @@ Tick = function() end if Atreides.Resources ~= CachedResources then - local parameters = { ["harvested"] = Atreides.Resources, ["goal"] = SpiceToHarvest } - local harvestedResources = UserInterface.Translate("harvested-resources", parameters) + local harvestedResources = UserInterface.Translate("harvested-resources", + { ["harvested"] = Atreides.Resources, ["goal"] = SpiceToHarvest }) UserInterface.SetMissionText(harvestedResources) CachedResources = Atreides.Resources end diff --git a/mods/d2k/maps/atreides-01b/atreides01b.lua b/mods/d2k/maps/atreides-01b/atreides01b.lua index e7210d2a3b..7963857b19 100644 --- a/mods/d2k/maps/atreides-01b/atreides01b.lua +++ b/mods/d2k/maps/atreides-01b/atreides01b.lua @@ -87,8 +87,8 @@ Tick = function() end if Atreides.Resources ~= CachedResources then - local parameters = { ["harvested"] = Atreides.Resources, ["goal"] = SpiceToHarvest } - local harvestedResources = UserInterface.Translate("harvested-resources", parameters) + local harvestedResources = UserInterface.Translate("harvested-resources", + { ["harvested"] = Atreides.Resources, ["goal"] = SpiceToHarvest }) UserInterface.SetMissionText(harvestedResources) CachedResources = Atreides.Resources end diff --git a/mods/d2k/maps/atreides-03a/atreides03a.lua b/mods/d2k/maps/atreides-03a/atreides03a.lua index 26748a198b..e8c9463cde 100644 --- a/mods/d2k/maps/atreides-03a/atreides03a.lua +++ b/mods/d2k/maps/atreides-03a/atreides03a.lua @@ -110,8 +110,8 @@ Tick = function() end if Atreides.Resources ~= CachedResources then - local parameters = { ["harvested"] = Atreides.Resources, ["goal"] = SpiceToHarvest } - local harvestedResources = UserInterface.Translate("harvested-resources", parameters) + local harvestedResources = UserInterface.Translate("harvested-resources", + { ["harvested"] = Atreides.Resources, ["goal"] = SpiceToHarvest }) UserInterface.SetMissionText(harvestedResources) CachedResources = Atreides.Resources end diff --git a/mods/d2k/maps/atreides-03b/atreides03b.lua b/mods/d2k/maps/atreides-03b/atreides03b.lua index d012402615..e14b62c770 100644 --- a/mods/d2k/maps/atreides-03b/atreides03b.lua +++ b/mods/d2k/maps/atreides-03b/atreides03b.lua @@ -110,8 +110,8 @@ Tick = function() end if Atreides.Resources ~= CachedResources then - local parameters = { ["harvested"] = Atreides.Resources, ["goal"] = SpiceToHarvest } - local harvestedResources = UserInterface.Translate("harvested-resources", parameters) + local harvestedResources = UserInterface.Translate("harvested-resources", + { ["harvested"] = Atreides.Resources, ["goal"] = SpiceToHarvest }) UserInterface.SetMissionText(harvestedResources) CachedResources = Atreides.Resources end diff --git a/mods/d2k/maps/atreides-05/atreides05.lua b/mods/d2k/maps/atreides-05/atreides05.lua index 94f4de2dfc..abc50966a1 100644 --- a/mods/d2k/maps/atreides-05/atreides05.lua +++ b/mods/d2k/maps/atreides-05/atreides05.lua @@ -305,8 +305,8 @@ WorldLoaded = function() Trigger.AfterDelay(DateTime.Seconds(2), function() TimerTicks = ContrabandTimes[Difficulty] - local time = { ["time"] = Utils.FormatTime(TimerTicks) } - local contrabandApproaching = UserInterface.Translate("contraband-approaching-starport-north-in", time) + local time = Utils.FormatTime(TimerTicks) + local contrabandApproaching = UserInterface.Translate("contraband-approaching-starport-north-in", { ["time"] = time }) Media.DisplayMessage(contrabandApproaching, Mentat) end) diff --git a/mods/d2k/maps/harkonnen-01a/harkonnen01a.lua b/mods/d2k/maps/harkonnen-01a/harkonnen01a.lua index e0e9daa883..7b4e88852f 100644 --- a/mods/d2k/maps/harkonnen-01a/harkonnen01a.lua +++ b/mods/d2k/maps/harkonnen-01a/harkonnen01a.lua @@ -87,8 +87,8 @@ Tick = function() end if Harkonnen.Resources ~= CachedResources then - local parameters = { ["harvested"] = Harkonnen.Resources, ["goal"] = SpiceToHarvest } - local harvestedResources = UserInterface.Translate("harvested-resources", parameters) + local harvestedResources = UserInterface.Translate("harvested-resources", + { ["harvested"] = Harkonnen.Resources, ["goal"] = SpiceToHarvest }) UserInterface.SetMissionText(harvestedResources) CachedResources = Harkonnen.Resources end diff --git a/mods/d2k/maps/harkonnen-01b/harkonnen01b.lua b/mods/d2k/maps/harkonnen-01b/harkonnen01b.lua index e0e9daa883..7b4e88852f 100644 --- a/mods/d2k/maps/harkonnen-01b/harkonnen01b.lua +++ b/mods/d2k/maps/harkonnen-01b/harkonnen01b.lua @@ -87,8 +87,8 @@ Tick = function() end if Harkonnen.Resources ~= CachedResources then - local parameters = { ["harvested"] = Harkonnen.Resources, ["goal"] = SpiceToHarvest } - local harvestedResources = UserInterface.Translate("harvested-resources", parameters) + local harvestedResources = UserInterface.Translate("harvested-resources", + { ["harvested"] = Harkonnen.Resources, ["goal"] = SpiceToHarvest }) UserInterface.SetMissionText(harvestedResources) CachedResources = Harkonnen.Resources end diff --git a/mods/d2k/maps/ordos-01a/ordos01a.lua b/mods/d2k/maps/ordos-01a/ordos01a.lua index 2431c880b0..b9c413b042 100644 --- a/mods/d2k/maps/ordos-01a/ordos01a.lua +++ b/mods/d2k/maps/ordos-01a/ordos01a.lua @@ -87,8 +87,8 @@ Tick = function() end if Ordos.Resources ~= CachedResources then - local parameters = { ["harvested"] = Ordos.Resources, ["goal"] = SpiceToHarvest } - local harvestedResources = UserInterface.Translate("harvested-resources", parameters) + local harvestedResources = UserInterface.Translate("harvested-resources", + { ["harvested"] = Ordos.Resources, ["goal"] = SpiceToHarvest }) UserInterface.SetMissionText(harvestedResources) CachedResources = Ordos.Resources end diff --git a/mods/d2k/maps/ordos-01b/ordos01b.lua b/mods/d2k/maps/ordos-01b/ordos01b.lua index da1525065f..263228a22e 100644 --- a/mods/d2k/maps/ordos-01b/ordos01b.lua +++ b/mods/d2k/maps/ordos-01b/ordos01b.lua @@ -87,8 +87,8 @@ Tick = function() end if Ordos.Resources ~= CachedResources then - local parameters = { ["harvested"] = Ordos.Resources, ["goal"] = SpiceToHarvest } - local harvestedResources = UserInterface.Translate("harvested-resources", parameters) + local harvestedResources = UserInterface.Translate("harvested-resources", + { ["harvested"] = Ordos.Resources, ["goal"] = SpiceToHarvest }) UserInterface.SetMissionText(harvestedResources) CachedResources = Ordos.Resources end diff --git a/mods/d2k/maps/ordos-05/ordos05.lua b/mods/d2k/maps/ordos-05/ordos05.lua index ed91cfcb7d..e6a9322015 100644 --- a/mods/d2k/maps/ordos-05/ordos05.lua +++ b/mods/d2k/maps/ordos-05/ordos05.lua @@ -132,8 +132,8 @@ Tick = function() if Ordos.IsObjectiveCompleted(CaptureStarport) then if Ordos.Resources ~= CachedResources then - local parameters = { ["harvested"] = Ordos.Resources, ["goal"] = SpiceToHarvest } - local harvestedResources = UserInterface.Translate("harvested-resources", parameters) + local harvestedResources = UserInterface.Translate("harvested-resources", + { ["harvested"] = Ordos.Resources, ["goal"] = SpiceToHarvest }) UserInterface.SetMissionText(harvestedResources) CachedResources = Ordos.Resources end diff --git a/mods/d2k/maps/ordos-06a/ordos06a.lua b/mods/d2k/maps/ordos-06a/ordos06a.lua index 78db7e7279..10868c4e0a 100644 --- a/mods/d2k/maps/ordos-06a/ordos06a.lua +++ b/mods/d2k/maps/ordos-06a/ordos06a.lua @@ -211,10 +211,10 @@ Tick = function() FirstIxiansArrived = true SendContraband() elseif (TimerTicks % DateTime.Seconds(1)) == 0 then - local time = { ["time"] = Utils.FormatTime(TimerTicks) } - local reinforcementsText = UserInterface.Translate("initial-reinforcements-arrive-in", time) + local time = Utils.FormatTime(TimerTicks) + local reinforcementsText = UserInterface.Translate("initial-reinforcements-arrive-in", { ["time"] = time }) if FirstIxiansArrived then - reinforcementsText = UserInterface.Translate("additional-reinforcements-arrive-in", time) + reinforcementsText = UserInterface.Translate("additional-reinforcements-arrive-in", { ["time"] = time }) end UserInterface.SetMissionText(reinforcementsText, Ordos.Color) @@ -244,8 +244,8 @@ WorldLoaded = function() Trigger.AfterDelay(DateTime.Seconds(2), function() TimerTicks = InitialContrabandTimes[Difficulty] - local time = { ["time"] = Utils.FormatTime(TimerTicks) } - Media.DisplayMessage(UserInterface.Translate("ixian-reinforcements-in", time), Mentat) + local time = Utils.FormatTime(TimerTicks) + Media.DisplayMessage(UserInterface.Translate("ixian-reinforcements-in", { ["time"] = time }), Mentat) end) Hunt(Atreides) diff --git a/mods/ra/maps/exodus/exodus.lua b/mods/ra/maps/exodus/exodus.lua index b269c317ab..01d677d197 100644 --- a/mods/ra/maps/exodus/exodus.lua +++ b/mods/ra/maps/exodus/exodus.lua @@ -90,7 +90,7 @@ ReinforcementsTicks2 = DateTime.Minutes(10) Reinforcements2 = { "mgg", "2tnk", "2tnk", "2tnk", "2tnk", "truk", "truk", "truk", - "truk", "truk", "truk", "1tnk", "1tnk", "jeep", "jeep" + "truk", "truk", "truk", "1tnk", "1tnk", "jeep", "jeep" } SovietUnits1 = @@ -230,8 +230,8 @@ ManageSovietAircraft = function() end SetEvacuateMissionText = function() - local attributes = { ["evacuated"] = UnitsEvacuated, ["threshold"] = UnitsEvacuatedThreshold[Difficulty] } - local unitsEvacuated = UserInterface.Translate("units-evacuated", attributes) + local unitsEvacuated = UserInterface.Translate("units-evacuated", + { ["evacuated"] = UnitsEvacuated, ["threshold"] = UnitsEvacuatedThreshold[Difficulty] }) UserInterface.SetMissionText(unitsEvacuated, TextColor) end diff --git a/mods/ra/maps/fall-of-greece-2-evacuation/evacuation.lua b/mods/ra/maps/fall-of-greece-2-evacuation/evacuation.lua index 156b3a868b..24f7c0b8ff 100644 --- a/mods/ra/maps/fall-of-greece-2-evacuation/evacuation.lua +++ b/mods/ra/maps/fall-of-greece-2-evacuation/evacuation.lua @@ -178,8 +178,8 @@ VillageSetup = function() end SetCivilianEvacuatedText = function() - local attributes = { ["evacuated"] = CiviliansEvacuated, ["threshold"] = CiviliansEvacuatedThreshold } - local civiliansEvacuated = UserInterface.Translate("civilians-evacuated", attributes) + local civiliansEvacuated = UserInterface.Translate("civilians-evacuated", + { ["evacuated"] = CiviliansEvacuated, ["threshold"] = CiviliansEvacuatedThreshold }) UserInterface.SetMissionText(civiliansEvacuated, TextColor) end