computed circular reference error messages

src/Firely.Fhir.Validation/Impl/ResourceSchema.cs

@@ -125,12 +125,13 @@ internal ResultReport ValidateResourceSchema(IScopedNode input, ValidationSettin
         {
             return state.Global.RunValidations.Start(
                 state,
-                Id.ToString(),  // is the same as the canonical for resource schemas
+                Id.ToString(), // is the same as the canonical for resource schemas
                 () =>
                 {
                     state.Global.ResourcesValidated += 1;
                     return base.ValidateInternal(input, vc, state);
-                });
+                },
+                () => input.ComputeReferenceCycle(vc));
         }
 
         /// <inheritdoc/>

src/Firely.Fhir.Validation/Schema/ValidationLogger.cs

@@ -14,8 +14,6 @@
 
 namespace Firely.Fhir.Validation
 {
-
-
     /// <summary>
     /// This class is used to track which resources/contained resources/bundled resources are already
     /// validated, and what the previous result was. Since it keeps track of running validations, it
@@ -68,9 +66,10 @@ public override string ToString() =>
         /// <param name="state">The validation state</param>
         /// <param name="profileUrl">Profile against which we are validating</param>
         /// <param name="validator">Validation to start when it has not been run before.</param>
+        /// <param name="refCycleVisualizer">A function which visualises reference cycles when called</param>
         /// <returns>The result of calling the validator, or a historic result if there is one.</returns>
 #pragma warning disable CS0618 // Type or member is obsolete
-        public ResultReport Start(ValidationState state, string profileUrl, Func<ResultReport> validator)
+        public ResultReport Start(ValidationState state, string profileUrl, Func<ResultReport> validator, Func<string?> refCycleVisualizer)
 #pragma warning restore CS0618 // Type or member is obsolete
         {
             var resourceUrl = state.Instance.ResourceUrl;
@@ -80,17 +79,17 @@ public ResultReport Start(ValidationState state, string profileUrl, Func<ResultR
 
             if (_data.TryGetValue(key, out var existing))
             {
-                if (existing.Result is null)
+                if (existing.Result is null && refCycleVisualizer() is { } referenceCycle) // check for loops: we do not include references to containers in the cycle
                     return new IssueAssertion(Issue.CONTENT_REFERENCE_CYCLE_DETECTED,
-                     $"Detected a loop: instance data inside '{fullLocation}' refers back to itself.")
+                            $"Detected a loop: instance data inside '{fullLocation}' refers back to itself (cycle structure: {referenceCycle}).")
                         .AsResult(fullLocation);
-                else
-                {
-                    // If the validation has been run before, return an outcome with the same result.
-                    // Note: we don't keep a copy of the original outcome since it has been included in the
-                    // total result (at the first run) and keeping them around costs a lot of memory.
-                    return new ResultReport(existing.Result.Value, new TraceAssertion(fullLocation, $"Repeated validation at {fullLocation} against profile {profileUrl}."));
-                }
+
+                // If the validation has been run before, return an outcome with the same result.
+                // Note: we don't keep a copy of the original outcome since it has been included in the
+                // total result (at the first run) and keeping them around costs a lot of memory.
+                return existing.Result is not null
+                    ? new ResultReport(existing.Result.Value, new TraceAssertion(fullLocation, $"Repeated validation at {fullLocation} against profile {profileUrl}."))
+                    : ResultReport.SUCCESS;
             }
             else
             {

src/Firely.Fhir.Validation/Support/IScopedNodeExtensions.cs

@@ -93,6 +93,44 @@ internal static bool IsExactlyEqualTo(this IScopedNode left, ITypedElement right
                 (childL, childR) => childL.Name == childR.Name && childL.IsExactlyEqualTo(childR, ignoreOrder)).All(t => t);
         }
 
+        internal static string? ComputeReferenceCycle(this IScopedNode current, ValidationSettings vc) =>
+            current.ToScopedNode().ComputeReferenceCycle(vc);
+
+        internal static string? ComputeReferenceCycle(this ScopedNode current, ValidationSettings vc, IList<(string, string)>? followed = null) // this is expensive, but only executed when a loop is detected. We accept this
+        {
+            followed ??= [];
+
+            foreach (var child in current.Children())
+            {
+                var childNode = child.ToScopedNode();
+
+                if (childNode.InstanceType == "Reference") 
+                {
+                    var target = childNode.Resolve(url => vc.ResolveExternalReference is { } resolve ? resolve(url, childNode.Location) : null)?.ToScopedNode();
+                    if (target is null || (childNode.Location.StartsWith(target.Location + ".contained["))) // external reference, or reference to parent container: we do not include these in the cycle
+                    {
+                        continue;
+                    }
+
+                    followed.Add((childNode.Location, target.Location)); // add the reference to the list of followed references
+
+                    if (followed.Count(tuple => tuple == (childNode.Location, target.Location)) is 2) // if we followed the same reference twice, we have a loop
+                    {
+                        return string.Join(" | ", followed.Select(reference => $"{reference.Item1} -> {reference.Item2}"));
+                    }
+
+                    if(ComputeReferenceCycle(target, vc, followed) is { } result) 
+                        return result; // if multiple paths are found, we only return the first one. Rerunning will show the next one. Let's hope that never happens.
+                }
+
+                if (ComputeReferenceCycle(childNode, vc, followed) is { } result2) // why is result still in scope? that makes no sense
+                {
+                    return result2;
+                }
+            }
+
+            return null;
+        }
     }
 }
 

