From 10d7436cf0f2515519b0678db5dad5a0a961f274 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Fri, 5 Apr 2024 18:48:17 +0100 Subject: [PATCH] CheckTranslationReference lint pass learns additional checks. - Test all translation languages, not just English. - Report any fields marked with TranslationReferenceAttribute that the lint pass lacked the knowledge to check. - Improve context provided by lint messages. Restructure code for readability. --- .../Lint/CheckTranslationReference.cs | 470 ++++++++++-------- 1 file changed, 268 insertions(+), 202 deletions(-) diff --git a/OpenRA.Mods.Common/Lint/CheckTranslationReference.cs b/OpenRA.Mods.Common/Lint/CheckTranslationReference.cs index 50e3371cceab..aebc862bfe46 100644 --- a/OpenRA.Mods.Common/Lint/CheckTranslationReference.cs +++ b/OpenRA.Mods.Common/Lint/CheckTranslationReference.cs @@ -14,6 +14,7 @@ using System.IO; using System.Linq; using System.Reflection; +using System.Text.RegularExpressions; using Linguini.Syntax.Ast; using Linguini.Syntax.Parser; using OpenRA.Traits; @@ -23,14 +24,80 @@ namespace OpenRA.Mods.Common.Lint { sealed class CheckTranslationReference : ILintPass, ILintMapPass { - const BindingFlags Binding = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static; + static readonly Regex TranslationFilenameRegex = new(@"(?[^\/\\]+)\.ftl$"); - readonly List referencedKeys = new(); - readonly Dictionary referencedVariablesPerKey = new(); - readonly List variableReferences = new(); + void ILintMapPass.Run(Action emitError, Action emitWarning, ModData modData, Map map) + { + if (map.TranslationDefinitions == null) + return; + + var usedKeys = GetUsedTranslationKeysInRuleset(map.Rules); + + foreach (var context in usedKeys.EmptyKeyContexts) + emitWarning($"Empty key in map translation files required by {context}"); + + var mapTranslations = FieldLoader.GetValue("value", map.TranslationDefinitions.Value); + + foreach (var language in GetTranslationLanguages(modData)) + { + var modTranslation = new Translation(language, modData.Manifest.Translations, modData.DefaultFileSystem, _ => { }); + var mapTranslation = new Translation(language, mapTranslations, map, error => emitError(error.Message)); + + foreach (var group in usedKeys.KeysWithContext) + { + if (modTranslation.HasMessage(group.Key)) + { + if (mapTranslation.HasMessage(group.Key)) + emitWarning($"Key `{group.Key}` in `{language}` language in map translation files already exists in mod translations and will not be used."); + } + else if (!mapTranslation.HasMessage(group.Key)) + { + foreach (var context in group) + emitWarning($"Missing key `{group.Key}` in `{language}` language in map translation files required by {context}"); + } + } + } + } + + void ILintPass.Run(Action emitError, Action emitWarning, ModData modData) + { + var (usedKeys, testedFields) = GetUsedTranslationKeysInMod(modData); + + foreach (var context in usedKeys.EmptyKeyContexts) + emitWarning($"Empty key in mod translation files required by {context}"); + + foreach (var language in GetTranslationLanguages(modData)) + { + 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); + + // Check if we couldn't test any fields. + const BindingFlags Binding = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static; + var allTranslatableFields = modData.ObjectCreator.GetTypes().SelectMany(t => + t.GetFields(Binding).Where(m => Utility.HasAttribute(m))).ToArray(); + var untestedFields = allTranslatableFields.Except(testedFields); + foreach (var field in untestedFields) + emitError( + $"Lint pass ({nameof(CheckTranslationReference)}) lacks the know-how to test translatable field " + + $"`{field.ReflectedType.Name}.{field.Name}` - previous warnings may be incorrect"); + } - void TestTraits(Ruleset rules, Action emitError, Action testKey) + static IEnumerable GetTranslationLanguages(ModData modData) { + return modData.Manifest.Translations + .Select(filename => TranslationFilenameRegex.Match(filename).Groups["language"].Value) + .Distinct() + .OrderBy(l => l); + } + + static TranslationKeys GetUsedTranslationKeysInRuleset(Ruleset rules) + { + var usedKeys = new TranslationKeys(); foreach (var actorInfo in rules.Actors) { foreach (var traitInfo in actorInfo.Value.TraitInfos()) @@ -38,274 +105,273 @@ void TestTraits(Ruleset rules, Action emitError, Action testKey) var traitType = traitInfo.GetType(); foreach (var field in Utility.GetFields(traitType)) { - var translationReference = Utility.GetCustomAttributes(field, true).FirstOrDefault(); + var translationReference = Utility.GetCustomAttributes(field, true).SingleOrDefault(); if (translationReference == null) continue; var keys = LintExts.GetFieldValues(traitInfo, field, translationReference.DictionaryReference); foreach (var key in keys) - { - if (key == null) - { - if (!translationReference.Optional) - emitError($"Actor type `{actorInfo.Key}` has an empty translation reference in trait `{traitType.Name[..^4]}.{field.Name}`."); - - continue; - } - - if (referencedKeys.Contains(key)) - continue; - - testKey(key); - referencedKeys.Add(key); - } + usedKeys.Add(key, translationReference, $"Actor `{actorInfo.Key}` trait `{traitType.Name[..^4]}.{field.Name}`"); } } } - } - - void ILintMapPass.Run(Action emitError, Action emitWarning, ModData modData, Map map) - { - if (map.TranslationDefinitions == null) - return; - - // TODO: Check all available languages. - const string Language = "en"; - var modTranslation = new Translation(Language, modData.Manifest.Translations, modData.DefaultFileSystem, _ => { }); - var mapTranslation = new Translation(Language, FieldLoader.GetValue("value", map.TranslationDefinitions.Value), map, error => emitError(error.ToString())); - TestTraits(map.Rules, emitError, key => - { - if (modTranslation.HasMessage(key)) - { - if (mapTranslation.HasMessage(key)) - emitWarning($"Map translation key `{key}` already exists in `{Language}` mod translations and will not be used."); - } - else if (!mapTranslation.HasMessage(key)) - emitWarning($"`{key}` is not present in `{Language}` translation."); - }); + return usedKeys; } - void ILintPass.Run(Action emitError, Action emitWarning, ModData modData) + static (TranslationKeys UsedKeys, List TestedFields) GetUsedTranslationKeysInMod(ModData modData) { - // TODO: Check all available languages. - const string Language = "en"; - Console.WriteLine($"Testing translation: {Language}"); - var translation = new Translation(Language, modData.Manifest.Translations, modData.DefaultFileSystem, error => emitError(error.ToString())); - - TestTraits(modData.DefaultRules, emitError, key => - { - if (!translation.HasMessage(key)) - emitWarning($"`{key}` is not present in `{Language}` translation."); - }); - + var usedKeys = GetUsedTranslationKeysInRuleset(modData.DefaultRules); + var testedFields = new List(); + testedFields.AddRange( + modData.ObjectCreator.GetTypes() + .Where(t => t.IsSubclassOf(typeof(TraitInfo))) + .SelectMany(t => t.GetFields().Where(f => f.HasAttribute()))); + + // HACK: Need to hardcode the custom loader for GameSpeeds. var gameSpeeds = modData.Manifest.Get(); + var gameSpeedNameField = typeof(GameSpeed).GetField(nameof(GameSpeed.Name)); + var gameSpeedTranslationReference = Utility.GetCustomAttributes(gameSpeedNameField, true)[0]; + testedFields.Add(gameSpeedNameField); foreach (var speed in gameSpeeds.Speeds.Values) - { - if (!translation.HasMessage(speed.Name)) - emitWarning($"`{speed.Name}` is not present in `{Language}` translation."); - - referencedKeys.Add(speed.Name); - } + usedKeys.Add(speed.Name, gameSpeedTranslationReference, $"`{nameof(GameSpeed)}.{nameof(GameSpeed.Name)}`"); foreach (var modType in modData.ObjectCreator.GetTypes()) { - foreach (var fieldInfo in modType.GetFields(Binding).Where(m => Utility.HasAttribute(m))) + const BindingFlags Binding = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static; + foreach (var field in modType.GetFields(Binding)) { - if (fieldInfo.IsInitOnly || !fieldInfo.IsStatic) + // Checking for constant string fields. + if (!field.IsLiteral) continue; - if (fieldInfo.FieldType != typeof(string)) - { - emitError($"Translation attribute on non string field `{fieldInfo.Name}`."); + var translationReference = Utility.GetCustomAttributes(field, true).SingleOrDefault(); + if (translationReference == null) continue; - } - - var key = (string)fieldInfo.GetValue(null); - if (referencedKeys.Contains(key)) - continue; - - if (!translation.HasMessage(key)) - emitWarning($"`{key}` is not present in `{Language}` translation."); - - var translationReference = Utility.GetCustomAttributes(fieldInfo, true)[0]; - if (translationReference.RequiredVariableNames != null && translationReference.RequiredVariableNames.Length > 0) - referencedVariablesPerKey.GetOrAdd(key, translationReference.RequiredVariableNames); - referencedKeys.Add(key); + testedFields.Add(field); + var keys = LintExts.GetFieldValues(null, field, translationReference.DictionaryReference); + foreach (var key in keys) + usedKeys.Add(key, translationReference, $"`{field.ReflectedType.Name}.{field.Name}`"); } } - var translatableFields = modData.ObjectCreator.GetTypes() + return (usedKeys, testedFields); + } + + static void CheckModWidgets(ModData modData, TranslationKeys usedKeys, List testedFields) + { + var chromeLayoutNodes = BuildChromeTree(modData); + + var widgetTypes = modData.ObjectCreator.GetTypes() .Where(t => t.Name.EndsWith("Widget", StringComparison.InvariantCulture) && t.IsSubclassOf(typeof(Widget))) + .ToList(); + var translationReferencesByWidgetField = widgetTypes.SelectMany(t => + { + var widgetName = t.Name[..^6]; + return Utility.GetFields(t) + .Select(f => + { + var attribute = Utility.GetCustomAttributes(f, true).SingleOrDefault(); + return (WidgetName: widgetName, FieldName: f.Name, TranslationReference: attribute); + }) + .Where(x => x.TranslationReference != null); + }) .ToDictionary( - t => t.Name[..^6], - t => t.GetFields().Where(f => f.HasAttribute()).ToArray()) - .Where(t => t.Value.Length > 0) - .ToDictionary( - t => t.Key, - t => t.Value.Select(f => (f.Name, f, Utility.GetCustomAttributes(f, true)[0])).ToArray()); + x => (x.WidgetName, x.FieldName), + x => x.TranslationReference); - foreach (var filename in modData.Manifest.ChromeLayout) + testedFields.AddRange(widgetTypes.SelectMany( + t => Utility.GetFields(t).Where(Utility.HasAttribute))); + + foreach (var node in chromeLayoutNodes) + CheckChrome(node, translationReferencesByWidgetField, usedKeys); + } + + static MiniYamlNode[] BuildChromeTree(ModData modData) + { + // Gather all the nodes together for evaluation. + var chromeLayoutNodes = modData.Manifest.ChromeLayout + .SelectMany(filename => MiniYaml.FromStream(modData.DefaultFileSystem.Open(filename), filename)) + .ToArray(); + + return chromeLayoutNodes; + } + + static void CheckChrome( + MiniYamlNode rootNode, + Dictionary<(string WidgetName, string FieldName), TranslationReferenceAttribute> translationReferencesByWidgetField, + TranslationKeys usedKeys) + { + var nodeType = rootNode.Key.Split('@')[0]; + foreach (var childNode in rootNode.Value.Nodes) { - var nodes = MiniYaml.FromStream(modData.DefaultFileSystem.Open(filename), filename); - foreach (var node in nodes) - CheckChrome(node, translation, Language, emitError, emitWarning, translatableFields); + var childType = childNode.Key.Split('@')[0]; + if (!translationReferencesByWidgetField.TryGetValue((nodeType, childType), out var translationReference)) + continue; + + var key = childNode.Value.Value; + usedKeys.Add(key, translationReference, $"Widget `{rootNode.Key}` field `{childType}` in {rootNode.Location}"); } - foreach (var file in modData.Manifest.Translations) + foreach (var childNode in rootNode.Value.Nodes) + if (childNode.Key == "Children") + foreach (var n in childNode.Value.Nodes) + CheckChrome(n, translationReferencesByWidgetField, usedKeys); + } + + static void CheckModTranslationFiles(ModData modData, TranslationKeys usedKeys, Action emitError, Action emitWarning) + { + foreach (var language in GetTranslationLanguages(modData)) { - var stream = modData.DefaultFileSystem.Open(file); - using (var reader = new StreamReader(stream)) + var keyWithAttrs = new HashSet(); + foreach (var file in modData.Manifest.Translations) { - var parser = new LinguiniParser(reader); - var result = parser.Parse(); + if (!file.EndsWith($"{language}.ftl", StringComparison.Ordinal)) + continue; - foreach (var entry in result.Entries) + var stream = modData.DefaultFileSystem.Open(file); + using (var reader = new StreamReader(stream)) { - // Don't flag definitions referenced (only) within the .ftl definitions as unused. - if (entry.GetType() == typeof(AstTerm)) - continue; + var parser = new LinguiniParser(reader); + var result = parser.Parse(); - variableReferences.Clear(); - var key = entry.GetId(); - - if (entry is AstMessage message) + foreach (var entry in result.Entries) { - var hasAttributes = message.Attributes.Count > 0; + if (entry is not AstMessage message) + continue; - if (!hasAttributes) - { - CheckUnusedKey(key, null, emitWarning, file); - CheckMessageValue(message.Value, key, null, emitWarning, file); - CheckMissingVariable(key, null, emitError, file); - } + 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) { - foreach (var attribute in message.Attributes) - { - var attrName = attribute.Id.Name.ToString(); - - CheckUnusedKey(key, attrName, emitWarning, file); - CheckMessageValue(attribute.Value, key, attrName, emitWarning, file); - CheckMissingVariable(key, attrName, emitError, file); - } + keyWithAttrs.Add(attributeName == null ? key : $"{key}.{attributeName}"); + CheckUnusedKey(key, attributeName, file, usedKeys, 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}"); + } } - } - void CheckChrome(MiniYamlNode node, Translation translation, string language, Action emitError, Action emitWarning, - Dictionary translatables) - { - var nodeType = node.Key.Split('@')[0]; - foreach (var childNode in node.Value.Nodes) + static void CheckUnusedKey(string key, string attribute, string file, TranslationKeys usedKeys, Action emitWarning) { - if (!translatables.TryGetValue(nodeType, out var translationNodes)) - continue; + var isAttribute = !string.IsNullOrEmpty(attribute); + var keyWithAtrr = isAttribute ? $"{key}.{attribute}" : key; - var childType = childNode.Key.Split('@')[0]; - var field = translationNodes.FirstOrDefault(t => t.Name == childType); - if (field.Name == null) - continue; + if (!usedKeys.Contains(keyWithAtrr)) + emitWarning(isAttribute ? + $"Unused attribute `{attribute}` of key `{key}` in {file}" : + $"Unused key `{key}` in {file}"); + } - var key = childNode.Value.Value; - if (key == null) + static void CheckVariables( + Pattern node, string key, string attribute, string file, TranslationKeys usedKeys, + Action emitError, Action emitWarning) + { + var isAttribute = !string.IsNullOrEmpty(attribute); + var keyWithAtrr = isAttribute ? $"{key}.{attribute}" : key; + + if (!usedKeys.TryGetRequiredVariables(keyWithAtrr, out var requiredVariables)) + return; + + var variableNames = new HashSet(); + foreach (var element in node.Elements) { - if (!field.Attribute.Optional) - emitError($"Widget `{node.Key}` in field `{childType}` has an empty translation reference."); + if (element is not Placeable placeable) + continue; - continue; + AddVariableAndCheckUnusedVariable(placeable); + if (placeable.Expression is SelectExpression selectExpression) + foreach (var variant in selectExpression.Variants) + foreach (var variantElement in variant.Value.Elements) + if (variantElement is Placeable variantPlaceable) + AddVariableAndCheckUnusedVariable(variantPlaceable); } - if (referencedKeys.Contains(key)) - continue; + void AddVariableAndCheckUnusedVariable(Placeable placeable) + { + if (placeable.Expression is not IInlineExpression inlineExpression || + inlineExpression is not VariableReference variableReference) + return; - if (!key.Any(char.IsLetter)) - continue; + var name = variableReference.Id.Name.ToString(); + variableNames.Add(name); - if (!translation.HasMessage(key)) - emitWarning($"`{key}` defined by `{node.Key}` is not present in `{language}` translation."); + if (!requiredVariables.Contains(name)) + emitWarning(isAttribute ? + $"Unused variable `{name}` for attribute `{attribute}` of key `{key}` in {file}" : + $"Unused variable `{name}` for key `{key}` in {file}"); + } - referencedKeys.Add(key); + foreach (var name in requiredVariables) + if (!variableNames.Contains(name)) + emitError(isAttribute ? + $"Missing variable `{name}` for attribute `{attribute}` of key `{key}` in {file}" : + $"Missing variable `{name}` for key `{key}` in {file}"); } - - foreach (var childNode in node.Value.Nodes) - if (childNode.Key == "Children") - foreach (var n in childNode.Value.Nodes) - CheckChrome(n, translation, language, emitError, emitWarning, translatables); } - void CheckUnusedKey(string key, string attribute, Action emitWarning, string file) + class TranslationKeys { - var isAttribute = !string.IsNullOrEmpty(attribute); - var keyWithAtrr = isAttribute ? $"{key}.{attribute}" : key; + readonly HashSet keys = new(); + readonly List<(string Key, string Context)> keysWithContext = new(); + readonly Dictionary> requiredVariablesByKey = new(); + readonly List contextForEmptyKeys = new(); - if (!referencedKeys.Contains(keyWithAtrr)) - emitWarning(isAttribute ? - $"Unused attribute `{attribute}` of key `{key}` in {file}" : - $"Unused key `{key}` in {file}"); - } - - void CheckMessageValue(Pattern node, string key, string attribute, Action emitWarning, string file) - { - foreach (var element in node.Elements) + public void Add(string key, TranslationReferenceAttribute translationReference, string context) { - if (element is Placeable placeable) + if (key == null) { - var expression = placeable.Expression; - if (expression is IInlineExpression inlineExpression && - inlineExpression is VariableReference variableReference) - CheckVariableReference(variableReference.Id.Name.ToString(), key, attribute, emitWarning, file); + if (!translationReference.Optional) + contextForEmptyKeys.Add(context); + return; + } - if (expression is SelectExpression selectExpression) - { - foreach (var variant in selectExpression.Variants) - { - foreach (var variantElement in variant.Value.Elements) - { - if (variantElement is Placeable variantPlaceable) - { - var variantExpression = variantPlaceable.Expression; - if (variantExpression is IInlineExpression variantInlineExpression && - variantInlineExpression is VariableReference variantVariableReference) - CheckVariableReference(variantVariableReference.Id.Name.ToString(), key, attribute, emitWarning, file); - } - } - } - } + if (translationReference.RequiredVariableNames != null) + { + var rv = requiredVariablesByKey.GetOrAdd(key, _ => new HashSet()); + rv.UnionWith(translationReference.RequiredVariableNames); } - } - } - void CheckMissingVariable(string key, string attribute, Action emitError, string file) - { - var isAttribute = !string.IsNullOrEmpty(attribute); - var keyWithAtrr = isAttribute ? $"{key}.{attribute}" : key; + keys.Add(key); + keysWithContext.Add((key, context)); + } - if (!referencedVariablesPerKey.TryGetValue(keyWithAtrr, out var referencedVariables)) - return; + public bool TryGetRequiredVariables(string key, out ISet requiredVariables) + { + if (requiredVariablesByKey.TryGetValue(key, out var rv)) + { + requiredVariables = rv; + return true; + } - foreach (var referencedVariable in referencedVariables) - if (!variableReferences.Contains(referencedVariable)) - emitError(isAttribute ? - $"Missing variable `{referencedVariable}` for attribute `{attribute}` of key `{key}` in {file}" : - $"Missing variable `{referencedVariable}` for key `{key}` in {file}"); - } + requiredVariables = null; + return false; + } - void CheckVariableReference(string varName, string key, string attribute, Action emitWarning, string file) - { - var isAttribute = !string.IsNullOrEmpty(attribute); - var keyWithAtrr = isAttribute ? $"{key}.{attribute}" : key; + public bool Contains(string key) + { + return keys.Contains(key); + } - variableReferences.Add(varName); + public ILookup KeysWithContext => keysWithContext.OrderBy(x => x.Key).ToLookup(x => x.Key, x => x.Context); - if (!referencedVariablesPerKey.TryGetValue(keyWithAtrr, out var referencedVariables) || !referencedVariables.Contains(varName)) - emitWarning(isAttribute ? - $"Unused variable `{varName}` for attribute `{attribute}` of key `{key}` in {file}" : - $"Unused variable `{varName}` for key `{key}` in {file}"); + public IEnumerable EmptyKeyContexts => contextForEmptyKeys; } } }