Skip to content

Root element for generated parts should be nullable #1922

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Apr 28, 2025
Merged

Root element for generated parts should be nullable #1922

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b4c3bde
make root element properties for derived parts to be nullable
tomjebo Apr 25, 2025
8d19536
Merge remote-tracking branch 'upstream/main' into issue995
tomjebo Apr 25, 2025
1b11370
update utility to check for null
tomjebo Apr 25, 2025
81ac063
remove [DisallowNull]
tomjebo Apr 25, 2025
eba7b64
turn on nullable for projects that include part root element tests
tomjebo Apr 25, 2025
10cbdbb
use First instead of FirstOrDefault
tomjebo Apr 28, 2025
bc91d0a
revert to previous code
tomjebo Apr 28, 2025
4cd86d7
Merge remote-tracking branch 'upstream/main' into issue995
tomjebo Apr 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion gen/DocumentFormat.OpenXml.Generator.Models/Generators/Parts/PartWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public static class PartWriter
"DocumentFormat.OpenXml.Framework",
"System",
"System.Collections.Generic",
"System.Diagnostics.CodeAnalysis",
};

private static readonly BlockOptions _options = new()
Expand Down Expand Up @@ -408,9 +409,10 @@ private static IEnumerable<Item> GetSchemaTypedParts(OpenXmlGeneratorServices se
yield return new(ItemType.Property, api.Class, writer =>
{
writer.WriteDocumentationComment("Gets or sets the root element of this part.");
writer.WriteLine("[DisallowNull]");
writer.Write("public ");
writer.Write(apiName);
writer.Write(" ");
writer.Write("? ");
Copy link
Preview

Copilot AI Apr 25, 2025

Choose a reason for hiding this comment

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

Applying [DisallowNull] on a property whose type is rendered as nullable (using '?') is contradictory; please ensure that the nullability annotations and attributes are consistent.

Suggested change
writer.Write("? ");
writer.Write(" ");

Copilot uses AI. Check for mistakes.

Copy link
Member

Choose a reason for hiding this comment

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

It's not contradictory (there are times you may want this), but @tomjebo why do have both? This means it may be null, but a user can't set it to that. Do we want that?

Copy link
Collaborator Author

@tomjebo tomjebo Apr 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@twsouthwick This was my misreading of your comment here in 959. I read that as [DisallowNull] and set the type to nullable with ?. I think you must have meant or but I was guessing there was a reason for wanting both. I'll take out the attribute [DisallowNull] as I think the root element property will have to be null at some point.

Copy link
Member

Choose a reason for hiding this comment

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

It also may have been early on in nullables and I misunderstood the point of it :) for now, just setting it to be nullable is good enough

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

so if the point is only to prevent assignment to null via static code analysis then I'm thinking we shouldn't use this as a way to enforce part root element minOccurs conformance. we should allow code to set to null (assuming eventually it will be set to a valid root element as required by the schemas) and then rely on our validation metadata to correctly catch the minOccurs constraint. that's my feeling. do you agree?

Copy link
Member

Choose a reason for hiding this comment

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

that sounds reasonable for now (that's the current behavior, right?)

writer.WriteLine(api.Class);

using (writer.AddBlock(_options))
Expand Down
1 change: 1 addition & 0 deletions ...r/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_AlternativeFormatImportPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand Down
4 changes: 3 additions & 1 deletion ...enerator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_CalculationChainPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand All @@ -32,7 +33,8 @@ internal CalculationChainPart()
/// <summary>
/// Gets or sets the root element of this part.
/// </summary>
public DocumentFormat.OpenXml.Spreadsheet.CalculationChain CalculationChain
[DisallowNull]
public DocumentFormat.OpenXml.Spreadsheet.CalculationChain? CalculationChain
{
get
{
Expand Down
4 changes: 3 additions & 1 deletion ...ml.Generator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_CellMetadataPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand Down Expand Up @@ -48,7 +49,8 @@ private protected override OpenXmlPartRootElement? InternalRootElement
/// <summary>
/// Gets or sets the root element of this part.
/// </summary>
public DocumentFormat.OpenXml.Spreadsheet.Metadata Metadata
[DisallowNull]
public DocumentFormat.OpenXml.Spreadsheet.Metadata? Metadata
{
get
{
Expand Down
4 changes: 3 additions & 1 deletion ...Generator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_ChartColorStylePart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand All @@ -32,7 +33,8 @@ internal ChartColorStylePart()
/// <summary>
/// Gets or sets the root element of this part.
/// </summary>
public DocumentFormat.OpenXml.Office2013.Drawing.ChartStyle.ColorStyle ColorStyle
[DisallowNull]
public DocumentFormat.OpenXml.Office2013.Drawing.ChartStyle.ColorStyle? ColorStyle
{
get
{
Expand Down
4 changes: 3 additions & 1 deletion ...ml.Generator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_ChartDrawingPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand Down Expand Up @@ -69,7 +70,8 @@ private protected override OpenXmlPartRootElement? InternalRootElement
/// <summary>
/// Gets or sets the root element of this part.
/// </summary>
public DocumentFormat.OpenXml.Drawing.Charts.UserShapes UserShapes
[DisallowNull]
public DocumentFormat.OpenXml.Drawing.Charts.UserShapes? UserShapes
{
get
{
Expand Down
4 changes: 3 additions & 1 deletion ...t.OpenXml.Generator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_ChartPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand Down Expand Up @@ -44,7 +45,8 @@ internal ChartPart()
/// <summary>
/// Gets or sets the root element of this part.
/// </summary>
public DocumentFormat.OpenXml.Drawing.Charts.ChartSpace ChartSpace
[DisallowNull]
public DocumentFormat.OpenXml.Drawing.Charts.ChartSpace? ChartSpace
{
get
{
Expand Down
4 changes: 3 additions & 1 deletion ...nXml.Generator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_ChartStylePart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand All @@ -32,7 +33,8 @@ internal ChartStylePart()
/// <summary>
/// Gets or sets the root element of this part.
/// </summary>
public DocumentFormat.OpenXml.Office2013.Drawing.ChartStyle.ChartStyle ChartStyle
[DisallowNull]
public DocumentFormat.OpenXml.Office2013.Drawing.ChartStyle.ChartStyle? ChartStyle
{
get
{
Expand Down
4 changes: 3 additions & 1 deletion ...nXml.Generator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_ChartsheetPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand All @@ -33,7 +34,8 @@ internal ChartsheetPart()
/// <summary>
/// Gets or sets the root element of this part.
/// </summary>
public DocumentFormat.OpenXml.Spreadsheet.Chartsheet Chartsheet
[DisallowNull]
public DocumentFormat.OpenXml.Spreadsheet.Chartsheet? Chartsheet
{
get
{
Expand Down
4 changes: 3 additions & 1 deletion ....Generator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_CommentAuthorsPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand All @@ -32,7 +33,8 @@ internal CommentAuthorsPart()
/// <summary>
/// Gets or sets the root element of this part.
/// </summary>
public DocumentFormat.OpenXml.Presentation.CommentAuthorList CommentAuthorList
[DisallowNull]
public DocumentFormat.OpenXml.Presentation.CommentAuthorList? CommentAuthorList
{
get
{
Expand Down
4 changes: 3 additions & 1 deletion ...Xml.Generator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_ConnectionsPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand All @@ -32,7 +33,8 @@ internal ConnectionsPart()
/// <summary>
/// Gets or sets the root element of this part.
/// </summary>
public DocumentFormat.OpenXml.Spreadsheet.Connections Connections
[DisallowNull]
public DocumentFormat.OpenXml.Spreadsheet.Connections? Connections
{
get
{
Expand Down
4 changes: 3 additions & 1 deletion ...nerator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_ControlPropertiesPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand All @@ -35,7 +36,8 @@ internal ControlPropertiesPart()
/// <summary>
/// Gets or sets the root element of this part.
/// </summary>
public DocumentFormat.OpenXml.Office2010.Excel.FormControlProperties FormControlProperties
[DisallowNull]
public DocumentFormat.OpenXml.Office2010.Excel.FormControlProperties? FormControlProperties
{
get
{
Expand Down
1 change: 1 addition & 0 deletions ...erator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_CoreFilePropertiesPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand Down
1 change: 1 addition & 0 deletions ...nXml.Generator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_CustomDataPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand Down
4 changes: 3 additions & 1 deletion ...ator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_CustomDataPropertiesPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand Down Expand Up @@ -40,7 +41,8 @@ internal CustomDataPropertiesPart()
/// <summary>
/// Gets or sets the root element of this part.
/// </summary>
public DocumentFormat.OpenXml.Office2010.Excel.DatastoreItem DatastoreItem
[DisallowNull]
public DocumentFormat.OpenXml.Office2010.Excel.DatastoreItem? DatastoreItem
{
get
{
Expand Down
4 changes: 3 additions & 1 deletion ...ator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_CustomFilePropertiesPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand Down Expand Up @@ -50,7 +51,8 @@ private protected override OpenXmlPartRootElement? InternalRootElement
/// <summary>
/// Gets or sets the root element of this part.
/// </summary>
public DocumentFormat.OpenXml.CustomProperties.Properties Properties
[DisallowNull]
public DocumentFormat.OpenXml.CustomProperties.Properties? Properties
{
get
{
Expand Down
1 change: 1 addition & 0 deletions ....Generator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_CustomPropertyPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand Down
4 changes: 3 additions & 1 deletion ...nerator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_CustomXmlMappingsPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand Down Expand Up @@ -48,7 +49,8 @@ private protected override OpenXmlPartRootElement? InternalRootElement
/// <summary>
/// Gets or sets the root element of this part.
/// </summary>
public DocumentFormat.OpenXml.Spreadsheet.MapInfo MapInfo
[DisallowNull]
public DocumentFormat.OpenXml.Spreadsheet.MapInfo? MapInfo
{
get
{
Expand Down
1 change: 1 addition & 0 deletions ...enXml.Generator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_CustomXmlPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand Down
4 changes: 3 additions & 1 deletion ...rator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_CustomXmlPropertiesPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand All @@ -35,7 +36,8 @@ internal CustomXmlPropertiesPart()
/// <summary>
/// Gets or sets the root element of this part.
/// </summary>
public DocumentFormat.OpenXml.CustomXmlDataProperties.DataStoreItem DataStoreItem
[DisallowNull]
public DocumentFormat.OpenXml.CustomXmlDataProperties.DataStoreItem? DataStoreItem
{
get
{
Expand Down
4 changes: 3 additions & 1 deletion ...l.Generator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_CustomizationPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand Down Expand Up @@ -53,7 +54,8 @@ private protected override OpenXmlPartRootElement? InternalRootElement
/// <summary>
/// Gets or sets the root element of this part.
/// </summary>
public DocumentFormat.OpenXml.Office.Word.TemplateCommandGroup TemplateCommandGroup
[DisallowNull]
public DocumentFormat.OpenXml.Office.Word.TemplateCommandGroup? TemplateCommandGroup
{
get
{
Expand Down
4 changes: 3 additions & 1 deletion ...l.Generator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_DiagramColorsPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand All @@ -32,7 +33,8 @@ internal DiagramColorsPart()
/// <summary>
/// Gets or sets the root element of this part.
/// </summary>
public DocumentFormat.OpenXml.Drawing.Diagrams.ColorsDefinition ColorsDefinition
[DisallowNull]
public DocumentFormat.OpenXml.Drawing.Diagrams.ColorsDefinition? ColorsDefinition
{
get
{
Expand Down
4 changes: 3 additions & 1 deletion ...Xml.Generator/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_DiagramDataPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand Down Expand Up @@ -36,7 +37,8 @@ internal DiagramDataPart()
/// <summary>
/// Gets or sets the root element of this part.
/// </summary>
public DocumentFormat.OpenXml.Drawing.Diagrams.DataModelRoot DataModelRoot
[DisallowNull]
public DocumentFormat.OpenXml.Drawing.Diagrams.DataModelRoot? DataModelRoot
{
get
{
Expand Down
4 changes: 3 additions & 1 deletion ...r/DocumentFormat.OpenXml.Generator.OpenXmlGenerator/Part_DiagramLayoutDefinitionPart.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using DocumentFormat.OpenXml.Framework;
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace DocumentFormat.OpenXml.Packaging
{
Expand Down Expand Up @@ -54,7 +55,8 @@ private protected override OpenXmlPartRootElement? InternalRootElement
/// <summary>
/// Gets or sets the root element of this part.
/// </summary>
public DocumentFormat.OpenXml.Drawing.Diagrams.LayoutDefinition LayoutDefinition
[DisallowNull]
public DocumentFormat.OpenXml.Drawing.Diagrams.LayoutDefinition? LayoutDefinition
{
get
{
Expand Down
Loading
Loading