Skip to content

[Fix] skip elements in a foreign namespace instead of reading them as ReqIF elements; fixes #97 - #100

Merged
samatstariongroup merged 2 commits into
developmentfrom
GH97
Oct 19, 2026
Merged

samatstariongroup merged 2 commits into
developmentfrom
GH97

Conversation

@samatstariongroup

Copy link
Copy Markdown
Member

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the ReqIFSharp code style guidelines
  • I have provided test coverage for my change (where applicable)

Description

Fixes #97.

The deserializer matched ReqIF elements on their local name only. An element in another namespace with the name of a ReqIF element, for example <other:THE-VALUE xmlns:other="..."> inside an ATTRIBUTE-VALUE-XHTML, was read as that ReqIF element.

Rule

An element is read as a ReqIF element when it is in the ReqIF namespace or in no namespace, in any document. Any other element is skipped, with its content, and a warning is logged. This differs from the proposal in the issue (the namespace of the REQ-IF root); the reason is in this comment. XHTML content and tool extensions are read as raw XML, as before.

How

  • New internal helpers in XmlReaderExtensions, each with an async variant that takes a cancellation token:
    • SkipForeignElement: skips a foreign element, logs the warning, and leaves the reader on its end tag, so the next Read() does not pass over a sibling in minified XML (see [Bug]: Elements are skipped when a ReqIF document has no whitespace between elements #94).
    • ReadToReqIFDescendant: like ReadToDescendant, but skips foreign elements on the way; used for the 30 reference reads.
    • SkipForeignElementsToEnd: reads the rest of a TYPE, OBJECT or DEFINITION subtree, so a foreign element after a reference is reported too.
  • Acceptance criterion "no element match uses the local name alone": instead of a per-match IsReqIFElement helper, SkipForeignElement runs at the top of every element loop, sync and async (58 places), before the existing switch (reader.LocalName). The switch statements in SpecRelation and RelationGroup.ReadObjectSpecificElements are reached only through the checked loop in SpecElementWithAttributes. The LocalName == "object" match in AttributeValueXHTML reads XHTML content and is left out on purpose.
  • The single-reference reads in the attribute definitions and enumeration values now use ReadElementContentAsStringToEndElement (from [Bug]: Elements are skipped when a ReqIF document has no whitespace between elements #94), so an element after a reference is not passed over in minified documents.

Also fixed

  • The AttributeValue constructors did not keep the ILoggerFactory, so attribute values never logged anything. With that fixed, enumeration values no longer report their own element and their DEFINITION and VALUES elements as not supported.
  • EnumValue, Specification and AttributeValueString got a logger; AlternativeId uses the logger of its owner; the private async readers now receive the cancellation token of their caller.

Tests

  • New ForeignNamespaceTestFixture:
    • a foreign element at 64 places (root, header, content sections, datatypes, spec types, all attribute definitions, spec objects, attribute values including default values, XHTML values, specifications, spec hierarchies, spec relations, relation groups, alternative ids, tool extensions, and before and after every kind of reference). Each case is read indented and minified, .reqif and .reqifz, sync and async, and gives the same object graph as the original and exactly one warning;
    • a reference container with only a foreign reference is read as empty, and the search does not run into the next container;
    • no element of any readable test document is skipped (original, minified, prefixed and without namespace copies), including XHTML and tool-extension content;
    • an element in no namespace in a document in the ReqIF namespace is still read;
    • attribute values created with a logger factory log a skipped element.
  • XmlReaderExtensionsTestFixture: the reader position, return values, warning text and cancellation of the three helpers.
  • AttributeValueEnumerationTestFixture: reading enumeration values logs no warnings.
  • Mutation checks were run for each helper and each fix (no skip, skip without consuming, Skip() moving past, search without skip, no subtree walk, depth check off by one, XHTML loop not moving on, the logger factory dropped, the enumeration cases removed, the old ReadElementContentAsString); every one makes tests fail.
  • dotnet build -c Release: 0 warnings; dotnet test: 550 passed.

Related, tracked separately: #98 (the public constructors of AttributeValueXHTML leave ExternalObjects or the logger null).

🤖 Generated with Claude Code

samatstarion and others added 2 commits October 19, 2026 13:19
… ReqIF elements; fixes #97

Elements were matched on their local name only, so an element in another
namespace with the name of a ReqIF element (for example
<other:THE-VALUE>) was read as that ReqIF element.

- Add XmlReaderExtensions.SkipForeignElement(Async): an element in a
  namespace other than the ReqIF namespace or no namespace is skipped with
  its content and a warning is logged; the reader is left on its end tag.
- Call it at the top of every element loop of the reader, sync and async.
- Read references with ReadToReqIFDescendant(Async), which skips foreign
  elements, and read the rest of TYPE, OBJECT and DEFINITION subtrees with
  SkipForeignElementsToEnd(Async), so a foreign element before or after a
  reference is reported.
- Read single references with ReadElementContentAsStringToEndElement in the
  attribute definitions and enumeration values, so a sibling after a
  reference is not passed over in minified documents (see #94).
- Elements in no namespace are still read in any document (decision noted
  on #97).
- Keep the logger factory in the AttributeValue constructors: attribute
  values did not log anything. Enumeration values no longer report their
  own element and DEFINITION and VALUES as not supported.
- Give EnumValue, Specification and AttributeValueString a logger, pass
  the owner's logger to AlternativeId, and pass the cancellation token to
  the private async readers.
- Add ForeignNamespaceTestFixture and tests for the new helpers; move the
  prefix and no-namespace helpers into TestDocuments.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…children and reference search positions; refs #97

- A foreign DEFINITION after the ReqIF one in Boolean, Date, Integer, Real
  and String attribute values is skipped with a warning.
- An unknown ReqIF element in an enumeration value is still reported as
  not supported.
- AttributeValueString created with a logger factory logs a skipped element.
- ReadToReqIFDescendant started on an end tag or on whitespace.
- A namespace that differs from the ReqIF namespace only in case is foreign.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
5.3% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@samatstariongroup
samatstariongroup merged commit b21f8fc into development Oct 19, 2026
7 of 8 checks passed
@samatstariongroup
samatstariongroup deleted the GH97 branch October 19, 2026 11:53
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

Successfully merging this pull request may close these issues.

[Bug]: Elements are matched on local name only, so elements in another namespace are read as ReqIF elements

1 participant