Fix inconsistent variable naming in deserialization methods for models with additional properties

[Copilot]

Fix Inconsistent Variable Naming in Deserialization Methods

Summary

Fixed variable naming inconsistency in deserialization methods when models have additional properties.

Root Cause

The original fix incorrectly assumed that when _additionalBinaryDataProperty.Value exists (a property with backing field _additionalBinaryDataProperties), the backing field would always be used in the constructor. However, the actual logic is:

• If the model supports BinaryData additional properties, the property parameter is added to the constructor (line 779-782 in ModelProvider.cs)
• Otherwise, the field parameter is added (line 785-786)

This meant:

1. When the property is in the constructor, it's declared as additionalProperties (using property's variable name)
2. My original fix tried to use additionalBinaryDataProperties (backing field's variable name)
3. This caused "undefined variable" errors

Solution

Check which one is actually in the constructor parameters:

• If the property itself is in the parameters → use property's variable expression (additionalProperties)
• If the backing field is in the parameters → use field's variable expression (additionalBinaryDataProperties)

Changes

• Fixed root cause analysis - property vs field in constructor
• Added logic to check constructor parameters
• Use correct variable expression based on what's in constructor
• Added test to validate variable naming consistency
• All 1010 unit tests pass
• All 98 Spector tests for additional-properties pass

Testing

✅ All ClientModel unit tests pass (1010 tests, including new test)
✅ All Spector additional-properties tests pass (98 tests)
✅ New test TestAdditionalPropertiesVariableNameConsistency validates the fix

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[Bug]: Inconsistent Variable Naming in Deserialization Methods</issue_title>
<issue_description>### Describe the bug

Description

The TypeSpec C# generator produces inconsistent variable names in the Deserialize* methods. A local variable is declared with one name but referenced with a different name later in the method.

Reproduction

In IndexingParametersConfiguration.Serialization.cs:

// Line 176 - variable declared as 'additionalProperties'
IDictionary<string, BinaryData> additionalProperties = new ChangeTrackingDictionary<string, BinaryData>();

// ... many lines later ...

// Line 301 - referenced as 'additionalBinaryDataProperties' (WRONG!)
additionalBinaryDataProperties.Add(prop.Name, BinaryData.FromString(prop.Value.GetRawText()));

Expected Behavior

The variable name should be consistent throughout the method. Either:

• Declare as additionalBinaryDataProperties and use additionalBinaryDataProperties, OR
• Declare as additionalProperties and use additionalProperties

Root Cause

The generator uses different naming conventions in different parts of the serialization template:

• One part uses the pattern matching the field name (_additionalBinaryDataProperties ? additionalBinaryDataProperties)
• Another part uses a simpler name (additionalProperties)

Affected Types

• IndexingParametersConfiguration
• FacetResult
• Potentially any type with additional/unknown properties handling

---

Reproduction

steps: Regenerate SDK on Azure.Search.Documents.

Existing: https://github.com/Azure/azure-sdk-for-net/blob/3df80e71cc22102f60910e9d188ca864ea18849d/sdk/search/Azure.Search.Documents/src/Generated/Models/FacetResult.Serialization.cs#L222C1-L223C1

Repo: https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/search/Azure.Search.Documents/src

Checklist

• Follow our Code of Conduct
• Check that there isn't already an issue that request the same bug to avoid creating a duplicate.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion.
• The provided reproduction is a minimal reproducible example of the bug.</issue_description>

<agent_instructions>This fix applies to packages\http-client-csharp</agent_instructions>

Comments on the Issue (you are @copilot in this section)

• Fixes [Bug]: Inconsistent Variable Naming in Deserialization Methods #9436

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[JonathanCrd]

Testes this fix on my local by running

./eng/scripts/Test-Spector.ps1 "http/type/property/additional-properties"

In .../TestProjects/Spector/http/type/property/additional-properties/src/Generated/Models/IsUnknownAdditionalPropertiesDiscriminatedDerived.Serialization.cs the additional properties are now being generated correctly:

 internal static IsUnknownAdditionalPropertiesDiscriminatedDerived DeserializeIsUnknownAdditionalPropertiesDiscriminatedDerived(JsonElement element, ModelReaderWriterOptions options)
{
    // ...
    IDictionary<string, BinaryData> additionalProperties = new ChangeTrackingDictionary<string, BinaryData>();
    // ...
    foreach (var prop in element.EnumerateObject())
    {
        // ...
        additionalProperties.Add(prop.Name, BinaryData.FromString(prop.Value.GetRawText()));
    }
    return new IsUnknownAdditionalPropertiesDiscriminatedDerived(name, kind, additionalProperties, index, age);
}

[JoshLove-msft]

Are we sure there is an actual bug here? When I tried to repro in the Search library the variable name was consistent.

[JonathanCrd]

Let me double check

[JonathanCrd]

@copilot can you include some tests for your fix?

[Copilot]

Added test in commit a85d580. The new test TestAdditionalPropertiesVariableNameConsistency validates that the variable name used in the declaration matches the name used in the .Add() calls, ensuring consistency regardless of whether the property or backing field is in the constructor parameters.

[JonathanCrd]

Solved on Microsoft teams. This is not a bug, it was happening because of custom code on the user's end.