Skip to content

Fix NRE in XLiffBody.AddTransUnit when tu.Source is null#153

Merged
imnasnainaec merged 11 commits into
masterfrom
fix/149-nre-addtransunit
Jul 8, 2026
Merged

Fix NRE in XLiffBody.AddTransUnit when tu.Source is null#153
imnasnainaec merged 11 commits into
masterfrom
fix/149-nre-addtransunit

Conversation

@imnasnainaec

@imnasnainaec imnasnainaec commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Adds null guard on tu.Source in AddTransUnit so a null Source doesn't throw when Target is also null
  • Adds null guard on tu.Source in the NumberTranslated property for the same case
  • Guards tu.Source/tuOld.Source in MergeXliffDocuments against the same NRE when a baseline XLIFF unit has no source variant; a blank old source is still counted as changed but no longer emits an empty OLD TEXT note
  • Simplifies the null checks across AddTransUnit, NumberTranslated, and NumberApproved to use null-conditional (?.) access. As part of this, the NumberTranslated "differs from source" heuristic now requires a non-blank source (IsNullOrWhiteSpace), so a unit with a blank source no longer trivially counts as translated. AddTransUnit keeps != null since an empty/whitespace value is still a legitimate stored translation.

Closes #149

Devin review: https://app.devin.ai/review/sillsdev/l10nsharp/pull/153


This change is Reviewable

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

Test Results

    7 files  ±0  124 suites  ±0   37s ⏱️ +13s
187 tests +2  182 ✔️ +2    5 💤 ±0  0 ±0 
714 runs  +8  699 ✔️ +8  15 💤 ±0  0 ±0 

Results for commit f574733. ± Comparison against base commit f008c92.

♻️ This comment has been updated with latest results.

@imnasnainaec imnasnainaec self-assigned this Jun 8, 2026
@imnasnainaec imnasnainaec marked this pull request as draft June 8, 2026 17:29
imnasnainaec and others added 8 commits June 8, 2026 13:31
A malformed baseline XLIFF can contain a trans-unit with no source variant, causing a NullReferenceException when MergeXliffDocuments compares it against the current scan. Use null-conditional access so a null old source is simply treated as differing from the new source.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When the baseline trans-unit's source is null/whitespace, the unit is still counted as changed, but skip the OLD TEXT note since it would carry no value. Uses IsNullOrWhiteSpace to match other XLiffUtils source-value checks.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collapse the Target/Source null guards in AddTransUnit, NumberTranslated, and NumberApproved to use ?. access. In the NumberTranslated 'differs from source' heuristic, also require a non-blank source (IsNullOrWhiteSpace) so a blank source no longer trivially counts a unit as translated. AddTransUnit keeps != null since an empty/whitespace value is still a legitimate stored translation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@imnasnainaec imnasnainaec marked this pull request as ready for review June 26, 2026 15:35

@tombogle tombogle left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tombogle reviewed 4 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on imnasnainaec).

@imnasnainaec imnasnainaec merged commit ae81ffb into master Jul 8, 2026
6 checks passed
@imnasnainaec imnasnainaec deleted the fix/149-nre-addtransunit branch July 8, 2026 09:36
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.

XLiffBody.AddTransUnit throws NRE when Source is null

2 participants