Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ExampleFromSpecificationIsHandledCorrectlyWithLateDefine() test should fail #889

Open
dougbu opened this issue Jan 7, 2024 · 0 comments

Comments

@dougbu
Copy link
Contributor

dougbu commented Jan 7, 2024

Describe the bug

After the fixes from #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
    throw new AnchorNotFoundException(alias.Start, alias.End, $"Alias ${alias.Value} cannot precede anchor declaration");
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

No branches or pull requests

1 participant