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

Fixed location property on TypedElementOnSourceNode.cs #2747

Merged
merged 12 commits into from
Mar 27, 2024

Conversation

Kasdejong
Copy link
Contributor

Description

TypedElementOnSourceNode.Location should now correctly omit choice type suffixes (like in PocoElementNode)

Related issues

Fixes issue #2642

Testing

Added new unit tests testing some edge cases

@Kasdejong Kasdejong requested a review from ewoutkramer March 26, 2024 17:13
@@ -45,7 +48,7 @@ public TypedElementOnSourceNode(ISourceNode source, string type, IStructureDefin
throw Error.Format(nameof(type), $"Cannot determine the type of the root element at '{_source.Location}', " +
$"please supply a type argument.");
else
return (rootType, null);
return (rootType!, null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's consider this for a bit - for the first time we are kind of lying here, and we're now saying: "in 98% of the cases, this is non-null - so it's not null". But is 98% good enough? Or should we drop de bang and add a #pragma to make it clear we're dodging the nullability warning?

src/Hl7.Fhir.Base/ElementModel/TypedElementOnSourceNode.cs Outdated Show resolved Hide resolved
src/Hl7.Fhir.Base/ElementModel/TypedElementOnSourceNode.cs Outdated Show resolved Hide resolved
@ewoutkramer ewoutkramer enabled auto-merge March 27, 2024 09:39
@ewoutkramer ewoutkramer merged commit 6731010 into develop Mar 27, 2024
16 checks passed
@ewoutkramer ewoutkramer deleted the feature/location-itypedelement branch March 27, 2024 10:05
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.

3 participants