dotnet / dotnet/aspnetcore

Invalid trim warning suppression in BindConverter

Open
#57,016 0 comments 0 reactions 0 assignees View on GitHub
area-blazor bug feature-trimming
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 5h
Merged PRs (30d)
276

Description

### Is there an existing issue for this?

- [X] I have searched the existing issues

### Describe the bug

`ParserDelegateCache.Get` method body suppresses trim analysis warnings with the following justification:

https://github.com/dotnet/aspnetcore/blob/98ee50279acfacff2b4ae4b6863956f3c65f34b2/src/Components/Components/src/BindConverter.cs#L1859-L1867

This is a lie. At minimum, the following code:

https://github.com/dotnet/aspnetcore/blob/98ee50279acfacff2b4ae4b6863956f3c65f34b2/src/Components/Components/src/BindConverter.cs#L1985-L1990

is doing MakeGenericMethod on a method that is annotated. I don't see anything obvious that would ensure this invariant holds:

https://github.com/dotnet/aspnetcore/blob/98ee50279acfacff2b4ae4b6863956f3c65f34b2/src/Components/Components/src/BindConverter.cs#L2002

Trim warning suppressions:

* Should be reviewed with the same level of suspicion as `unsafe`. Assume it's a bug unless proven otherwise.
* Should never be used on a 130 line method that does tons of reflection. Extract the suppressed block to a small helper method and suppress there.

### Expected Behavior

_No response_

### Steps To Reproduce

_No response_

### Exceptions (if any)

_No response_

### .NET Version

_No response_

### Anything else?

_No response_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.