test/Firely.Fhir.Validation.Compilation.Tests.Shared/FhirTests/ValidationManifestTest.cs

@@ -32,7 +32,7 @@ public class ValidationManifestTest
         /// </summary>
         //[Ignore]
         [DataTestMethod]
-        [ValidationManifestDataSource(TEST_CASES_MANIFEST, singleTest: "mr-m-simple-nossystem")]
+        [ValidationManifestDataSource(TEST_CASES_MANIFEST, singleTest: "message-empty-entry")]
         public void RunSingleTest(TestCase testCase, string baseDirectory)
             => _runner.RunTestCase(testCase, DotNetValidator.Create(), baseDirectory);
 
@@ -59,7 +59,7 @@ public void RunFirelySdkTests(TestCase testCase, string baseDirectory)
         /// - The method `ClassCleanup` will gather all the testcases and serialize those to disk. The filename can be altered in
         /// that method
         /// </summary>
-        [Ignore("This test is only used to generate the Firely SDK results in the manifest. See the method for more info")]
+        // [Ignore("This test is only used to generate the Firely SDK results in the manifest. See the method for more info")]
         [TestMethod]
         public void AddFirelySdkValidatorResults()
                     => _runner.AddOrEditValidatorResults(TEST_CASES_MANIFEST, new[] { DotNetValidator.Create() });

test/Firely.Fhir.Validation.Tests/Impl/CircularReferenceTests.cs

@@ -10,6 +10,7 @@
 using Hl7.Fhir.ElementModel;
 using Hl7.Fhir.Support;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
+using System.Linq;
 
 namespace Firely.Fhir.Validation.Tests
 {
@@ -90,6 +91,7 @@ public void CircularInContainedResources()
             var result = test(SCHEMA, pat.DictionaryToTypedElement("Patient"));
             result.IsSuccessful.Should().BeTrue();
             result.Evidence.Should().Contain(ass => (ass as IssueAssertion)!.IssueNumber == Issue.CONTENT_REFERENCE_CYCLE_DETECTED.Code);
+            result.Warnings.Should().ContainSingle(ass => ass.IssueNumber == Issue.CONTENT_REFERENCE_CYCLE_DETECTED.Code).Subject.Message.Should().ContainAll(["Patient.contained[1]", "Patient.contained[0]"]);
         }
 
         [TestMethod]