From 8add96e3ce7d368856163e4d6008f38921759a1d Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 27 Jul 2026 22:14:42 -0700 Subject: [PATCH 1/7] Emit hygienic generated code The generated files carried nothing but a bare attribution comment, so the compiler and every analyzer treated them as hand written code. In projects with `enable`, `` and a strict analysis level, that made the build fail outright (fixes #44, fixes #45). Changes: - Every emitted file now starts with `// `, followed by the attribution comment and an explicit `#nullable enable`. The marker is what Roslyn, StyleCop and third party analyzers look for to skip generated code, and the directive makes the output independent from the consumer's nullable setting. - The attribution comment of the strongly typed class was silently dropped before: it was attached to the compilation unit and then discarded by the following `WithUsings` call. - The XML documentation of the generated members was silently dropped too: the `#region` trivia replaced the leading trivia instead of extending it. - Every visible generated member is now documented (CS1591), including all of its parameters (CS1573), and resource values are XML escaped before being inlined into a `` element (CS1570). - `Converter` and `ConverterParameter` on the generated markup extension are annotated as nullable, and the plural and macro templates are made nullable correct. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../CodeGenerators/CsharpCodeGenerator.cs | 172 +++++++++++++----- .../CodeGenerators/GeneratedCode.cs | 40 ++++ src/ReswPlus.SourceGenerator/ReswGenerator.cs | 5 +- .../ReswPlus.SourceGenerator.csproj | 3 + .../Templates/Macros/Macros.txt | 46 ++--- .../Templates/Plurals/IPluralProvider.txt | 8 + .../Templates/Plurals/PluralTypeEnum.txt | 9 + .../Plurals/ResourceLoaderExtension.txt | 12 +- tests/ReswPlusUnitTests/FormatTagPlurals.cs | 1 + .../ReswPlusUnitTests/GeneratedCodeHygiene.cs | 140 ++++++++++++++ .../ReswPlusUnitTests.csproj | 2 + tests/ReswPlusUnitTests/ReswTestHelpers.cs | 69 +++++++ 12 files changed, 431 insertions(+), 76 deletions(-) create mode 100644 src/ReswPlus.SourceGenerator/CodeGenerators/GeneratedCode.cs create mode 100644 tests/ReswPlusUnitTests/GeneratedCodeHygiene.cs create mode 100644 tests/ReswPlusUnitTests/ReswTestHelpers.cs diff --git a/src/ReswPlus.SourceGenerator/CodeGenerators/CsharpCodeGenerator.cs b/src/ReswPlus.SourceGenerator/CodeGenerators/CsharpCodeGenerator.cs index 7e91b9b..04bd724 100644 --- a/src/ReswPlus.SourceGenerator/CodeGenerators/CsharpCodeGenerator.cs +++ b/src/ReswPlus.SourceGenerator/CodeGenerators/CsharpCodeGenerator.cs @@ -22,7 +22,10 @@ namespace ReswPlus.SourceGenerator.CodeGenerators; /// /// The generated code may look similar to the following: /// +/// // <auto-generated/> /// // File generated automatically by ReswPlus. https://github.com/DotNetPlus/ReswPlus +/// #nullable enable +/// /// using System; /// using Windows.UI.Xaml.Markup; /// using Windows.UI.Xaml.Data; @@ -56,6 +59,11 @@ namespace ReswPlus.SourceGenerator.CodeGenerators; /// internal sealed class CSharpCodeGenerator : ICodeGenerator { + /// + /// The text used for the <returns> element of the generated lookup members. + /// + private const string LocalizedStringReturns = "The localized string for the current UI culture."; + /// /// Generates a collection of files containing the source code. /// @@ -65,15 +73,8 @@ internal sealed class CSharpCodeGenerator : ICodeGenerator /// A collection of generated files. public IEnumerable GetGeneratedFiles(string? baseFilename, StronglyTypedClass info, ResourceFileInfo resourceFileInfo) { - // Create a header comment that will be placed at the top of the generated file. - var headerTrivia = TriviaList( - Comment("// File generated automatically by ReswPlus. https://github.com/DotNetPlus/ReswPlus"), - CarriageReturnLineFeed - ); - // Build the compilation unit (the root node of a C# file) and add required using directives. var compilationUnit = CompilationUnit() - .WithLeadingTrivia(headerTrivia) .WithUsings(List(GetUsings(info.AppType))); // Create the strongly-typed static class declaration (the class that will provide resource lookup). @@ -115,10 +116,10 @@ public IEnumerable GetGeneratedFiles(string? baseFilename, Strong ) ) ); - // Attach the region directives as leading and trailing trivia. + // Attach the region directives, keeping the documentation comment already attached to the member. membersForItem[i] = membersForItem[i] - .WithLeadingTrivia(SyntaxFactory.TriviaList(regionDirectiveTrivia)) - .WithTrailingTrivia(SyntaxFactory.TriviaList(endRegionDirectiveTrivia)); + .WithLeadingTrivia(membersForItem[i].GetLeadingTrivia().Insert(0, regionDirectiveTrivia)) + .WithTrailingTrivia(membersForItem[i].GetTrailingTrivia().Add(endRegionDirectiveTrivia)); } formatMembers.AddRange(membersForItem); } @@ -143,10 +144,98 @@ public IEnumerable GetGeneratedFiles(string? baseFilename, Strong } // Normalize the whitespace (formatting) and return the generated source code. - var code = compilationUnit.NormalizeWhitespace().ToFullString(); + var code = GeneratedCode.AddFileHeader(compilationUnit.NormalizeWhitespace().ToFullString()); yield return new GeneratedFile(baseFilename + ".cs", code); } + /// + /// Creates the leading trivia holding the XML documentation comment of a generated member. + /// + /// The content of the <summary> element. + /// The content of the <returns> element, if any. + /// The <param> elements to add, as name/description pairs. + /// The trivia to use as the leading trivia of the documented member. + /// + /// Every parameter needs its own <param> element, otherwise the compiler reports CS1573 for + /// projects that set GenerateDocumentationFile. Likewise, members with no documentation at all are + /// reported as CS1591, which is why every visible generated member is documented. + /// + private static SyntaxTriviaList CreateDocumentation(string summary, string? returns = null, params (string Name, string Description)[] parameters) + { + var lines = new List(); + + AddLine($"{EscapeXml(summary)}"); + + foreach (var (name, description) in parameters) + { + AddLine($"{EscapeXml(description)}"); + } + + if (returns is not null) + { + AddLine($"{EscapeXml(returns)}"); + } + + return TriviaList(lines); + + void AddLine(string content) + { + lines.Add(Comment($"/// {content}")); + lines.Add(ElasticCarriageReturnLineFeed); + } + } + + /// + /// Escapes the characters that are not allowed in the content of an XML element. + /// + /// The text to escape. + /// The escaped text. + /// + /// Resource values are copied into the documentation of the generated members, and they routinely contain + /// markup. Leaving them unescaped would produce malformed documentation, reported as CS1570. + /// + private static string EscapeXml(string text) + { + return text + .Replace("&", "&") + .Replace("<", "<") + .Replace(">", ">"); + } + + /// + /// Creates the leading trivia holding the XML documentation comment of a generated lookup method. + /// + /// The content of the <summary> element. + /// The localization item the method is generated for. + /// The parameters of the generated method, in declaration order. + /// The content of the <returns> element. + /// The trivia to use as the leading trivia of the documented method. + private static SyntaxTriviaList CreateDocumentation(string summary, Localization item, IEnumerable parameters, string returns) + { + return CreateDocumentation(summary, returns, parameters.Select(p => (p.Name, DescribeParameter(item, p))).ToArray()); + } + + /// + /// Returns the documentation text describing the role of a parameter of a generated lookup method. + /// + /// The localization item the method is generated for. + /// The parameter to describe. + /// The description to use for the <param> element of the parameter. + private static string DescribeParameter(Localization item, FunctionFormatTagParameter parameter) + { + if (parameter.IsVariantId) + { + return "The identifier of the variant of the string to look up."; + } + + if (item is PluralLocalization plural && ReferenceEquals(plural.ParameterToUseForPluralization, parameter)) + { + return "The quantity used to select the plural form of the string."; + } + + return $"The value to substitute for the '{parameter.Name}' placeholder."; + } + /// /// Returns a collection of using directives based on the application type. /// @@ -238,6 +327,10 @@ private ClassDeclarationSyntax CreateStronglyTypedClass(StronglyTypedClass info) Token(SyntaxKind.PublicKeyword), Token(SyntaxKind.StaticKeyword) )) + .WithLeadingTrivia(CreateDocumentation( + "Looks up the localized string with the given resource key.", + LocalizedStringReturns, + ("key", "The key of the resource to look up."))) .WithParameterList( ParameterList( SingletonSeparatedList( @@ -268,6 +361,7 @@ private ClassDeclarationSyntax CreateStronglyTypedClass(StronglyTypedClass info) // Build and return the complete class declaration. var classDecl = ClassDeclaration(info.ClassName) .WithAttributeLists(attributes) + .WithLeadingTrivia(CreateDocumentation($"Provides strongly-typed access to the strings of the '{info.ResoureFile}' resource file.")) .WithModifiers(TokenList( Token(SyntaxKind.PublicKeyword), Token(SyntaxKind.StaticKeyword) @@ -287,29 +381,6 @@ private IEnumerable CreateFormatMethodSyntax(Localizati // Create XML documentation for the member using the summary from the localization item. var summaryText = item.Summary ?? string.Empty; - var xmlComment = TriviaList( - Trivia( - DocumentationCommentTrivia(SyntaxKind.SingleLineDocumentationCommentTrivia, - List(new XmlNodeSyntax[] - { - // Leading "/// " text. - XmlText().WithTextTokens( - TokenList(XmlTextLiteral("/// ")) - ), - // The summary element. - XmlElement( - XmlElementStartTag(XmlName("summary")), - XmlElementEndTag(XmlName("summary")) - ).WithContent( - List(new XmlNodeSyntax[] - { - XmlText().WithTextTokens(TokenList(XmlTextLiteral(summaryText))) - }) - ) - }) - ) - ) - ); if (item.IsProperty) { @@ -319,7 +390,7 @@ private IEnumerable CreateFormatMethodSyntax(Localizati Token(SyntaxKind.PublicKeyword), Token(SyntaxKind.StaticKeyword) )) - .WithLeadingTrivia(xmlComment) + .WithLeadingTrivia(CreateDocumentation(summaryText)) .WithAccessorList( AccessorList( SingletonList( @@ -375,7 +446,7 @@ private IEnumerable CreateFormatMethodSyntax(Localizati Token(SyntaxKind.StaticKeyword) )) .WithParameterList(ParameterList(overloadParameters)) - .WithLeadingTrivia(xmlComment) + .WithLeadingTrivia(CreateDocumentation(summaryText, item, functionParameters, LocalizedStringReturns)) .WithBody( Block( // try { return {Key}(...); } catch { return string.Empty; } @@ -424,7 +495,7 @@ private IEnumerable CreateFormatMethodSyntax(Localizati Token(SyntaxKind.StaticKeyword) )) .WithParameterList(ParameterList(stdParameters)) - .WithLeadingTrivia(xmlComment) + .WithLeadingTrivia(CreateDocumentation(summaryText, item, functionParameters, LocalizedStringReturns)) .WithBody(Block(GenerateFormatMethodBody(item))); members.Add(stdMethod); } @@ -443,7 +514,7 @@ private IEnumerable CreateFormatMethodSyntax(Localizati Token(SyntaxKind.StaticKeyword) )) .WithParameterList(ParameterList(parametersList)) - .WithLeadingTrivia(xmlComment) + .WithLeadingTrivia(CreateDocumentation(summaryText, item, functionParameters, LocalizedStringReturns)) .WithBody(Block(GenerateFormatMethodBody(item))); members.Add(methodDecl); } @@ -660,15 +731,19 @@ private ClassDeclarationSyntax CreateMarkupExtensionSyntax(string resourceFileNa { // The default _Undefined member with value 0. EnumMemberDeclaration(Identifier("_Undefined")) + .WithLeadingTrivia(CreateDocumentation("No resource key is selected, the markup extension resolves to an empty string.")) .WithEqualsValue(EqualsValueClause(LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(0)))) }; foreach (var key in keys) { - enumMembers.Add(EnumMemberDeclaration(Identifier(key))); + enumMembers.Add( + EnumMemberDeclaration(Identifier(key)) + .WithLeadingTrivia(CreateDocumentation($"Identifies the '{key}' resource."))); } var keyEnum = EnumDeclaration("KeyEnum") .WithModifiers(TokenList(Token(SyntaxKind.PublicKeyword))) - .WithMembers(SeparatedList(enumMembers)); + .WithMembers(SeparatedList(enumMembers)) + .WithLeadingTrivia(CreateDocumentation($"Enumerates the resource keys available in the '{resourceFileName}' resource file.")); // Create a private static field for the resource provider. var resourceField = FieldDeclaration( @@ -710,6 +785,9 @@ private ClassDeclarationSyntax CreateMarkupExtensionSyntax(string resourceFileNa Token(SyntaxKind.ProtectedKeyword), Token(SyntaxKind.OverrideKeyword) )) + .WithLeadingTrivia(CreateDocumentation( + "Returns the localized string identified by the Key property, converted with the Converter property if one is set.", + "The value to assign to the target of the markup extension.")) .WithBody(Block( // Declare a local variable 'value' that checks if the key is _Undefined. LocalDeclarationStatement( @@ -797,6 +875,7 @@ private ClassDeclarationSyntax CreateMarkupExtensionSyntax(string resourceFileNa ) ) .WithAttributeLists(attributes) + .WithLeadingTrivia(CreateDocumentation($"A XAML markup extension that looks up the strings of the '{resourceFileName}' resource file.")) .WithModifiers(TokenList( Token(SyntaxKind.PublicKeyword), Token(SyntaxKind.PartialKeyword) @@ -818,9 +897,10 @@ private ClassDeclarationSyntax CreateMarkupExtensionSyntax(string resourceFileNa .WithSemicolonToken(Token(SyntaxKind.SemicolonToken)) ]) ) - ), + ) + .WithLeadingTrivia(CreateDocumentation("Gets or sets the key of the resource to look up.")), // Create the Converter property. - PropertyDeclaration(ParseTypeName("IValueConverter"), "Converter") + PropertyDeclaration(NullableType(ParseTypeName("IValueConverter")), "Converter") .WithModifiers(TokenList(Token(SyntaxKind.PublicKeyword))) .WithAccessorList( AccessorList( @@ -832,9 +912,10 @@ private ClassDeclarationSyntax CreateMarkupExtensionSyntax(string resourceFileNa .WithSemicolonToken(Token(SyntaxKind.SemicolonToken)) ]) ) - ), + ) + .WithLeadingTrivia(CreateDocumentation("Gets or sets the converter to apply to the localized string, if any.")), // Create the ConverterParameter property. - PropertyDeclaration(PredefinedType(Token(SyntaxKind.ObjectKeyword)), "ConverterParameter") + PropertyDeclaration(NullableType(PredefinedType(Token(SyntaxKind.ObjectKeyword))), "ConverterParameter") .WithModifiers(TokenList(Token(SyntaxKind.PublicKeyword))) .WithAccessorList( AccessorList( @@ -846,7 +927,8 @@ private ClassDeclarationSyntax CreateMarkupExtensionSyntax(string resourceFileNa .WithSemicolonToken(Token(SyntaxKind.SemicolonToken)) ]) ) - ), + ) + .WithLeadingTrivia(CreateDocumentation("Gets or sets the parameter to pass to the converter, if any.")), provideValueMethod ); diff --git a/src/ReswPlus.SourceGenerator/CodeGenerators/GeneratedCode.cs b/src/ReswPlus.SourceGenerator/CodeGenerators/GeneratedCode.cs new file mode 100644 index 0000000..53e76fe --- /dev/null +++ b/src/ReswPlus.SourceGenerator/CodeGenerators/GeneratedCode.cs @@ -0,0 +1,40 @@ +namespace ReswPlus.SourceGenerator.CodeGenerators; + +/// +/// Defines the conventions shared by every file emitted by ReswPlus. +/// +internal static class GeneratedCode +{ + /// + /// The header prepended to every emitted file, terminated by a blank line. + /// + /// + /// The header serves two purposes: + /// + /// + /// The <auto-generated/> marker on the very first line is how Roslyn, StyleCop and most + /// third party analyzers detect generated code. Analyzers skip generated code by default, so the marker + /// keeps the emitted output out of the consumer's analysis results. + /// + /// + /// The explicit #nullable enable directive makes the emitted code independent from the + /// <Nullable> setting of the consuming project, so it always compiles the same way. + /// + /// + /// + public const string FileHeader = + "// \r\n" + + "// File generated automatically by ReswPlus. https://github.com/DotNetPlus/ReswPlus\r\n" + + "#nullable enable\r\n" + + "\r\n"; + + /// + /// Prepends to the given source code. + /// + /// The source code to prepend the header to. + /// The source code, preceded by the generated file header. + public static string AddFileHeader(string sourceCode) + { + return FileHeader + sourceCode; + } +} diff --git a/src/ReswPlus.SourceGenerator/ReswGenerator.cs b/src/ReswPlus.SourceGenerator/ReswGenerator.cs index 5ee02a6..7fdfbd9 100644 --- a/src/ReswPlus.SourceGenerator/ReswGenerator.cs +++ b/src/ReswPlus.SourceGenerator/ReswGenerator.cs @@ -8,6 +8,7 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.Text; using ReswPlus.SourceGenerator.ClassGenerators; +using ReswPlus.SourceGenerator.CodeGenerators; using ReswPlus.SourceGenerator.Models; using Microsoft.CodeAnalysis.Diagnostics; @@ -363,7 +364,7 @@ private static void AddLanguageSupport(SourceProductionContext spc, string[] lan var resourceLoaderResourceName = $"{assemblyName}.Templates.Plurals.ResourceLoaderExtension.txt"; var resourceLoaderTemplate = ReadAllText(Assembly.GetExecutingAssembly().GetManifestResourceStream(resourceLoaderResourceName)); var resourceLoaderCode = resourceLoaderTemplate.Replace("{{PluralProviderSelector}}", pluralSelectorCode); - spc.AddSource("ResourceLoaderExtension.cs", SourceText.From(resourceLoaderCode, Encoding.UTF8)); + spc.AddSource("ResourceLoaderExtension.cs", SourceText.From(GeneratedCode.AddFileHeader(resourceLoaderCode), Encoding.UTF8)); } /// @@ -378,7 +379,7 @@ private static void AddSourceFromResource(SourceProductionContext spc, string re return; } var sourceText = ReadAllText(stream); - spc.AddSource(itemName, SourceText.From(sourceText, Encoding.UTF8)); + spc.AddSource(itemName, SourceText.From(GeneratedCode.AddFileHeader(sourceText), Encoding.UTF8)); } /// diff --git a/src/ReswPlus.SourceGenerator/ReswPlus.SourceGenerator.csproj b/src/ReswPlus.SourceGenerator/ReswPlus.SourceGenerator.csproj index aaee5c8..514f802 100644 --- a/src/ReswPlus.SourceGenerator/ReswPlus.SourceGenerator.csproj +++ b/src/ReswPlus.SourceGenerator/ReswPlus.SourceGenerator.csproj @@ -11,6 +11,9 @@ true Generated + + + diff --git a/src/ReswPlus.SourceGenerator/Templates/Macros/Macros.txt b/src/ReswPlus.SourceGenerator/Templates/Macros/Macros.txt index 74a03e3..f6863e4 100644 --- a/src/ReswPlus.SourceGenerator/Templates/Macros/Macros.txt +++ b/src/ReswPlus.SourceGenerator/Templates/Macros/Macros.txt @@ -16,7 +16,7 @@ namespace _ReswPlus_AutoGenerated internal static class Macros { #region ShortDate - private static string _shortDate = null; + private static string? _shortDate; public static string ShortDate { get @@ -31,7 +31,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region LongDate - private static string _longDate = null; + private static string? _longDate; public static string LongDate { get @@ -46,7 +46,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region ShortTime - private static string _shortTime = null; + private static string? _shortTime; public static string ShortTime { get @@ -61,7 +61,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region LongTime - private static string _longTime = null; + private static string? _longTime; public static string LongTime { get @@ -76,7 +76,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region WeekDay - private static string _weekDay = null; + private static string? _weekDay; public static string WeekDay { get @@ -91,7 +91,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region ShortWeekDay - private static string _shortWeekDay = null; + private static string? _shortWeekDay; public static string ShortWeekDay { get @@ -106,7 +106,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region Year - private static string _year = null; + private static string? _year; public static string Year { get @@ -121,7 +121,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region YearTwoDigits - private static string _yearTwoDigits = null; + private static string? _yearTwoDigits; public static string YearTwoDigits { get @@ -136,7 +136,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region LocaleName - private static string _localeName = null; + private static string? _localeName; public static string LocaleName { get @@ -151,7 +151,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region LocaleId - private static string _localeId = null; + private static string? _localeId; public static string LocaleId { get @@ -166,7 +166,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region LocaleTwoLetters - private static string _localeTwoLetters = null; + private static string? _localeTwoLetters; public static string LocaleTwoLetters { get @@ -183,7 +183,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || DOTNETCORE || NETCOREAPP #region App Version: Full - private static string _appVersionFull = null; + private static string? _appVersionFull; public static string AppVersionFull { get @@ -206,7 +206,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || DOTNETCORE || NETCOREAPP #region App Version: Major.Minor.Build - private static string _appVersionMajorMinorBuild = null; + private static string? _appVersionMajorMinorBuild; public static string AppVersionMajorMinorBuild { get @@ -229,7 +229,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || DOTNETCORE || NETCOREAPP #region App Version: Major.Minor - private static string _appVersionMajorMinor = null; + private static string? _appVersionMajorMinor; public static string AppVersionMajorMinor { get @@ -252,7 +252,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || DOTNETCORE || NETCOREAPP #region App Version: Major - private static string _appVersionMajor = null; + private static string? _appVersionMajor; public static string AppVersionMajor { get @@ -275,7 +275,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || NETCOREAPP #region Architecture - private static string _architecture = null; + private static string? _architecture; public static string Architecture { get @@ -300,7 +300,7 @@ namespace _ReswPlus_AutoGenerated case Windows.System.ProcessorArchitecture.Neutral: _architecture = "Any"; break; - case Windows.System.ProcessorArchitecture.Unknown: + default: _architecture = "Unknown"; break; } @@ -313,7 +313,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || DOTNETCORE || NETCOREAPP #region ApplicationName - private static string _applicationName = null; + private static string? _applicationName; public static string ApplicationName { get @@ -335,7 +335,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || NETCOREAPP #region ApplicationName - private static string _publisherName = null; + private static string? _publisherName; public static string PublisherName { get @@ -352,7 +352,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || NETCOREAPP #region DeviceFamily - private static string _deviceFamily = null; + private static string? _deviceFamily; public static string DeviceFamily { get @@ -369,7 +369,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || NETCOREAPP #region DeviceManufacturer - private static string _deviceManufacturer = null; + private static string? _deviceManufacturer; public static string DeviceManufacturer { get @@ -386,7 +386,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || NETCOREAPP #region DeviceModel - private static string _deviceModel = null; + private static string? _deviceModel; public static string DeviceModel { get @@ -403,7 +403,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || NETCOREAPP #region OperatingSystem - private static string _operatingSystemVersion = null; + private static string? _operatingSystemVersion; public static string OperatingSystemVersion { get diff --git a/src/ReswPlus.SourceGenerator/Templates/Plurals/IPluralProvider.txt b/src/ReswPlus.SourceGenerator/Templates/Plurals/IPluralProvider.txt index e2e8b65..47803e6 100644 --- a/src/ReswPlus.SourceGenerator/Templates/Plurals/IPluralProvider.txt +++ b/src/ReswPlus.SourceGenerator/Templates/Plurals/IPluralProvider.txt @@ -1,7 +1,15 @@ namespace _ReswPlus_AutoGenerated.Plurals { + /// + /// Computes the CLDR plural category of a number for a given language. + /// public interface IPluralProvider { + /// + /// Computes the plural category to use for the given quantity. + /// + /// The quantity to compute the plural category for. + /// The plural category matching . PluralTypeEnum ComputePlural(double n); } } diff --git a/src/ReswPlus.SourceGenerator/Templates/Plurals/PluralTypeEnum.txt b/src/ReswPlus.SourceGenerator/Templates/Plurals/PluralTypeEnum.txt index 88e1317..742a0f8 100644 --- a/src/ReswPlus.SourceGenerator/Templates/Plurals/PluralTypeEnum.txt +++ b/src/ReswPlus.SourceGenerator/Templates/Plurals/PluralTypeEnum.txt @@ -1,12 +1,21 @@ namespace _ReswPlus_AutoGenerated.Plurals { + /// + /// The CLDR plural categories a string can be declined in. + /// public enum PluralTypeEnum { + /// The 'zero' plural category. ZERO, + /// The 'one' plural category. ONE, + /// The 'two' plural category. TWO, + /// The 'other' plural category, used when no other category applies. OTHER, + /// The 'few' plural category. FEW, + /// The 'many' plural category. MANY }; } diff --git a/src/ReswPlus.SourceGenerator/Templates/Plurals/ResourceLoaderExtension.txt b/src/ReswPlus.SourceGenerator/Templates/Plurals/ResourceLoaderExtension.txt index db8a57f..3280bad 100644 --- a/src/ReswPlus.SourceGenerator/Templates/Plurals/ResourceLoaderExtension.txt +++ b/src/ReswPlus.SourceGenerator/Templates/Plurals/ResourceLoaderExtension.txt @@ -6,7 +6,7 @@ namespace _ReswPlus_AutoGenerated.Plurals { internal static class ResourceLoaderExtension { - private static _ReswPlus_AutoGenerated.Plurals.IPluralProvider _pluralProvider; + private static _ReswPlus_AutoGenerated.Plurals.IPluralProvider? _pluralProvider; private static readonly object _objLock = new object(); public static string GetPlural(this _ReswPlus_AutoGenerated.ResourceStringProvider resourceStringProvider, string key, double number, bool supportNoneState = false) @@ -24,7 +24,7 @@ namespace _ReswPlus_AutoGenerated.Plurals return ""; } } - string selectedSentence = null; + string? selectedSentence = null; var pluralType = _pluralProvider.ComputePlural(number); try { @@ -54,19 +54,19 @@ namespace _ReswPlus_AutoGenerated.Plurals return selectedSentence ?? ""; } - private static void CreatePluralProvider(string forcedCultureName = null) + private static void CreatePluralProvider(string? forcedCultureName = null) { lock (_objLock) { if (_pluralProvider is null) { var cultureToUse = CultureInfo.CurrentUICulture.TwoLetterISOLanguageName; - if (!string.IsNullOrEmpty(forcedCultureName)) + if (forcedCultureName is { Length: > 0 }) { try { - var forcedCulture = new CultureInfo(forcedCultureName)?.TwoLetterISOLanguageName; - if (!string.IsNullOrEmpty(forcedCulture)) + var forcedCulture = new CultureInfo(forcedCultureName).TwoLetterISOLanguageName; + if (forcedCulture is { Length: > 0 }) { cultureToUse = forcedCulture; } diff --git a/tests/ReswPlusUnitTests/FormatTagPlurals.cs b/tests/ReswPlusUnitTests/FormatTagPlurals.cs index f47c887..6806e0a 100644 --- a/tests/ReswPlusUnitTests/FormatTagPlurals.cs +++ b/tests/ReswPlusUnitTests/FormatTagPlurals.cs @@ -47,6 +47,7 @@ public void TestParseParameters_OneValidPluralParameter() { var basicLocalizedItems = new ReswItem[0]; var res = FormatTag.ParseParameters("test", new[] { "Plural " + type.Key }, basicLocalizedItems, "test", null); + Assert.NotNull(res); Assert.True(res.Parameters.Count == 1); _ = Assert.IsType(res.Parameters[0]); var functionParam = (FunctionFormatTagParameter)res.Parameters[0]; diff --git a/tests/ReswPlusUnitTests/GeneratedCodeHygiene.cs b/tests/ReswPlusUnitTests/GeneratedCodeHygiene.cs new file mode 100644 index 0000000..14d5108 --- /dev/null +++ b/tests/ReswPlusUnitTests/GeneratedCodeHygiene.cs @@ -0,0 +1,140 @@ +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Xunit; + +namespace ReswPlusUnitTests; + +/// +/// Tests covering the hygiene of the code emitted by the generator: the auto-generated header, the explicit +/// nullable context and the XML documentation, all of which are required for the generated code to build +/// cleanly in projects with analyzers and warnings-as-errors enabled. +/// +public class GeneratedCodeHygiene +{ + private static string GenerateSample() + { + return ReswTestHelpers.GenerateCode(ReswTestHelpers.CreateResw( + ("Welcome", "Welcome!", null), + ("HtmlString", "Click here & win", null), + ("Greeting", "Hello {0}, you are {1} years old", "#Format[String name, Int age]"), + ("WithMacro", "Running {0}", "#Format[APP_NAME]"), + ("FileCount_None", "No files", null), + ("FileCount_One", "{0} file", "#Format[Plural Int count]"), + ("FileCount_Other", "{0} files", "#Format[Plural Int count]"), + ("Theme_Variant0", "Light theme", null), + ("Theme_Variant1", "Dark theme", null))); + } + + [Fact] + public void GeneratedFileStartsWithAutoGeneratedMarker() + { + var code = GenerateSample(); + + // Many analyzers only inspect the very first line of a file to decide whether it is generated code. + Assert.StartsWith("// \r\n", code); + } + + [Fact] + public void GeneratedFileDeclaresAnExplicitNullableContext() + { + var code = GenerateSample(); + + Assert.Contains("#nullable enable", code); + } + + [Fact] + public void GeneratedFileIsSyntacticallyValid() + { + var code = GenerateSample(); + + var diagnostics = CSharpSyntaxTree.ParseText(code).GetDiagnostics().ToArray(); + + Assert.Empty(diagnostics); + } + + [Fact] + public void EveryVisibleMemberIsDocumented() + { + var root = CSharpSyntaxTree.ParseText(GenerateSample()).GetRoot(); + + var visibleMembers = root.DescendantNodes() + .OfType() + .Where(member => member is EnumMemberDeclarationSyntax || + member.Modifiers.Any(SyntaxKind.PublicKeyword) || + member.Modifiers.Any(SyntaxKind.ProtectedKeyword)) + .ToArray(); + + Assert.NotEmpty(visibleMembers); + + foreach (var member in visibleMembers) + { + // A visible member without documentation is reported as CS1591 when the consuming project + // sets GenerateDocumentationFile. + Assert.True( + GetDocumentation(member) is not null, + $"Missing documentation on: {member.ToString().Split('\n')[0].Trim()}"); + } + } + + [Fact] + public void EveryParameterOfADocumentedMethodIsDocumented() + { + var root = CSharpSyntaxTree.ParseText(GenerateSample()).GetRoot(); + + var methods = root.DescendantNodes().OfType().ToArray(); + + Assert.NotEmpty(methods); + + foreach (var method in methods) + { + var documentation = GetDocumentation(method); + + Assert.NotNull(documentation); + + var documentedParameters = documentation! + .DescendantNodes() + .OfType() + .Where(element => element.StartTag.Name.LocalName.Text == "param") + .SelectMany(element => element.StartTag.Attributes.OfType()) + .Select(attribute => attribute.Identifier.Identifier.Text) + .ToArray(); + + foreach (var parameter in method.ParameterList.Parameters) + { + // A documented member that omits one of its parameters is reported as CS1573. + Assert.Contains(parameter.Identifier.Text, documentedParameters); + } + } + } + + [Fact] + public void ResourceValuesAreEscapedInTheDocumentation() + { + var code = GenerateSample(); + + // A resource value containing markup would produce invalid XML documentation (CS1570) if left unescaped. + Assert.Contains("Click <b>here</b> & win", code); + Assert.DoesNotContain("Click here & win", code); + } + + [Fact] + public void MarkupExtensionPropertiesThatCanBeNullAreAnnotated() + { + var code = GenerateSample(); + + // Both properties are left unset unless the user assigns them in XAML, so they must be nullable + // to avoid CS8618 in projects that enable nullable reference types. + Assert.Contains("public IValueConverter? Converter", code); + Assert.Contains("public object? ConverterParameter", code); + } + + private static DocumentationCommentTriviaSyntax? GetDocumentation(SyntaxNode member) + { + return member.GetLeadingTrivia() + .Select(trivia => trivia.GetStructure()) + .OfType() + .FirstOrDefault(); + } +} diff --git a/tests/ReswPlusUnitTests/ReswPlusUnitTests.csproj b/tests/ReswPlusUnitTests/ReswPlusUnitTests.csproj index adb26bc..530f38e 100644 --- a/tests/ReswPlusUnitTests/ReswPlusUnitTests.csproj +++ b/tests/ReswPlusUnitTests/ReswPlusUnitTests.csproj @@ -2,10 +2,12 @@ net9.0-windows10.0.19041.0 + enable false + diff --git a/tests/ReswPlusUnitTests/ReswTestHelpers.cs b/tests/ReswPlusUnitTests/ReswTestHelpers.cs new file mode 100644 index 0000000..3ed6f8c --- /dev/null +++ b/tests/ReswPlusUnitTests/ReswTestHelpers.cs @@ -0,0 +1,69 @@ +using System.Collections.Generic; +using System.Linq; +using ReswPlus.SourceGenerator; +using ReswPlus.SourceGenerator.ClassGenerators; +using ReswPlus.SourceGenerator.Models; +using Xunit; + +namespace ReswPlusUnitTests; + +/// +/// Helpers to build .resw documents and run the ReswPlus generators over them from tests. +/// +internal static class ReswTestHelpers +{ + /// + /// Builds the content of a .resw file from the given entries. + /// + /// The entries to add, as key/value/comment triples. A comment is omitted. + /// The content of the resulting .resw file. + public static string CreateResw(params (string Key, string Value, string? Comment)[] entries) + { + var elements = entries.Select(entry => + $""" + + {Escape(entry.Value)}{(entry.Comment is null ? "" : $"\r\n {Escape(entry.Comment)}")} + + """); + + return $""" + + + + text/microsoft-resx + + {string.Join("\r\n", elements)} + + """; + } + + /// + /// Generates the C# code for a .resw file. + /// + /// The content of the .resw file. + /// The type of the consuming application. + /// The generated C# code. + public static string GenerateCode(string reswContent, AppType appType = AppType.WindowsAppSDK) + { + var resourceFileInfo = new ResourceFileInfo(@"C:\Project\Strings\en-US\Resources.resw", new Project("TestProject", isLibrary: false)); + var generator = ReswClassGenerator.CreateGenerator(resourceFileInfo, logger: null); + + Assert.NotNull(generator); + + var result = generator!.GenerateCode( + baseFilename: "Resources", + content: reswContent, + defaultNamespace: "TestProject.Strings", + isAdvanced: true, + appType: appType); + + Assert.NotNull(result); + + return result!.Files.Single().Content; + } + + private static string Escape(string text) + { + return text.Replace("&", "&").Replace("<", "<").Replace(">", ">"); + } +} From c4cefeb4ad778f510d7dccbcbe1c9845ca70e581 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 27 Jul 2026 22:19:43 -0700 Subject: [PATCH 2/7] Use the '.g.cs' extension for the generated files Tooling recognizes generated code either by the `` marker in the file header or by the file name. Roslyn's own generated code detection, StyleCop and most code coverage tools all treat a file whose name ends with `.g.cs` as generated, so using that extension for the hint names adds a second, independent signal on top of the header. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../CodeGenerators/CsharpCodeGenerator.cs | 2 +- .../CodeGenerators/GeneratedCode.cs | 10 +++++++ src/ReswPlus.SourceGenerator/ReswGenerator.cs | 29 ++++++++++--------- .../ReswPlusUnitTests/GeneratedCodeHygiene.cs | 9 ++++++ tests/ReswPlusUnitTests/ReswTestHelpers.cs | 14 ++++++++- 5 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/ReswPlus.SourceGenerator/CodeGenerators/CsharpCodeGenerator.cs b/src/ReswPlus.SourceGenerator/CodeGenerators/CsharpCodeGenerator.cs index 04bd724..1cdda08 100644 --- a/src/ReswPlus.SourceGenerator/CodeGenerators/CsharpCodeGenerator.cs +++ b/src/ReswPlus.SourceGenerator/CodeGenerators/CsharpCodeGenerator.cs @@ -145,7 +145,7 @@ public IEnumerable GetGeneratedFiles(string? baseFilename, Strong // Normalize the whitespace (formatting) and return the generated source code. var code = GeneratedCode.AddFileHeader(compilationUnit.NormalizeWhitespace().ToFullString()); - yield return new GeneratedFile(baseFilename + ".cs", code); + yield return new GeneratedFile(baseFilename + GeneratedCode.FileExtension, code); } /// diff --git a/src/ReswPlus.SourceGenerator/CodeGenerators/GeneratedCode.cs b/src/ReswPlus.SourceGenerator/CodeGenerators/GeneratedCode.cs index 53e76fe..84dea49 100644 --- a/src/ReswPlus.SourceGenerator/CodeGenerators/GeneratedCode.cs +++ b/src/ReswPlus.SourceGenerator/CodeGenerators/GeneratedCode.cs @@ -28,6 +28,16 @@ internal static class GeneratedCode "#nullable enable\r\n" + "\r\n"; + /// + /// The extension used for the hint name of every emitted file. + /// + /// + /// On top of the <auto-generated/> marker in , tooling also recognizes + /// generated code by its file name. Roslyn's own generated code detection, StyleCop and most code coverage + /// tools all treat a file whose name ends with .g.cs as generated. + /// + public const string FileExtension = ".g.cs"; + /// /// Prepends to the given source code. /// diff --git a/src/ReswPlus.SourceGenerator/ReswGenerator.cs b/src/ReswPlus.SourceGenerator/ReswGenerator.cs index 7fdfbd9..f28be2d 100644 --- a/src/ReswPlus.SourceGenerator/ReswGenerator.cs +++ b/src/ReswPlus.SourceGenerator/ReswGenerator.cs @@ -159,10 +159,10 @@ public void Initialize(IncrementalGeneratorInitializationContext context) switch (appType) { case AppType.WindowsAppSDK: - AddSourceFromResource(spc, $"{assemblyName}.Templates.ResourceStringProviders.MicrosoftResourceStringProvider.txt", "ResourceStringProvider.cs"); + AddSourceFromResource(spc, $"{assemblyName}.Templates.ResourceStringProviders.MicrosoftResourceStringProvider.txt", "ResourceStringProvider"); break; case AppType.UWP: - AddSourceFromResource(spc, $"{assemblyName}.Templates.ResourceStringProviders.WindowsResourceStringProvider.txt", "ResourceStringProvider.cs"); + AddSourceFromResource(spc, $"{assemblyName}.Templates.ResourceStringProviders.WindowsResourceStringProvider.txt", "ResourceStringProvider"); break; default: spc.ReportDiagnostic(Diagnostic.Create(UnrecognizedAppTypeDiagnostic, Location.None)); @@ -251,22 +251,22 @@ into fileGroup // Add each generated file as a new source. foreach (var generatedFile in generatedData.Files) { - spc.AddSource($"{Path.GetFileName(filePath)}.cs", SourceText.From(generatedFile.Content, Encoding.UTF8)); + spc.AddSource($"{Path.GetFileName(filePath)}{GeneratedCode.FileExtension}", SourceText.From(generatedFile.Content, Encoding.UTF8)); } // If macros were used, include the Macros source file. if (generatedData.ContainsMacro) { - AddSourceFromResource(spc, "ReswPlus.SourceGenerator.Templates.Macros.Macros.txt", "Macros.cs"); + AddSourceFromResource(spc, "ReswPlus.SourceGenerator.Templates.Macros.Macros.txt", "Macros"); } // If plural forms are detected, add plural-related support sources. if (generatedData.ContainsPlural) { - AddSourceFromResource(spc, $"{assemblyName}.Templates.Plurals.IPluralProvider.txt", "IPluralProvider.cs"); - AddSourceFromResource(spc, $"{assemblyName}.Templates.Plurals.PluralTypeEnum.txt", "PluralTypeEnum.cs"); - AddSourceFromResource(spc, $"{assemblyName}.Templates.Utils.IntExt.txt", "IntExt.cs"); - AddSourceFromResource(spc, $"{assemblyName}.Templates.Utils.DoubleExt.txt", "DoubleExt.cs"); + AddSourceFromResource(spc, $"{assemblyName}.Templates.Plurals.IPluralProvider.txt", "IPluralProvider"); + AddSourceFromResource(spc, $"{assemblyName}.Templates.Plurals.PluralTypeEnum.txt", "PluralTypeEnum"); + AddSourceFromResource(spc, $"{assemblyName}.Templates.Utils.IntExt.txt", "IntExt"); + AddSourceFromResource(spc, $"{assemblyName}.Templates.Utils.DoubleExt.txt", "DoubleExt"); AddLanguageSupport(spc, allLanguages); } } @@ -347,7 +347,7 @@ private static void AddLanguageSupport(SourceProductionContext spc, string[] lan foreach (var pluralFile in PluralFormsRetriever.RetrievePluralFormsForLanguages(languagesSupported)) { var resourceName = $"{assemblyName}.Templates.Plurals.{pluralFile.Id}Provider.txt"; - AddSourceFromResource(spc, resourceName, $"{pluralFile.Id}Provider.cs"); + AddSourceFromResource(spc, resourceName, $"{pluralFile.Id}Provider"); // Add each language handled by this provider. foreach (var lng in pluralFile.Languages) @@ -358,19 +358,22 @@ private static void AddLanguageSupport(SourceProductionContext spc, string[] lan } // Add the fallback provider. - AddSourceFromResource(spc, $"{assemblyName}.Templates.Plurals.OtherProvider.txt", "OtherProvider.cs"); + AddSourceFromResource(spc, $"{assemblyName}.Templates.Plurals.OtherProvider.txt", "OtherProvider"); // Build and add the ResourceLoaderExtension with the plural selector injected. var resourceLoaderResourceName = $"{assemblyName}.Templates.Plurals.ResourceLoaderExtension.txt"; var resourceLoaderTemplate = ReadAllText(Assembly.GetExecutingAssembly().GetManifestResourceStream(resourceLoaderResourceName)); var resourceLoaderCode = resourceLoaderTemplate.Replace("{{PluralProviderSelector}}", pluralSelectorCode); - spc.AddSource("ResourceLoaderExtension.cs", SourceText.From(GeneratedCode.AddFileHeader(resourceLoaderCode), Encoding.UTF8)); + spc.AddSource($"ResourceLoaderExtension{GeneratedCode.FileExtension}", SourceText.From(GeneratedCode.AddFileHeader(resourceLoaderCode), Encoding.UTF8)); } /// /// Reads a resource stream and adds its content as a source file. /// - private static void AddSourceFromResource(SourceProductionContext spc, string resourcePath, string itemName) + /// The context to add the source to. + /// The path of the embedded resource holding the template. + /// The name of the type declared by the template, used as the hint name. + private static void AddSourceFromResource(SourceProductionContext spc, string resourcePath, string typeName) { using var stream = Assembly.GetExecutingAssembly().GetManifestResourceStream(resourcePath); if (stream is null) @@ -379,7 +382,7 @@ private static void AddSourceFromResource(SourceProductionContext spc, string re return; } var sourceText = ReadAllText(stream); - spc.AddSource(itemName, SourceText.From(GeneratedCode.AddFileHeader(sourceText), Encoding.UTF8)); + spc.AddSource($"{typeName}{GeneratedCode.FileExtension}", SourceText.From(GeneratedCode.AddFileHeader(sourceText), Encoding.UTF8)); } /// diff --git a/tests/ReswPlusUnitTests/GeneratedCodeHygiene.cs b/tests/ReswPlusUnitTests/GeneratedCodeHygiene.cs index 14d5108..3ebe3d2 100644 --- a/tests/ReswPlusUnitTests/GeneratedCodeHygiene.cs +++ b/tests/ReswPlusUnitTests/GeneratedCodeHygiene.cs @@ -36,6 +36,15 @@ public void GeneratedFileStartsWithAutoGeneratedMarker() Assert.StartsWith("// \r\n", code); } + [Fact] + public void GeneratedFileUsesTheGeneratedCodeExtension() + { + var file = ReswTestHelpers.GenerateFile(ReswTestHelpers.CreateResw(("Welcome", "Welcome!", null))); + + // Tooling also recognizes generated code by its file name, not just by the header. + Assert.EndsWith(".g.cs", file.Filename); + } + [Fact] public void GeneratedFileDeclaresAnExplicitNullableContext() { diff --git a/tests/ReswPlusUnitTests/ReswTestHelpers.cs b/tests/ReswPlusUnitTests/ReswTestHelpers.cs index 3ed6f8c..a96ce71 100644 --- a/tests/ReswPlusUnitTests/ReswTestHelpers.cs +++ b/tests/ReswPlusUnitTests/ReswTestHelpers.cs @@ -2,6 +2,7 @@ using System.Linq; using ReswPlus.SourceGenerator; using ReswPlus.SourceGenerator.ClassGenerators; +using ReswPlus.SourceGenerator.CodeGenerators; using ReswPlus.SourceGenerator.Models; using Xunit; @@ -44,6 +45,17 @@ public static string CreateResw(params (string Key, string Value, string? Commen /// The type of the consuming application. /// The generated C# code. public static string GenerateCode(string reswContent, AppType appType = AppType.WindowsAppSDK) + { + return GenerateFile(reswContent, appType).Content; + } + + /// + /// Generates the file for a .resw file. + /// + /// The content of the .resw file. + /// The type of the consuming application. + /// The generated file. + public static GeneratedFile GenerateFile(string reswContent, AppType appType = AppType.WindowsAppSDK) { var resourceFileInfo = new ResourceFileInfo(@"C:\Project\Strings\en-US\Resources.resw", new Project("TestProject", isLibrary: false)); var generator = ReswClassGenerator.CreateGenerator(resourceFileInfo, logger: null); @@ -59,7 +71,7 @@ public static string GenerateCode(string reswContent, AppType appType = AppType. Assert.NotNull(result); - return result!.Files.Single().Content; + return result!.Files.Single(); } private static string Escape(string text) From 8c4feb5e35d661c958fb0a139a822c03aa86bbc6 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 27 Jul 2026 22:46:35 -0700 Subject: [PATCH 3/7] Add diagnostics for the content of the resw files The generator parses every `.resw` and already knows the plural categories each language requires, but it reported nothing about the resources themselves. Every failure mode below used to ship silently and surface as a crash, or as wrong text, in a language the team may not read. New rules, all reported on the offending line of the `.resw` file: - RESWP0006: a translated value doesn't use the same placeholders as the default language. This is the classic cause of a runtime `FormatException` in an untested language. - RESWP0007: a value uses a placeholder that has no matching parameter in its `#Format` tag. - RESWP0008: a pluralized resource is missing plural forms its language requires (Polish needs `_Few` and `_Many`, Arabic needs more). The lookup silently returns an empty string for a missing form. - RESWP0009: two resources of the same file are generated as the same member, either because their names only differ by case, or because a plain resource collides with a pluralized one, which doesn't even compile. - RESWP0010: a value used as a composite format string is malformed. All five are warnings, not errors: raising the severity would break the build of every project that already has an inconsistency the moment it updates the package. Projects that want a rule to be fatal can escalate it per rule from their `.editorconfig`. The rules err on the side of not firing, since a noisy analyzer gets disabled wholesale: `{{` and `}}` are treated as the escaped braces they are, placeholders are compared as sets so that a translation is free to reorder them, values of resources without a `#Format` tag are never parsed as format strings since they are never formatted, and resources that only exist in the default language are skipped. RESWP0006 immediately found a real bug in the French resources of both samples: the `AnimalTreat_Variant1` strings substitute `{1}`, the variant id, where they mean `{2}`, the name of the pet. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 17 + .../Strings/fr/Resources.resw | 4 +- .../Strings/fr/Resources.resw | 4 +- .../Analysis/CompositeFormatString.cs | 169 ++++++++++ .../Analysis/ReswDocument.cs | 269 +++++++++++++++ .../Analysis/ReswFileGrouping.cs | 72 ++++ .../Analysis/ReswResourceAnalyzer.cs | 282 ++++++++++++++++ .../Analysis/ReswResourceModel.cs | 209 ++++++++++++ .../AnalyzerReleases.Shipped.md | 7 +- .../ClassGenerators/PluralCategory.cs | 29 ++ .../ClassGenerators/PluralFormsRetriever.cs | 62 +++- .../ClassGenerators/ReswClassGenerator.cs | 4 +- src/ReswPlus.SourceGenerator/Diagnostics.cs | 133 ++++++++ src/ReswPlus.SourceGenerator/ReswGenerator.cs | 135 +++----- .../ReswPlusUnitTests/ResourceDiagnostics.cs | 314 ++++++++++++++++++ tests/ReswPlusUnitTests/ReswTestHelpers.cs | 33 ++ 16 files changed, 1638 insertions(+), 105 deletions(-) create mode 100644 src/ReswPlus.SourceGenerator/Analysis/CompositeFormatString.cs create mode 100644 src/ReswPlus.SourceGenerator/Analysis/ReswDocument.cs create mode 100644 src/ReswPlus.SourceGenerator/Analysis/ReswFileGrouping.cs create mode 100644 src/ReswPlus.SourceGenerator/Analysis/ReswResourceAnalyzer.cs create mode 100644 src/ReswPlus.SourceGenerator/Analysis/ReswResourceModel.cs create mode 100644 src/ReswPlus.SourceGenerator/ClassGenerators/PluralCategory.cs create mode 100644 src/ReswPlus.SourceGenerator/Diagnostics.cs create mode 100644 tests/ReswPlusUnitTests/ResourceDiagnostics.cs diff --git a/README.md b/README.md index 86a8670..dd5d9da 100644 --- a/README.md +++ b/README.md @@ -66,6 +66,23 @@ ReswPlus allows multiple variants of a string based on different criteria, such 🗨 [How to Use Variants](https://github.com/reswplus/ReswPlus/wiki/Features:-Variants) +### Resource Diagnostics +ReswPlus checks the content of your `.resw` files while it generates the code, and reports the inconsistencies that would otherwise only show up at runtime, in a language your team may not read. + +| Rule | Description | +| --- | --- | +| `RESWP0006` | A translated value doesn't use the same placeholders as the default language, which throws a `FormatException` at runtime. | +| `RESWP0007` | A value uses a placeholder that has no matching parameter in its `#Format` tag. | +| `RESWP0008` | A pluralized resource is missing the plural forms its language requires, which silently produces grammatically wrong text. | +| `RESWP0009` | Two resources of the same file are generated as the same member. | +| `RESWP0010` | A value that is used as a composite format string is malformed. | + +These are reported as **warnings**, so that updating the package never breaks a build that already has an inconsistency. Escalate the ones you want to be fatal from your `.editorconfig`: + +```ini +dotnet_diagnostic.RESWP0006.severity = error +``` + ## Tools In addition to features to enrich resw files, ReswPlus also provides some interesting tools to improve your productivity or make it easier to use/support resw files in your workflow and localization process. diff --git a/samples/UWP/ReswPlusUWPSample/Strings/fr/Resources.resw b/samples/UWP/ReswPlusUWPSample/Strings/fr/Resources.resw index 9fa3c7a..58cce61 100644 --- a/samples/UWP/ReswPlusUWPSample/Strings/fr/Resources.resw +++ b/samples/UWP/ReswPlusUWPSample/Strings/fr/Resources.resw @@ -121,11 +121,11 @@ Contoso for Android - Récompensez votre chiot, donnez {0} biscuit à {1} ! + Récompensez votre chiot, donnez {0} biscuit à {2} ! #Format[Plural Double treatNumber, Variant petType, String petName] - Récompensez votre chiot, donnez {0} biscuits à {1} ! + Récompensez votre chiot, donnez {0} biscuits à {2} ! Récompensez votre chaton, donnez {0} biscuit à votre chat {2} ! diff --git a/samples/WinAppSDK/ReswPlusWinAppSDKSample/Strings/fr/Resources.resw b/samples/WinAppSDK/ReswPlusWinAppSDKSample/Strings/fr/Resources.resw index 9fa3c7a..58cce61 100644 --- a/samples/WinAppSDK/ReswPlusWinAppSDKSample/Strings/fr/Resources.resw +++ b/samples/WinAppSDK/ReswPlusWinAppSDKSample/Strings/fr/Resources.resw @@ -121,11 +121,11 @@ Contoso for Android - Récompensez votre chiot, donnez {0} biscuit à {1} ! + Récompensez votre chiot, donnez {0} biscuit à {2} ! #Format[Plural Double treatNumber, Variant petType, String petName] - Récompensez votre chiot, donnez {0} biscuits à {1} ! + Récompensez votre chiot, donnez {0} biscuits à {2} ! Récompensez votre chaton, donnez {0} biscuit à votre chat {2} ! diff --git a/src/ReswPlus.SourceGenerator/Analysis/CompositeFormatString.cs b/src/ReswPlus.SourceGenerator/Analysis/CompositeFormatString.cs new file mode 100644 index 0000000..4ecbce6 --- /dev/null +++ b/src/ReswPlus.SourceGenerator/Analysis/CompositeFormatString.cs @@ -0,0 +1,169 @@ +using System.Collections.Generic; +using System.Globalization; + +namespace ReswPlus.SourceGenerator.Analysis; + +/// +/// Parses the composite format strings used by the values of formatted resources. +/// +/// +/// The generated code passes the value of a formatted resource to , +/// so a value that this parser rejects is guaranteed to throw a at runtime. +/// The parser is deliberately permissive where the runtime is: it accepts anything the runtime accepts, so that +/// no valid value is ever reported. +/// +internal static class CompositeFormatString +{ + /// + /// Extracts the set of argument indexes referenced by a composite format string. + /// + /// The composite format string to parse. + /// The distinct argument indexes it references, in ascending order. + /// Whether is a valid composite format string. + public static bool TryGetArgumentIndexes(string value, out SortedSet indexes) + { + indexes = new SortedSet(); + + var position = 0; + + while (position < value.Length) + { + var character = value[position]; + + if (character == '}') + { + // Outside of a format item, a closing brace only stands for itself when it is doubled. + if (position + 1 < value.Length && value[position + 1] == '}') + { + position += 2; + continue; + } + + return false; + } + + if (character != '{') + { + position++; + continue; + } + + // A doubled opening brace stands for a literal brace, it doesn't open a format item. + if (position + 1 < value.Length && value[position + 1] == '{') + { + position += 2; + continue; + } + + if (!TryReadFormatItem(value, ref position, out var index)) + { + return false; + } + + _ = indexes.Add(index); + } + + return true; + } + + /// + /// Reads a single {index[,alignment][:format]} item, starting from its opening brace. + /// + private static bool TryReadFormatItem(string value, ref int position, out int index) + { + index = 0; + + // Skip the opening brace. + position++; + + if (!TryReadInteger(value, ref position, out index)) + { + return false; + } + + SkipWhiteSpace(value, ref position); + + if (position < value.Length && value[position] == ',') + { + position++; + + SkipWhiteSpace(value, ref position); + + if (position < value.Length && (value[position] == '-' || value[position] == '+')) + { + position++; + } + + if (!TryReadInteger(value, ref position, out _)) + { + return false; + } + + SkipWhiteSpace(value, ref position); + } + + if (position < value.Length && value[position] == ':' && !TrySkipFormatSpecifier(value, ref position)) + { + return false; + } + + if (position >= value.Length || value[position] != '}') + { + return false; + } + + position++; + + return true; + } + + /// + /// Skips the :format part of a format item, stopping on the closing brace of the item. + /// + private static bool TrySkipFormatSpecifier(string value, ref int position) + { + // Skip the colon. + position++; + + while (position < value.Length) + { + var character = value[position]; + + if (character is '{' or '}') + { + // Braces are escaped by doubling them inside a format specifier as well. + if (position + 1 < value.Length && value[position + 1] == character) + { + position += 2; + continue; + } + + return character == '}'; + } + + position++; + } + + return false; + } + + private static bool TryReadInteger(string value, ref int position, out int result) + { + var start = position; + + while (position < value.Length && char.IsDigit(value[position])) + { + position++; + } + + return int.TryParse(value.Substring(start, position - start), NumberStyles.None, CultureInfo.InvariantCulture, out result); + } + + private static void SkipWhiteSpace(string value, ref int position) + { + while (position < value.Length && value[position] == ' ') + { + position++; + } + } +} diff --git a/src/ReswPlus.SourceGenerator/Analysis/ReswDocument.cs b/src/ReswPlus.SourceGenerator/Analysis/ReswDocument.cs new file mode 100644 index 0000000..88517db --- /dev/null +++ b/src/ReswPlus.SourceGenerator/Analysis/ReswDocument.cs @@ -0,0 +1,269 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Text; +using System.Threading; +using System.Xml; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Text; +using ReswPlus.Core.ResourceParser; + +namespace ReswPlus.SourceGenerator.Analysis; + +/// +/// A single <data> entry of a .resw file, together with its position in the file. +/// +internal sealed class ReswEntry +{ + public ReswEntry(ReswItem item, Location location) + { + Item = item; + Location = location; + } + + /// + /// Gets the parsed resource, in the shape consumed by the generation pipeline. + /// + public ReswItem Item { get; } + + /// + /// Gets the location of the name of the resource, so diagnostics can point at the offending line. + /// + public Location Location { get; } + + /// + /// Gets the name of the resource. + /// + public string Key => Item.Key; + + /// + /// Gets the value of the resource. + /// + public string Value => Item.Value; +} + +/// +/// A parsed .resw file. +/// +/// +/// This mirrors , but it is based on so that the position of +/// every entry can be recorded. Diagnostics that can be double clicked are much more useful than diagnostics +/// reported on the project. +/// +internal sealed class ReswDocument +{ + private ReswDocument(string path, string? language, IReadOnlyList entries) + { + Path = path; + Language = language; + Entries = entries; + } + + /// + /// Gets the path of the .resw file. + /// + public string Path { get; } + + /// + /// Gets the language of the file, taken from the name of the folder containing it, or + /// if the file is not inside a folder. + /// + /// + /// This is the primary language subtag, matching the granularity of the plural providers: the region of a + /// bcp47 tag such as en-US doesn't influence pluralization. + /// + public string? Language { get; } + + /// + /// Gets the entries of the file, in document order. + /// + public IReadOnlyList Entries { get; } + + /// + /// Parses a .resw file. + /// + /// The path of the file. + /// The content of the file. + /// The token used to cancel the operation. + /// The parsed file, or if the content is not valid XML. + public static ReswDocument? Parse(string path, SourceText text, CancellationToken cancellationToken) + { + var entries = new List(); + + try + { + using var reader = XmlReader.Create(new StringReader(text.ToString()), new XmlReaderSettings { DtdProcessing = DtdProcessing.Prohibit }); + var lineInfo = reader as IXmlLineInfo; + + while (reader.Read()) + { + cancellationToken.ThrowIfCancellationRequested(); + + if (reader.NodeType != XmlNodeType.Element || reader.LocalName != "data" || reader.NamespaceURI.Length != 0) + { + continue; + } + + if (TryReadEntry(reader, lineInfo, path, text, out var entry)) + { + entries.Add(entry!); + } + } + } + catch (XmlException) + { + // The .resw file is not valid XML, which the generation pipeline reports on its own. There is + // nothing useful this analysis can add, and every rule would fire on incomplete data. + return null; + } + + return new ReswDocument(path, GetLanguage(path), entries); + } + + /// + /// Reads the <data> element the reader is positioned on, leaving the reader on its end tag. + /// + private static bool TryReadEntry(XmlReader reader, IXmlLineInfo? lineInfo, string path, SourceText text, out ReswEntry? entry) + { + entry = null; + + var dataDepth = reader.Depth; + var isEmpty = reader.IsEmptyElement; + + if (!reader.MoveToAttribute("name")) + { + return false; + } + + var key = reader.Value; + var location = GetAttributeValueLocation(reader, lineInfo, path, text, key.Length); + + _ = reader.MoveToElement(); + + if (isEmpty) + { + return false; + } + + string? value = null; + string? comment = null; + + while (reader.Read()) + { + if (reader.NodeType == XmlNodeType.EndElement && reader.Depth == dataDepth) + { + break; + } + + if (reader.NodeType != XmlNodeType.Element || reader.Depth != dataDepth + 1 || reader.NamespaceURI.Length != 0) + { + continue; + } + + if (value is null && reader.LocalName == "value") + { + value = ReadTextContent(reader); + } + else if (comment is null && reader.LocalName == "comment") + { + comment = ReadTextContent(reader); + } + } + + // A element with no child is skipped by the parser used for generation as well. + if (value is null) + { + return false; + } + + entry = new ReswEntry(new ReswItem(key, value, comment), location); + + return true; + } + + /// + /// Reads the concatenated text of the element the reader is positioned on, leaving the reader on its end tag. + /// + private static string ReadTextContent(XmlReader reader) + { + if (reader.IsEmptyElement) + { + return string.Empty; + } + + var elementDepth = reader.Depth; + var builder = new StringBuilder(); + + while (reader.Read()) + { + if (reader.NodeType == XmlNodeType.EndElement && reader.Depth == elementDepth) + { + break; + } + + switch (reader.NodeType) + { + case XmlNodeType.Text: + case XmlNodeType.CDATA: + case XmlNodeType.Whitespace: + case XmlNodeType.SignificantWhitespace: + _ = builder.Append(reader.Value); + break; + } + } + + return builder.ToString(); + } + + /// + /// Builds the location of the value of the attribute the reader is currently positioned on. + /// + private static Location GetAttributeValueLocation(XmlReader reader, IXmlLineInfo? lineInfo, string path, SourceText text, int length) + { + // Moving to the value of the attribute makes the line info point inside the quotes, which is where the + // name of the resource actually starts. + if (lineInfo is null || !lineInfo.HasLineInfo() || !reader.ReadAttributeValue()) + { + return CreateFileLocation(path); + } + + var line = lineInfo.LineNumber - 1; + var character = lineInfo.LinePosition - 1; + + if (line < 0 || character < 0 || line >= text.Lines.Count) + { + return CreateFileLocation(path); + } + + var startOffset = Math.Min(text.Lines[line].Start + character, text.Length); + var endOffset = Math.Min(startOffset + length, text.Length); + + return Location.Create( + path, + TextSpan.FromBounds(startOffset, endOffset), + new LinePositionSpan(new LinePosition(line, character), new LinePosition(line, character + length))); + } + + /// + /// Builds a location pointing at the beginning of a file, used when no precise position is available. + /// + private static Location CreateFileLocation(string path) + { + return Location.Create(path, default, default); + } + + /// + /// Returns the language a .resw file is written in, based on the folder containing it. + /// + private static string? GetLanguage(string path) + { + var folder = System.IO.Path.GetFileName(System.IO.Path.GetDirectoryName(path)); + + if (string.IsNullOrEmpty(folder)) + { + return null; + } + + return folder.Split('-')[0].ToLowerInvariant(); + } +} diff --git a/src/ReswPlus.SourceGenerator/Analysis/ReswFileGrouping.cs b/src/ReswPlus.SourceGenerator/Analysis/ReswFileGrouping.cs new file mode 100644 index 0000000..14e5740 --- /dev/null +++ b/src/ReswPlus.SourceGenerator/Analysis/ReswFileGrouping.cs @@ -0,0 +1,72 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; + +namespace ReswPlus.SourceGenerator.Analysis; + +/// +/// Groups the .resw files of a project into the sets of files that translate one another. +/// +internal static class ReswFileGrouping +{ + /// + /// Groups the given .resw files by the resource file they are a translation of. + /// + /// The paths of the .resw files of the project. + /// The groups of files, keyed by the language independent path of the resource. + /// + /// Translations of the same resource live in sibling language folders, so dropping the language folder from + /// the path of a file yields a key shared by all of its translations. + /// + public static IEnumerable> GroupByResource(IEnumerable reswFiles) + { + return reswFiles.GroupBy(static path => Path.Combine( + Path.GetDirectoryName(Path.GetDirectoryName(path)) ?? string.Empty, + Path.GetFileName(path))); + } + + /// + /// Retrieve the default resource file from the given list that matches one of the preferred languages. + /// + /// The paths of the files of one group. + /// The default language of the project, if it declares one. + /// The path of the file holding the resources of the default language. + public static string? RetrieveDefaultResourceFile(IEnumerable reswFiles, string? defaultLanguage) + { + // Build a list of candidate languages. + var candidateLanguages = new List(); + if (defaultLanguage is { Length: > 0 }) + { + candidateLanguages.Add(defaultLanguage); + } + + // Ensure "en-us" and "en" are included if not already the default. + if (!"en-us".Equals(defaultLanguage, StringComparison.OrdinalIgnoreCase)) + { + candidateLanguages.Add("en-us"); + } + + if (!"en".Equals(defaultLanguage, StringComparison.OrdinalIgnoreCase)) + { + candidateLanguages.Add("en"); + } + + // Iterate candidates and files to find a match. + foreach (var language in candidateLanguages) + { + foreach (var reswFile in reswFiles) + { + // Get the immediate parent folder name (e.g. "en-us"). + var parentFolderName = Path.GetFileName(Path.GetDirectoryName(reswFile)); + if (parentFolderName.Equals(language, StringComparison.OrdinalIgnoreCase)) + { + return reswFile; + } + } + } + + // Fallback to the first available resource file. + return reswFiles.FirstOrDefault(); + } +} diff --git a/src/ReswPlus.SourceGenerator/Analysis/ReswResourceAnalyzer.cs b/src/ReswPlus.SourceGenerator/Analysis/ReswResourceAnalyzer.cs new file mode 100644 index 0000000..89b0c96 --- /dev/null +++ b/src/ReswPlus.SourceGenerator/Analysis/ReswResourceAnalyzer.cs @@ -0,0 +1,282 @@ +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Linq; +using System.Threading; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Text; +using ReswPlus.SourceGenerator.ClassGenerators; + +namespace ReswPlus.SourceGenerator.Analysis; + +/// +/// Reports the problems of the .resw files of a project that would otherwise only surface at runtime, in +/// a language the team may not read. +/// +/// +/// Every rule is reported as a warning rather than an error: raising the severity would break the build of every +/// project that already has an inconsistency the moment it updates the package. Projects that want a rule to be +/// fatal can escalate it through .editorconfig. +/// +/// The rules err on the side of not firing. A noisy analyzer gets disabled wholesale, taking the valuable rules +/// with it, so a rule that cannot decide stays silent. +/// +/// +internal static class ReswResourceAnalyzer +{ + /// + /// The suffixes that identify a plural form of a resource, including the ReswPlus specific empty state. + /// + private static readonly string[] PluralSuffixes = ["Zero", "One", "Two", "Few", "Many", "Other", "None"]; + + /// + /// Analyzes the .resw files of a project. + /// + /// The .resw files of the project, with their content. + /// The default language of the project, if it declares one. + /// The callback invoked for every problem found. + /// The token used to cancel the operation. + public static void Analyze( + IReadOnlyList<(string Path, SourceText Text)> reswFiles, + string? defaultLanguage, + Action reportDiagnostic, + CancellationToken cancellationToken) + { + var textsByPath = new Dictionary(); + + foreach (var (path, text) in reswFiles) + { + textsByPath[path] = text; + } + + foreach (var group in ReswFileGrouping.GroupByResource(textsByPath.Keys)) + { + cancellationToken.ThrowIfCancellationRequested(); + + var defaultPath = ReswFileGrouping.RetrieveDefaultResourceFile(group, defaultLanguage); + + if (defaultPath is null || ReswDocument.Parse(defaultPath, textsByPath[defaultPath], cancellationToken) is not { } defaultDocument) + { + continue; + } + + var defaultModel = ReswResourceModel.Create(defaultDocument); + + AnalyzeDocument(defaultModel, defaultModel, reportDiagnostic); + + foreach (var path in group) + { + cancellationToken.ThrowIfCancellationRequested(); + + if (path == defaultPath || ReswDocument.Parse(path, textsByPath[path], cancellationToken) is not { } document) + { + continue; + } + + AnalyzeDocument(ReswResourceModel.Create(document), defaultModel, reportDiagnostic); + } + } + } + + private static void AnalyzeDocument(ReswResourceModel model, ReswResourceModel defaultModel, Action reportDiagnostic) + { + ReportDuplicateMembers(model, reportDiagnostic); + ReportMissingPluralForms(model, reportDiagnostic); + ReportFormattingProblems(model, defaultModel, reportDiagnostic); + } + + /// + /// RESWP0009: reports the resources that are generated as a member another resource already generates. + /// + /// + /// The comparison is case insensitive because resource lookup is: two resources whose names only differ by + /// case resolve to the same string at runtime, even though the members generated for them do not collide. + /// + private static void ReportDuplicateMembers(ReswResourceModel model, Action reportDiagnostic) + { + var membersByName = new Dictionary(StringComparer.OrdinalIgnoreCase); + + foreach (var member in model.Members) + { + if (membersByName.TryGetValue(member.Name, out var existing)) + { + reportDiagnostic(Diagnostic.Create( + Diagnostics.DuplicateResource, + member.Entries[0].Location, + member.Entries[0].Key, + member.Name, + existing.Entries[0].Key)); + } + else + { + membersByName.Add(member.Name, member); + } + } + } + + /// + /// RESWP0008: reports the pluralized resources that don't define every plural form their language requires. + /// + /// + /// A missing form is not a build failure: the lookup simply returns an empty string at runtime, which makes + /// this the kind of problem that ships unnoticed. + /// + private static void ReportMissingPluralForms(ReswResourceModel model, Action reportDiagnostic) + { + if (model.Document.Language is not { Length: > 0 } language || + PluralFormsRetriever.RetrievePluralCategoriesForLanguage(language) is not { } requiredCategories) + { + return; + } + + foreach (var member in model.Members) + { + if (!member.IsPlural) + { + continue; + } + + // A pluralized resource that also has variants is declined once per variant, and every variant needs + // the full set of plural forms of the language. + foreach (var declension in GetDeclensions(member)) + { + var missingCategories = requiredCategories + .Where(category => !model.TryGetEntry($"{declension.Prefix}_{category}", out _)) + .ToArray(); + + if (missingCategories.Length == 0) + { + continue; + } + + reportDiagnostic(Diagnostic.Create( + Diagnostics.MissingPluralForms, + declension.Location, + declension.Prefix, + string.Join(", ", missingCategories.Select(category => $"'_{category}'")), + language)); + } + } + } + + /// + /// RESWP0006, RESWP0007 and RESWP0010: reports the values that are not usable as the composite format string + /// the generated code passes them to. + /// + /// The file being analyzed. + /// + /// The file of the default language, which is the only one carrying the #Format tags and therefore the + /// only one that determines whether, and with how many arguments, a resource is formatted. + /// + /// The callback invoked for every problem found. + private static void ReportFormattingProblems(ReswResourceModel model, ReswResourceModel defaultModel, Action reportDiagnostic) + { + var isDefaultLanguage = ReferenceEquals(model, defaultModel); + + foreach (var member in model.Members) + { + // Values of resources that are never formatted are returned verbatim, so braces in them are literal. + if (!defaultModel.TryGetMember(member.Name, out var defaultMember) || !defaultMember.IsFormatted) + { + continue; + } + + foreach (var entry in member.Entries) + { + if (!CompositeFormatString.TryGetArgumentIndexes(entry.Value, out var indexes)) + { + reportDiagnostic(Diagnostic.Create(Diagnostics.InvalidFormatString, entry.Location, entry.Key)); + + continue; + } + + if (TryGetUndeclaredIndex(indexes, defaultMember.FormatParameterCount, out var undeclaredIndex)) + { + reportDiagnostic(Diagnostic.Create( + Diagnostics.UndeclaredFormatParameter, + entry.Location, + entry.Key, + undeclaredIndex, + defaultMember.FormatParameterCount)); + + continue; + } + + // Comparing a translation against itself would always match, and a resource that only exists in + // the default language has nothing to be compared with. + if (isDefaultLanguage || + !defaultModel.TryGetEntry(entry.Key, out var defaultEntry) || + !CompositeFormatString.TryGetArgumentIndexes(defaultEntry.Value, out var defaultIndexes) || + indexes.SetEquals(defaultIndexes)) + { + continue; + } + + reportDiagnostic(Diagnostic.Create( + Diagnostics.PlaceholderMismatch, + entry.Location, + entry.Key, + DescribePlaceholders(indexes), + DescribePlaceholders(defaultIndexes))); + } + } + } + + /// + /// Looks for a placeholder that has no matching argument in the generated call to string.Format. + /// + /// The argument indexes referenced by the value, in ascending order. + /// The number of arguments the generated code passes. + /// The first index that has no matching argument. + /// Whether the value would throw a at runtime. + private static bool TryGetUndeclaredIndex(IEnumerable indexes, int parameterCount, out int undeclaredIndex) + { + foreach (var index in indexes) + { + if (index >= parameterCount) + { + undeclaredIndex = index; + + return true; + } + } + + undeclaredIndex = 0; + + return false; + } + + /// + /// Returns the resource name prefixes a pluralized resource is declined from, one per variant. + /// + private static IEnumerable<(string Prefix, Location Location)> GetDeclensions(ReswMember member) + { + var locationsByPrefix = new Dictionary(StringComparer.Ordinal); + + foreach (var entry in member.Entries) + { + var separator = entry.Key.LastIndexOf('_'); + + if (separator <= 0 || !PluralSuffixes.Contains(entry.Key.Substring(separator + 1))) + { + continue; + } + + var prefix = entry.Key.Substring(0, separator); + + if (!locationsByPrefix.ContainsKey(prefix)) + { + locationsByPrefix.Add(prefix, entry.Location); + } + } + + return locationsByPrefix.Select(pair => (pair.Key, pair.Value)); + } + + private static string DescribePlaceholders(IEnumerable indexes) + { + var placeholders = indexes.Select(index => $"{{{index.ToString(CultureInfo.InvariantCulture)}}}").ToArray(); + + return placeholders.Length == 0 ? "no placeholder" : string.Join(", ", placeholders); + } +} diff --git a/src/ReswPlus.SourceGenerator/Analysis/ReswResourceModel.cs b/src/ReswPlus.SourceGenerator/Analysis/ReswResourceModel.cs new file mode 100644 index 0000000..86166d8 --- /dev/null +++ b/src/ReswPlus.SourceGenerator/Analysis/ReswResourceModel.cs @@ -0,0 +1,209 @@ +using System.Collections.Generic; +using System.IO; +using System.Linq; +using ReswPlus.Core.ResourceParser; +using ReswPlus.SourceGenerator.ClassGenerators; + +namespace ReswPlus.SourceGenerator.Analysis; + +/// +/// A member the generator produces for a .resw file, and the entries it is produced from. +/// +internal sealed class ReswMember +{ + public ReswMember(string name, bool isPlural, IReadOnlyList entries, int formatParameterCount) + { + Name = name; + IsPlural = isPlural; + Entries = entries; + FormatParameterCount = formatParameterCount; + } + + /// + /// Gets the name of the generated member. + /// + public string Name { get; } + + /// + /// Gets whether the member is generated from a set of pluralized resources. + /// + public bool IsPlural { get; } + + /// + /// Gets the entries the member is generated from, in document order. A member generated from a plain + /// resource has a single entry, a pluralized or varianted one has an entry per form. + /// + public IReadOnlyList Entries { get; } + + /// + /// Gets the number of parameters declared by the #Format tag of the resource, or 0 when the + /// resource has no usable tag and its value is therefore returned without being formatted. + /// + public int FormatParameterCount { get; } + + /// + /// Gets whether the value of the resource is passed to . + /// + public bool IsFormatted => FormatParameterCount > 0; +} + +/// +/// The members a .resw file is generated as. +/// +/// +/// This mirrors the classification done by when it parses a resource file, so +/// that the diagnostics reason about exactly what the generator emits. It is deliberately a separate, read only +/// pass: the diagnostics are additive and must not influence generation. +/// +internal sealed class ReswResourceModel +{ + private readonly Dictionary _membersByName; + private readonly Dictionary _entriesByKey; + + private ReswResourceModel(ReswDocument document, IReadOnlyList members) + { + Document = document; + Members = members; + + _membersByName = new Dictionary(); + _entriesByKey = new Dictionary(); + + foreach (var member in members) + { + if (!_membersByName.ContainsKey(member.Name)) + { + _membersByName.Add(member.Name, member); + } + } + + foreach (var entry in document.Entries) + { + if (!_entriesByKey.ContainsKey(entry.Key)) + { + _entriesByKey.Add(entry.Key, entry); + } + } + } + + /// + /// Gets the file the members are generated from. + /// + public ReswDocument Document { get; } + + /// + /// Gets the generated members, ordered by the position of their first entry in the file. + /// + public IReadOnlyList Members { get; } + + /// + /// Looks up a generated member by name. + /// + /// The name of the member to look up. + /// The member, if it was found. + /// Whether a member with that name is generated. + public bool TryGetMember(string name, out ReswMember member) + { + return _membersByName.TryGetValue(name, out member); + } + + /// + /// Looks up an entry of the file by resource name. + /// + /// The name of the resource to look up. + /// The entry, if it was found. + /// Whether the file declares that resource. + public bool TryGetEntry(string key, out ReswEntry entry) + { + return _entriesByKey.TryGetValue(key, out entry); + } + + /// + /// Builds the model of a parsed .resw file. + /// + /// The file to build the model of. + /// The members the file is generated as. + public static ReswResourceModel Create(ReswDocument document) + { + var entriesByItem = new Dictionary(); + var positions = new Dictionary(); + + for (var i = 0; i < document.Entries.Count; i++) + { + var entry = document.Entries[i]; + + entriesByItem[entry.Item] = entry; + positions[entry.Item] = i; + } + + var items = document.Entries.Select(entry => entry.Item).ToArray(); + + // The classification below follows ReswClassGenerator.Parse: pluralized and varianted resources are + // grouped first, out of all the items, and whatever is left and has a usable name becomes a plain member. + var stringItems = items + .Where(item => ReswClassGenerator.IsValidPropertyName(item.Key) && !(item.Comment?.Contains(ReswClassGenerator.TagIgnore) ?? false)) + .ToArray(); + + var groups = items.GetItemsWithVariantOrPlural().ToArray(); + var basicItems = stringItems.Except(groups.SelectMany(group => group.Items)).ToArray(); + var resourceFileName = Path.GetFileName(document.Path); + + var members = new List(); + + foreach (var group in groups) + { + // Only one of the resources of a group carries the #Format tag, and it is not necessarily the first. + var comment = group.Items.FirstOrDefault(item => HasFormatTag(item.Comment))?.Comment; + + members.Add(new ReswMember( + group.Key, + group.SupportPlural, + group.Items.Where(entriesByItem.ContainsKey).Select(item => entriesByItem[item]).ToArray(), + CountFormatParameters(group.Key, comment, basicItems, resourceFileName))); + } + + foreach (var item in basicItems) + { + members.Add(new ReswMember( + item.Key, + isPlural: false, + [entriesByItem[item]], + CountFormatParameters(item.Key, item.Comment, stringItems, resourceFileName))); + } + + return new ReswResourceModel( + document, + members.Where(member => member.Entries.Count > 0) + .OrderBy(member => positions[member.Entries[0].Item]) + .ToArray()); + } + + private static bool HasFormatTag(string? comment) + { + return ReswClassGenerator.ParseTag(comment).format is not null; + } + + /// + /// Counts the arguments the generated code passes to . + /// + /// The name of the resource, used for diagnostics of the tag parser. + /// The comment carrying the #Format tag, if any. + /// The resources a Reference() parameter of the tag can point at. + /// The name of the resource file, used for diagnostics of the tag parser. + /// + /// The number of declared parameters, or 0 when there is no tag or the tag cannot be parsed, since + /// in both cases the generated member returns the value of the resource without formatting it. + /// + private static int CountFormatParameters(string key, string? comment, IEnumerable knownItems, string resourceFileName) + { + var (format, _) = ReswClassGenerator.ParseTag(comment); + + if (format is null) + { + return 0; + } + + var parameters = FormatTag.ParseParameters(key, FormatTag.SplitParameters(format), knownItems, resourceFileName, logger: null); + + return parameters?.Parameters.Count ?? 0; + } +} diff --git a/src/ReswPlus.SourceGenerator/AnalyzerReleases.Shipped.md b/src/ReswPlus.SourceGenerator/AnalyzerReleases.Shipped.md index e349588..bd05fc8 100644 --- a/src/ReswPlus.SourceGenerator/AnalyzerReleases.Shipped.md +++ b/src/ReswPlus.SourceGenerator/AnalyzerReleases.Shipped.md @@ -8,4 +8,9 @@ RESWP0001 | Compatibility | Error | ReswPlus source generator only support RESWP0002 | Compatibility | Error | ReswPlus cannot determine the namespace. RESWP0003 | Compatibility | Error | Can't retrieve the root path of the project. RESWP0004 | Compatibility | Info | ReswPlus cannot determine the project type, defaulting to application. -RESWP0005 | Compatibility | Error | ReswPlus only supports UWP and WinAppSDK applications/libraries. \ No newline at end of file +RESWP0005 | Compatibility | Error | ReswPlus only supports UWP and WinAppSDK applications/libraries. +RESWP0006 | Resources | Warning | A translated value doesn't use the same placeholders as the default language. +RESWP0007 | Resources | Warning | A value uses a placeholder that has no matching parameter in its #Format tag. +RESWP0008 | Resources | Warning | A pluralized resource is missing plural forms its language requires. +RESWP0009 | Resources | Warning | Two resources of the same file are generated as the same member. +RESWP0010 | Resources | Warning | A value that is used as a composite format string is malformed. \ No newline at end of file diff --git a/src/ReswPlus.SourceGenerator/ClassGenerators/PluralCategory.cs b/src/ReswPlus.SourceGenerator/ClassGenerators/PluralCategory.cs new file mode 100644 index 0000000..283886f --- /dev/null +++ b/src/ReswPlus.SourceGenerator/ClassGenerators/PluralCategory.cs @@ -0,0 +1,29 @@ +namespace ReswPlus.SourceGenerator.ClassGenerators; + +/// +/// The CLDR plural categories a resource can be declined in. +/// +/// +/// The names match the suffixes ReswPlus appends to the key of a pluralized resource at runtime, so a category +/// maps directly to the resource named <key>_<category>. +/// +internal enum PluralCategory +{ + /// The 'zero' plural category. + Zero, + + /// The 'one' plural category. + One, + + /// The 'two' plural category. + Two, + + /// The 'few' plural category. + Few, + + /// The 'many' plural category. + Many, + + /// The 'other' plural category, used when no other category applies. + Other +} diff --git a/src/ReswPlus.SourceGenerator/ClassGenerators/PluralFormsRetriever.cs b/src/ReswPlus.SourceGenerator/ClassGenerators/PluralFormsRetriever.cs index d5144bf..0af3e26 100644 --- a/src/ReswPlus.SourceGenerator/ClassGenerators/PluralFormsRetriever.cs +++ b/src/ReswPlus.SourceGenerator/ClassGenerators/PluralFormsRetriever.cs @@ -7,15 +7,32 @@ namespace ReswPlus.SourceGenerator.ClassGenerators; /// internal sealed class PluralFormsRetriever { + /// + /// A plural form supported by a set of languages. + /// internal record PluralForm { - public PluralForm(string id, string[] languages) + public PluralForm(string id, PluralCategory[] categories, string[] languages) { Id = id; + Categories = categories; Languages = languages; } + /// + /// Gets the identifier of the provider implementing this plural form. + /// public string Id { get; set; } + + /// + /// Gets the plural categories the provider of this form can return, and which a resource declined in a + /// language using this form therefore has to define. + /// + public PluralCategory[] Categories { get; set; } + + /// + /// Gets the languages using this plural form. + /// public string[] Languages { get; set; } } @@ -26,6 +43,7 @@ public PluralForm(string id, string[] languages) [ new PluralForm( "IntOneOrZero", + [PluralCategory.One, PluralCategory.Other], [ "ak", // Akan "bh", // Bihari @@ -40,6 +58,7 @@ public PluralForm(string id, string[] languages) ), new PluralForm( "ZeroToOne", + [PluralCategory.One, PluralCategory.Other], [ "am", // Amharic "bn", // Bengali @@ -54,6 +73,7 @@ public PluralForm(string id, string[] languages) ), new PluralForm( "ZeroToTwoExcluded", + [PluralCategory.One, PluralCategory.Other], [ "hy", // Armenian "fr", // French @@ -62,6 +82,7 @@ public PluralForm(string id, string[] languages) ), new PluralForm( "OnlyOne", + [PluralCategory.One, PluralCategory.Other], [ "af", // Afrikaans "sq", // Albanian @@ -166,12 +187,14 @@ public PluralForm(string id, string[] languages) ), new PluralForm( "Sinhala", + [PluralCategory.One, PluralCategory.Other], [ "si" // Sinhala ] ), new PluralForm( "Latvian", + [PluralCategory.Zero, PluralCategory.One, PluralCategory.Other], [ "lv", // Latvian "prg" // Prussian @@ -179,12 +202,14 @@ public PluralForm(string id, string[] languages) ), new PluralForm( "Irish", + [PluralCategory.One, PluralCategory.Two, PluralCategory.Few, PluralCategory.Many, PluralCategory.Other], [ "ga" // Irish ] ), new PluralForm( "Romanian", + [PluralCategory.One, PluralCategory.Few, PluralCategory.Other], [ "ro", // Romanian "mo" // Moldavian @@ -192,12 +217,14 @@ public PluralForm(string id, string[] languages) ), new PluralForm( "Lithuanian", + [PluralCategory.One, PluralCategory.Few, PluralCategory.Many, PluralCategory.Other], [ "lt" // Lithuanian ] ), new PluralForm( "Slavic", + [PluralCategory.One, PluralCategory.Few, PluralCategory.Many, PluralCategory.Other], [ "ru", // Russian "uk", // Ukrainian @@ -206,6 +233,7 @@ public PluralForm(string id, string[] languages) ), new PluralForm( "Czech", + [PluralCategory.One, PluralCategory.Few, PluralCategory.Many, PluralCategory.Other], [ "cs", // Czech "sk" // Slovak @@ -213,24 +241,28 @@ public PluralForm(string id, string[] languages) ), new PluralForm( "Polish", + [PluralCategory.One, PluralCategory.Few, PluralCategory.Many, PluralCategory.Other], [ "pl" // Polish ] ), new PluralForm( "Slovenian", + [PluralCategory.One, PluralCategory.Two, PluralCategory.Few, PluralCategory.Other], [ "sl" // Slovenian ] ), new PluralForm( "Arabic", + [PluralCategory.Zero, PluralCategory.One, PluralCategory.Two, PluralCategory.Few, PluralCategory.Many, PluralCategory.Other], [ "ar" // Arabic ] ), new PluralForm( "Hebrew", + [PluralCategory.One, PluralCategory.Two, PluralCategory.Many, PluralCategory.Other], [ "he", // Hebrew "iw" // (old code for Hebrew) @@ -238,6 +270,7 @@ public PluralForm(string id, string[] languages) ), new PluralForm( "Filipino", + [PluralCategory.One, PluralCategory.Other], [ "fil", // Filipino "tl" // Tagalog @@ -245,36 +278,42 @@ public PluralForm(string id, string[] languages) ), new PluralForm( "Macedonian", + [PluralCategory.One, PluralCategory.Other], [ "mk" // Macedonian ] ), new PluralForm( "Breizh", + [PluralCategory.One, PluralCategory.Two, PluralCategory.Few, PluralCategory.Many, PluralCategory.Other], [ "br" // Breton ] ), new PluralForm( "CentralAtlasTamazight", + [PluralCategory.One, PluralCategory.Other], [ "tzm" // Central Atlas Tamazight ] ), new PluralForm( "OneOrZero", + [PluralCategory.Zero, PluralCategory.One, PluralCategory.Other], [ "ksh" // Colognian ] ), new PluralForm( "OneOrZeroToOneExcluded", + [PluralCategory.Zero, PluralCategory.One, PluralCategory.Other], [ "lag" // Langi ] ), new PluralForm( "OneOrTwo", + [PluralCategory.One, PluralCategory.Two, PluralCategory.Other], [ "kw", // Cornish "smn", // Inari Sami @@ -289,6 +328,7 @@ public PluralForm(string id, string[] languages) ), new PluralForm( "Croat", + [PluralCategory.One, PluralCategory.Few, PluralCategory.Other], [ "bs", // Bosnian "hr", // Croatian @@ -298,42 +338,49 @@ public PluralForm(string id, string[] languages) ), new PluralForm( "Tachelhit", + [PluralCategory.One, PluralCategory.Few, PluralCategory.Other], [ "shi" // Tachelhit ] ), new PluralForm( "Icelandic", + [PluralCategory.One, PluralCategory.Other], [ "is" // Icelandic ] ), new PluralForm( "Manx", + [PluralCategory.One, PluralCategory.Two, PluralCategory.Few, PluralCategory.Many, PluralCategory.Other], [ "gv" // Manx ] ), new PluralForm( "ScottishGaelic", + [PluralCategory.One, PluralCategory.Two, PluralCategory.Few, PluralCategory.Other], [ "gd" // Scottish Gaelic ] ), new PluralForm( "Maltese", + [PluralCategory.One, PluralCategory.Few, PluralCategory.Many, PluralCategory.Other], [ "mt" // Maltese ] ), new PluralForm( "Welsh", + [PluralCategory.Zero, PluralCategory.One, PluralCategory.Two, PluralCategory.Few, PluralCategory.Many, PluralCategory.Other], [ "cy" // Welsh ] ), new PluralForm( "Danish", + [PluralCategory.One, PluralCategory.Other], [ "da" // Danish ] @@ -378,4 +425,17 @@ public static IEnumerable RetrievePluralFormsForLanguages(IEnumerabl } return result.Values; } + + /// + /// Retrieves the plural categories a resource has to define to be correctly declined in a language. + /// + /// The primary language subtag to retrieve the categories for. + /// + /// The plural categories required by , or if the language + /// has no dedicated plural provider, in which case no plural form can be assumed to be required. + /// + public static PluralCategory[]? RetrievePluralCategoriesForLanguage(string language) + { + return LanguageToPluralForm.TryGetValue(language, out var pluralForm) ? pluralForm.Categories : null; + } } diff --git a/src/ReswPlus.SourceGenerator/ClassGenerators/ReswClassGenerator.cs b/src/ReswPlus.SourceGenerator/ClassGenerators/ReswClassGenerator.cs index f526ee6..cee01a1 100644 --- a/src/ReswPlus.SourceGenerator/ClassGenerators/ReswClassGenerator.cs +++ b/src/ReswPlus.SourceGenerator/ClassGenerators/ReswClassGenerator.cs @@ -17,7 +17,7 @@ namespace ReswPlus.SourceGenerator.ClassGenerators; /// public sealed class ReswClassGenerator { - private const string TagIgnore = "#ReswPlusIgnore"; + internal const string TagIgnore = "#ReswPlusIgnore"; private const string Deprecated_TagStrongType = "#ReswPlusTyped"; private const string TagFormat = "#Format"; private const string TagFormatDotNet = "#FormatNet"; @@ -163,7 +163,7 @@ private StronglyTypedClass Parse(string content, string defaultNamespace, bool i /// /// The property name to validate. /// True if the property name is valid; otherwise, false. - private static bool IsValidPropertyName(string propertyName) + internal static bool IsValidPropertyName(string propertyName) { return !string.IsNullOrWhiteSpace(propertyName) && diff --git a/src/ReswPlus.SourceGenerator/Diagnostics.cs b/src/ReswPlus.SourceGenerator/Diagnostics.cs new file mode 100644 index 0000000..f1db9b1 --- /dev/null +++ b/src/ReswPlus.SourceGenerator/Diagnostics.cs @@ -0,0 +1,133 @@ +using Microsoft.CodeAnalysis; + +namespace ReswPlus.SourceGenerator; + +/// +/// The descriptors of all the diagnostics reported by ReswPlus. +/// +/// +/// Every rule declared here must also be listed in AnalyzerReleases.Unshipped.md until it ships, +/// otherwise the build fails with RS2008. +/// +internal static class Diagnostics +{ + /// + /// The category of the diagnostics reporting an unsupported or misconfigured project. + /// + private const string CompatibilityCategory = "Compatibility"; + + /// + /// The category of the diagnostics reporting a problem in the content of the .resw files. + /// + private const string ResourcesCategory = "Resources"; + + /// + /// RESWP0001: the compilation is not a C# compilation. + /// + public static readonly DiagnosticDescriptor UnsupportedLanguage = new( + "RESWP0001", + "Language not supported", + "ReswPlus source generator only supports C#", + CompatibilityCategory, + DiagnosticSeverity.Error, + isEnabledByDefault: true); + + /// + /// RESWP0002: the root namespace of the project could not be determined. + /// + public static readonly DiagnosticDescriptor UnknownNamespace = new( + "RESWP0002", + "Unknown namespace", + "ReswPlus cannot determine the namespace", + CompatibilityCategory, + DiagnosticSeverity.Error, + isEnabledByDefault: true); + + /// + /// RESWP0003: the root path of the project could not be determined. + /// + public static readonly DiagnosticDescriptor MissingRootPath = new( + "RESWP0003", + "Root path missing", + "Can't retrieve the root path of the project", + CompatibilityCategory, + DiagnosticSeverity.Error, + isEnabledByDefault: true); + + /// + /// RESWP0004: the project type could not be determined. + /// + public static readonly DiagnosticDescriptor UnknownProjectType = new( + "RESWP0004", + "Unknown Project Type", + "ReswPlus cannot determine the project type, defaulting to application", + CompatibilityCategory, + DiagnosticSeverity.Info, + isEnabledByDefault: true); + + /// + /// RESWP0005: the project is neither a UWP nor a WinAppSDK project. + /// + public static readonly DiagnosticDescriptor UnrecognizedAppType = new( + "RESWP0005", + "Project type not recognized", + "ReswPlus only supports UWP and WinAppSDK applications/libraries", + CompatibilityCategory, + DiagnosticSeverity.Error, + isEnabledByDefault: true); + + /// + /// RESWP0006: a translated value doesn't use the same placeholders as the default language. + /// + public static readonly DiagnosticDescriptor PlaceholderMismatch = new( + "RESWP0006", + "Placeholder mismatch between languages", + "The value of the resource '{0}' uses {1}, while its value in the default language uses {2}", + ResourcesCategory, + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + /// + /// RESWP0007: a value uses a placeholder that has no matching parameter in its #Format tag. + /// + public static readonly DiagnosticDescriptor UndeclaredFormatParameter = new( + "RESWP0007", + "Undeclared format parameter", + "The value of the resource '{0}' uses the placeholder '{{{1}}}', but its '#Format' tag only declares {2} parameter(s)", + ResourcesCategory, + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + /// + /// RESWP0008: a pluralized resource is missing plural forms its language requires. + /// + public static readonly DiagnosticDescriptor MissingPluralForms = new( + "RESWP0008", + "Missing plural forms", + "The pluralized resource '{0}' is missing the {1} form(s) required by the '{2}' language", + ResourcesCategory, + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + /// + /// RESWP0009: two resources of the same file are generated as the same member. + /// + public static readonly DiagnosticDescriptor DuplicateResource = new( + "RESWP0009", + "Duplicate resource", + "The resource '{0}' generates the member '{1}', which is already generated by the resource '{2}'", + ResourcesCategory, + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + /// + /// RESWP0010: a value that is used as a composite format string is malformed. + /// + public static readonly DiagnosticDescriptor InvalidFormatString = new( + "RESWP0010", + "Invalid composite format string", + "The value of the resource '{0}' is used as a composite format string, but it is not a valid one", + ResourcesCategory, + DiagnosticSeverity.Warning, + isEnabledByDefault: true); +} diff --git a/src/ReswPlus.SourceGenerator/ReswGenerator.cs b/src/ReswPlus.SourceGenerator/ReswGenerator.cs index f28be2d..64f1a5a 100644 --- a/src/ReswPlus.SourceGenerator/ReswGenerator.cs +++ b/src/ReswPlus.SourceGenerator/ReswGenerator.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.IO; using System.Linq; using System.Reflection; @@ -7,6 +8,7 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.Text; +using ReswPlus.SourceGenerator.Analysis; using ReswPlus.SourceGenerator.ClassGenerators; using ReswPlus.SourceGenerator.CodeGenerators; using ReswPlus.SourceGenerator.Models; @@ -28,52 +30,6 @@ public enum AppType [Generator] public partial class ReswSourceGenerator : IIncrementalGenerator { - // Diagnostic descriptors (could be moved to a central location if reused) - private static readonly DiagnosticDescriptor UnsupportedLanguageDiagnostic = - new( - "RESWP0001", - "Language not supported", - "ReswPlus source generator only supports C#", - "Compatibility", - DiagnosticSeverity.Error, - isEnabledByDefault: true); - - private static readonly DiagnosticDescriptor UnknownNamespaceDiagnostic = - new( - "RESWP0002", - "Unknown namespace", - "ReswPlus cannot determine the namespace", - "Compatibility", - DiagnosticSeverity.Error, - isEnabledByDefault: true); - - private static readonly DiagnosticDescriptor MissingRootPathDiagnostic = - new( - "RESWP0003", - "Root path missing", - "Can't retrieve the root path of the project", - "Compatibility", - DiagnosticSeverity.Error, - isEnabledByDefault: true); - - private static readonly DiagnosticDescriptor UnknownProjectTypeDiagnostic = - new( - "RESWP0004", - "Unknown Project Type", - "ReswPlus cannot determine the project type, defaulting to application", - "Compatibility", - DiagnosticSeverity.Info, - isEnabledByDefault: true); - - private static readonly DiagnosticDescriptor UnrecognizedAppTypeDiagnostic = - new( - "RESWP0005", - "Project type not recognized", - "ReswPlus only supports UWP and WinAppSDK applications/libraries", - "Compatibility", - DiagnosticSeverity.Error, - isEnabledByDefault: true); - public void Initialize(IncrementalGeneratorInitializationContext context) { #if DEBUG @@ -100,6 +56,16 @@ public void Initialize(IncrementalGeneratorInitializationContext context) .Where(file => Path.GetExtension(file.Path).Equals(".resw", StringComparison.OrdinalIgnoreCase)) .Collect(); + // The resource diagnostics only depend on the .resw files and on the default language of the project, so + // they are registered separately: combining them with the compilation would re-run the whole analysis on + // every keystroke, for every language of the project. + var defaultLanguageProvider = context.AnalyzerConfigOptionsProvider + .Select((options, cancellationToken) => GetOption(options.GlobalOptions, "build_property.DefaultLanguage")); + + context.RegisterSourceOutput( + reswFilesProvider.Combine(defaultLanguageProvider), + static (spc, source) => ReportResourceDiagnostics(spc, source.Left, source.Right)); + // Combine the Compilation, the global options, and the additional files. var combinedProvider = context.CompilationProvider .Combine(globalOptionsProvider) @@ -118,7 +84,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) // Only support C# if (compilation is not CSharpCompilation) { - spc.ReportDiagnostic(Diagnostic.Create(UnsupportedLanguageDiagnostic, Location.None)); + spc.ReportDiagnostic(Diagnostic.Create(Diagnostics.UnsupportedLanguage, Location.None)); return; } @@ -131,7 +97,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) if (string.IsNullOrEmpty(projectRootPath)) { - spc.ReportDiagnostic(Diagnostic.Create(MissingRootPathDiagnostic, Location.None)); + spc.ReportDiagnostic(Diagnostic.Create(Diagnostics.MissingRootPath, Location.None)); return; } @@ -149,7 +115,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) } else { - spc.ReportDiagnostic(Diagnostic.Create(UnknownProjectTypeDiagnostic, Location.None)); + spc.ReportDiagnostic(Diagnostic.Create(Diagnostics.UnknownProjectType, Location.None)); } // Determine AppType based on referenced assemblies. @@ -165,7 +131,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) AddSourceFromResource(spc, $"{assemblyName}.Templates.ResourceStringProviders.WindowsResourceStringProvider.txt", "ResourceStringProvider"); break; default: - spc.ReportDiagnostic(Diagnostic.Create(UnrecognizedAppTypeDiagnostic, Location.None)); + spc.ReportDiagnostic(Diagnostic.Create(Diagnostics.UnrecognizedAppType, Location.None)); return; } @@ -175,7 +141,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) // Retrieve the project's root namespace. if (string.IsNullOrEmpty(options.RootNamespace)) { - spc.ReportDiagnostic(Diagnostic.Create(UnknownNamespaceDiagnostic, Location.None)); + spc.ReportDiagnostic(Diagnostic.Create(Diagnostics.UnknownNamespace, Location.None)); return; } var projectRootNamespace = options.RootNamespace!; @@ -184,14 +150,9 @@ public void Initialize(IncrementalGeneratorInitializationContext context) var allResourceFiles = additionalFiles.Distinct().ToArray(); // Group files and retrieve the default resource file per group. - var defaultLanguageResourceFiles = (from file in allResourceFiles - group file by - Path.Combine( - Path.GetDirectoryName(Path.GetDirectoryName(file.Path)), - Path.GetFileName(file.Path)) - into fileGroup - let defaultFile = RetrieveDefaultResourceFile( - fileGroup.Select(f => f.Path), + var defaultLanguageResourceFiles = (from fileGroup in ReswFileGrouping.GroupByResource(allResourceFiles.Select(file => file.Path)) + let defaultFile = ReswFileGrouping.RetrieveDefaultResourceFile( + fileGroup, projectDefaultLanguage) where defaultFile != null select defaultFile).ToArray(); @@ -274,52 +235,32 @@ into fileGroup } /// - /// Helper method to retrieve an option value. + /// Reports the diagnostics about the content of the .resw files of the project. /// - private static string? GetOption(AnalyzerConfigOptions globalOptions, string key) + /// The context to report the diagnostics to. + /// The .resw files of the project. + /// The default language of the project, if it declares one. + private static void ReportResourceDiagnostics(SourceProductionContext spc, ImmutableArray reswFiles, string? defaultLanguage) { - return globalOptions.TryGetValue(key, out var value) ? value : null; - } + var documents = new List<(string Path, SourceText Text)>(); - /// - /// Retrieve the default resource file from the given list that matches one of the preferred languages. - /// - private static string? RetrieveDefaultResourceFile(IEnumerable reswFiles, string? defaultLanguage) - { - // Build a list of candidate languages. - var candidateLanguages = new List(); - if (defaultLanguage is { Length: > 0 }) + foreach (var file in reswFiles.Distinct()) { - candidateLanguages.Add(defaultLanguage); - } - - // Ensure "en-us" and "en" are included if not already the default. - if (!"en-us".Equals(defaultLanguage, StringComparison.OrdinalIgnoreCase)) - { - candidateLanguages.Add("en-us"); - } - - if (!"en".Equals(defaultLanguage, StringComparison.OrdinalIgnoreCase)) - { - candidateLanguages.Add("en"); - } - - // Iterate candidates and files to find a match. - foreach (var language in candidateLanguages) - { - foreach (var reswFile in reswFiles) + if (file.GetText(spc.CancellationToken) is { } text) { - // Get the immediate parent folder name (e.g. "en-us"). - var parentFolderName = Path.GetFileName(Path.GetDirectoryName(reswFile)); - if (parentFolderName.Equals(language, StringComparison.OrdinalIgnoreCase)) - { - return reswFile; - } + documents.Add((file.Path, text)); } } - // Fallback to the first available resource file. - return reswFiles.FirstOrDefault(); + ReswResourceAnalyzer.Analyze(documents, defaultLanguage, spc.ReportDiagnostic, spc.CancellationToken); + } + + /// + /// Helper method to retrieve an option value. + /// + private static string? GetOption(AnalyzerConfigOptions globalOptions, string key) + { + return globalOptions.TryGetValue(key, out var value) ? value : null; } /// diff --git a/tests/ReswPlusUnitTests/ResourceDiagnostics.cs b/tests/ReswPlusUnitTests/ResourceDiagnostics.cs new file mode 100644 index 0000000..dc6097c --- /dev/null +++ b/tests/ReswPlusUnitTests/ResourceDiagnostics.cs @@ -0,0 +1,314 @@ +using System.Linq; +using Xunit; + +namespace ReswPlusUnitTests; + +/// +/// Tests for the diagnostics reported on the content of the .resw files. +/// +/// +/// Every rule is covered in both directions: on input that is genuinely broken, and on the valid input that is +/// most likely to be mistaken for it. False positives are worse than missing diagnostics, because an analyzer +/// that reports valid resources gets disabled wholesale. +/// +public class ResourceDiagnostics +{ + [Fact] + public void PlaceholderMismatch_IsReportedWhenATranslationDropsAPlaceholder() + { + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw(("Greeting", "Hello {0}, you are {1}", "#Format[String name, Int age]"))), + ("fr", ReswTestHelpers.CreateResw(("Greeting", "Bonjour {0}", null)))); + + var diagnostic = Assert.Single(diagnostics); + + Assert.Equal("RESWP0006", diagnostic.Id); + Assert.Contains(@"Strings\fr\Resources.resw", diagnostic.Location.GetLineSpan().Path); + } + + [Fact] + public void PlaceholderMismatch_IsNotReportedWhenATranslationReordersPlaceholders() + { + // Languages routinely need a different word order, which is exactly what indexed placeholders are for. + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw(("Greeting", "Hello {0}, you are {1}", "#Format[String name, Int age]"))), + ("fr", ReswTestHelpers.CreateResw(("Greeting", "Vous avez {1} ans, {0}", null)))); + + Assert.Empty(diagnostics); + } + + [Fact] + public void PlaceholderMismatch_IsNotReportedForEscapedBraces() + { + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw(("Greeting", "Hello {0}", "#Format[String name]"))), + ("fr", ReswTestHelpers.CreateResw(("Greeting", "Bonjour {{{0}}}", null)))); + + Assert.Empty(diagnostics); + } + + [Fact] + public void PlaceholderMismatch_IsNotReportedForResourcesThatAreNeverFormatted() + { + // Without a #Format tag the value is returned verbatim, so the braces are literal text. + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw(("Sample", "Use {0} as a placeholder", null))), + ("fr", ReswTestHelpers.CreateResw(("Sample", "Utilisez un espace réservé", null)))); + + Assert.Empty(diagnostics); + } + + [Fact] + public void PlaceholderMismatch_IsNotReportedForResourcesMissingFromATranslation() + { + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw(("Greeting", "Hello {0}", "#Format[String name]"))), + ("fr", ReswTestHelpers.CreateResw(("Other", "Autre", null)))); + + Assert.Empty(diagnostics); + } + + [Fact] + public void UndeclaredFormatParameter_IsReportedWhenAValueUsesMoreParametersThanDeclared() + { + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw(("Greeting", "Hello {0}, you are {1}", "#Format[String name]")))); + + var diagnostic = Assert.Single(diagnostics); + + Assert.Equal("RESWP0007", diagnostic.Id); + Assert.Contains("{1}", diagnostic.GetMessage()); + } + + [Fact] + public void UndeclaredFormatParameter_IsReportedForTranslationsToo() + { + // The tag lives in the default language, but every translation is formatted with the same arguments. + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw(("Greeting", "Hello {0}", "#Format[String name]"))), + ("fr", ReswTestHelpers.CreateResw(("Greeting", "Bonjour {1}", null)))); + + var diagnostic = Assert.Single(diagnostics); + + Assert.Equal("RESWP0007", diagnostic.Id); + } + + [Fact] + public void UndeclaredFormatParameter_IsNotReportedWhenAValueUsesFewerParametersThanDeclared() + { + // The empty state of a pluralized resource legitimately doesn't show the quantity. + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw( + ("FileCount_None", "No files", null), + ("FileCount_One", "{0} file", "#Format[Plural Int count]"), + ("FileCount_Other", "{0} files", null)))); + + Assert.Empty(diagnostics); + } + + [Fact] + public void UndeclaredFormatParameter_IsNotReportedForEscapedBraces() + { + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw(("Greeting", "Hello {0}, use {{1}} to escape", "#Format[String name]")))); + + Assert.Empty(diagnostics); + } + + [Fact] + public void MissingPluralForms_AreReportedForALanguageThatNeedsMoreForms() + { + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw( + ("FileCount_One", "{0} file", "#Format[Plural Int count]"), + ("FileCount_Other", "{0} files", null))), + ("pl", ReswTestHelpers.CreateResw( + ("FileCount_One", "{0} plik", null), + ("FileCount_Other", "{0} pliku", null)))); + + var diagnostic = Assert.Single(diagnostics); + + Assert.Equal("RESWP0008", diagnostic.Id); + Assert.Contains("'_Few'", diagnostic.GetMessage()); + Assert.Contains("'_Many'", diagnostic.GetMessage()); + } + + [Fact] + public void MissingPluralForms_AreNotReportedWhenTheLanguageHasThemAll() + { + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw( + ("FileCount_One", "{0} file", "#Format[Plural Int count]"), + ("FileCount_Other", "{0} files", null))), + ("pl", ReswTestHelpers.CreateResw( + ("FileCount_One", "{0} plik", null), + ("FileCount_Few", "{0} pliki", null), + ("FileCount_Many", "{0} plików", null), + ("FileCount_Other", "{0} pliku", null)))); + + Assert.Empty(diagnostics); + } + + [Fact] + public void MissingPluralForms_AreNotReportedForALanguageWithoutAPluralProvider() + { + // Without a known plural provider no form can be assumed to be required. + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw( + ("FileCount_One", "{0} file", "#Format[Plural Int count]"), + ("FileCount_Other", "{0} files", null))), + ("qqq", ReswTestHelpers.CreateResw(("FileCount_One", "{0}", null)))); + + Assert.Empty(diagnostics); + } + + [Fact] + public void MissingPluralForms_AreReportedPerVariantOfAPluralizedResource() + { + var diagnostics = ReswTestHelpers.Analyze( + "pl", + ("pl", ReswTestHelpers.CreateResw( + ("Treat_Variant1_One", "{0} smakołyk", "#Format[Plural Int count, Variant petType]"), + ("Treat_Variant1_Few", "{0} smakołyki", null), + ("Treat_Variant1_Many", "{0} smakołyków", null), + ("Treat_Variant1_Other", "{0} smakołyku", null), + ("Treat_Variant2_One", "{0} smakołyk", null), + ("Treat_Variant2_Other", "{0} smakołyku", null)))); + + var diagnostic = Assert.Single(diagnostics); + + Assert.Equal("RESWP0008", diagnostic.Id); + Assert.Contains("Treat_Variant2", diagnostic.GetMessage()); + } + + [Fact] + public void DuplicateResource_IsReportedForKeysThatOnlyDifferByCase() + { + // Resource lookup is case insensitive, so both members resolve to the same string at runtime. + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw( + ("Welcome", "Welcome!", null), + ("welcome", "Welcome?", null)))); + + var diagnostic = Assert.Single(diagnostics); + + Assert.Equal("RESWP0009", diagnostic.Id); + } + + [Fact] + public void DuplicateResource_IsReportedWhenAPlainResourceCollidesWithAPluralizedOne() + { + // This one doesn't even compile: both the property and the pluralized method are named 'FileCount'. + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw( + ("FileCount_One", "{0} file", "#Format[Plural Int count]"), + ("FileCount_Other", "{0} files", null), + ("FileCount", "Some files", null)))); + + var diagnostic = Assert.Single(diagnostics); + + Assert.Equal("RESWP0009", diagnostic.Id); + Assert.Contains("FileCount", diagnostic.GetMessage()); + } + + [Fact] + public void DuplicateResource_IsNotReportedForDistinctResources() + { + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw( + ("Welcome", "Welcome!", null), + ("WelcomeBack", "Welcome back!", null), + ("FileCount_One", "{0} file", "#Format[Plural Int count]"), + ("FileCount_Other", "{0} files", null)))); + + Assert.Empty(diagnostics); + } + + [Fact] + public void InvalidFormatString_IsReportedForAMalformedFormattedValue() + { + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw(("Greeting", "Hello {0, you are {1}", "#Format[String name, Int age]")))); + + var diagnostic = Assert.Single(diagnostics); + + Assert.Equal("RESWP0010", diagnostic.Id); + } + + [Fact] + public void InvalidFormatString_IsReportedForTranslationsToo() + { + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw(("Greeting", "Hello {0}", "#Format[String name]"))), + ("fr", ReswTestHelpers.CreateResw(("Greeting", "Bonjour {0", null)))); + + var diagnostic = Assert.Single(diagnostics); + + Assert.Equal("RESWP0010", diagnostic.Id); + } + + [Fact] + public void InvalidFormatString_IsNotReportedForResourcesThatAreNeverFormatted() + { + // A value that is never passed to string.Format can contain any brace it wants. + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw(("Shortcut", "Press { to open the menu", null)))); + + Assert.Empty(diagnostics); + } + + [Fact] + public void InvalidFormatString_IsNotReportedForValidFormatItems() + { + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw(("Donate", "Hey {1}, donate {2:C2} to {0}!", "#Format[\"WWF\", String username, Int amount]")))); + + Assert.Empty(diagnostics); + } + + [Fact] + public void IgnoredResourcesAreNotAnalyzed() + { + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw(("Welcome", "Welcome!", "#ReswPlusIgnore"), ("welcome", "Welcome?", null)))); + + Assert.Empty(diagnostics); + } + + [Fact] + public void DiagnosticsPointAtTheLineOfTheOffendingResource() + { + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw( + ("First", "First", null), + ("Greeting", "Hello {0} {1}", "#Format[String name]")))); + + var lineSpan = Assert.Single(diagnostics).Location.GetLineSpan(); + var line = ReswTestHelpers.CreateResw(("First", "First", null), ("Greeting", "Hello {0} {1}", "#Format[String name]")) + .Split('\n')[lineSpan.StartLinePosition.Line]; + + Assert.Contains(@"name=""Greeting""", line); + Assert.Equal("Greeting".Length, lineSpan.EndLinePosition.Character - lineSpan.StartLinePosition.Character); + } +} diff --git a/tests/ReswPlusUnitTests/ReswTestHelpers.cs b/tests/ReswPlusUnitTests/ReswTestHelpers.cs index a96ce71..ae283a2 100644 --- a/tests/ReswPlusUnitTests/ReswTestHelpers.cs +++ b/tests/ReswPlusUnitTests/ReswTestHelpers.cs @@ -1,6 +1,10 @@ using System.Collections.Generic; using System.Linq; +using System.Threading; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Text; using ReswPlus.SourceGenerator; +using ReswPlus.SourceGenerator.Analysis; using ReswPlus.SourceGenerator.ClassGenerators; using ReswPlus.SourceGenerator.CodeGenerators; using ReswPlus.SourceGenerator.Models; @@ -78,4 +82,33 @@ private static string Escape(string text) { return text.Replace("&", "&").Replace("<", "<").Replace(">", ">"); } + + /// + /// Runs the resource analysis over a set of in-memory .resw files. + /// + /// The default language declared by the project. + /// The files of the project, as language folder name and content pairs. + /// The diagnostics reported for those files. + public static IReadOnlyList Analyze(string? defaultLanguage, params (string Language, string Content)[] files) + { + var documents = files + .Select(file => ($@"C:\Project\Strings\{file.Language}\Resources.resw", SourceText.From(file.Content))) + .ToArray(); + + var diagnostics = new List(); + + ReswResourceAnalyzer.Analyze(documents, defaultLanguage, diagnostics.Add, CancellationToken.None); + + return diagnostics; + } + + /// + /// Returns the identifiers of the given diagnostics, in the order they were reported. + /// + /// The diagnostics to describe. + /// The identifiers of the diagnostics. + public static string[] GetIds(this IEnumerable diagnostics) + { + return diagnostics.Select(diagnostic => diagnostic.Id).ToArray(); + } } From 6a528a55107ee02e2b1d8a5cc09107188cfe44a4 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 27 Jul 2026 22:55:30 -0700 Subject: [PATCH 4/7] Report the resource diagnostics from an analyzer, not the generator A source generator runs on every keystroke in the IDE, so it has to stay as cheap as possible. Parsing every `.resw` file of every language of a project and cross checking them against the default language is far too expensive to do there, and it is not what a generator is for. The rules are therefore moved to a proper `DiagnosticAnalyzer`, which the compiler schedules independently of the generator pipeline, runs out of process, and which consumers can disable per rule. The check logic itself is unchanged, it simply moved from `ReswResourceAnalyzer` to `ReswResourceRules` so that the analyzer is a thin entry point over it, and it is now also covered by a test that goes through `CompilationWithAnalyzers`, which is the only thing that can catch a registration mistake. While reviewing the pipeline: the generator held on to the `Compilation` itself, which is a different object after every edit, so every downstream step was invalidated on every keystroke. It now projects it to an equatable `CompilationInfo` holding only the three things the generation actually reads, so the pipeline is only invalidated when one of them changes. Note that a compilation end action is skipped once the compilation already has errors, so RESWP0009 does not appear on the build that a resource conflict makes fail. Reporting it from the generator instead would have made it visible there, but at a cost paid by every keystroke of every consumer, which is not a good trade. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 2 +- .../Analysis/ReswResourceAnalyzer.cs | 285 +++--------------- .../Analysis/ReswResourceRules.cs | 281 +++++++++++++++++ .../AnalyzerReleases.Shipped.md | 2 +- .../CompilationInfo.cs | 34 +++ src/ReswPlus.SourceGenerator/Diagnostics.cs | 6 +- src/ReswPlus.SourceGenerator/ReswGenerator.cs | 51 +--- .../ReswPlusUnitTests/ResourceDiagnostics.cs | 17 ++ tests/ReswPlusUnitTests/ReswTestHelpers.cs | 60 +++- 9 files changed, 436 insertions(+), 302 deletions(-) create mode 100644 src/ReswPlus.SourceGenerator/Analysis/ReswResourceRules.cs create mode 100644 src/ReswPlus.SourceGenerator/CompilationInfo.cs diff --git a/README.md b/README.md index dd5d9da..29a385f 100644 --- a/README.md +++ b/README.md @@ -74,7 +74,7 @@ ReswPlus checks the content of your `.resw` files while it generates the code, a | `RESWP0006` | A translated value doesn't use the same placeholders as the default language, which throws a `FormatException` at runtime. | | `RESWP0007` | A value uses a placeholder that has no matching parameter in its `#Format` tag. | | `RESWP0008` | A pluralized resource is missing the plural forms its language requires, which silently produces grammatically wrong text. | -| `RESWP0009` | Two resources of the same file are generated as the same member. | +| `RESWP0009` | Two resources of the same file conflict with each other, because their names only differ by case or because a plain resource collides with a pluralized one. | | `RESWP0010` | A value that is used as a composite format string is malformed. | These are reported as **warnings**, so that updating the package never breaks a build that already has an inconsistency. Escalate the ones you want to be fatal from your `.editorconfig`: diff --git a/src/ReswPlus.SourceGenerator/Analysis/ReswResourceAnalyzer.cs b/src/ReswPlus.SourceGenerator/Analysis/ReswResourceAnalyzer.cs index 89b0c96..3c043bb 100644 --- a/src/ReswPlus.SourceGenerator/Analysis/ReswResourceAnalyzer.cs +++ b/src/ReswPlus.SourceGenerator/Analysis/ReswResourceAnalyzer.cs @@ -1,11 +1,10 @@ using System; using System.Collections.Generic; -using System.Globalization; -using System.Linq; -using System.Threading; +using System.Collections.Immutable; +using System.IO; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Text; -using ReswPlus.SourceGenerator.ClassGenerators; namespace ReswPlus.SourceGenerator.Analysis; @@ -14,269 +13,63 @@ namespace ReswPlus.SourceGenerator.Analysis; /// a language the team may not read. /// /// -/// Every rule is reported as a warning rather than an error: raising the severity would break the build of every -/// project that already has an inconsistency the moment it updates the package. Projects that want a rule to be -/// fatal can escalate it through .editorconfig. -/// -/// The rules err on the side of not firing. A noisy analyzer gets disabled wholesale, taking the valuable rules -/// with it, so a rule that cannot decide stays silent. -/// +/// This is deliberately an analyzer and not part of the source generator. A generator runs on every keystroke in +/// the IDE, and inspecting every .resw file of every language of a project is far too expensive to do +/// there. Analyzers are scheduled independently of the generator pipeline, run out of process, and can be turned +/// off per rule by the consumer, so the cost is both smaller and opt out. /// -internal static class ReswResourceAnalyzer +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class ReswResourceAnalyzer : DiagnosticAnalyzer { - /// - /// The suffixes that identify a plural form of a resource, including the ReswPlus specific empty state. - /// - private static readonly string[] PluralSuffixes = ["Zero", "One", "Two", "Few", "Many", "Other", "None"]; - - /// - /// Analyzes the .resw files of a project. - /// - /// The .resw files of the project, with their content. - /// The default language of the project, if it declares one. - /// The callback invoked for every problem found. - /// The token used to cancel the operation. - public static void Analyze( - IReadOnlyList<(string Path, SourceText Text)> reswFiles, - string? defaultLanguage, - Action reportDiagnostic, - CancellationToken cancellationToken) + /// + public override ImmutableArray SupportedDiagnostics { get; } = + [ + Diagnostics.PlaceholderMismatch, + Diagnostics.UndeclaredFormatParameter, + Diagnostics.MissingPluralForms, + Diagnostics.DuplicateResource, + Diagnostics.InvalidFormatString + ]; + + /// + public override void Initialize(AnalysisContext context) { - var textsByPath = new Dictionary(); - - foreach (var (path, text) in reswFiles) - { - textsByPath[path] = text; - } - - foreach (var group in ReswFileGrouping.GroupByResource(textsByPath.Keys)) - { - cancellationToken.ThrowIfCancellationRequested(); - - var defaultPath = ReswFileGrouping.RetrieveDefaultResourceFile(group, defaultLanguage); - - if (defaultPath is null || ReswDocument.Parse(defaultPath, textsByPath[defaultPath], cancellationToken) is not { } defaultDocument) - { - continue; - } - - var defaultModel = ReswResourceModel.Create(defaultDocument); - - AnalyzeDocument(defaultModel, defaultModel, reportDiagnostic); - - foreach (var path in group) - { - cancellationToken.ThrowIfCancellationRequested(); + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); - if (path == defaultPath || ReswDocument.Parse(path, textsByPath[path], cancellationToken) is not { } document) - { - continue; - } - - AnalyzeDocument(ReswResourceModel.Create(document), defaultModel, reportDiagnostic); - } - } + // The rules compare the resources of a language against the resources of the default language, so they + // need the whole set of files at once and are registered as a single action per compilation. + context.RegisterCompilationAction(AnalyzeCompilation); } - private static void AnalyzeDocument(ReswResourceModel model, ReswResourceModel defaultModel, Action reportDiagnostic) + private static void AnalyzeCompilation(CompilationAnalysisContext context) { - ReportDuplicateMembers(model, reportDiagnostic); - ReportMissingPluralForms(model, reportDiagnostic); - ReportFormattingProblems(model, defaultModel, reportDiagnostic); - } + var reswFiles = new List<(string Path, SourceText Text)>(); - /// - /// RESWP0009: reports the resources that are generated as a member another resource already generates. - /// - /// - /// The comparison is case insensitive because resource lookup is: two resources whose names only differ by - /// case resolve to the same string at runtime, even though the members generated for them do not collide. - /// - private static void ReportDuplicateMembers(ReswResourceModel model, Action reportDiagnostic) - { - var membersByName = new Dictionary(StringComparer.OrdinalIgnoreCase); - - foreach (var member in model.Members) + foreach (var additionalFile in context.Options.AdditionalFiles) { - if (membersByName.TryGetValue(member.Name, out var existing)) - { - reportDiagnostic(Diagnostic.Create( - Diagnostics.DuplicateResource, - member.Entries[0].Location, - member.Entries[0].Key, - member.Name, - existing.Entries[0].Key)); - } - else - { - membersByName.Add(member.Name, member); - } - } - } + context.CancellationToken.ThrowIfCancellationRequested(); - /// - /// RESWP0008: reports the pluralized resources that don't define every plural form their language requires. - /// - /// - /// A missing form is not a build failure: the lookup simply returns an empty string at runtime, which makes - /// this the kind of problem that ships unnoticed. - /// - private static void ReportMissingPluralForms(ReswResourceModel model, Action reportDiagnostic) - { - if (model.Document.Language is not { Length: > 0 } language || - PluralFormsRetriever.RetrievePluralCategoriesForLanguage(language) is not { } requiredCategories) - { - return; - } - - foreach (var member in model.Members) - { - if (!member.IsPlural) + if (!Path.GetExtension(additionalFile.Path).Equals(".resw", StringComparison.OrdinalIgnoreCase)) { continue; } - // A pluralized resource that also has variants is declined once per variant, and every variant needs - // the full set of plural forms of the language. - foreach (var declension in GetDeclensions(member)) + if (additionalFile.GetText(context.CancellationToken) is { } text) { - var missingCategories = requiredCategories - .Where(category => !model.TryGetEntry($"{declension.Prefix}_{category}", out _)) - .ToArray(); - - if (missingCategories.Length == 0) - { - continue; - } - - reportDiagnostic(Diagnostic.Create( - Diagnostics.MissingPluralForms, - declension.Location, - declension.Prefix, - string.Join(", ", missingCategories.Select(category => $"'_{category}'")), - language)); + reswFiles.Add((additionalFile.Path, text)); } } - } - - /// - /// RESWP0006, RESWP0007 and RESWP0010: reports the values that are not usable as the composite format string - /// the generated code passes them to. - /// - /// The file being analyzed. - /// - /// The file of the default language, which is the only one carrying the #Format tags and therefore the - /// only one that determines whether, and with how many arguments, a resource is formatted. - /// - /// The callback invoked for every problem found. - private static void ReportFormattingProblems(ReswResourceModel model, ReswResourceModel defaultModel, Action reportDiagnostic) - { - var isDefaultLanguage = ReferenceEquals(model, defaultModel); - foreach (var member in model.Members) + if (reswFiles.Count == 0) { - // Values of resources that are never formatted are returned verbatim, so braces in them are literal. - if (!defaultModel.TryGetMember(member.Name, out var defaultMember) || !defaultMember.IsFormatted) - { - continue; - } - - foreach (var entry in member.Entries) - { - if (!CompositeFormatString.TryGetArgumentIndexes(entry.Value, out var indexes)) - { - reportDiagnostic(Diagnostic.Create(Diagnostics.InvalidFormatString, entry.Location, entry.Key)); - - continue; - } - - if (TryGetUndeclaredIndex(indexes, defaultMember.FormatParameterCount, out var undeclaredIndex)) - { - reportDiagnostic(Diagnostic.Create( - Diagnostics.UndeclaredFormatParameter, - entry.Location, - entry.Key, - undeclaredIndex, - defaultMember.FormatParameterCount)); - - continue; - } - - // Comparing a translation against itself would always match, and a resource that only exists in - // the default language has nothing to be compared with. - if (isDefaultLanguage || - !defaultModel.TryGetEntry(entry.Key, out var defaultEntry) || - !CompositeFormatString.TryGetArgumentIndexes(defaultEntry.Value, out var defaultIndexes) || - indexes.SetEquals(defaultIndexes)) - { - continue; - } - - reportDiagnostic(Diagnostic.Create( - Diagnostics.PlaceholderMismatch, - entry.Location, - entry.Key, - DescribePlaceholders(indexes), - DescribePlaceholders(defaultIndexes))); - } - } - } - - /// - /// Looks for a placeholder that has no matching argument in the generated call to string.Format. - /// - /// The argument indexes referenced by the value, in ascending order. - /// The number of arguments the generated code passes. - /// The first index that has no matching argument. - /// Whether the value would throw a at runtime. - private static bool TryGetUndeclaredIndex(IEnumerable indexes, int parameterCount, out int undeclaredIndex) - { - foreach (var index in indexes) - { - if (index >= parameterCount) - { - undeclaredIndex = index; - - return true; - } - } - - undeclaredIndex = 0; - - return false; - } - - /// - /// Returns the resource name prefixes a pluralized resource is declined from, one per variant. - /// - private static IEnumerable<(string Prefix, Location Location)> GetDeclensions(ReswMember member) - { - var locationsByPrefix = new Dictionary(StringComparer.Ordinal); - - foreach (var entry in member.Entries) - { - var separator = entry.Key.LastIndexOf('_'); - - if (separator <= 0 || !PluralSuffixes.Contains(entry.Key.Substring(separator + 1))) - { - continue; - } - - var prefix = entry.Key.Substring(0, separator); - - if (!locationsByPrefix.ContainsKey(prefix)) - { - locationsByPrefix.Add(prefix, entry.Location); - } + return; } - return locationsByPrefix.Select(pair => (pair.Key, pair.Value)); - } - - private static string DescribePlaceholders(IEnumerable indexes) - { - var placeholders = indexes.Select(index => $"{{{index.ToString(CultureInfo.InvariantCulture)}}}").ToArray(); + var defaultLanguage = context.Options.AnalyzerConfigOptionsProvider.GlobalOptions.TryGetValue("build_property.DefaultLanguage", out var value) + ? value + : null; - return placeholders.Length == 0 ? "no placeholder" : string.Join(", ", placeholders); + ReswResourceRules.Analyze(reswFiles, defaultLanguage, context.ReportDiagnostic, context.CancellationToken); } } diff --git a/src/ReswPlus.SourceGenerator/Analysis/ReswResourceRules.cs b/src/ReswPlus.SourceGenerator/Analysis/ReswResourceRules.cs new file mode 100644 index 0000000..edcb2d7 --- /dev/null +++ b/src/ReswPlus.SourceGenerator/Analysis/ReswResourceRules.cs @@ -0,0 +1,281 @@ +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Linq; +using System.Threading; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Text; +using ReswPlus.SourceGenerator.ClassGenerators; + +namespace ReswPlus.SourceGenerator.Analysis; + +/// +/// Implements the rules reported on the content of the .resw files of a project. +/// +/// +/// Every rule is reported as a warning rather than an error: raising the severity would break the build of every +/// project that already has an inconsistency the moment it updates the package. Projects that want a rule to be +/// fatal can escalate it through .editorconfig. +/// +/// The rules err on the side of not firing. A noisy analyzer gets disabled wholesale, taking the valuable rules +/// with it, so a rule that cannot decide stays silent. +/// +/// +internal static class ReswResourceRules +{ + /// + /// The suffixes that identify a plural form of a resource, including the ReswPlus specific empty state. + /// + private static readonly string[] PluralSuffixes = ["Zero", "One", "Two", "Few", "Many", "Other", "None"]; + + /// + /// Analyzes the .resw files of a project. + /// + /// The .resw files of the project, with their content. + /// The default language of the project, if it declares one. + /// The callback invoked for every problem found. + /// The token used to cancel the operation. + public static void Analyze( + IReadOnlyList<(string Path, SourceText Text)> reswFiles, + string? defaultLanguage, + Action reportDiagnostic, + CancellationToken cancellationToken) + { + var textsByPath = new Dictionary(); + + foreach (var (path, text) in reswFiles) + { + textsByPath[path] = text; + } + + foreach (var group in ReswFileGrouping.GroupByResource(textsByPath.Keys)) + { + cancellationToken.ThrowIfCancellationRequested(); + + var defaultPath = ReswFileGrouping.RetrieveDefaultResourceFile(group, defaultLanguage); + + if (defaultPath is null || ReswDocument.Parse(defaultPath, textsByPath[defaultPath], cancellationToken) is not { } defaultDocument) + { + continue; + } + + var defaultModel = ReswResourceModel.Create(defaultDocument); + + AnalyzeDocument(defaultModel, defaultModel, reportDiagnostic); + + foreach (var path in group) + { + cancellationToken.ThrowIfCancellationRequested(); + + if (path == defaultPath || ReswDocument.Parse(path, textsByPath[path], cancellationToken) is not { } document) + { + continue; + } + + AnalyzeDocument(ReswResourceModel.Create(document), defaultModel, reportDiagnostic); + } + } + } + + private static void AnalyzeDocument(ReswResourceModel model, ReswResourceModel defaultModel, Action reportDiagnostic) + { + ReportDuplicateMembers(model, reportDiagnostic); + ReportMissingPluralForms(model, reportDiagnostic); + ReportFormattingProblems(model, defaultModel, reportDiagnostic); + } + + /// + /// RESWP0009: reports the resources that conflict with a resource declared earlier in the same file. + /// + /// + /// The comparison is case insensitive because resource lookup is: two resources whose names only differ by + /// case resolve to the same string at runtime. A plain resource can also conflict with a pluralized or + /// varianted one, in which case the generated members collide and the project no longer compiles. + /// + private static void ReportDuplicateMembers(ReswResourceModel model, Action reportDiagnostic) + { + var membersByName = new Dictionary(StringComparer.OrdinalIgnoreCase); + + foreach (var member in model.Members) + { + if (membersByName.TryGetValue(member.Name, out var existing)) + { + reportDiagnostic(Diagnostic.Create( + Diagnostics.DuplicateResource, + member.Entries[0].Location, + member.Entries[0].Key, + existing.Entries[0].Key)); + } + else + { + membersByName.Add(member.Name, member); + } + } + } + + /// + /// RESWP0008: reports the pluralized resources that don't define every plural form their language requires. + /// + /// + /// A missing form is not a build failure: the lookup simply returns an empty string at runtime, which makes + /// this the kind of problem that ships unnoticed. + /// + private static void ReportMissingPluralForms(ReswResourceModel model, Action reportDiagnostic) + { + if (model.Document.Language is not { Length: > 0 } language || + PluralFormsRetriever.RetrievePluralCategoriesForLanguage(language) is not { } requiredCategories) + { + return; + } + + foreach (var member in model.Members) + { + if (!member.IsPlural) + { + continue; + } + + // A pluralized resource that also has variants is declined once per variant, and every variant needs + // the full set of plural forms of the language. + foreach (var declension in GetDeclensions(member)) + { + var missingCategories = requiredCategories + .Where(category => !model.TryGetEntry($"{declension.Prefix}_{category}", out _)) + .ToArray(); + + if (missingCategories.Length == 0) + { + continue; + } + + reportDiagnostic(Diagnostic.Create( + Diagnostics.MissingPluralForms, + declension.Location, + declension.Prefix, + string.Join(", ", missingCategories.Select(category => $"'_{category}'")), + language)); + } + } + } + + /// + /// RESWP0006, RESWP0007 and RESWP0010: reports the values that are not usable as the composite format string + /// the generated code passes them to. + /// + /// The file being analyzed. + /// + /// The file of the default language, which is the only one carrying the #Format tags and therefore the + /// only one that determines whether, and with how many arguments, a resource is formatted. + /// + /// The callback invoked for every problem found. + private static void ReportFormattingProblems(ReswResourceModel model, ReswResourceModel defaultModel, Action reportDiagnostic) + { + var isDefaultLanguage = ReferenceEquals(model, defaultModel); + + foreach (var member in model.Members) + { + // Values of resources that are never formatted are returned verbatim, so braces in them are literal. + if (!defaultModel.TryGetMember(member.Name, out var defaultMember) || !defaultMember.IsFormatted) + { + continue; + } + + foreach (var entry in member.Entries) + { + if (!CompositeFormatString.TryGetArgumentIndexes(entry.Value, out var indexes)) + { + reportDiagnostic(Diagnostic.Create(Diagnostics.InvalidFormatString, entry.Location, entry.Key)); + + continue; + } + + if (TryGetUndeclaredIndex(indexes, defaultMember.FormatParameterCount, out var undeclaredIndex)) + { + reportDiagnostic(Diagnostic.Create( + Diagnostics.UndeclaredFormatParameter, + entry.Location, + entry.Key, + undeclaredIndex, + defaultMember.FormatParameterCount)); + + continue; + } + + // Comparing a translation against itself would always match, and a resource that only exists in + // the default language has nothing to be compared with. + if (isDefaultLanguage || + !defaultModel.TryGetEntry(entry.Key, out var defaultEntry) || + !CompositeFormatString.TryGetArgumentIndexes(defaultEntry.Value, out var defaultIndexes) || + indexes.SetEquals(defaultIndexes)) + { + continue; + } + + reportDiagnostic(Diagnostic.Create( + Diagnostics.PlaceholderMismatch, + entry.Location, + entry.Key, + DescribePlaceholders(indexes), + DescribePlaceholders(defaultIndexes))); + } + } + } + + /// + /// Looks for a placeholder that has no matching argument in the generated call to string.Format. + /// + /// The argument indexes referenced by the value, in ascending order. + /// The number of arguments the generated code passes. + /// The first index that has no matching argument. + /// Whether the value would throw a at runtime. + private static bool TryGetUndeclaredIndex(IEnumerable indexes, int parameterCount, out int undeclaredIndex) + { + foreach (var index in indexes) + { + if (index >= parameterCount) + { + undeclaredIndex = index; + + return true; + } + } + + undeclaredIndex = 0; + + return false; + } + + /// + /// Returns the resource name prefixes a pluralized resource is declined from, one per variant. + /// + private static IEnumerable<(string Prefix, Location Location)> GetDeclensions(ReswMember member) + { + var locationsByPrefix = new Dictionary(StringComparer.Ordinal); + + foreach (var entry in member.Entries) + { + var separator = entry.Key.LastIndexOf('_'); + + if (separator <= 0 || !PluralSuffixes.Contains(entry.Key.Substring(separator + 1))) + { + continue; + } + + var prefix = entry.Key.Substring(0, separator); + + if (!locationsByPrefix.ContainsKey(prefix)) + { + locationsByPrefix.Add(prefix, entry.Location); + } + } + + return locationsByPrefix.Select(pair => (pair.Key, pair.Value)); + } + + private static string DescribePlaceholders(IEnumerable indexes) + { + var placeholders = indexes.Select(index => $"{{{index.ToString(CultureInfo.InvariantCulture)}}}").ToArray(); + + return placeholders.Length == 0 ? "no placeholder" : string.Join(", ", placeholders); + } +} diff --git a/src/ReswPlus.SourceGenerator/AnalyzerReleases.Shipped.md b/src/ReswPlus.SourceGenerator/AnalyzerReleases.Shipped.md index bd05fc8..f630a4d 100644 --- a/src/ReswPlus.SourceGenerator/AnalyzerReleases.Shipped.md +++ b/src/ReswPlus.SourceGenerator/AnalyzerReleases.Shipped.md @@ -12,5 +12,5 @@ RESWP0005 | Compatibility | Error | ReswPlus only supports UWP and WinAppS RESWP0006 | Resources | Warning | A translated value doesn't use the same placeholders as the default language. RESWP0007 | Resources | Warning | A value uses a placeholder that has no matching parameter in its #Format tag. RESWP0008 | Resources | Warning | A pluralized resource is missing plural forms its language requires. -RESWP0009 | Resources | Warning | Two resources of the same file are generated as the same member. +RESWP0009 | Resources | Warning | Two resources of the same file conflict with each other. RESWP0010 | Resources | Warning | A value that is used as a composite format string is malformed. \ No newline at end of file diff --git a/src/ReswPlus.SourceGenerator/CompilationInfo.cs b/src/ReswPlus.SourceGenerator/CompilationInfo.cs new file mode 100644 index 0000000..ca18dca --- /dev/null +++ b/src/ReswPlus.SourceGenerator/CompilationInfo.cs @@ -0,0 +1,34 @@ +namespace ReswPlus.SourceGenerator; + +/// +/// The parts of a compilation the generation depends on. +/// +/// +/// A Compilation is a different object after every edit, so keeping one in the incremental pipeline would +/// invalidate every downstream step on every keystroke. Projecting it to this equatable model instead means the +/// pipeline is only invalidated when something the generation actually reads has changed. +/// +internal sealed record CompilationInfo +{ + public CompilationInfo(bool isCSharp, AppType appType, string? assemblyName) + { + IsCSharp = isCSharp; + AppType = appType; + AssemblyName = assemblyName; + } + + /// + /// Gets whether the compilation is a C# compilation, the only language the generator supports. + /// + public bool IsCSharp { get; } + + /// + /// Gets the type of application being built, determined from the references of the compilation. + /// + public AppType AppType { get; } + + /// + /// Gets the name of the assembly being built, used to build the resource identifier of a library. + /// + public string? AssemblyName { get; } +} diff --git a/src/ReswPlus.SourceGenerator/Diagnostics.cs b/src/ReswPlus.SourceGenerator/Diagnostics.cs index f1db9b1..02600e3 100644 --- a/src/ReswPlus.SourceGenerator/Diagnostics.cs +++ b/src/ReswPlus.SourceGenerator/Diagnostics.cs @@ -110,12 +110,12 @@ internal static class Diagnostics isEnabledByDefault: true); /// - /// RESWP0009: two resources of the same file are generated as the same member. + /// RESWP0009: two resources of the same file conflict with each other. /// public static readonly DiagnosticDescriptor DuplicateResource = new( "RESWP0009", - "Duplicate resource", - "The resource '{0}' generates the member '{1}', which is already generated by the resource '{2}'", + "Conflicting resources", + "The resource '{0}' conflicts with the resource '{1}' declared earlier in the same file", ResourcesCategory, DiagnosticSeverity.Warning, isEnabledByDefault: true); diff --git a/src/ReswPlus.SourceGenerator/ReswGenerator.cs b/src/ReswPlus.SourceGenerator/ReswGenerator.cs index 64f1a5a..e83b0d8 100644 --- a/src/ReswPlus.SourceGenerator/ReswGenerator.cs +++ b/src/ReswPlus.SourceGenerator/ReswGenerator.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Collections.Immutable; using System.IO; using System.Linq; using System.Reflection; @@ -56,33 +55,28 @@ public void Initialize(IncrementalGeneratorInitializationContext context) .Where(file => Path.GetExtension(file.Path).Equals(".resw", StringComparison.OrdinalIgnoreCase)) .Collect(); - // The resource diagnostics only depend on the .resw files and on the default language of the project, so - // they are registered separately: combining them with the compilation would re-run the whole analysis on - // every keystroke, for every language of the project. - var defaultLanguageProvider = context.AnalyzerConfigOptionsProvider - .Select((options, cancellationToken) => GetOption(options.GlobalOptions, "build_property.DefaultLanguage")); + // Only the parts of the compilation the generation actually depends on are captured, so that an unrelated + // edit in the project doesn't invalidate the whole pipeline: a Compilation is a new object every time. + var compilationInfoProvider = context.CompilationProvider.Select(static (compilation, cancellationToken) => + new CompilationInfo(compilation is CSharpCompilation, RetrieveAppType(compilation), compilation.AssemblyName)); - context.RegisterSourceOutput( - reswFilesProvider.Combine(defaultLanguageProvider), - static (spc, source) => ReportResourceDiagnostics(spc, source.Left, source.Right)); - - // Combine the Compilation, the global options, and the additional files. - var combinedProvider = context.CompilationProvider + // Combine the compilation information, the global options, and the additional files. + var combinedProvider = compilationInfoProvider .Combine(globalOptionsProvider) .Combine(reswFilesProvider); - context.RegisterSourceOutput(combinedProvider, (spc, source) => + context.RegisterSourceOutput(combinedProvider, static (spc, source) => { // Unpack the combined tuple. - var ((compilation, options), additionalFiles) = source; + var ((compilationInfo, options), additionalFiles) = source; - if (compilation is null || options is null) + if (options is null) { return; } // Only support C# - if (compilation is not CSharpCompilation) + if (!compilationInfo.IsCSharp) { spc.ReportDiagnostic(Diagnostic.Create(Diagnostics.UnsupportedLanguage, Location.None)); return; @@ -119,7 +113,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) } // Determine AppType based on referenced assemblies. - var appType = RetrieveAppType(compilation); + var appType = compilationInfo.AppType; var assemblyName = Assembly.GetExecutingAssembly().GetName().Name; switch (appType) @@ -188,7 +182,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context) } // Generate code for the resource file. - var resourceFileInfo = new ResourceFileInfo(filePath, new Project(compilation.AssemblyName!, isLibrary)); + var resourceFileInfo = new ResourceFileInfo(filePath, new Project(compilationInfo.AssemblyName!, isLibrary)); var codeGenerator = ReswClassGenerator.CreateGenerator(resourceFileInfo, null); if (codeGenerator is null) { @@ -234,27 +228,6 @@ public void Initialize(IncrementalGeneratorInitializationContext context) }); } - /// - /// Reports the diagnostics about the content of the .resw files of the project. - /// - /// The context to report the diagnostics to. - /// The .resw files of the project. - /// The default language of the project, if it declares one. - private static void ReportResourceDiagnostics(SourceProductionContext spc, ImmutableArray reswFiles, string? defaultLanguage) - { - var documents = new List<(string Path, SourceText Text)>(); - - foreach (var file in reswFiles.Distinct()) - { - if (file.GetText(spc.CancellationToken) is { } text) - { - documents.Add((file.Path, text)); - } - } - - ReswResourceAnalyzer.Analyze(documents, defaultLanguage, spc.ReportDiagnostic, spc.CancellationToken); - } - /// /// Helper method to retrieve an option value. /// diff --git a/tests/ReswPlusUnitTests/ResourceDiagnostics.cs b/tests/ReswPlusUnitTests/ResourceDiagnostics.cs index dc6097c..817ac08 100644 --- a/tests/ReswPlusUnitTests/ResourceDiagnostics.cs +++ b/tests/ReswPlusUnitTests/ResourceDiagnostics.cs @@ -1,4 +1,6 @@ using System.Linq; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; using Xunit; namespace ReswPlusUnitTests; @@ -295,6 +297,21 @@ public void IgnoredResourcesAreNotAnalyzed() Assert.Empty(diagnostics); } + [Fact] + public async Task TheAnalyzerIsRegisteredAndReportsThroughTheCompiler() + { + // The rules are exercised directly by the other tests, this one covers the wiring: without the right + // registration the analyzer would silently report nothing when the compiler runs it. + var diagnostics = await ReswTestHelpers.RunAnalyzerAsync( + ("en-US", ReswTestHelpers.CreateResw(("Greeting", "Hello {0}, you are {1}", "#Format[String name, Int age]"))), + ("fr", ReswTestHelpers.CreateResw(("Greeting", "Bonjour {0}", null)))); + + var diagnostic = Assert.Single(diagnostics); + + Assert.Equal("RESWP0006", diagnostic.Id); + Assert.Equal(DiagnosticSeverity.Warning, diagnostic.Severity); + } + [Fact] public void DiagnosticsPointAtTheLineOfTheOffendingResource() { diff --git a/tests/ReswPlusUnitTests/ReswTestHelpers.cs b/tests/ReswPlusUnitTests/ReswTestHelpers.cs index ae283a2..9c6221d 100644 --- a/tests/ReswPlusUnitTests/ReswTestHelpers.cs +++ b/tests/ReswPlusUnitTests/ReswTestHelpers.cs @@ -1,7 +1,11 @@ using System.Collections.Generic; +using System.Collections.Immutable; using System.Linq; using System.Threading; +using System.Threading.Tasks; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Text; using ReswPlus.SourceGenerator; using ReswPlus.SourceGenerator.Analysis; @@ -78,11 +82,6 @@ public static GeneratedFile GenerateFile(string reswContent, AppType appType = A return result!.Files.Single(); } - private static string Escape(string text) - { - return text.Replace("&", "&").Replace("<", "<").Replace(">", ">"); - } - /// /// Runs the resource analysis over a set of in-memory .resw files. /// @@ -92,23 +91,60 @@ private static string Escape(string text) public static IReadOnlyList Analyze(string? defaultLanguage, params (string Language, string Content)[] files) { var documents = files - .Select(file => ($@"C:\Project\Strings\{file.Language}\Resources.resw", SourceText.From(file.Content))) + .Select(file => (GetPath(file.Language), SourceText.From(file.Content))) .ToArray(); var diagnostics = new List(); - ReswResourceAnalyzer.Analyze(documents, defaultLanguage, diagnostics.Add, CancellationToken.None); + ReswResourceRules.Analyze(documents, defaultLanguage, diagnostics.Add, CancellationToken.None); return diagnostics; } /// - /// Returns the identifiers of the given diagnostics, in the order they were reported. + /// Runs the over a set of in-memory .resw files, exercising the + /// same path the compiler takes. /// - /// The diagnostics to describe. - /// The identifiers of the diagnostics. - public static string[] GetIds(this IEnumerable diagnostics) + /// The files of the project, as language folder name and content pairs. + /// The diagnostics reported for those files. + public static async Task> RunAnalyzerAsync(params (string Language, string Content)[] files) + { + var additionalFiles = files + .Select(file => (AdditionalText)new InMemoryAdditionalText(GetPath(file.Language), file.Content)) + .ToImmutableArray(); + + var compilation = CSharpCompilation.Create("TestProject"); + + return await compilation + .WithAnalyzers([new ReswResourceAnalyzer()], new AnalyzerOptions(additionalFiles)) + .GetAnalyzerDiagnosticsAsync(CancellationToken.None); + } + + private static string GetPath(string language) { - return diagnostics.Select(diagnostic => diagnostic.Id).ToArray(); + return $@"C:\Project\Strings\{language}\Resources.resw"; + } + + private static string Escape(string text) + { + return text.Replace("&", "&").Replace("<", "<").Replace(">", ">"); + } + + private sealed class InMemoryAdditionalText : AdditionalText + { + private readonly SourceText _text; + + public InMemoryAdditionalText(string path, string content) + { + Path = path; + _text = SourceText.From(content); + } + + public override string Path { get; } + + public override SourceText GetText(CancellationToken cancellationToken = default) + { + return _text; + } } } From b75e34349a5c5abdff9c6c27b9d045fae21c1f0b Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Mon, 27 Jul 2026 23:59:12 -0700 Subject: [PATCH 5/7] Address review feedback on the diagnostics Fixes found by two review passes over the branch: - RESWP0006 no longer fires when a translation uses *more* placeholders than the default language. That direction is valid and common: English spells the singular out as `One file` while French still needs `{0} fichier`, and `string.Format` ignores the arguments a value doesn't reference. The rule now reports exactly the placeholders a translation drops, which is what actually loses information, and its message says so. - RESWP0008 no longer fires for a pluralized resource that only exists in a translation. The default language is what drives generation, so a resource left behind after it was dropped there generates nothing and its plural forms are never looked up. - RESWP0008 no longer requires `_Zero` for a resource that declares `_None`, unless the language can return the zero category for a non-zero quantity, which of all the providers only Latvian does. `GetPlural` short circuits a zero quantity to `_None`, so for every other language the `_Zero` resource is unreachable. - The analysis now identifies a resource case insensitively, the way resource lookup does, when it matches a plural form or a translation back to the resource a generated member reads. - The tag of a plain resource is parsed against the same set of resources the generator uses, so a `Reference()` that the generator cannot resolve, and which therefore makes it emit the value unformatted, no longer makes the analysis treat that value as a format string. The composite format parser is also now checked against `string.Format` itself, over every string of length four or less built from the alphabet that makes up a format item. Accepting a value the runtime rejects means missing a guaranteed crash, and rejecting one it accepts means reporting a perfectly good resource. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 2 +- .../Analysis/ReswResourceModel.cs | 11 +- .../Analysis/ReswResourceRules.cs | 45 ++++++-- .../AnalyzerReleases.Shipped.md | 2 +- .../ClassGenerators/PluralFormsRetriever.cs | 26 +++-- src/ReswPlus.SourceGenerator/Diagnostics.cs | 6 +- .../CompositeFormatStringParsing.cs | 108 ++++++++++++++++++ .../ReswPlusUnitTests/ResourceDiagnostics.cs | 95 +++++++++++++++ 8 files changed, 268 insertions(+), 27 deletions(-) create mode 100644 tests/ReswPlusUnitTests/CompositeFormatStringParsing.cs diff --git a/README.md b/README.md index 29a385f..a6568c7 100644 --- a/README.md +++ b/README.md @@ -71,7 +71,7 @@ ReswPlus checks the content of your `.resw` files while it generates the code, a | Rule | Description | | --- | --- | -| `RESWP0006` | A translated value doesn't use the same placeholders as the default language, which throws a `FormatException` at runtime. | +| `RESWP0006` | A translated value drops placeholders its value in the default language uses, silently losing information. | | `RESWP0007` | A value uses a placeholder that has no matching parameter in its `#Format` tag. | | `RESWP0008` | A pluralized resource is missing the plural forms its language requires, which silently produces grammatically wrong text. | | `RESWP0009` | Two resources of the same file conflict with each other, because their names only differ by case or because a plain resource collides with a pluralized one. | diff --git a/src/ReswPlus.SourceGenerator/Analysis/ReswResourceModel.cs b/src/ReswPlus.SourceGenerator/Analysis/ReswResourceModel.cs index 86166d8..65bd27f 100644 --- a/src/ReswPlus.SourceGenerator/Analysis/ReswResourceModel.cs +++ b/src/ReswPlus.SourceGenerator/Analysis/ReswResourceModel.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.IO; using System.Linq; @@ -65,8 +66,10 @@ private ReswResourceModel(ReswDocument document, IReadOnlyList membe Document = document; Members = members; - _membersByName = new Dictionary(); - _entriesByKey = new Dictionary(); + // Resource lookup is case insensitive, so a resource is identified the same way here: it is how the + // runtime matches a plural form or a translation back to the resource the generated member reads. + _membersByName = new Dictionary(StringComparer.OrdinalIgnoreCase); + _entriesByKey = new Dictionary(StringComparer.OrdinalIgnoreCase); foreach (var member in members) { @@ -163,11 +166,13 @@ public static ReswResourceModel Create(ReswDocument document) foreach (var item in basicItems) { + // The generator resolves the references of a plain resource against the same set: ReswClassGenerator + // narrows its own list of items down to the ones left after grouping before it parses their tags. members.Add(new ReswMember( item.Key, isPlural: false, [entriesByItem[item]], - CountFormatParameters(item.Key, item.Comment, stringItems, resourceFileName))); + CountFormatParameters(item.Key, item.Comment, basicItems, resourceFileName))); } return new ReswResourceModel( diff --git a/src/ReswPlus.SourceGenerator/Analysis/ReswResourceRules.cs b/src/ReswPlus.SourceGenerator/Analysis/ReswResourceRules.cs index edcb2d7..e95f7da 100644 --- a/src/ReswPlus.SourceGenerator/Analysis/ReswResourceRules.cs +++ b/src/ReswPlus.SourceGenerator/Analysis/ReswResourceRules.cs @@ -80,7 +80,7 @@ public static void Analyze( private static void AnalyzeDocument(ReswResourceModel model, ReswResourceModel defaultModel, Action reportDiagnostic) { ReportDuplicateMembers(model, reportDiagnostic); - ReportMissingPluralForms(model, reportDiagnostic); + ReportMissingPluralForms(model, defaultModel, reportDiagnostic); ReportFormattingProblems(model, defaultModel, reportDiagnostic); } @@ -120,17 +120,19 @@ private static void ReportDuplicateMembers(ReswResourceModel model, Action - private static void ReportMissingPluralForms(ReswResourceModel model, Action reportDiagnostic) + private static void ReportMissingPluralForms(ReswResourceModel model, ReswResourceModel defaultModel, Action reportDiagnostic) { if (model.Document.Language is not { Length: > 0 } language || - PluralFormsRetriever.RetrievePluralCategoriesForLanguage(language) is not { } requiredCategories) + PluralFormsRetriever.RetrievePluralFormForLanguage(language) is not { } pluralForm) { return; } foreach (var member in model.Members) { - if (!member.IsPlural) + // A resource left behind in a translation after the default language dropped it generates nothing, + // so its plural forms are never looked up and its missing ones don't matter. + if (!member.IsPlural || !defaultModel.TryGetMember(member.Name, out var defaultMember) || !defaultMember.IsPlural) { continue; } @@ -139,7 +141,12 @@ private static void ReportMissingPluralForms(ReswResourceModel model, Action !(category == PluralCategory.Zero && hasNoneForm && pluralForm.ZeroIsOnlyForZeroQuantity)) .Where(category => !model.TryGetEntry($"{declension.Prefix}_{category}", out _)) .ToArray(); @@ -205,8 +212,18 @@ private static void ReportFormattingProblems(ReswResourceModel model, ReswResour // the default language has nothing to be compared with. if (isDefaultLanguage || !defaultModel.TryGetEntry(entry.Key, out var defaultEntry) || - !CompositeFormatString.TryGetArgumentIndexes(defaultEntry.Value, out var defaultIndexes) || - indexes.SetEquals(defaultIndexes)) + !CompositeFormatString.TryGetArgumentIndexes(defaultEntry.Value, out var defaultIndexes)) + { + continue; + } + + // A translation is free to use more placeholders than the default language: string.Format ignores + // the arguments a format string doesn't reference, and the indexes that have no matching argument + // at all are already covered above. Only the placeholders a translation drops are a problem, since + // they silently remove information from the localized string. + var missingPlaceholders = defaultIndexes.Where(index => !indexes.Contains(index)).ToArray(); + + if (missingPlaceholders.Length == 0) { continue; } @@ -215,8 +232,7 @@ private static void ReportFormattingProblems(ReswResourceModel model, ReswResour Diagnostics.PlaceholderMismatch, entry.Location, entry.Key, - DescribePlaceholders(indexes), - DescribePlaceholders(defaultIndexes))); + DescribePlaceholders(missingPlaceholders))); } } } @@ -272,10 +288,15 @@ private static bool TryGetUndeclaredIndex(IEnumerable indexes, int paramete return locationsByPrefix.Select(pair => (pair.Key, pair.Value)); } - private static string DescribePlaceholders(IEnumerable indexes) + /// + /// Describes a set of placeholders the way they appear in the value of a resource. + /// + /// The argument indexes to describe, which must not be empty. + /// The description of the placeholders, to embed in a diagnostic message. + private static string DescribePlaceholders(IReadOnlyList indexes) { - var placeholders = indexes.Select(index => $"{{{index.ToString(CultureInfo.InvariantCulture)}}}").ToArray(); + var placeholders = indexes.Select(index => $"{{{index.ToString(CultureInfo.InvariantCulture)}}}"); - return placeholders.Length == 0 ? "no placeholder" : string.Join(", ", placeholders); + return $"the placeholder{(indexes.Count == 1 ? "" : "s")} {string.Join(", ", placeholders)}"; } } diff --git a/src/ReswPlus.SourceGenerator/AnalyzerReleases.Shipped.md b/src/ReswPlus.SourceGenerator/AnalyzerReleases.Shipped.md index f630a4d..37b2f89 100644 --- a/src/ReswPlus.SourceGenerator/AnalyzerReleases.Shipped.md +++ b/src/ReswPlus.SourceGenerator/AnalyzerReleases.Shipped.md @@ -9,7 +9,7 @@ RESWP0002 | Compatibility | Error | ReswPlus cannot determine the namespac RESWP0003 | Compatibility | Error | Can't retrieve the root path of the project. RESWP0004 | Compatibility | Info | ReswPlus cannot determine the project type, defaulting to application. RESWP0005 | Compatibility | Error | ReswPlus only supports UWP and WinAppSDK applications/libraries. -RESWP0006 | Resources | Warning | A translated value doesn't use the same placeholders as the default language. +RESWP0006 | Resources | Warning | A translated value drops placeholders its value in the default language uses. RESWP0007 | Resources | Warning | A value uses a placeholder that has no matching parameter in its #Format tag. RESWP0008 | Resources | Warning | A pluralized resource is missing plural forms its language requires. RESWP0009 | Resources | Warning | Two resources of the same file conflict with each other. diff --git a/src/ReswPlus.SourceGenerator/ClassGenerators/PluralFormsRetriever.cs b/src/ReswPlus.SourceGenerator/ClassGenerators/PluralFormsRetriever.cs index 0af3e26..c87bfbb 100644 --- a/src/ReswPlus.SourceGenerator/ClassGenerators/PluralFormsRetriever.cs +++ b/src/ReswPlus.SourceGenerator/ClassGenerators/PluralFormsRetriever.cs @@ -30,6 +30,17 @@ public PluralForm(string id, PluralCategory[] categories, string[] languages) /// public PluralCategory[] Categories { get; set; } + /// + /// Gets whether the provider of this form only returns for a quantity + /// that is itself zero. + /// + /// + /// A resource that declares a _None form short circuits a zero quantity to it, so for such a form + /// the _Zero resource becomes unreachable and is not required. Latvian is the exception: its + /// provider also returns for quantities such as 11 or 20. + /// + public bool ZeroIsOnlyForZeroQuantity { get; set; } = true; + /// /// Gets the languages using this plural form. /// @@ -199,7 +210,8 @@ public PluralForm(string id, PluralCategory[] categories, string[] languages) "lv", // Latvian "prg" // Prussian ] - ), + ) + { ZeroIsOnlyForZeroQuantity = false }, new PluralForm( "Irish", [PluralCategory.One, PluralCategory.Two, PluralCategory.Few, PluralCategory.Many, PluralCategory.Other], @@ -427,15 +439,15 @@ public static IEnumerable RetrievePluralFormsForLanguages(IEnumerabl } /// - /// Retrieves the plural categories a resource has to define to be correctly declined in a language. + /// Retrieves the plural form of a language. /// - /// The primary language subtag to retrieve the categories for. + /// The primary language subtag to retrieve the plural form for. /// - /// The plural categories required by , or if the language - /// has no dedicated plural provider, in which case no plural form can be assumed to be required. + /// The plural form of , or if the language has no dedicated + /// plural provider, in which case no plural form can be assumed to be required. /// - public static PluralCategory[]? RetrievePluralCategoriesForLanguage(string language) + public static PluralForm? RetrievePluralFormForLanguage(string language) { - return LanguageToPluralForm.TryGetValue(language, out var pluralForm) ? pluralForm.Categories : null; + return LanguageToPluralForm.TryGetValue(language, out var pluralForm) ? pluralForm : null; } } diff --git a/src/ReswPlus.SourceGenerator/Diagnostics.cs b/src/ReswPlus.SourceGenerator/Diagnostics.cs index 02600e3..c4908b9 100644 --- a/src/ReswPlus.SourceGenerator/Diagnostics.cs +++ b/src/ReswPlus.SourceGenerator/Diagnostics.cs @@ -77,12 +77,12 @@ internal static class Diagnostics isEnabledByDefault: true); /// - /// RESWP0006: a translated value doesn't use the same placeholders as the default language. + /// RESWP0006: a translated value drops placeholders its value in the default language uses. /// public static readonly DiagnosticDescriptor PlaceholderMismatch = new( "RESWP0006", - "Placeholder mismatch between languages", - "The value of the resource '{0}' uses {1}, while its value in the default language uses {2}", + "Missing placeholder in a translation", + "The value of the resource '{0}' doesn't use {1} that its value in the default language uses", ResourcesCategory, DiagnosticSeverity.Warning, isEnabledByDefault: true); diff --git a/tests/ReswPlusUnitTests/CompositeFormatStringParsing.cs b/tests/ReswPlusUnitTests/CompositeFormatStringParsing.cs new file mode 100644 index 0000000..4efa6b2 --- /dev/null +++ b/tests/ReswPlusUnitTests/CompositeFormatStringParsing.cs @@ -0,0 +1,108 @@ +using System; +using System.Collections.Generic; +using ReswPlus.SourceGenerator.Analysis; +using Xunit; + +namespace ReswPlusUnitTests; + +/// +/// Tests for the parser used to decide whether the value of a formatted resource is a usable composite format +/// string, and which arguments it references. +/// +public class CompositeFormatStringParsing +{ + [Theory] + [InlineData("", new int[0])] + [InlineData("no placeholder at all", new int[0])] + [InlineData("{0}", new[] { 0 })] + [InlineData("{1} {0}", new[] { 0, 1 })] + [InlineData("{0} and {0} again", new[] { 0 })] + [InlineData("{0,10}", new[] { 0 })] + [InlineData("{0,-10}", new[] { 0 })] + [InlineData("{0:C2}", new[] { 0 })] + [InlineData("{2,-10:yyyy-MM-dd}", new[] { 2 })] + [InlineData("{{0}}", new int[0])] + [InlineData("{{{0}}}", new[] { 0 })] + [InlineData("100% of {0}", new[] { 0 })] + public void ValidFormatStringsAreParsed(string value, int[] expectedIndexes) + { + Assert.True(CompositeFormatString.TryGetArgumentIndexes(value, out var indexes)); + Assert.Equal(expectedIndexes, indexes); + } + + [Theory] + [InlineData("{")] + [InlineData("}")] + [InlineData("{0")] + [InlineData("0}")] + [InlineData("{}")] + [InlineData("{name}")] + [InlineData("{0,}")] + [InlineData("{0 0}")] + [InlineData("{{0}")] + [InlineData("{0:{}")] + public void MalformedFormatStringsAreRejected(string value) + { + Assert.False(CompositeFormatString.TryGetArgumentIndexes(value, out _)); + } + + [Fact] + public void TheParserAgreesWithStringFormat() + { + // Accepting a value the runtime rejects means missing a guaranteed crash, and rejecting a value the + // runtime accepts means reporting a perfectly good resource. Both are checked exhaustively over the + // alphabet that makes up a format item. + var alphabet = new[] { '{', '}', '0', '1', 'a', ',', ':', '-', ' ' }; + var values = new object[1000]; + var buffer = new char[4]; + var mismatches = new List(); + + for (var i = 0; i < values.Length; i++) + { + // A string argument ignores the format specifier, so only the shape of the value is under test. + values[i] = "x"; + } + + void Walk(int depth, int length) + { + if (depth == length) + { + var candidate = new string(buffer, 0, length); + + bool isAcceptedByTheRuntime; + + try + { + _ = string.Format(candidate, values); + + isAcceptedByTheRuntime = true; + } + catch (FormatException) + { + isAcceptedByTheRuntime = false; + } + + if (CompositeFormatString.TryGetArgumentIndexes(candidate, out _) != isAcceptedByTheRuntime) + { + mismatches.Add(candidate); + } + + return; + } + + foreach (var character in alphabet) + { + buffer[depth] = character; + + Walk(depth + 1, length); + } + } + + for (var length = 0; length <= buffer.Length; length++) + { + Walk(0, length); + } + + Assert.Empty(mismatches); + } +} diff --git a/tests/ReswPlusUnitTests/ResourceDiagnostics.cs b/tests/ReswPlusUnitTests/ResourceDiagnostics.cs index 817ac08..e1e08c1 100644 --- a/tests/ReswPlusUnitTests/ResourceDiagnostics.cs +++ b/tests/ReswPlusUnitTests/ResourceDiagnostics.cs @@ -75,6 +75,52 @@ public void PlaceholderMismatch_IsNotReportedForResourcesMissingFromATranslation Assert.Empty(diagnostics); } + [Fact] + public void PlaceholderMismatch_IsNotReportedWhenATranslationAddsAPlaceholder() + { + // English routinely spells the singular out while other languages still need the number, and + // string.Format ignores the arguments a value doesn't reference, so this is valid. + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw( + ("FileCount_One", "One file", "#Format[Plural Int count]"), + ("FileCount_Other", "{0} files", null))), + ("fr", ReswTestHelpers.CreateResw( + ("FileCount_One", "{0} fichier", null), + ("FileCount_Other", "{0} fichiers", null)))); + + Assert.Empty(diagnostics); + } + + [Fact] + public void PlaceholderMismatch_IsReportedWhenATranslationSubstitutesTheWrongArgument() + { + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw(("Treat", "give {0} biscuits to {2}", "#Format[Int count, Variant petType, String petName]"))), + ("fr", ReswTestHelpers.CreateResw(("Treat", "donnez {0} biscuits à {1}", null)))); + + var diagnostic = Assert.Single(diagnostics); + + Assert.Equal("RESWP0006", diagnostic.Id); + Assert.Contains("{2}", diagnostic.GetMessage()); + } + + [Fact] + public void FormattingProblems_AreNotReportedForATagThatTheGeneratorCannotResolve() + { + // A Reference() pointing at a pluralized resource cannot be resolved, so the generator discards the whole + // tag and emits the value unformatted: nothing about it can throw, and its braces are literal. + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw( + ("FileCount_One", "{0} file", "#Format[Plural Int count]"), + ("FileCount_Other", "{0} files", null), + ("Message", "{0} and {1} and {2}", "#Format[FileCount_One(), String other]")))); + + Assert.Empty(diagnostics); + } + [Fact] public void UndeclaredFormatParameter_IsReportedWhenAValueUsesMoreParametersThanDeclared() { @@ -195,6 +241,55 @@ public void MissingPluralForms_AreReportedPerVariantOfAPluralizedResource() Assert.Contains("Treat_Variant2", diagnostic.GetMessage()); } + [Fact] + public void MissingPluralForms_AreNotReportedForAResourceThatOnlyExistsInATranslation() + { + // A resource left behind in a translation after the default language dropped it generates no member, so + // its plural forms are never looked up. + var diagnostics = ReswTestHelpers.Analyze( + "en-US", + ("en-US", ReswTestHelpers.CreateResw(("Welcome", "Welcome!", null))), + ("pl", ReswTestHelpers.CreateResw(("Orphan_One", "{0} plik", null)))); + + Assert.Empty(diagnostics); + } + + [Fact] + public void MissingPluralForms_DoNotIncludeZeroWhenTheResourceHasAnEmptyState() + { + // GetPlural short circuits a zero quantity to the _None form, and the Arabic provider only returns the + // zero category for a quantity that is itself zero, so _Zero is unreachable here. + var diagnostics = ReswTestHelpers.Analyze( + "ar", + ("ar", ReswTestHelpers.CreateResw( + ("FileCount_None", "لا ملفات", "#Format[Plural Int count]"), + ("FileCount_One", "{0}", null), + ("FileCount_Two", "{0}", null), + ("FileCount_Few", "{0}", null), + ("FileCount_Many", "{0}", null), + ("FileCount_Other", "{0}", null)))); + + Assert.Empty(diagnostics); + } + + [Fact] + public void MissingPluralForms_StillIncludeZeroForALanguageThatUsesItForOtherQuantities() + { + // The Latvian provider returns the zero category for quantities such as 11 or 20 as well, so the _None + // form does not make _Zero unreachable. + var diagnostics = ReswTestHelpers.Analyze( + "lv", + ("lv", ReswTestHelpers.CreateResw( + ("FileCount_None", "Nav failu", "#Format[Plural Int count]"), + ("FileCount_One", "{0} fails", null), + ("FileCount_Other", "{0} faili", null)))); + + var diagnostic = Assert.Single(diagnostics); + + Assert.Equal("RESWP0008", diagnostic.Id); + Assert.Contains("'_Zero'", diagnostic.GetMessage()); + } + [Fact] public void DuplicateResource_IsReportedForKeysThatOnlyDifferByCase() { From 6fe8ec064ac1594d6032807475cbe187d794f7be Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 28 Jul 2026 10:45:08 -0700 Subject: [PATCH 6/7] Keep the generated code compatible with C# 7.3 The first commit of this branch annotated the generated code for nullable reference types, which needs C# 8, while projects created from the UWP templates default to C# 7.3. That would have forced every UWP consumer to raise their language version just to update the package, which is too much to ask of a fix for a warning. It also turns out not to be necessary. The compiler skips its own nullable analysis for a file marked ``, exactly as analyzers skip it, so the header added by that same commit already keeps the consumer's nullable warnings off the generated code. The sample consumer used to check this builds with zero warnings either way, with `enable` and `` on, so #44 is still closed. The nullable annotations, the `#nullable enable` directive and the changes made to the templates for their sake are therefore all reverted, and a test now parses the generated code as C# 7.3 to keep it that way. Annotating the API properly is worth doing, but it is a breaking change that deserves its own pull request rather than riding along with this one. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../CodeGenerators/CsharpCodeGenerator.cs | 5 +- .../CodeGenerators/GeneratedCode.cs | 17 ++----- .../Templates/Macros/Macros.txt | 46 +++++++++---------- .../Plurals/ResourceLoaderExtension.txt | 12 ++--- .../ReswPlusUnitTests/GeneratedCodeHygiene.cs | 26 +++++++---- 5 files changed, 52 insertions(+), 54 deletions(-) diff --git a/src/ReswPlus.SourceGenerator/CodeGenerators/CsharpCodeGenerator.cs b/src/ReswPlus.SourceGenerator/CodeGenerators/CsharpCodeGenerator.cs index 1cdda08..b491f82 100644 --- a/src/ReswPlus.SourceGenerator/CodeGenerators/CsharpCodeGenerator.cs +++ b/src/ReswPlus.SourceGenerator/CodeGenerators/CsharpCodeGenerator.cs @@ -24,7 +24,6 @@ namespace ReswPlus.SourceGenerator.CodeGenerators; /// /// // <auto-generated/> /// // File generated automatically by ReswPlus. https://github.com/DotNetPlus/ReswPlus -/// #nullable enable /// /// using System; /// using Windows.UI.Xaml.Markup; @@ -900,7 +899,7 @@ private ClassDeclarationSyntax CreateMarkupExtensionSyntax(string resourceFileNa ) .WithLeadingTrivia(CreateDocumentation("Gets or sets the key of the resource to look up.")), // Create the Converter property. - PropertyDeclaration(NullableType(ParseTypeName("IValueConverter")), "Converter") + PropertyDeclaration(ParseTypeName("IValueConverter"), "Converter") .WithModifiers(TokenList(Token(SyntaxKind.PublicKeyword))) .WithAccessorList( AccessorList( @@ -915,7 +914,7 @@ private ClassDeclarationSyntax CreateMarkupExtensionSyntax(string resourceFileNa ) .WithLeadingTrivia(CreateDocumentation("Gets or sets the converter to apply to the localized string, if any.")), // Create the ConverterParameter property. - PropertyDeclaration(NullableType(PredefinedType(Token(SyntaxKind.ObjectKeyword))), "ConverterParameter") + PropertyDeclaration(PredefinedType(Token(SyntaxKind.ObjectKeyword)), "ConverterParameter") .WithModifiers(TokenList(Token(SyntaxKind.PublicKeyword))) .WithAccessorList( AccessorList( diff --git a/src/ReswPlus.SourceGenerator/CodeGenerators/GeneratedCode.cs b/src/ReswPlus.SourceGenerator/CodeGenerators/GeneratedCode.cs index 84dea49..235615c 100644 --- a/src/ReswPlus.SourceGenerator/CodeGenerators/GeneratedCode.cs +++ b/src/ReswPlus.SourceGenerator/CodeGenerators/GeneratedCode.cs @@ -9,23 +9,14 @@ internal static class GeneratedCode /// The header prepended to every emitted file, terminated by a blank line. /// /// - /// The header serves two purposes: - /// - /// - /// The <auto-generated/> marker on the very first line is how Roslyn, StyleCop and most - /// third party analyzers detect generated code. Analyzers skip generated code by default, so the marker - /// keeps the emitted output out of the consumer's analysis results. - /// - /// - /// The explicit #nullable enable directive makes the emitted code independent from the - /// <Nullable> setting of the consuming project, so it always compiles the same way. - /// - /// + /// The <auto-generated/> marker on the very first line is how Roslyn, StyleCop and most third + /// party analyzers detect generated code. Analyzers skip generated code by default, and the compiler skips + /// its own nullable analysis for it, so the marker keeps the emitted output out of the consumer's build + /// results without the code having to declare a nullable context of its own. /// public const string FileHeader = "// \r\n" + "// File generated automatically by ReswPlus. https://github.com/DotNetPlus/ReswPlus\r\n" + - "#nullable enable\r\n" + "\r\n"; /// diff --git a/src/ReswPlus.SourceGenerator/Templates/Macros/Macros.txt b/src/ReswPlus.SourceGenerator/Templates/Macros/Macros.txt index f6863e4..74a03e3 100644 --- a/src/ReswPlus.SourceGenerator/Templates/Macros/Macros.txt +++ b/src/ReswPlus.SourceGenerator/Templates/Macros/Macros.txt @@ -16,7 +16,7 @@ namespace _ReswPlus_AutoGenerated internal static class Macros { #region ShortDate - private static string? _shortDate; + private static string _shortDate = null; public static string ShortDate { get @@ -31,7 +31,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region LongDate - private static string? _longDate; + private static string _longDate = null; public static string LongDate { get @@ -46,7 +46,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region ShortTime - private static string? _shortTime; + private static string _shortTime = null; public static string ShortTime { get @@ -61,7 +61,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region LongTime - private static string? _longTime; + private static string _longTime = null; public static string LongTime { get @@ -76,7 +76,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region WeekDay - private static string? _weekDay; + private static string _weekDay = null; public static string WeekDay { get @@ -91,7 +91,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region ShortWeekDay - private static string? _shortWeekDay; + private static string _shortWeekDay = null; public static string ShortWeekDay { get @@ -106,7 +106,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region Year - private static string? _year; + private static string _year = null; public static string Year { get @@ -121,7 +121,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region YearTwoDigits - private static string? _yearTwoDigits; + private static string _yearTwoDigits = null; public static string YearTwoDigits { get @@ -136,7 +136,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region LocaleName - private static string? _localeName; + private static string _localeName = null; public static string LocaleName { get @@ -151,7 +151,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region LocaleId - private static string? _localeId; + private static string _localeId = null; public static string LocaleId { get @@ -166,7 +166,7 @@ namespace _ReswPlus_AutoGenerated #endregion #region LocaleTwoLetters - private static string? _localeTwoLetters; + private static string _localeTwoLetters = null; public static string LocaleTwoLetters { get @@ -183,7 +183,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || DOTNETCORE || NETCOREAPP #region App Version: Full - private static string? _appVersionFull; + private static string _appVersionFull = null; public static string AppVersionFull { get @@ -206,7 +206,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || DOTNETCORE || NETCOREAPP #region App Version: Major.Minor.Build - private static string? _appVersionMajorMinorBuild; + private static string _appVersionMajorMinorBuild = null; public static string AppVersionMajorMinorBuild { get @@ -229,7 +229,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || DOTNETCORE || NETCOREAPP #region App Version: Major.Minor - private static string? _appVersionMajorMinor; + private static string _appVersionMajorMinor = null; public static string AppVersionMajorMinor { get @@ -252,7 +252,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || DOTNETCORE || NETCOREAPP #region App Version: Major - private static string? _appVersionMajor; + private static string _appVersionMajor = null; public static string AppVersionMajor { get @@ -275,7 +275,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || NETCOREAPP #region Architecture - private static string? _architecture; + private static string _architecture = null; public static string Architecture { get @@ -300,7 +300,7 @@ namespace _ReswPlus_AutoGenerated case Windows.System.ProcessorArchitecture.Neutral: _architecture = "Any"; break; - default: + case Windows.System.ProcessorArchitecture.Unknown: _architecture = "Unknown"; break; } @@ -313,7 +313,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || DOTNETCORE || NETCOREAPP #region ApplicationName - private static string? _applicationName; + private static string _applicationName = null; public static string ApplicationName { get @@ -335,7 +335,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || NETCOREAPP #region ApplicationName - private static string? _publisherName; + private static string _publisherName = null; public static string PublisherName { get @@ -352,7 +352,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || NETCOREAPP #region DeviceFamily - private static string? _deviceFamily; + private static string _deviceFamily = null; public static string DeviceFamily { get @@ -369,7 +369,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || NETCOREAPP #region DeviceManufacturer - private static string? _deviceManufacturer; + private static string _deviceManufacturer = null; public static string DeviceManufacturer { get @@ -386,7 +386,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || NETCOREAPP #region DeviceModel - private static string? _deviceModel; + private static string _deviceModel = null; public static string DeviceModel { get @@ -403,7 +403,7 @@ namespace _ReswPlus_AutoGenerated #if WINDOWS_UWP || NETCOREAPP #region OperatingSystem - private static string? _operatingSystemVersion; + private static string _operatingSystemVersion = null; public static string OperatingSystemVersion { get diff --git a/src/ReswPlus.SourceGenerator/Templates/Plurals/ResourceLoaderExtension.txt b/src/ReswPlus.SourceGenerator/Templates/Plurals/ResourceLoaderExtension.txt index 3280bad..db8a57f 100644 --- a/src/ReswPlus.SourceGenerator/Templates/Plurals/ResourceLoaderExtension.txt +++ b/src/ReswPlus.SourceGenerator/Templates/Plurals/ResourceLoaderExtension.txt @@ -6,7 +6,7 @@ namespace _ReswPlus_AutoGenerated.Plurals { internal static class ResourceLoaderExtension { - private static _ReswPlus_AutoGenerated.Plurals.IPluralProvider? _pluralProvider; + private static _ReswPlus_AutoGenerated.Plurals.IPluralProvider _pluralProvider; private static readonly object _objLock = new object(); public static string GetPlural(this _ReswPlus_AutoGenerated.ResourceStringProvider resourceStringProvider, string key, double number, bool supportNoneState = false) @@ -24,7 +24,7 @@ namespace _ReswPlus_AutoGenerated.Plurals return ""; } } - string? selectedSentence = null; + string selectedSentence = null; var pluralType = _pluralProvider.ComputePlural(number); try { @@ -54,19 +54,19 @@ namespace _ReswPlus_AutoGenerated.Plurals return selectedSentence ?? ""; } - private static void CreatePluralProvider(string? forcedCultureName = null) + private static void CreatePluralProvider(string forcedCultureName = null) { lock (_objLock) { if (_pluralProvider is null) { var cultureToUse = CultureInfo.CurrentUICulture.TwoLetterISOLanguageName; - if (forcedCultureName is { Length: > 0 }) + if (!string.IsNullOrEmpty(forcedCultureName)) { try { - var forcedCulture = new CultureInfo(forcedCultureName).TwoLetterISOLanguageName; - if (forcedCulture is { Length: > 0 }) + var forcedCulture = new CultureInfo(forcedCultureName)?.TwoLetterISOLanguageName; + if (!string.IsNullOrEmpty(forcedCulture)) { cultureToUse = forcedCulture; } diff --git a/tests/ReswPlusUnitTests/GeneratedCodeHygiene.cs b/tests/ReswPlusUnitTests/GeneratedCodeHygiene.cs index 3ebe3d2..22b7241 100644 --- a/tests/ReswPlusUnitTests/GeneratedCodeHygiene.cs +++ b/tests/ReswPlusUnitTests/GeneratedCodeHygiene.cs @@ -7,8 +7,8 @@ namespace ReswPlusUnitTests; /// -/// Tests covering the hygiene of the code emitted by the generator: the auto-generated header, the explicit -/// nullable context and the XML documentation, all of which are required for the generated code to build +/// Tests covering the hygiene of the code emitted by the generator: the auto-generated header and the XML +/// documentation, both of which are required for the generated code to build /// cleanly in projects with analyzers and warnings-as-errors enabled. /// public class GeneratedCodeHygiene @@ -46,11 +46,19 @@ public void GeneratedFileUsesTheGeneratedCodeExtension() } [Fact] - public void GeneratedFileDeclaresAnExplicitNullableContext() + public void GeneratedFileIsValidForAProjectWithoutNullableReferenceTypes() { + // Projects created from the UWP templates default to C# 7.3, so the generated code stays within it: it + // relies on the marker to keep the consumer's nullable analysis off its output rather + // than on a nullable context of its own, which would need C# 8. var code = GenerateSample(); - Assert.Contains("#nullable enable", code); + Assert.DoesNotContain("#nullable", code); + + var options = CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp7_3); + var diagnostics = CSharpSyntaxTree.ParseText(code, options).GetDiagnostics().ToArray(); + + Assert.Empty(diagnostics); } [Fact] @@ -129,14 +137,14 @@ public void ResourceValuesAreEscapedInTheDocumentation() } [Fact] - public void MarkupExtensionPropertiesThatCanBeNullAreAnnotated() + public void MarkupExtensionPropertiesAreNotAnnotatedForNullability() { var code = GenerateSample(); - // Both properties are left unset unless the user assigns them in XAML, so they must be nullable - // to avoid CS8618 in projects that enable nullable reference types. - Assert.Contains("public IValueConverter? Converter", code); - Assert.Contains("public object? ConverterParameter", code); + // Annotating them would need C# 8, which projects created from the UWP templates don't use by default. + // The marker keeps the consumer's nullable analysis off the file instead. + Assert.Contains("public IValueConverter Converter", code); + Assert.Contains("public object ConverterParameter", code); } private static DocumentationCommentTriviaSyntax? GetDocumentation(SyntaxNode member) From 4cf023b1edcb1f6a38c7f38696e4abdf0b92eace Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 28 Jul 2026 10:45:24 -0700 Subject: [PATCH 7/7] Fix the build of the UWP samples Both UWP samples pinned `TargetPlatformVersion` to `10.0.22000.0`, which is no longer installed on the hosted runners: they now ship the 26100 SDK. Without a matching SDK none of the platform references resolve, which is why the build failed with RESWP0005, the generator correctly reporting that it could not see a UWP or WinAppSDK project. Both samples now target `10.0.26100.0`. Package signing is also disabled for the UWP sample: its temporary certificate expired in February, which fails the build, and a sample doesn't need a signed package to be built. The WinAppSDK sample has the same expired certificate but only warns about it, so it is left alone. Both problems predate this branch, the build of the solution just never got far enough to reach them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- samples/UWP/ReswPlusUWPSample/ReswPlusUWPSample.csproj | 7 ++++--- .../ReswPlusUWPSampleExternalLibrary.csproj | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/samples/UWP/ReswPlusUWPSample/ReswPlusUWPSample.csproj b/samples/UWP/ReswPlusUWPSample/ReswPlusUWPSample.csproj index ae7c3d5..8326a68 100644 --- a/samples/UWP/ReswPlusUWPSample/ReswPlusUWPSample.csproj +++ b/samples/UWP/ReswPlusUWPSample/ReswPlusUWPSample.csproj @@ -1,4 +1,4 @@ - + @@ -12,7 +12,7 @@ ReswPlusUWPSample en-US UAP - 10.0.22000.0 + 10.0.26100.0 10.0.17763.0 14 512 @@ -21,7 +21,8 @@ ReswPlusUWPSample_TemporaryKey.pfx - True + + False true False SHA256 diff --git a/samples/UWP/ReswPlusUWPSampleExternalLibrary/ReswPlusUWPSampleExternalLibrary.csproj b/samples/UWP/ReswPlusUWPSampleExternalLibrary/ReswPlusUWPSampleExternalLibrary.csproj index 01675a6..d043a52 100644 --- a/samples/UWP/ReswPlusUWPSampleExternalLibrary/ReswPlusUWPSampleExternalLibrary.csproj +++ b/samples/UWP/ReswPlusUWPSampleExternalLibrary/ReswPlusUWPSampleExternalLibrary.csproj @@ -1,4 +1,4 @@ - + @@ -12,7 +12,7 @@ ReswPlusUWPSampleExternalLibrary en UAP - 10.0.22000.0 + 10.0.26100.0 10.0.17763.0 14 512