Skip to content

Conversation

twsouthwick
Copy link
Member

Fixes #1325

@twsouthwick twsouthwick requested a review from Copilot March 10, 2025 17:13
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This PR refactors code to remove CA1851 warnings by replacing multiple enumerations of IEnumerable with a new extension method and removing suppression attributes.

  • Introduced the FirstOrDefaultAndMaxOne extension method in EnumerableExtensions.cs
  • Removed CA1851 suppression attributes and replaced enumerations with the new extension method in SemanticConstraint.cs and RelationshipTypeConstraint.cs

Reviewed Changes

File Description
src/DocumentFormat.OpenXml.Framework/EnumerableExtensions.cs Added extension method to safely return the first (or default) element while ensuring at most one element exists.
src/DocumentFormat.OpenXml.Framework/Validation/Semantic/SemanticConstraint.cs Removed a suppression attribute and refactored part lookup to use LINQ with FirstOrDefaultAndMaxOne.
src/DocumentFormat.OpenXml.Framework/Validation/Semantic/RelationshipTypeConstraint.cs Removed a suppression attribute and refactored relationship lookup to use FirstOrDefaultAndMaxOne.

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (3)

src/DocumentFormat.OpenXml.Framework/EnumerableExtensions.cs:15

  • [nitpick] Consider renaming the parameter 'exThrow' to 'exceptionFactory' for improved clarity.
public static T? FirstOrDefaultAndMaxOne<T>(this IEnumerable<T> enumerable, Func<Exception>? exThrow = null)

src/DocumentFormat.OpenXml.Framework/EnumerableExtensions.cs:25

  • [nitpick] The exception message may be improved for clarity; consider rephrasing it to better indicate that the enumerable contains more than one element.
throw exThrow?.Invoke() ?? throw new InvalidOperationException("Max of a single item should be in the enumerable");

src/DocumentFormat.OpenXml.Framework/Validation/Semantic/RelationshipTypeConstraint.cs:67

  • [nitpick] Consider passing a custom exception factory lambda to FirstOrDefaultAndMaxOne to provide a more context-specific error message.
.FirstOrDefaultAndMaxOne();

Copy link

Test Results

    70 files      70 suites   1h 8m 48s ⏱️
 2 041 tests  2 038 ✅  3 💤 0 ❌
32 418 runs  32 382 ✅ 36 💤 0 ❌

Results for commit 923a052.

@twsouthwick twsouthwick enabled auto-merge (squash) March 10, 2025 17:53
@twsouthwick twsouthwick merged commit d596354 into main Mar 17, 2025
22 checks passed
@twsouthwick twsouthwick deleted the remove-warning branch March 17, 2025 17:37
@twsouthwick
Copy link
Member Author

See #1899

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.

Clean up CA1851 warnings
4 participants