Write Errors when a duplicate localization key is found. (#4885)

* Update Linguini to v0.8.1

* Add tests and verify desired behavior has been reached.

* Remove duplicate messages.

* Minor fix to message output. Add Wrapper for Fluent errors.

* Restart the test pipeline.

* Restart the test pipeline.

* Make so test don't do an early bailout.

* Ensure all errors get written rather than bailing on first.

* Fix text breakage.

* Remove obsolete // TODO LINGUINI

* line wrapping conventions

---------

Co-authored-by: metalgearsloth <comedian_vs_clown@hotmail.com>
Co-authored-by: ElectroJr <leonsfriedrich@gmail.com>
This commit is contained in:
Ygg01
2024-06-05 07:23:41 +02:00
committed by GitHub
parent a95ba9f181
commit 9240c94e59
8 changed files with 368 additions and 69 deletions

View File

@@ -15,10 +15,10 @@
<PackageVersion Include="ILReader.Core" Version="1.0.0.4" />
<PackageVersion Include="JetBrains.Annotations" Version="2023.3.0" />
<PackageVersion Include="JetBrains.Profiler.Api" Version="1.4.0" />
<PackageVersion Include="Linguini.Bundle" Version="0.1.3" />
<PackageVersion Include="Linguini.Bundle" Version="0.8.1" />
<PackageVersion Include="Microsoft.CodeAnalysis.Analyzers" Version="3.3.4" />
<PackageVersion Include="Microsoft.CodeAnalysis.Analyzer.Testing" Version="1.1.1"/>
<PackageVersion Include="Microsoft.CodeAnalysis.CSharp.Analyzer.Testing.NUnit" Version="1.1.1"/>
<PackageVersion Include="Microsoft.CodeAnalysis.Analyzer.Testing" Version="1.1.1" />
<PackageVersion Include="Microsoft.CodeAnalysis.CSharp.Analyzer.Testing.NUnit" Version="1.1.1" />
<PackageVersion Include="Microsoft.CodeAnalysis.CSharp" Version="4.8.0" />
<PackageVersion Include="Microsoft.CodeAnalysis.CSharp.Features" Version="4.8.0" />
<PackageVersion Include="Microsoft.CodeAnalysis.CSharp.Scripting" Version="4.8.0" />
@@ -71,4 +71,4 @@
<PackageVersion Include="prometheus-net.DotNetRuntime" Version="4.4.0" />
<PackageVersion Include="PolySharp" Version="1.14.1" />
</ItemGroup>
</Project>
</Project>

View File

@@ -252,9 +252,6 @@ cmd-bind-arg-command = <InputCommand>
cmd-net-draw-interp-desc = Toggles the debug drawing of the network interpolation.
cmd-net-draw-interp-help = Usage: net_draw_interp
cmd-net-draw-interp-desc = Toggles the debug drawing of the network interpolation.
cmd-net-draw-interp-help = Usage: net_draw_interp
cmd-net-watch-ent-desc = Dumps all network updates for an EntityId to the console.
cmd-net-watch-ent-help = Usage: net_watchent <0|EntityUid>
@@ -306,16 +303,9 @@ cmd-savegrid-help = savegrid <gridID> <Path>
cmd-testbed-desc = Loads a physics testbed on the specified map.
cmd-testbed-help = testbed <mapid> <test>
cmd-saveconfig-desc = Saves the client configuration to the config file.
cmd-saveconfig-help = saveconfig
## 'flushcookies' command
# Note: the flushcookies command is from Robust.Client.WebView, it's not in the main engine code.
cmd-flushcookies-desc = Flush CEF cookie storage to disk
cmd-flushcookies-help = This ensure cookies are properly saved to disk in the event of unclean shutdowns.
Note that the actual operation is asynchronous.
## 'addcomp' command
cmd-addcomp-desc = Adds a component to an entity.
cmd-addcomp-help = addcomp <uid> <componentName>
@@ -453,9 +443,6 @@ cmd-showanchored-help = Usage: showanchored
cmd-dmetamem-desc = Dumps a type's members in a format suitable for the sandbox configuration file.
cmd-dmetamem-help = Usage: dmetamem <type>
cmd-dmetamem-desc = Displays chunk bounds for the purposes of rendering.
cmd-dmetamem-help = Usage: showchunkbb <type>
cmd-launchauth-desc = Load authentication tokens from launcher data to aid in testing of live servers.
cmd-launchauth-help = Usage: launchauth <account name>
@@ -522,9 +509,6 @@ cmd-profsnap-help = Usage: profsnap
cmd-devwindow-desc = Dev Window
cmd-devwindow-help = Usage: devwindow
cmd-devwindow-desc = Open file
cmd-devwindow-help = Usage: testopenfile
cmd-scene-desc = Immediately changes the UI scene/state.
cmd-scene-help = Usage: scene <className>
@@ -535,14 +519,11 @@ cmd-hwid-desc = Returns the current HWID (HardWare ID).
cmd-hwid-help = Usage: hwid
cmd-vvread-desc = Retrieve a path's value using VV (View Variables).
cmd-vvread-desc = Usage: vvread <path>
cmd-vvread-help = Usage: vvread <path>
cmd-vvwrite-desc = Modify a path's value using VV (View Variables).
cmd-vvwrite-help = Usage: vvwrite <path>
cmd-vv-desc = Opens View Variables (VV).
cmd-vv-help = Usage: vv <path|entity ID|guihover>
cmd-vvinvoke-desc = Invoke/Call a path with arguments using VV.
cmd-vvinvoke-help = Usage: vvinvoke <path> [arguments...]

View File

@@ -1,6 +1,10 @@
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Text;
using Linguini.Bundle;
using Linguini.Bundle.Errors;
using Linguini.Syntax.Ast;
using Linguini.Syntax.Parser.Error;
using Robust.Shared.Collections;
using Robust.Shared.Utility;
@@ -17,6 +21,24 @@ internal static class LocHelper
return FormatErrors(self.Message, span, resource, newLine);
}
public static bool InsertResourcesAndReport(this FluentBundle bundle, Resource resource,
ResPath path, [NotNullWhen(false)] out List<LocError>? errors)
{
if (!bundle.AddResource(resource, out var parseErrors))
{
errors = new List<LocError>();
foreach (var fluentError in parseErrors)
{
errors.Add(new LocError(path, fluentError));
}
return false;
}
errors = null;
return true;
}
private static string FormatErrors(string message, ErrorSpan span, ReadOnlyMemory<char> resource, string? newLine)
{
newLine ??= Environment.NewLine;
@@ -69,3 +91,28 @@ internal static class LocHelper
return sb.ToString();
}
}
/// <summary>
/// Wrapper around Fluent Error, that adds path to the list of values.
/// Work in progress, FluentErrors need to be modified to be more accessible.
/// </summary>
internal record LocError
{
public readonly ResPath Path;
public readonly FluentError Error;
/// <summary>
/// Basic constructor.
/// </summary>
/// <param name="path">path of resource being added.</param>
/// <param name="fluentError">FluentError encountered.</param>
public LocError(ResPath path, FluentError fluentError)
{
Path = path;
Error = fluentError;
}
public override string ToString()
{
return $"[{Path.CanonPath}]: {Error}";
}
}

