Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Bubble up formatter trim warnings #11023

Merged
merged 9 commits into from
Mar 18, 2024
Merged

Bubble up formatter trim warnings #11023

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b3285f1
Bubble up formatter trim warnings
LakshanF Mar 11, 2024
a608b1a
Merge branch 'main' of https://github.com/dotnet/winforms into TrimTe…
LakshanF Mar 12, 2024
87970d5
Add feature checks on the binding boundary
LakshanF Mar 15, 2024
bdf9ea1
Merge branch 'main' into TrimTestWrnFix2
LakshanF Mar 15, 2024
2b3b1cf
Build break fix
LakshanF Mar 15, 2024
415789c
Merge branch 'main' of https://github.com/dotnet/winforms into TrimTe…
LakshanF Mar 18, 2024
7415482
revert TrimTest project changes
LakshanF Mar 18, 2024
6d292c1
Apply suggestions from code review
LakshanF Mar 18, 2024
7ca07fb
Fix build break
LakshanF Mar 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/System.Windows.Forms/src/System/Windows/Forms/Internal/Formatter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ namespace System.Windows.Forms;

internal static class Formatter
{
internal const string ComponentModelTrimIncompatibilityMessage = "ComponentModel APIs are trim incompatible.";
LakshanF marked this conversation as resolved.
Show resolved Hide resolved

private static readonly Type stringType = typeof(string);
private static readonly Type booleanType = typeof(bool);
private static readonly Type checkStateType = typeof(CheckState);
Expand All @@ -25,6 +27,7 @@ internal static class Formatter
/// If the caller is expecting a nullable value back, we must also re-wrap the final result
/// inside a nullable value before returning.
/// </summary>
[RequiresUnreferencedCode(ComponentModelTrimIncompatibilityMessage)]
public static object? FormatObject(
object? value,
Type targetType,
Expand Down Expand Up @@ -82,6 +85,7 @@ internal static class Formatter
/// - Uses TypeConverters or IConvertible where appropriate
/// - Throws a FormatException is no suitable conversion can be found
/// </summary>
[RequiresUnreferencedCode(ComponentModelTrimIncompatibilityMessage)]
private static object? FormatObjectInternal(
object? value,
Type targetType,
Expand Down Expand Up @@ -212,6 +216,7 @@ internal static class Formatter
/// If the caller is expecting a nullable value back, we must also re-wrap the final result
/// inside a nullable value before returning.
/// </summary>
[RequiresUnreferencedCode(ComponentModelTrimIncompatibilityMessage)]
public static object? ParseObject(
object? value,
Type targetType,
Expand Down Expand Up @@ -263,6 +268,7 @@ internal static class Formatter
/// - Uses TypeConverters or IConvertible where appropriate
/// - Throws a FormatException if no suitable conversion can be found
/// </summary>
[RequiresUnreferencedCode(ComponentModelTrimIncompatibilityMessage)]
Copy link
Member

Choose a reason for hiding this comment

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

Is this code path hit at runtime by the test app? I don't know whether we should bubble up warnings, or carve out a trim-compatible path through this method. If we bubble it up, I'd like to understand where we are going to cut off the annotations so that the test app doesn't hit warnings due to this.

Copy link
Member Author

Choose a reason for hiding this comment

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

ParseObjectInternal path is not hit at runtime. Added feature guards on the call stack path that reach the binding boundary. FormatObjectInternal is hit at runtime by the runtime app and bubbled up its RUC to publish methods.

private static object? ParseObjectInternal(
object? value,
Type targetType,
Expand Down Expand Up @@ -431,7 +437,7 @@ private static CultureInfo GetFormatterCulture(IFormatProvider? formatInfo)
/// <summary>
/// Converts a value to the specified type using best Parse() method on that type
/// </summary>
public static object? InvokeStringParseMethod(object? value, Type targetType, IFormatProvider? formatInfo)
public static object? InvokeStringParseMethod(object? value, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] Type targetType, IFormatProvider? formatInfo)
{
try
{
Expand Down
6 changes: 5 additions & 1 deletion src/System.Windows.Forms/tests/IntegrationTests/TrimTest/TrimTest.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<OutputType>WinExe</OutputType>
<OutputType>Exe</OutputType>
<Nullable>enable</Nullable>
<OutputType>WinExe</OutputType>
<ImplicitUsings>enable</ImplicitUsings>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<!-- These are needed to suppress the localization step picked up from Arcade targets -->
<EnableXlfLocalization>false</EnableXlfLocalization>
<UpdateXlfOnBuild>false</UpdateXlfOnBuild>
<PublishTrimmed>true</PublishTrimmed>
<TrimmerSingleWarn>false</TrimmerSingleWarn>
<_SuppressWinFormsTrimError>true</_SuppressWinFormsTrimError>
<SuppressTrimAnalysisWarnings>false</SuppressTrimAnalysisWarnings>
</PropertyGroup>

<!-- These normally come from $(UseWindowsForms) when $(ImplicitUsings) is enabled -->
Expand Down