Fix formal review findings: external relationship validation, snippet deserialization, package enhance, and doc corrections

docs/design/spdx-model/spdx-element.md

@@ -8,8 +8,9 @@ identity handling across all element types.
 
 ## Design
 
-`SpdxElement` is a public abstract class. Concrete element types (`SpdxDocument`, `SpdxPackage`,
-`SpdxFile`, `SpdxSnippet`, `SpdxRelationship`, `SpdxAnnotation`) inherit from it.
+`SpdxElement` is a public abstract class. It is directly inherited by `SpdxDocument`, `SpdxRelationship`,
+and `SpdxAnnotation`. `SpdxLicenseElement` is an abstract class that also inherits from `SpdxElement`, and
+is in turn inherited by `SpdxPackage`, `SpdxFile`, and `SpdxSnippet`.
 
 Data members:
 

src/DemaConsulting.SpdxModel/IO/Spdx2JsonDeserializer.cs

@@ -249,14 +249,14 @@ public static SpdxSnippet DeserializeSnippet(JsonNode? json)
         {
             Id = ParseString(json, SpdxConstants.FieldSpdxId),
             SnippetFromFile = ParseString(json, SpdxConstants.FieldSnippetFromFile),
-            SnippetByteStart = Convert.ToInt32(Find(json, SpdxConstants.FieldRanges, SpdxConstants.FieldStartPointer,
-                SpdxConstants.FieldOffset)?.ToString() ?? ""),
-            SnippetByteEnd = Convert.ToInt32(Find(json, SpdxConstants.FieldRanges, SpdxConstants.FieldEndPointer,
-                SpdxConstants.FieldOffset)?.ToString() ?? ""),
-            SnippetLineStart = Convert.ToInt32(Find(json, SpdxConstants.FieldRanges, SpdxConstants.FieldStartPointer,
-                SpdxConstants.FieldLineNumber)?.ToString() ?? ""),
-            SnippetLineEnd = Convert.ToInt32(Find(json, SpdxConstants.FieldRanges, SpdxConstants.FieldEndPointer,
-                SpdxConstants.FieldLineNumber)?.ToString() ?? ""),
+            SnippetByteStart = int.TryParse(Find(json, SpdxConstants.FieldRanges, SpdxConstants.FieldStartPointer,
+                SpdxConstants.FieldOffset)?.ToString(), out var byteStart) ? byteStart : 0,
+            SnippetByteEnd = int.TryParse(Find(json, SpdxConstants.FieldRanges, SpdxConstants.FieldEndPointer,
+                SpdxConstants.FieldOffset)?.ToString(), out var byteEnd) ? byteEnd : 0,
+            SnippetLineStart = int.TryParse(Find(json, SpdxConstants.FieldRanges, SpdxConstants.FieldStartPointer,
+                SpdxConstants.FieldLineNumber)?.ToString(), out var lineStart) ? lineStart : 0,
+            SnippetLineEnd = int.TryParse(Find(json, SpdxConstants.FieldRanges, SpdxConstants.FieldEndPointer,
+                SpdxConstants.FieldLineNumber)?.ToString(), out var lineEnd) ? lineEnd : 0,
             ConcludedLicense = ParseString(json, SpdxConstants.FieldLicenseConcluded),
             LicenseInfoInSnippet = ParseStringArray(json, SpdxConstants.FieldLicenseInfoInSnippets),
             LicenseComments = ParseOptionalString(json, SpdxConstants.FieldLicenseComments),

src/DemaConsulting.SpdxModel/SpdxExternalDocumentReference.cs

@@ -33,7 +33,7 @@ public sealed class SpdxExternalDocumentReference
     ///     Equality comparer for the same external document reference
     /// </summary>
     /// <remarks>
-    ///     This considers packages as being the same if they have the same document.
+    ///     This considers external document references as being the same if they have the same document.
     /// </remarks>
     public static readonly IEqualityComparer<SpdxExternalDocumentReference> Same =
         new SpdxExternalDocumentReferenceSame();

src/DemaConsulting.SpdxModel/SpdxPackage.cs

@@ -304,7 +304,7 @@
         {
             VerificationCode.Enhance(other.VerificationCode);
         }
-        else
+        else if (VerificationCode == null)
         {
             VerificationCode = other.VerificationCode?.DeepCopy();
         }
@@ -387,7 +387,7 @@
     /// <param name="issues">List to populate with issues</param>
     /// <param name="doc">Optional document for checking file-references</param>
     /// <param name="ntia">Perform NTIA validation</param>
     public void Validate(List<string> issues, SpdxDocument? doc, bool ntia = false)
     {
         // Validate Package Name Field
         if (Name.Length == 0)

src/DemaConsulting.SpdxModel/Transform/SpdxRelationships.cs

@@ -45,8 +45,10 @@ public static void Add(SpdxDocument document, SpdxRelationship relationship)
             throw new ArgumentException($"Element {relationship.Id} not found in SPDX document", nameof(relationship));
         }
 
-        // Ensure the relationship related-element ID matches an element
-        if (document.GetElement(relationship.RelatedSpdxElement) == null)
+        // Ensure the relationship related-element ID matches an element, is NOASSERTION, or uses the DocumentRef- external-reference prefix
+        if (!relationship.RelatedSpdxElement.StartsWith("DocumentRef-") &&
+            relationship.RelatedSpdxElement != SpdxElement.NoAssertion &&
+            document.GetElement(relationship.RelatedSpdxElement) == null)
         {
             throw new ArgumentException($"Element {relationship.RelatedSpdxElement} not found in SPDX document",
                 nameof(relationship));

test/DemaConsulting.SpdxModel.Tests/IO/Spdx2JsonDeserializeSnippet.cs

@@ -105,6 +105,50 @@ public void Spdx2JsonDeserializer_DeserializeSnippet_CorrectResults()
         Assert.AreEqual("SPDXRef-DoapSource", snippet.SnippetFromFile);
     }
 
+    /// <summary>
+    ///     Tests deserializing a snippet that has only byte ranges and no optional line-number ranges.
+    ///     This is a regression test for a <see cref="FormatException" /> thrown when line-number
+    ///     range fields were absent and the old code used <c>Convert.ToInt32("")</c>.
+    /// </summary>
+    [TestMethod]
+    public void Spdx2JsonDeserializer_DeserializeSnippet_WithoutLineRanges_DefaultsToZero()
+    {
+        // Arrange: Create a JSON snippet with only byte ranges (no lineNumber entries)
+        var json = new JsonObject
+        {
+            ["SPDXID"] = "SPDXRef-Snippet",
+            ["copyrightText"] = "Copyright 2008-2010 John Smith",
+            ["licenseConcluded"] = "GPL-2.0-only",
+            ["licenseInfoInSnippets"] = new JsonArray { "GPL-2.0-only" },
+            ["ranges"] = new JsonArray
+            {
+                new JsonObject
+                {
+                    ["endPointer"] = new JsonObject
+                    {
+                        ["offset"] = 420,
+                        ["reference"] = "SPDXRef-DoapSource"
+                    },
+                    ["startPointer"] = new JsonObject
+                    {
+                        ["offset"] = 310,
+                        ["reference"] = "SPDXRef-DoapSource"
+                    }
+                }
+            },
+            ["snippetFromFile"] = "SPDXRef-DoapSource"
+        };
+
+        // Act: Deserialize the JSON object to an SpdxSnippet object (must not throw)
+        var snippet = Spdx2JsonDeserializer.DeserializeSnippet(json);
+
+        // Assert: Byte ranges are correct and absent line ranges default to 0
+        Assert.AreEqual(310, snippet.SnippetByteStart);
+        Assert.AreEqual(420, snippet.SnippetByteEnd);
+        Assert.AreEqual(0, snippet.SnippetLineStart);
+        Assert.AreEqual(0, snippet.SnippetLineEnd);
+    }
+
     /// <summary>
     ///     Tests deserializing multiple snippets.
     /// </summary>

test/DemaConsulting.SpdxModel.Tests/SpdxExternalDocumentReferenceTests.cs

@@ -172,7 +172,7 @@ public void SpdxExternalDocumentReference_Enhance_AddsOrUpdatesInformationCorrec
     }
 
     /// <summary>
-    ///     Tests the <see cref="SpdxExternalDocumentReference.Validate" /> method reports bad annotators
+    ///     Tests the <see cref="SpdxExternalDocumentReference.Validate" /> method reports missing external document ID
     /// </summary>
     [TestMethod]
     public void SpdxExternalDocumentReference_Validate_MissingId()
@@ -193,7 +193,7 @@ public void SpdxExternalDocumentReference_Validate_MissingId()
     }
 
     /// <summary>
-    ///     Tests the <see cref="SpdxExternalDocumentReference.Validate" /> method reports bad annotators
+    ///     Tests the <see cref="SpdxExternalDocumentReference.Validate" /> method reports missing document URI
     /// </summary>
     [TestMethod]
     public void SpdxExternalDocumentReference_Validate_MissingDocument()