Skip to content

C#: Add neutral model for System.ComponentModel.PropertyDescriptor.GetValue - #20627

Merged
aschackmull merged 1 commit into
github:mainfrom
hvitved:csharp/property-descriptor-get-value-neutral
Oct 14, 2025
Merged

C#: Add neutral model for System.ComponentModel.PropertyDescriptor.GetValue#20627
aschackmull merged 1 commit into
github:mainfrom
hvitved:csharp/property-descriptor-get-value-neutral

Conversation

@hvitved

@hvitvedhvitved commented Oct 12, 2025

Copy link
Copy Markdown
Contributor

This PR disables the flow-generated summary for PropertyDescriptor.GetValue. The problem with this summary is that it has type object -> object, which means that we risk first upcasting a known specific type t to object, and then subsequently downcasting it to a different type, which does not correspond to the type of any property of t.

@hvitved
hvitvedforce-pushed the csharp/property-descriptor-get-value-neutral branch from 49f2c4d to 2577452CompareOctober 12, 2025 07:07
@hvitvedhvitved added the no-change-note-required This PR does not need a change note label Oct 12, 2025
@hvitved
hvitved marked this pull request as ready for review October 12, 2025 13:21
@hvitved
hvitved requested a review from a team as a code ownerOctober 12, 2025 13:21
@hvitved
hvitved requested review from Copilot and removed request for a teamOctober 12, 2025 13:21

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

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 disables the automatically generated flow summary for System.ComponentModel.PropertyDescriptor.GetValue by adding a neutral model. The change prevents potential type safety issues where the method's object -> object signature could lead to incorrect upcasting and downcasting of known specific types to incompatible property types.

  • Adds a manual neutral model for PropertyDescriptor.GetValue to override the auto-generated flow summary
  • Removes the problematic dfc-generated flow summaries from test expectations for both System.ComponentModel.PropertyDescriptor and Newtonsoft.Json.Linq.JPropertyDescriptor

Reviewed Changes

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

FileDescription
System.ComponentModel.model.ymlAdds neutral model configuration to disable flow summary for PropertyDescriptor.GetValue
FlowSummaries.expectedUpdates test expectations by removing auto-generated summaries and adding neutral model entries
FlowSummariesFiltered.expectedRemoves the filtered flow summary entry for PropertyDescriptor.GetValue

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

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

@aschackmullaschackmull left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@aschackmull

Copy link
Copy Markdown
Contributor

Dca shows zero impact. Merging.

@aschackmull
aschackmull merged commit da0b168 into github:mainOct 14, 2025
24 checks passed
@hvitved
hvitved deleted the csharp/property-descriptor-get-value-neutral branch October 19, 2025 14:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C#no-change-note-requiredThis PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@hvitved@aschackmull