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

Not-Null Arrays when using --nullableReferenceAttributes #338

Merged
merged 4 commits into from
Jun 16, 2022

Conversation

las-nsc
Copy link
Contributor

@las-nsc las-nsc commented Jun 9, 2022

When applying the latest release to our project, all of the NRT warnings were fixed except one. The difference for this is one was that the complex type was referenced rather than inline, and just changing that has removed the nullable attributes, however since it is also never null in the initial state, I thought this change should be made. There's no rush to complete this, and I think it may need some more thought.

Combine IsCollection, IsArray and IsList into IsEnumerable, and use for nullable attribute prevention.
Moved GetSet into GetAccessors to avoid nested ternary
Tidied up ModelBuilder by encapsulating common groups of property setters on PropertyModel, using IXmlSchemaNode.

@codecov
Copy link

codecov bot commented Jun 9, 2022

Codecov Report

Merging #338 (ebe7649) into master (8204bee) will increase coverage by 0.05%.
The diff coverage is 97.35%.

@@            Coverage Diff             @@
##           master     #338      +/-   ##
==========================================
+ Coverage   95.53%   95.59%   +0.05%     
==========================================
  Files          19       20       +1     
  Lines        3044     2880     -164     
  Branches      465      445      -20     
==========================================
- Hits         2908     2753     -155     
+ Misses         78       71       -7     
+ Partials       58       56       -2     
Impacted Files Coverage Δ
XmlSchemaClassGenerator/ModelBuilder.cs 98.03% <96.94%> (+0.34%) ⬆️
XmlSchemaClassGenerator/TypeModel.cs 96.79% <98.21%> (+0.19%) ⬆️
XmlSchemaClassGenerator/Extensions.cs 100.00% <100.00%> (ø)
XmlSchemaClassGenerator/IXmlSchemaNode.cs 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8204bee...ebe7649. Read the comment docs.

var privateSetter = isPrivateSetter ? "private " : string.Empty;
member.Name += $" {{ get; {privateSetter}set; }}"; // hack to generate automatic property
}
member.Name += backingField != null
Copy link
Owner

Choose a reason for hiding this comment

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

Can you revert this? Sonarcloud complains about the nested ternary operator and I think they have a point.

@mganss
Copy link
Owner

mganss commented Jun 9, 2022

  • IsArray determines whether we can apply the XmlArrayItemAttribute optimization where the intermediate sequence disappears.
  • IsList signals that the element is an xs:list
  • IsCollection simply means that the corresponding element has maxOccurs > 1 and therefore we're mapping to a C# collection

las-nsc added 2 commits June 9, 2022 18:53
…idied up ModelBuilder (encapsulated common groups of property setters on PropertyModel, using IXmlSchemaNode).
@las-nsc
Copy link
Contributor Author

las-nsc commented Jun 9, 2022

No idea why a test failed (they pass locally), and can't figure out how to retry...

@mganss
Copy link
Owner

mganss commented Jun 10, 2022

Thanks for your hard work. Would love to merge this. Could you remove the remaining nested ternaries?

@mganss
Copy link
Owner

mganss commented Jun 10, 2022

No idea why a test failed (they pass locally), and can't figure out how to retry...

You can click on the ❌ then Details to view the output from AppVeyor. Manual retry doesn't seem to possible unless you're the owner.

@Nintynuts
Copy link

(This is my personal account)

I'm unable to make changes to the PR for 2 weeks. If there are particular ternary statements you want to revert, then go ahead, but personally I think if they're properly chained and formatted, then they're pretty clear and shouldn't be a problem. E.g.

return conditionA ? valueA
    : conditionB ? valueB
    : valueC;

@mganss mganss merged commit ebe7649 into mganss:master Jun 16, 2022
@las-nsc las-nsc deleted the las/non-nullable-arrays branch July 6, 2022 12:49
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