Skip to content

Add do...while loop in the SetValue method of SerializationResourceManager.cs until resourceName is unique #13578

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

Conversation

LeafShi1
Copy link
Member

@LeafShi1 LeafShi1 commented Jun 10, 2025

Fixes #12595

Root Cause

This issue is caused by replacing for (;;) with if (appendCount || _nameTable.TryGetValue(nameBase, out count)) in PR #8332, which leads to resource name conflicts.

Proposed changes

  • Add do...while loop in the SetValue method until resourceName is unique

Customer Impact

  • When serializing collection values, the resource numeric suffix can be incremented without duplication

Regression?

  • Yes

Risk

  • Minimal

Screenshots

Before

Paste after the second time, the resource will end up using the same number 1 over and over
Image

After

Resource suffixes are increasing

AfterChange

Test methodology

  • Manually

Test environment(s)

  • .net 10.0.0-preview.6.25304.106
Microsoft Reviewers: Open in CodeFlow

Copy link
Contributor

@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.

Pull Request Overview

This PR ensures that SetValue in SerializationResourceManager.cs repeatedly generates a new resourceName until it’s unique, fixing duplicate suffixes when serializing collections.

  • Replaces single conditional append with a do…while loop to retry name generation
  • Changes the existence check from TryGetValue on the base name to ContainsKey on the current resourceName
  • Updates how count is retrieved and used when building the new name
Comments suppressed due to low confidence (1)

src/System.Windows.Forms.Design/src/System/ComponentModel/Design/Serialization/ResourceCodeDomSerializer.SerializationResourceManager.cs:787

  • Because count is appended before it’s incremented, the first generated name can be nameBase0 when appendCount is true but no existing entries are found. Move count++ before formatting resourceName (or restructure the loop) so the suffix starts at 1.
resourceName = $"{nameBase}{count}";

@LeafShi1 LeafShi1 force-pushed the Issue_12595_fix_SetValue_SerializationResourceManager.cs branch from 3395dff to 55afccb Compare June 10, 2025 08:21
Copy link

codecov bot commented Jun 10, 2025

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.

Project coverage is 76.59527%. Comparing base (8732cfb) to head (78091bf).
Report is 34 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #13578         +/-   ##
===================================================
- Coverage   76.59901%   76.59527%   -0.00375%     
===================================================
  Files           3230        3230                 
  Lines         639161      639217         +56     
  Branches       47297       47304          +7     
===================================================
+ Hits          489591      489610         +19     
- Misses        145988      146038         +50     
+ Partials        3582        3569         -13     
Flag Coverage Δ
Debug 76.59527% <0.00000%> (-0.00375%) ⬇️
integration 18.80145% <0.00000%> (+0.00602%) ⬆️
production 51.00335% <0.00000%> (-0.00293%) ⬇️
test 97.40114% <ø> (-0.00298%) ⬇️
unit 48.37601% <0.00000%> (-0.02737%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Member

@Epica3055 Epica3055 left a comment

Choose a reason for hiding this comment

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

👍

@ricardobossan
Copy link
Member

All LGTM!

Copy link
Member

@KlausLoeffelmann KlausLoeffelmann left a comment

Choose a reason for hiding this comment

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

Looks good!

@LeafShi1 LeafShi1 merged commit 75ff80c into dotnet:main Jun 16, 2025
8 checks passed
@LeafShi1 LeafShi1 deleted the Issue_12595_fix_SetValue_SerializationResourceManager.cs branch June 16, 2025 01:15
KlausLoeffelmann pushed a commit to KlausLoeffelmann/winforms that referenced this pull request Jun 16, 2025
…nager.cs until resourceName is unique (dotnet#13578)

* Add do...while loop in the SetValue method of SerializationResourceManager.cs until resourceName is unique

* Check nameBase directly rather than resourceName for clarity
@LeafShi1
Copy link
Member Author

/backport to release/8.0

Copy link
Contributor

Started backporting to release/8.0: https://github.com/dotnet/winforms/actions/runs/15725589917

@LeafShi1
Copy link
Member Author

/backport to release/9.0

Copy link
Contributor

Started backporting to release/9.0: https://github.com/dotnet/winforms/actions/runs/15725601101

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ResourceCodeDomSerializer fails to assign distinct number suffixes to resources when serializing collection values
4 participants