Skip to content

Fix potential NullReferenceException in C# union model discriminator generation#7641

Merged
baywet merged 7 commits intomicrosoft:mainfrom
hobostay:fix/csharp-discriminator-nre
Apr 23, 2026
Merged

Fix potential NullReferenceException in C# union model discriminator generation#7641
baywet merged 7 commits intomicrosoft:mainfrom
hobostay:fix/csharp-discriminator-nre

Conversation

@hobostay
Copy link
Copy Markdown
Contributor

Summary

  • Add null/empty check for mappedType.Key before generating discriminator condition in C# union model factory method

Problem

In WriteFactoryMethodBodyForUnionModel, when iterating over properties of a union/discriminated type:

var mappedType = parentClass.DiscriminatorInformation.DiscriminatorMappings
    .FirstOrDefault(x => x.Value.Name.Equals(propertyType.Name, StringComparison.OrdinalIgnoreCase));
writer.WriteLine($"...\"{mappedType.Key.SanitizeDoubleQuote()}\".Equals(...)");

If FirstOrDefault finds no matching mapping, it returns default(KeyValuePair<string, CodeType>) with a null key. The next line unconditionally accesses mappedType.Key, generating invalid C# code like "".Equals(mappingValue, ...) instead of a proper discriminator check.

Fix

Guard the WriteLine call with a null/empty check:

if (!string.IsNullOrEmpty(mappedType.Key))
    writer.WriteLine(...);

Test plan

  • Verify existing C# writer tests pass
  • Verify union model generation handles unmapped types gracefully

🤖 Generated with Claude Code

@hobostay hobostay requested a review from a team as a code owner April 21, 2026 12:13
Test User and others added 2 commits April 22, 2026 12:10
…generation

When no discriminator mapping is found for a property type, `FirstOrDefault` returns a default KeyValuePair with a null key. Accessing `mappedType.Key` without a null check could generate invalid C# code with empty discriminator strings.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@baywet baywet force-pushed the fix/csharp-discriminator-nre branch from 7846692 to 2747093 Compare April 22, 2026 16:10
baywet
baywet previously approved these changes Apr 22, 2026
Copy link
Copy Markdown
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

@baywet baywet enabled auto-merge (squash) April 22, 2026 16:14
Copy link
Copy Markdown
Contributor

@adrian05-ms adrian05-ms left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution! Some of the integration tests are failing. Please fix them and I'll approve and merge the PR

Copy link
Copy Markdown

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 hardens union-model factory-method generation against missing discriminator mappings by avoiding code emission when the discriminator mapping key can’t be resolved, preventing writer-time exceptions and malformed conditional chains across multiple language emitters.

Changes:

  • Add null/empty guards around discriminator mapping keys before emitting discriminator-based branches in union factory methods (C#, Python, Dart; skip/continue in Go/Java/PHP).
  • Fix conditional-chain state (includeElse) and closing-brace behavior to avoid emitting invalid else if/dangling blocks when no discriminator branches are generated.
  • Update CHANGELOG.md under Unreleased to document the fix (but currently includes unrelated entries).

Reviewed changes

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

Show a summary per file
File Description
src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs Skip discriminator branch generation when the mapping key can’t be found; keep else chaining valid.
src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs Guard mapped discriminator key usage and ensure elif chaining only begins after an emitted branch.
src/Kiota.Builder/Writers/Dart/CodeMethodWriter.cs Avoid reading discriminator unless needed; skip branches with missing mapping keys and fix else chaining.
src/Kiota.Builder/Writers/Go/CodeMethodWriter.cs Skip discriminated-type branches when mapping key is null/empty; only close the chain when started.
src/Kiota.Builder/Writers/Java/CodeMethodWriter.cs Skip discriminated-type branches when mapping key is null/empty; only close the chain when started.
src/Kiota.Builder/Writers/Php/CodeMethodWriter.cs Skip discriminated-type branches when mapping key is null/empty; only close the chain when started.
CHANGELOG.md Add an Unreleased changelog entry for the fix (currently also adds unrelated items).

Comment thread src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs
Comment thread src/Kiota.Builder/Writers/Dart/CodeMethodWriter.cs
Comment thread CHANGELOG.md
Remove unrelated CHANGELOG entries (microsoft#7643, microsoft#7642, microsoft#7639) that don't
belong to this PR. Add C# and Dart writer tests verifying that union
model factory methods skip discriminator branches when the mapping key
is null/empty (i.e. an unmapped complex type).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
auto-merge was automatically disabled April 23, 2026 12:46

Head branch was pushed to by a user without write access

@baywet baywet requested a review from adrian05-ms April 23, 2026 12:58
@baywet baywet enabled auto-merge (squash) April 23, 2026 13:19
@baywet baywet merged commit 6ce5e7d into microsoft:main Apr 23, 2026
299 checks passed
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.

4 participants