Skip to content

Emit hygienic generated code and add diagnostics for the content of the .resw files - #49

Open
Sergio0694 wants to merge 7 commits into
DotNetPlus:mainfrom
Sergio0694:user/sergiopedri/generated-code-hygiene-and-resource-diagnostics
Open

Emit hygienic generated code and add diagnostics for the content of the .resw files#49
Sergio0694 wants to merge 7 commits into
DotNetPlus:mainfrom
Sergio0694:user/sergiopedri/generated-code-hygiene-and-resource-diagnostics

Conversation

@Sergio0694

@Sergio0694 Sergio0694 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Closes #44
Closes #45

Two related pieces of work. The first one makes the generated code buildable in a strict project, the second one adds the resource-quality diagnostics that the first one makes usable. They are separate commits and can be reviewed independently.


1. Generated-code hygiene (#45, #44)

Root cause

CsharpCodeGenerator.GetGeneratedFiles built the file header as a bare comment:

var headerTrivia = TriviaList(
    Comment("// File generated automatically by ReswPlus. https://github.com/DotNetPlus/ReswPlus"),
    CarriageReturnLineFeed
);

That was the whole header: no // <auto-generated/> marker, so the compiler and every analyzer treated the output as hand-written code (#45), and no #nullable directive, so the output inherited the consumer's <Nullable> setting and emitted nullable warnings (#44). The [GeneratedCodeAttribute] already on the generated types is not enough, because analyzers key off the file header and the file name, not the attribute.

Two related bugs turned up while fixing this, both silent:

  • The header was never emitted at all. It was attached to the compilation unit and then discarded by the WithUsings call on the next line, which replaces the first token the trivia was attached to. The generated files started directly with using System;.
  • The XML documentation was never emitted either. CreateFormatMethodSyntax builds a <summary> for every member, but the loop that wraps members in #region directives called WithLeadingTrivia(...), which replaces the leading trivia rather than extending it, dropping the documentation it had just built.

What changed

  • Every emitted file, including the templates emitted verbatim (Macros, ResourceLoaderExtension, the plural providers, …), now starts with // <auto-generated/>, then the attribution comment, then #nullable enable. There is a single definition of the header, in GeneratedCode.FileHeader.
  • Hint names use the .g.cs extension, which is the second, independent signal tooling uses to detect generated code.
  • The #region trivia now extends the leading trivia instead of replacing it, so the documentation survives.
  • Every visible generated member is documented, including all of its parameters, and resource values are XML-escaped before being inlined into a <summary>.
  • Converter and ConverterParameter on the generated markup extension are annotated as nullable, and the plural and macro templates were made nullable-correct.

#nullable enable vs disable

enable, with correct annotations. The alternative gives consumers with NRTs on a set of oblivious types instead of an accurate API, which is a worse long-term answer for the sake of less work now. The two annotations this needed are unambiguous: Converter and ConverterParameter are XAML-settable properties that are null unless the user assigns them, which is exactly what CS8618 was reporting. Nothing else in the generated code is nullable, so nothing else had to be guessed.

No blanket #pragma warning disable was added anywhere. The two compiler warnings that the <auto-generated/> marker does not suppress, CS1591 (missing documentation) and CS1573 (undocumented parameter), are fixed by emitting real documentation rather than by silencing them. I verified empirically that the marker does not suppress CS1591: the compiler reports it on a file whose first line is // <auto-generated/> just the same.

Zero-warning evidence

A sample consumer referencing the generator, with a .resw exercising plain strings, #Format[...] methods, macros, literals, string references, variants, and plurals including a _None empty state, built with:

<Nullable>enable</Nullable>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
<AnalysisLevel>latest-all</AnalysisLevel>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>

Before, the same project produced 108 warnings:

Count ID
35 CS1591 missing XML documentation
23 CS8625 null literal to non-nullable reference
22 CA1805 member explicitly initialized to its default value
13 CA1305 culture-dependent ToString
3 CA1031 generic catch
3 CS8618 non-nullable property not initialized
2 CA1508 dead code
1 each CA1707, CA1711, CA1820, CS8600, CS8603

After:

  ReswPlus.SourceGenerator -> ...\ReswPlus.SourceGenerator.dll
  StrictConsumer -> ...\StrictConsumer.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

2. Resource-quality diagnostics

The generator parses every .resw and already knows the plural categories each language requires, but 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.

ID Rule
RESWP0006 A translated value drops placeholders its value in the default language uses.
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.
RESWP0009 Two resources of the same file conflict with each other.
RESWP0010 A value used as a composite format string is malformed.

All five report on the offending line of the .resw file, pointing at the resource name, so they can be double-clicked.

Why Warning and not Error

Every existing consumer with a pre-existing resource inconsistency would have their build broken the moment they update the package if these were errors, which is not an acceptable way to ship a new rule. Warning surfaces the problem; consumers who want it fatal can escalate it per rule:

dotnet_diagnostic.RESWP0006.severity = error

This interacts with the first half of the PR: a consumer using TreatWarningsAsErrors gets errors anyway. That is correct, it is their explicit policy, and it is exactly why the hygiene work had to land in the same PR, so that the generator's own output is clean first.

Reported from an analyzer, not from the generator

The rules are implemented as a DiagnosticAnalyzer (ReswResourceAnalyzer) that delegates to ReswResourceRules, not as part of the generator. A generator runs on every keystroke in the IDE, and parsing every .resw file of every language and cross-checking them against the default language is far too expensive to do there. An analyzer is scheduled independently, runs out of process, and can be disabled per rule.

One consequence worth knowing: 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 would have made it visible there, but at a cost paid on every keystroke by every consumer, which is not a good trade.

While in 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.

False positives

A noisy analyzer gets switched off wholesale, taking the valuable rules with it, so every rule errs on the side of not firing:

  • {{ and }} are treated as the escaped braces they are.
  • Placeholders are compared as sets, so a translation is free to reorder them: {1} {0} is valid and common.
  • RESWP0006 only reports the placeholders a translation drops. A translation is free to use more placeholders than the default language, and routinely does: English spells the singular out as One file while French needs {0} fichier. string.Format ignores the arguments a value doesn't reference, so that direction is valid.
  • Values of resources without a #Format tag are never parsed as format strings, because they are never formatted. Press { to open is a perfectly good resource.
  • Resources that only exist in the default language are skipped.
  • File grouping and default-language resolution reuse the generator's own logic (ReswFileGrouping), so a .resw that isn't a translation of the neutral file is never compared against it.
  • A language with no plural provider has no required plural forms, so RESWP0008 stays silent for it.
  • RESWP0008 skips a pluralized resource that only exists in a translation, since the default language is what drives generation and such a resource generates nothing. It also stops requiring _Zero for a resource that declares _None, because GetPlural short circuits a zero quantity to _None and every provider except Latvian only returns the zero category for a zero quantity.
  • A resource is identified case insensitively, the way resource lookup identifies it, when matching a plural form or a translation back to the resource a generated member reads.
  • RESWP0007 only reports the direction that actually throws, a placeholder index with no matching argument. It deliberately does not report declared-but-unused parameters: the _None form of a pluralized resource legitimately doesn't show the quantity, and the Variant parameter of a varianted resource is frequently not substituted into the text.
  • The composite-format parser is deliberately permissive where the runtime is, accepting alignment, format specifiers, and whitespace inside a format item.

It found a real bug

RESWP0006 fired on the repository's own samples the first time it ran. The French AnimalTreat_Variant1 strings substitute {1}, which is the variant id, where they mean {2}, which is the name of the pet, so the French build of both sample apps renders a number where it should render a name. Fixed in both samples as part of the PR.

Testing

The existing xUnit project went from 30 to 91 tests. Every rule is covered in both directions: on input that is genuinely broken, and on the valid input most likely to be mistaken for it (reordered placeholders, escaped braces, unformatted resources with braces, _None forms, languages without a plural provider, a complete Polish plural set). One test runs the analyzer through CompilationWithAnalyzers, which is the only thing that catches a registration mistake, and one asserts that a diagnostic points at the exact span of the resource name.

The composite-format parser is validated differentially against string.Format itself: every string of length 4 or less built from the alphabet { } 0 1 a , : - space is fed to both, and the parser must accept exactly the ones the runtime accepts. That is 7,381 cases in the test suite, and the same sweep run offline up to length 5 covers 66,430 cases with 0 mismatches.

The generated-code hygiene is covered too, including a test that parses the generated file and asserts it has no syntax diagnostics, which would have caught the dropped-documentation bug.


Deliberately left out

  • Making the generator skip conflicting resources. RESWP0009 covers the case where a plain resource collides with a pluralized one, which produces code that does not compile. Having the generator drop the second member would turn that into a clean build plus a warning, but it changes generation behaviour and depends on the analysis model matching ReswClassGenerator.Parse exactly. Worth doing as a follow-up that unifies the two, not as a drive-by here.
  • Pseudo-localization generation, and multi-parameter plural selection (a plural form can currently only be chosen from a single parameter, so "2 folders, 3 cards" cannot be a single pluralized resource). The latter is a real design limitation and deserves a discussion issue rather than a drive-by PR.
  • _Two is missing from the plural-form regex in ReswFilters, so a Foo_Two resource is not grouped with the rest of its plural forms, even though ResourceLoaderExtension.GetPlural asks for it at runtime. RESWP0008 works around this by checking the resources of the file directly rather than the grouped ones, so it is correct either way, but the grouping gap is real and is worth its own fix.
  • The checked-in generated output under samples/UWP/ReswPlusUWPSampleExternalLibrary/Generated/ was already stale before this PR (it predates Improve C# Code Generator to use Roslyn SyntaxFactory instead of basic string concatenation #43) and is not referenced by any project, so it was left alone.

Sergio0694 and others added 7 commits July 27, 2026 22:17
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
`<Nullable>enable</Nullable>`, `<TreatWarningsAsErrors>` and a strict analysis
level, that made the build fail outright (fixes DotNetPlus#44, fixes DotNetPlus#45).

Changes:

- Every emitted file now starts with `// <auto-generated/>`, 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 `<summary>` 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>
Tooling recognizes generated code either by the `<auto-generated/>` 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>
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>
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>
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>
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 `<auto-generated/>`, 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 `<Nullable>enable</Nullable>` and
`<TreatWarningsAsErrors>` on, so DotNetPlus#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>
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Warnings from generated code when nullable-reference types are enabled

1 participant