Skip to content

feat(LC0100): report malformed translation files - #589

Open
tkroes wants to merge 1 commit into
ALCops:mainfrom
tkroes:feat/lc0100-translation-files-must-be-well-formed
Open

tkroes wants to merge 1 commit into
ALCops:mainfrom
tkroes:feat/lc0100-translation-files-must-be-well-formed

Conversation

@tkroes

@tkroes tkroes commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds LinterCop rule LC0100 TranslationFilesMustBeWellFormed to report malformed or unreadable translation files for the current app.

  • Checks XML well-formedness for compiler-discovered XLIFF files (no XLIFF schema/semantic validation)
  • Reports every malformed/unreadable file, with its path and details at Location.None; XML parse errors include line and column
  • Remains silent when translation generation is disabled or the translation directory is absent
  • Category Style, severity Error, enabled by default; no CodeFix and no setting
  • Supports all three installed SDK target assemblies (netstandard2.1, net8.0, net10.0); no version gate

Validation

  • Targeted tests: 4 passed
  • Full LinterCop suite: 400 passed
  • Release CI build across all target frameworks: succeeded
  • dotnet format ALCops.sln --verify-no-changes: clean
  • .claude rule validation: passed
  • /code-review: no significant issues found

Docs-site page in the sibling alcops.dev repo remains to be written.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Arthurvdv

Copy link
Copy Markdown
Member

Thanks for this one, @tkroes. A malformed .xlf is a nasty thing to track down, so a rule for it is more then welcome 🤗

A few points I would like to discuss with you before we merge.

PlatformCop vs LinterCop

I tend to place rules by what happens if they're ignored (see how the analyzers differ):

  • LinterCop covers code that is correct but could be shorter, clearer or faster. Ignoring a LinterCop rule breaks nothing; the cost falls on whoever maintains the code next.
  • PlatformCop covers constructs the platform rejects, ignores, or runs differently from what the code says. The tie-breaker is: "If ignoring the rule produces a runtime error or a silently wrong result, it is a PlatformCop rule."

The rule doc justifies with "failed translation loading affects the whole extension". That is the PlatformCop consequence: the platform doesn't apply the translations and users see the base-language captions. It doesn't depend on any Base Application convention, so it isn't ApplicationCop either. It also isn't about procedure bodies, which is what LinterCop inspects.

Could you move the rule to PlatformCop with the next free PC ID? LC0100 hasn't been released, so nothing is lost by leaving it unused. And while moving it, don't forget to change the category from Style to something like Usage: a broken translation file isn't a style issue.

Severity: Warning rather than Error

The rule is on by default at Error, so anyone who upgrades ALCops with a malformed .xlf in their project gets a new build error straight away. That can break CI/CD pipelines that update the analyzers automatically. Could you ship it as a Warning? Teams that want it to block the build can raise it to Error in their ruleset.

Code Review

Some remarks on the implementaion of the code itself

1. Stream the file instead of building a DOM. XmlDocument.Load builds the whole document in memory just to check well-formedness. Translation files can be several MB per language, with 20+ languages. It also keeps the default DTD handling, so a DOCTYPE with entity declarations gets expanded. An XmlReader read to the end does the same check at streaming cost, and lets us turn DTDs off:

private static readonly XmlReaderSettings WellFormedOnly = new()
{
    DtdProcessing = DtdProcessing.Prohibit,
    XmlResolver = null,
    IgnoreComments = true,
    IgnoreWhitespace = true,
};

using Stream stream = fileSystem.OpenRead(translationFile);
using var reader = XmlReader.Create(stream, WellFormedOnly);
while (reader.Read())
{
}

XmlException keeps its LineNumber/LinePosition, so the message stays the same.

2. Discovery only catches DirectoryNotFoundException. Each file read handles IOException and UnauthorizedAccessException, but listing the files doesn't. An unreadable Translations folder or a path-too-long error then escapes the compilation action, and the user gets an analyzer crash instead of a translation-file diagnostic. For example:

catch (DirectoryNotFoundException)
{
    return;
}
catch (Exception exception) when (exception is IOException or UnauthorizedAccessException)
{
    ReportFileError(ctx, "Translations", $"Unable to enumerate translation files: {exception.Message}");
    return;
}

... or return silently if you'd rather not report this case. Either way it shouldn't crash.

3. Missing manifest: this rule and LC0091 behave differently. When the app's manifest (app.json) can't be resolved, this rule returns, while TranslatableTextShouldBeTranslated carries on: it only skips when a manifest is present and turns translation-file generation off and otherwise uses an empty app name. What do you think is the best choice here? Should this rule follow the same pattern, so both translation rules agree on when they run?

NavAppManifest? manifest;
try
{
    manifest = ManifestHelper.GetManifest(ctx.Compilation);
}
catch (FileNotFoundException)
{
    manifest = null;
}

if (manifest is not null && !manifest.CompilerFeatures.ShouldGenerateTranslationFile())
    return;

string appName = manifest?.AppName ?? string.Empty;

Tests

I see some errors in src/ALCops.LinterCop.Test/Rules/TranslationFilesMustBeWellFormed/TranslationFilesMustBeWellFormed.cs. Could you bring in #592 (88f251a) from main? That will show whether they're still errors or just came from the test-build issue #592 fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants