search

aaubry / YamlDotNet

YamlDotNet is a .NET library for YAML
MIT License
2.48k stars 466 forks source link

`ExampleFromSpecificationIsHandledCorrectlyWithLateDefine()` test should fail #889

Open dougbu opened 5 months ago

dougbu commented 5 months ago

Describe the bug

After the fixes from https://github.com/aaubry/YamlDotNet/pull/491, the SerializationTests.ExampleFromSpecificationIsHandledCorrectlyWithLateDefine() test should fail. Placement of anchors after aliases referring to them is not spec-compliant.

To Reproduce

Execute the SerializationTests.ExampleFromSpecificationIsHandledCorrectlyWithLateDefine() test.

Expected

Test should fail due to an AnchorNotFoundException or ForwardAnchorNotSupportedException (I'm not sure which). It's not clear whether forward references should always be disallowed or if the IList<> constraint is correct but over-zealous. If IList<> types should indeed be special cased, perhaps aliases must reference anchors defined in the same list or earlier in the document❔

My suspicion is special-casing IList<> is incorrect because ordering within a sequence node is semantically important. I haven't found when ForwardAnchorNotSupportedException or the IList<> constraint were introduced to get a sense of the reasoning. Nor have I found anything in the spec indicating a contextual relaxation of "first occurrence", "the most recent event in the serialization having the specified anchor", and similar wording.

Nits

  1. References to AnchorNotFoundException in ForwardAnchorNotSupportedException constructor doc comments are incorrect.
  2. Wording of the $"The anchor '{anchor}' does not exists" message should be $"The anchor '{anchor}' does not exist" (singular).
  3. Even better, should consistently use $"Anchor '{alias.Value}' not found" when AnchorNotFoundException is thrown and use ForwardAnchorNotSupportedException at https://github.com/aaubry/YamlDotNet/blob/847230593e95750d4294ca72c98a4bd46bdcf265/YamlDotNet/Serialization/ValueDeserializers/AliasValueDeserializer.cs#L107