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

CHANGELOG.md

@@ -39,13 +39,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - [L10NSharp.Windows.Forms] Restored project-local Resources support for `FallbackLanguagesDlgBase` button images (`Move`, `Move_up`, and `Move_down`).
 - [L10NSharp.Windows.Forms] Corrected resource manager base name to `L10NSharp.Windows.Forms.Properties.Resources`.
 - [L10NSharp.Windows.Forms.Tests] Corrected resource manager base name to `L10NSharp.Windows.Forms.Tests.Properties.Resources`.
-- [L10NSharp] Fixed file handle leak in `XliffLocalizationManager.CreateOrUpdateDefaultXliffFileIfNecessary` when an exception was thrown between `File.Open` and `Close`. (#151)
-- [L10NSharp] Fixed race condition in `XLiffBody.AddTransUnitRaw` where two concurrent threads with the same translation-unit ID could both bypass the duplicate check and silently overwrite each other's entry. (#150)
-- [L10NSharp] Eliminated unnecessary cross-instance lock contention in `XLiffBody` by making `_transUnitIdLock` instance-level instead of static. (#150)
-- [L10NSharp] Fixed `FilenamesToAddToCache` yielding both the custom and installed XLIFF for the same language when `UseLanguageCodeFolders` is `true`, causing custom translations to be silently overwritten by installed ones. (#140)
-- [L10NSharp] Fixed `ExtractXliff` accumulating duplicate "Not found in static scan" notes on successive runs; the note is now replaced rather than appended, and removed when the string is subsequently found. (#113)
-- [L10NSharp] Fixed `LocalizationManager.GetString` silently falling back to English when called with a one-shot `IEnumerable<string>` for `preferredLanguageIds`; the sequence is now materialized before use. (#139)
-- [L10NSharp] Fixed malformed XLIFF file produced when `GetDynamicString`/`GetString` was called with a null, empty, or whitespace string ID; the entry is now silently skipped rather than written with a blank `id` attribute that caused a crash on next launch. (#104)
+- [L10NSharp] Fixed file handle leak in `XliffLocalizationManager.CreateOrUpdateDefaultXliffFileIfNecessary` when an exception was thrown between `File.Open` and `Close`.
+- [L10NSharp] Fixed race condition in `XLiffBody.AddTransUnitRaw` where two concurrent threads with the same translation-unit ID could both bypass the duplicate check and silently overwrite each other's entry.
+- [L10NSharp] Eliminated unnecessary cross-instance lock contention in `XLiffBody` by making `_transUnitIdLock` instance-level instead of static.
+- [L10NSharp] Fixed `FilenamesToAddToCache` yielding both the custom and installed XLIFF for the same language when `UseLanguageCodeFolders` is `true`, causing custom translations to be silently overwritten by installed ones.
+- [L10NSharp] Fixed `ExtractXliff` accumulating duplicate "Not found in static scan" notes on successive runs; the note is now replaced rather than appended, and removed when the string is subsequently found.
+- [L10NSharp] Fixed `LocalizationManager.GetString` silently falling back to English when called with a one-shot `IEnumerable<string>` for `preferredLanguageIds`; the sequence is now materialized before use.
+- [L10NSharp] Fixed malformed XLIFF file produced when `GetDynamicString`/`GetString` was called with a null, empty, or whitespace string ID; the entry is now silently skipped rather than written with a blank `id` attribute that caused a crash on next launch.
+- [L10NSharp] Fixed `NullReferenceException` in `XLiffBody.AddTransUnit`, `XLiffBody.NumberTranslated`, and `XliffLocalizationManager.MergeXliffDocuments` when a trans-unit has no source variant (e.g. from a malformed XLIFF file).
 
 ### Removed
 

src/L10NSharp.Tests/XLiffLocalizationManagerTests.cs

@@ -272,7 +272,38 @@ public void MergeXliffDocuments_StringFoundAfterBeingMissing_RemovesNotFoundNote
 			Assert.That(tu.Notes.Any(n => n.Text.Contains("Not found")), Is.False);
 		}
 
-		private void CheckMergedTransUnit(XLiffTransUnit tu, string sourceText, string[] notes, bool isDynamic)
+		[Test]
+		public void AddTransUnit_NullSourceNullTarget_DoesNotThrow()
+		{
+			var body = new XLiffBody();
+			var tu = new XLiffTransUnit { Id = "some-id", Source = null, Target = null };
+			Assert.DoesNotThrow(() => body.AddTransUnit(tu));
+		}
+
+		[Test]
+		public void MergeXliffDocuments_BaselineUnitHasNullSource_DoesNotThrow()
+		{
+			var newDoc = new XLiffDocument();
+			newDoc.AddTransUnit(new XLiffTransUnit {
+				Id = "Some.id",
+				Source = new XLiffTransUnitVariant { Lang = "en", Value = "Current text." }
+			});
+
+			var oldDoc = new XLiffDocument();
+			oldDoc.AddTransUnit(new XLiffTransUnit { Id = "Some.id", Source = null });
+
+			XLiffDocument mergedDoc = null;
+			Assert.DoesNotThrow(() =>
+				mergedDoc = XliffLocalizationManager.MergeXliffDocuments(newDoc, oldDoc, true));
+
+			// A null old source has no value to report, so no "OLD TEXT" note is added.
+			var tu = mergedDoc.GetTransUnitForId("Some.id");
+			Assert.IsNotNull(tu);
+			Assert.That(tu.Notes.Any(n => n.Text.StartsWith("OLD TEXT")), Is.False);
+		}
+
+		private static void CheckMergedTransUnit(
+			XLiffTransUnit tu, string sourceText, string[] notes, bool isDynamic)
 		{
 			Assert.IsNotNull(tu);
 			Assert.That("en", Is.EqualTo(tu.Source.Lang));

src/L10NSharp/XLiffUtils/XLiffBody.cs

@@ -164,12 +164,12 @@ public bool AddTransUnit(XLiffTransUnit tu)
 		{
 			if (!AddTransUnitRaw(tu))
 				return false;
-		
+
 			// If the target exists, store its value in the dictionary lookup.  Otherwise, store
 			// the source value there.
-			if (tu.Target != null && tu.Target.Value != null)
+			if (tu.Target?.Value != null)
 				TranslationsById[tu.Id] = tu.Target.Value;
-			else
+			else if (tu.Source?.Value != null)
 				TranslationsById[tu.Id] = tu.Source.Value;
 			return true;
 		}
@@ -234,14 +234,15 @@ internal int NumberTranslated
 						_translatedCount = 0;
 						foreach (var tu in TransUnitsUnordered)
 						{
-							if (tu.Target == null || string.IsNullOrWhiteSpace(tu.Target.Value))
+							if (string.IsNullOrWhiteSpace(tu.Target?.Value))
 								continue;
 							if (tu.TranslationStatus == TranslationStatus.Approved ||
 							    tu.Target.TargetState == XLiffTransUnitVariant.TranslationState.Translated)
 							{
 								++_translatedCount;
 							}
-							else if (tu.Target.Value != tu.Source.Value &&
+							else if (!string.IsNullOrWhiteSpace(tu.Source?.Value) &&
+							         tu.Target.Value != tu.Source.Value &&
 							         tu.Target.TargetState == XLiffTransUnitVariant.TranslationState.Undefined)
 							{
 								++_translatedCount;
@@ -271,7 +272,7 @@ internal int NumberApproved
 						_approvedCount = 0;
 						foreach (var tu in TransUnitsUnordered)
 						{
-							if (tu.Target == null || string.IsNullOrWhiteSpace(tu.Target.Value))
+							if (string.IsNullOrWhiteSpace(tu.Target?.Value))
 								continue;
 							if (tu.TranslationStatus == TranslationStatus.Approved)
 								++_approvedCount;

src/L10NSharp/XLiffUtils/XliffLocalizationManager.cs

@@ -630,11 +630,12 @@ internal static XLiffDocument MergeXliffDocuments(XLiffDocument xliffNew, XLiffD
 									tu.AddNote(note.NoteLang, "[OLD NOTE] " + note.Text);
 							}
 						}
-						if (tu.Source.Value != tuOld.Source.Value)
+						if (tu.Source?.Value != tuOld.Source?.Value)
 						{
 							++changedStringCount;
 							changedStringIds.Add(tu.Id);
-							tu.AddNote("en", $"OLD TEXT (before {xliffNew.File.ProductVersion}): {tuOld.Source.Value}");
+							if (!string.IsNullOrWhiteSpace(tuOld.Source?.Value))
+								tu.AddNote("en", $"OLD TEXT (before {xliffNew.File.ProductVersion}): {tuOld.Source.Value}");
 						}
 						if (tuOld.Dynamic && !tu.Dynamic)
 						{