View File

@@ -3,6 +3,7 @@ using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using Linguini.Bundle;
using Linguini.Bundle.Errors;
using Robust.Shared.GameObjects;
using Robust.Shared.GameObjects.Components.Localization;
@@ -92,14 +93,14 @@ namespace Robust.Shared.Localization
var allErrors = new List<FluentError>();
if (desc == null
&& !bundle.TryGetMsg(locId, "desc", null, out var err1, out desc))
&& !bundle.TryGetMessage(locId, "desc", null, out var err1, out desc))
{
desc = null;
allErrors.AddRange(err1);
}
if (suffix == null
&& !bundle.TryGetMsg(locId, "suffix", null, out var err, out suffix))
&& !bundle.TryGetMessage(locId, "suffix", null, out var err, out suffix))
{
suffix = null;
}

View File

@@ -1,16 +1,13 @@
#nullable enable
using System;
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Text.RegularExpressions;
using Linguini.Bundle;
using Linguini.Bundle.Types;
using Linguini.Shared.Types.Bundle;
using Robust.Shared.Enums;
using Robust.Shared.GameObjects;
using Robust.Shared.GameObjects.Components.Localization;
using Robust.Shared.IoC;
using Robust.Shared.Maths;
namespace Robust.Shared.Localization
@@ -326,8 +323,7 @@ namespace Robust.Shared.Localization
private void AddCtxFunction(FluentBundle ctx, string name, LocFunction function)
{
ctx.AddFunction(name, (args, options)
=> CallFunction(function, ctx, args, options), out _, InsertBehavior.Overriding);
ctx.AddFunctionOverriding(name, (args, options) => CallFunction(function, ctx, args, options));
}
private IFluentType CallFunction(
@@ -356,8 +352,8 @@ namespace Robust.Shared.Localization
{
var bundle = _contexts[culture];
bundle.AddFunction(name, (args, options)
=> CallFunction(function, bundle, args, options), out _, InsertBehavior.Overriding);
bundle.AddFunctionOverriding(name, (args, options)
=> CallFunction(function, bundle, args, options));
}
}
@@ -377,6 +373,32 @@ namespace Robust.Shared.Localization
return WrappedValue.Format(_context);
}
public bool IsError()
{
return false;
}
public bool Matches(IFluentType other, IScope scope)
{
if (other is FluentLocWrapperType otherWrapper)
{
return (WrappedValue, otherWrapper.WrappedValue) switch
{
(LocValueNone, LocValueNone) => true,
(LocValueDateTime l, LocValueDateTime d) => l.Value.Equals(d.Value),
(LocValueTimeSpan l, LocValueTimeSpan d) => l.Value.Equals(d.Value),
(LocValueNumber l, LocValueNumber d) => l.Value.Equals(d.Value),
(LocValueString l, LocValueString d) => l.Value.Equals(d.Value),
(LocValueEntity l, LocValueEntity d) => l.Value.Equals(d.Value),
({ } l, { } d) => Equals(l, d),
_ => false,
};
}
return false;
}
public IFluentType Copy()
{
return this;

View File

@@ -4,6 +4,7 @@ using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.IO;
using System.Linq;
using System.Text;
using Linguini.Bundle;
using Linguini.Bundle.Builder;
using Linguini.Bundle.Errors;
@@ -47,7 +48,8 @@ namespace Robust.Shared.Localization
if (!TryGetString(messageId, out var msg))
{
_logSawmill.Debug("Unknown messageId ({culture}): {messageId}", _defaultCulture.Value.Item1.Name, messageId);
_logSawmill.Debug("Unknown messageId ({culture}): {messageId}", _defaultCulture.Value.Item1.Name,
messageId);
msg = messageId;
}
@@ -64,8 +66,9 @@ namespace Robust.Shared.Localization
if (TryGetString(messageId, out var argMsg, arg))
return argMsg;
_logSawmill.Debug("Unknown messageId ({culture}): {messageId}", _defaultCulture.Value.Item1.Name, messageId);
return messageId;
_logSawmill.Debug("Unknown messageId ({culture}): {messageId}", _defaultCulture.Value.Item1.Name,
messageId);
return messageId;
}
public string GetString(string messageId, (string, object) arg1, (string, object) arg2)
@@ -76,8 +79,9 @@ namespace Robust.Shared.Localization
if (TryGetString(messageId, out var argMsg, arg1, arg2))
return argMsg;
_logSawmill.Debug("Unknown messageId ({culture}): {messageId}", _defaultCulture.Value.Item1.Name, messageId);
return messageId;
_logSawmill.Debug("Unknown messageId ({culture}): {messageId}", _defaultCulture.Value.Item1.Name,
messageId);
return messageId;
}
public string GetString(string messageId, params (string, object)[] args)
@@ -88,8 +92,9 @@ namespace Robust.Shared.Localization
if (TryGetString(messageId, out var argMsg, args))
return argMsg;
_logSawmill.Debug("Unknown messageId ({culture}): {messageId}", _defaultCulture.Value.Item1.Name, messageId);
return messageId;
_logSawmill.Debug("Unknown messageId ({culture}): {messageId}", _defaultCulture.Value.Item1.Name,
messageId);
return messageId;
}
#endregion
@@ -101,7 +106,7 @@ namespace Robust.Shared.Localization
#region TryGetString
public bool TryGetString(string messageId, [NotNullWhen(true)] out string? value)
public bool TryGetString(string messageId, [NotNullWhen(true)] out string? value)
{
if (_defaultCulture == null)
{
@@ -112,7 +117,7 @@ public bool TryGetString(string messageId, [NotNullWhen(true)] out string? value
if (TryGetString(messageId, _defaultCulture.Value, out value))
return true;
foreach (var fallback in _fallbackCultures)
foreach (var fallback in _fallbackCultures)
{
if (TryGetString(messageId, fallback, out value))
return true;
@@ -122,18 +127,24 @@ public bool TryGetString(string messageId, [NotNullWhen(true)] out string? value
return false;
}
public bool TryGetString(string messageId, (CultureInfo, FluentBundle) bundle, [NotNullWhen(true)] out string? value)
public bool TryGetString(string messageId,
(CultureInfo, FluentBundle) bundle,
[NotNullWhen(true)] out string? value)
{
try
{
// TODO LINGUINI error list nullable.
var result = bundle.Item2.TryGetAttrMsg(messageId, null, out var errs, out value);
foreach (var err in errs)
if (bundle.Item2.TryGetAttrMessage(messageId, null, out var errors, out value))
return true;
if (errors != null)
{
_logSawmill.Error("{culture}/{messageId}: {error}", bundle.Item1.Name, messageId, err);
foreach (var err in errors)
{
_logSawmill.Error("{culture}/{messageId}: {error}", bundle.Item1.Name, messageId, err);
}
}
return result;
return false;
}
catch (Exception e)
{
@@ -143,7 +154,8 @@ public bool TryGetString(string messageId, [NotNullWhen(true)] out string? value
}
}
public bool TryGetString(string messageId, [NotNullWhen(true)] out string? value,
public bool TryGetString(string messageId,
[NotNullWhen(true)] out string? value,
(string, object) arg)
{
// TODO LINGUINI add try-get-message variant that takes in a (string, object)[]
@@ -165,8 +177,10 @@ public bool TryGetString(string messageId, [NotNullWhen(true)] out string? value
return TryGetString(messageId, out value, args, bundle, info);
}
public bool TryGetString(string messageId, [NotNullWhen(true)] out string? value,
(string, object) arg1, (string, object) arg2)
public bool TryGetString(string messageId,
[NotNullWhen(true)] out string? value,
(string, object) arg1,
(string, object) arg2)
{
// TODO LINGUINI add try-get-message variant that takes in a (string, object)[]
// I.e., have it automatically call FluentFromObject(context) with the right context if the message exists
@@ -188,7 +202,8 @@ public bool TryGetString(string messageId, [NotNullWhen(true)] out string? value
return TryGetString(messageId, out value, args, bundle, info);
}
public bool TryGetString(string messageId, [NotNullWhen(true)] out string? value,
public bool TryGetString(string messageId,
[NotNullWhen(true)] out string? value,
params (string, object)[] keyArgs)
{
// TODO LINGUINI add try-get-message variant that takes in a (string, object)[]
@@ -216,10 +231,13 @@ public bool TryGetString(string messageId, [NotNullWhen(true)] out string? value
{
try
{
var result = bundle.TryGetAttrMsg(messageId, args, out var errs, out value);
foreach (var err in errs)
var result = bundle.TryGetAttrMessage(messageId, args, out var errs, out value);
if (errs != null)
{
_logSawmill.Error("{culture}/{messageId}: {error}", culture.Name, messageId, err);
foreach (var err in errs)
{
_logSawmill.Error("{culture}/{messageId}: {error}", culture.Name, messageId, err);
}
}
return result;
@@ -245,14 +263,14 @@ public bool TryGetString(string messageId, [NotNullWhen(true)] out string? value
}
var idx = messageId.IndexOf('.');
if (idx != -1 )
if (idx != -1)
messageId = messageId.Remove(idx);
culture = _defaultCulture;
if (culture.Value.Item2.HasMessage(messageId))
return true;
foreach (var fallback in _fallbackCultures)
foreach (var fallback in _fallbackCultures)
{
culture = fallback;
if (culture.Value.Item2.HasMessage(messageId))
@@ -279,7 +297,7 @@ public bool TryGetString(string messageId, [NotNullWhen(true)] out string? value
if (bundle.TryGetAstMessage(messageId, out message))
return true;
foreach (var fallback in _fallbackCultures)
foreach (var fallback in _fallbackCultures)
{
bundle = fallback.Item2;
if (bundle.TryGetAstMessage(messageId, out message))
@@ -331,13 +349,13 @@ public bool TryGetString(string messageId, [NotNullWhen(true)] out string? value
_contexts.Add(culture, bundle);
AddBuiltInFunctions(bundle);
_loadData(_res, culture, bundle);
_initData(_res, culture, bundle);
DefaultCulture ??= culture;
}
public void SetFallbackCluture(params CultureInfo[] cultures)
{
_fallbackCultures = Array.Empty<(CultureInfo, FluentBundle)>();;
_fallbackCultures = Array.Empty<(CultureInfo, FluentBundle)>();
var tuples = new (CultureInfo, FluentBundle)[cultures.Length];
var i = 0;
foreach (var culture in cultures)
@@ -376,6 +394,41 @@ public bool TryGetString(string messageId, [NotNullWhen(true)] out string? value
}
private void _loadData(IResourceManager resourceManager, CultureInfo culture, FluentBundle context)
{
var resources = ReadLocaleFolder(resourceManager, culture);
foreach (var (path, resource, data) in resources)
{
var errors = resource.Errors;
context.AddResourceOverriding(resource);
WriteWarningForErrs(path, errors, data);
}
}
private void _initData(IResourceManager resourceManager, CultureInfo culture, FluentBundle context)
{
var resources = ReadLocaleFolder(resourceManager, culture);
var resErrors = new List<LocError>();
foreach (var (path, resource, data) in resources)
{
var errors = resource.Errors;
WriteWarningForErrs(path, errors, data);
if (!context.InsertResourcesAndReport(resource, path, out var errs))
{
resErrors.AddRange(errs);
}
}
if (resErrors.Count > 0)
{
WriteLocErrors(resErrors);
}
}
private static ParallelQuery<(ResPath path, Resource resource, string contents)> ReadLocaleFolder(
IResourceManager resourceManager, CultureInfo culture)
{
// Load data from .ftl files.
// Data is loaded from /Locale/<language-code>/*
@@ -400,13 +453,7 @@ public bool TryGetString(string messageId, [NotNullWhen(true)] out string? value
var resource = parser.Parse();
return (path, resource, contents);
});
foreach (var (path, resource, data) in resources)
{
var errors = resource.Errors;
context.AddResourceOverriding(resource);
WriteWarningForErrs(path, errors, data);
}
return resources;
}
private void WriteWarningForErrs(ResPath path, List<ParseError> errs, string resource)
@@ -417,12 +464,24 @@ public bool TryGetString(string messageId, [NotNullWhen(true)] out string? value
}
}
private void WriteWarningForErrs(IList<FluentError> errs, string locId)
private void WriteWarningForErrs(IList<FluentError>? errs, string locId)
{
if (errs == null) return;
foreach (var err in errs)
{
_logSawmill.Error("Error extracting `{locId}`\n{e1}", locId, err);
}
}
private void WriteLocErrors(IList<LocError>? errs)
{
if (errs == null) return;
var sbErr = new StringBuilder();
foreach (var err in errs)
{
sbErr.Append(err).AppendLine();
}
_logSawmill.Error(sbErr.ToString());
}
}
}

View File

@@ -0,0 +1,43 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using NUnit.Framework;
using Robust.Shared.ContentPack;
using Robust.Shared.IoC;
using Robust.Shared.Localization;
using Robust.Shared.Log;
namespace Robust.UnitTesting.Shared.Localization;
[TestFixture]
public sealed class LoadLocalizationTest : RobustUnitTest
{
private const string DuplicateTerm = @"
term1 = 1
term1 = 2
";
protected override void OverrideIoC()
{
base.OverrideIoC();
IoCManager.Register<ILogManager, SpyLogManager>(overwrite: true);
}
[Test]
public void TestLoadLocalization()
{
var res = IoCManager.Resolve<IResourceManagerInternal>();
res.MountString("/Locale/en-US/a.ftl", DuplicateTerm);
var loc = IoCManager.Resolve<ILocalizationManager>();
var spyLog = (SpyLogManager) IoCManager.Resolve<ILogManager>();
var culture = new CultureInfo("en-US", false);
var x = spyLog.CountError;
loc.LoadCulture(culture);
Assert.That(spyLog.CountError, NUnit.Framework.Is.GreaterThan(x));
}
}

View File

@@ -0,0 +1,146 @@
using System;
using System.Collections.Generic;
using Robust.Shared.Log;
namespace Robust.UnitTesting;
/// <summary>
/// Represents a log manager that spies on logged messages. Use this to check if side effect
/// like logging is being correctly done.
/// </summary>
public sealed class SpyLogManager : ILogManager
{
private readonly SpyLogger _spyLogger = new();
public ISawmill RootSawmill => _spyLogger;
public ISawmill GetSawmill(string name)
{
return _spyLogger;
}
public IEnumerable<ISawmill> AllSawmills => new[]
{
_spyLogger
};
public int CountError => _spyLogger.ErrorMessages.Count;
public void Clear()
{
_spyLogger.Clear();
}
}
/// <summary>
/// Represents a logger used for spying on log messages.
/// </summary>
public sealed class SpyLogger : ISawmill
{
public string Name { get; } = "SpyLogger";
public LogLevel? Level { get; set; } = LogLevel.Debug;
public List<string> DebugMessages = new();
public List<string> ErrorMessages = new();
public List<string> WarningMessages = new();
public List<string> InfoMessages = new();
public List<string> FatalMessages = new();
public List<string> VerboseMessages = new();
public void AddHandler(ILogHandler handler)
{
// NOT NEEDED
}
public void RemoveHandler(ILogHandler handler)
{
// NOT NEEDED
}
public void Log(LogLevel level, string message, params object?[] args)
{
Log(level, null, message, args);
}
public void Log(LogLevel level, Exception? exception, string message, params object?[] args)
{
var msg = string.Format(message, args);
var list = level switch
{
LogLevel.Verbose => VerboseMessages,
LogLevel.Debug => DebugMessages,
LogLevel.Info => InfoMessages,
LogLevel.Warning => WarningMessages,
LogLevel.Error => ErrorMessages,
LogLevel.Fatal => FatalMessages,
_ => VerboseMessages,
};
list.Add(msg);
}
public void Log(LogLevel level, string message)
{
Log(level, null, message, []);
}
public void Debug(string message, params object?[] args)
{
Log(LogLevel.Debug, null, message, args);
}
public void Debug(string message)
{
Log(LogLevel.Debug, message);
}
public void Info(string message, params object?[] args)
{
Log(LogLevel.Info, null, message, args);
}
public void Info(string message)
{
Log(LogLevel.Debug, message);
}
public void Warning(string message, params object?[] args)
{
Log(LogLevel.Warning, message, args);
}
public void Warning(string message)
{
Log(LogLevel.Warning, message);
}
public void Error(string message, params object?[] args)
{
Log(LogLevel.Error, message, args);
}
public void Error(string message)
{
Log(LogLevel.Error, message);
}
public void Fatal(string message, params object?[] args)
{
Log(LogLevel.Fatal, message, args);
}
public void Fatal(string message)
{
Log(LogLevel.Fatal, message);
}
public void Clear()
{
DebugMessages.Clear();
ErrorMessages.Clear();
WarningMessages.Clear();
InfoMessages.Clear();
FatalMessages.Clear();
VerboseMessages.Clear();
}
}