From bb8d0df0eac29f0cb96774a4b47774de50233522 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Mon, 10 Mar 2025 10:02:28 -0700 Subject: [PATCH 1/3] Refactor to clean up CA1851 warnings --- .../EnumerableExtensions.cs | 34 +++++++++++++++++++ .../Semantic/RelationshipTypeConstraint.cs | 20 +++++------ .../Validation/Semantic/SemanticConstraint.cs | 30 ++++++---------- 3 files changed, 55 insertions(+), 29 deletions(-) create mode 100644 src/DocumentFormat.OpenXml.Framework/EnumerableExtensions.cs diff --git a/src/DocumentFormat.OpenXml.Framework/EnumerableExtensions.cs b/src/DocumentFormat.OpenXml.Framework/EnumerableExtensions.cs new file mode 100644 index 000000000..286663937 --- /dev/null +++ b/src/DocumentFormat.OpenXml.Framework/EnumerableExtensions.cs @@ -0,0 +1,34 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Collections.Generic; +using System.Linq; + +namespace DocumentFormat.OpenXml; + +internal static class EnumerableExtensions +{ + /// + /// Similar to but will also verify that at most there is one. + /// + public static T? FirstOrDefaultAndMaxOne(this IEnumerable enumerable, Func? exThrow = null) + { + using var e = enumerable.GetEnumerator(); + + if (e.MoveNext()) + { + var first = e.Current; + + if (e.MoveNext()) + { + throw exThrow?.Invoke() ?? throw new InvalidOperationException("Max of a single item should be in the enumerable"); + } + + return first; + } + + return default; + } +} + diff --git a/src/DocumentFormat.OpenXml.Framework/Validation/Semantic/RelationshipTypeConstraint.cs b/src/DocumentFormat.OpenXml.Framework/Validation/Semantic/RelationshipTypeConstraint.cs index 9c63d462e..f8166c6b7 100644 --- a/src/DocumentFormat.OpenXml.Framework/Validation/Semantic/RelationshipTypeConstraint.cs +++ b/src/DocumentFormat.OpenXml.Framework/Validation/Semantic/RelationshipTypeConstraint.cs @@ -21,7 +21,6 @@ public RelationshipTypeConstraint(OpenXmlQualifiedName attribute, string type) _type = type; } - [System.Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1851:Possible multiple enumerations of 'IEnumerable' collection", Justification = "https://github.com/dotnet/Open-XML-SDK/issues/1325")] public override ValidationErrorInfo? ValidateCore(ValidationContext context) { var current = context.Stack.Current; @@ -57,20 +56,21 @@ public RelationshipTypeConstraint(OpenXmlQualifiedName attribute, string type) var rels = current.Part.ExternalRelationships.Where(r => r.Id == attribute.Value.InnerText); - if (!rels.Any()) + if (rels.FirstOrDefault() is { } rel) { - var pairs = current.Part.Parts.Where(p => p.RelationshipId == attribute.Value.InnerText); + actualType = rel.RelationshipType; - if (pairs.Any()) - { - Debug.Assert(pairs.Count() == 1); - actualType = pairs.First().OpenXmlPart.RelationshipType; - } } else { - Debug.Assert(rels.Count() == 1); - actualType = rels.First().RelationshipType; + var pair = current.Part.Parts + .Where(p => p.RelationshipId == attribute.Value.InnerText) + .FirstOrDefaultAndMaxOne(); + + if (pair is { }) + { + actualType = pair.OpenXmlPart.RelationshipType; + } } if (actualType == _type) diff --git a/src/DocumentFormat.OpenXml.Framework/Validation/Semantic/SemanticConstraint.cs b/src/DocumentFormat.OpenXml.Framework/Validation/Semantic/SemanticConstraint.cs index 4d58f8851..6793e17ec 100644 --- a/src/DocumentFormat.OpenXml.Framework/Validation/Semantic/SemanticConstraint.cs +++ b/src/DocumentFormat.OpenXml.Framework/Validation/Semantic/SemanticConstraint.cs @@ -16,7 +16,6 @@ namespace DocumentFormat.OpenXml.Validation.Semantic /// /// Base class for each semantic constraint category. /// - [System.Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1851:Possible multiple enumerations of 'IEnumerable' collection", Justification = "https://github.com/dotnet/Open-XML-SDK/issues/1325")] internal abstract class SemanticConstraint : IValidator { public SemanticConstraint(SemanticValidationLevel level) @@ -130,13 +129,10 @@ private static void Get(ValidationContext context, out SemanticValidationLevel l } else if (parts[0] == "..") { - var refParts = current.Package + return current.Package .GetAllParts() - .Where(p => p.Parts.Any(r => r.OpenXmlPart.PackagePart.Uri == current.Part.PackagePart.Uri)); - - Debug.Assert(refParts.Count() == 1); - - return refParts.First(); + .Where(p => p.Parts.Any(r => r.OpenXmlPart.PackagePart.Uri == current.Part.PackagePart.Uri)) + .First(); } else { @@ -247,29 +243,25 @@ protected static bool GetAttrNumVal(OpenXmlSimpleType attributeValue, out double private static OpenXmlPart? GetPartThroughPartPath(IEnumerable pairs, string[] path) { - var temp = default(OpenXmlPart); + var foundPart = default(OpenXmlPart); var parts = pairs; for (int i = 0; i < path.Length; i++) { - var s = parts.Where(p => p.OpenXmlPart.GetType().Name == path[i]).Select(t => t.OpenXmlPart); - var count = s.Count(); - - if (count > 1) - { - throw new System.IO.FileFormatException(ValidationResources.MoreThanOnePartForOneUri); - } + foundPart = parts + .Where(p => p.OpenXmlPart.GetType().Name == path[i]) + .Select(t => t.OpenXmlPart) + .FirstOrDefaultAndMaxOne(static () => new System.IO.FileFormatException(ValidationResources.MoreThanOnePartForOneUri)); - if (count == 0) + if (foundPart is not { }) { return null; } - temp = s.First(); - parts = temp.Parts; + parts = foundPart.Parts; } - return temp; + return foundPart; } protected readonly struct PartHolder From fb9ea52016b866441168331bab347087d52620ac Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Mon, 10 Mar 2025 10:10:59 -0700 Subject: [PATCH 2/3] fix whitespace --- src/DocumentFormat.OpenXml.Framework/EnumerableExtensions.cs | 1 - .../Validation/Semantic/RelationshipTypeConstraint.cs | 1 - 2 files changed, 2 deletions(-) diff --git a/src/DocumentFormat.OpenXml.Framework/EnumerableExtensions.cs b/src/DocumentFormat.OpenXml.Framework/EnumerableExtensions.cs index 286663937..86ef68cc6 100644 --- a/src/DocumentFormat.OpenXml.Framework/EnumerableExtensions.cs +++ b/src/DocumentFormat.OpenXml.Framework/EnumerableExtensions.cs @@ -31,4 +31,3 @@ internal static class EnumerableExtensions return default; } } - diff --git a/src/DocumentFormat.OpenXml.Framework/Validation/Semantic/RelationshipTypeConstraint.cs b/src/DocumentFormat.OpenXml.Framework/Validation/Semantic/RelationshipTypeConstraint.cs index f8166c6b7..2e04de7dc 100644 --- a/src/DocumentFormat.OpenXml.Framework/Validation/Semantic/RelationshipTypeConstraint.cs +++ b/src/DocumentFormat.OpenXml.Framework/Validation/Semantic/RelationshipTypeConstraint.cs @@ -59,7 +59,6 @@ public RelationshipTypeConstraint(OpenXmlQualifiedName attribute, string type) if (rels.FirstOrDefault() is { } rel) { actualType = rel.RelationshipType; - } else { From 923a0526ae58bc57c60c1c4aa9adff194c36025a Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Mon, 10 Mar 2025 10:19:16 -0700 Subject: [PATCH 3/3] per feedback --- .../EnumerableExtensions.cs | 4 ++-- .../Resources/ExceptionMessages.Designer.cs | 9 +++++++++ .../Resources/ExceptionMessages.resx | 3 +++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/DocumentFormat.OpenXml.Framework/EnumerableExtensions.cs b/src/DocumentFormat.OpenXml.Framework/EnumerableExtensions.cs index 86ef68cc6..c13d46d06 100644 --- a/src/DocumentFormat.OpenXml.Framework/EnumerableExtensions.cs +++ b/src/DocumentFormat.OpenXml.Framework/EnumerableExtensions.cs @@ -12,7 +12,7 @@ internal static class EnumerableExtensions /// /// Similar to but will also verify that at most there is one. /// - public static T? FirstOrDefaultAndMaxOne(this IEnumerable enumerable, Func? exThrow = null) + public static T? FirstOrDefaultAndMaxOne(this IEnumerable enumerable, Func? exceptionFactory = null) { using var e = enumerable.GetEnumerator(); @@ -22,7 +22,7 @@ internal static class EnumerableExtensions if (e.MoveNext()) { - throw exThrow?.Invoke() ?? throw new InvalidOperationException("Max of a single item should be in the enumerable"); + throw exceptionFactory?.Invoke() ?? throw new InvalidOperationException(ExceptionMessages.FirstOrDefaultMaxOne); } return first; diff --git a/src/DocumentFormat.OpenXml.Framework/Resources/ExceptionMessages.Designer.cs b/src/DocumentFormat.OpenXml.Framework/Resources/ExceptionMessages.Designer.cs index 8419ed6c2..cf1eaed38 100644 --- a/src/DocumentFormat.OpenXml.Framework/Resources/ExceptionMessages.Designer.cs +++ b/src/DocumentFormat.OpenXml.Framework/Resources/ExceptionMessages.Designer.cs @@ -330,6 +330,15 @@ internal static string FileNotFound { } } + /// + /// Looks up a localized string similar to The enumerable contained more than a single element when only zero or one are allowed.. + /// + internal static string FirstOrDefaultMaxOne { + get { + return ResourceManager.GetString("FirstOrDefaultMaxOne", resourceCulture); + } + } + /// /// Looks up a localized string similar to The root XML element "{0}" in the part is incorrect. The expected root XML element is: "{1}".. /// diff --git a/src/DocumentFormat.OpenXml.Framework/Resources/ExceptionMessages.resx b/src/DocumentFormat.OpenXml.Framework/Resources/ExceptionMessages.resx index 7ce73d0a8..b0aaa58a4 100644 --- a/src/DocumentFormat.OpenXml.Framework/Resources/ExceptionMessages.resx +++ b/src/DocumentFormat.OpenXml.Framework/Resources/ExceptionMessages.resx @@ -411,4 +411,7 @@ Package could not be opened for stream. See inner exception for details and be aware that there are behavior differences in stream support between .NET Framework and Core. + + The enumerable contained more than a single element when only zero or one are allowed. + \ No newline at end of file