Disable default JSON reflection when PublishTrimmed == true

[eiriktsarpalis]

Following up on feedback from dotnet/sdk#33757 this updates from Microsoft.NET.Sdk.targets in the sdk repo to Microsoft.NET.ILLink.targets.

I might need some guidance on the testing strategy here, my assumption is that we would need to test this from the sdk repo once the change has been merged.

Fix #84378

[ghost]

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Following up on feedback from dotnet/sdk#33757 this updates from Microsoft.NET.Sdk.targets in the sdk repo to Microsoft.NET.ILLink.targets.

I might need some guidance on the testing strategy here, my assumption is that we would need to test this from the sdk repo once the change has been merged.

Author:	eiriktsarpalis
Assignees:	-
Labels:	area-Tools-ILLink
Milestone:	-

[eerhardt]

Looks like ILLink.RoslynAnalyzer.Tests are failing. Here's one:

    ILLink.RoslynAnalyzer.Tests.UnconditionalSuppressMessageCodeFixTests.FixInPropertyAccessor [FAIL]
      Context: Iterative code fix application
      content of '/0/Test0.cs' did not match. Diff shown with expected as baseline:
       using System;
       using System.Diagnostics.CodeAnalysis;
       
       public class C
       {
       	[RequiresUnreferencedCodeAttribute("message")]
       	public int M1() => 0;
       
       	public int field;
       
       	private int M2 {
               [UnconditionalSuppressMessage("Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = "<Pending>")]
      +        [UnconditionalSuppressMessage("Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = "<Pending>")]
      +        [UnconditionalSuppressMessage("Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = "<Pending>")]
               get { return M1(); }
      -
      +		set { field = M1(); }
      -        [UnconditionalSuppressMessage("Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = "<Pending>")]
      -        set { field = M1(); }
       	}
       }
      
      Expected: True
      Actual:   False
      Stack Trace:
        /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Testing.Verifiers.XUnit/XUnitVerifier.cs(87,0): at Microsoft.CodeAnalysis.Testing.Verifiers.XUnitVerifier.Fail(String message)
        /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/Extensions/IVerifierExtensions.cs(67,0): at Microsoft.CodeAnalysis.Testing.IVerifierExtensions.EqualOrDiff(IVerifier verifier, String expected, String actual, String message)
        /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.CodeFix.Testing/CodeFixTest`1.cs(531,0): at Microsoft.CodeAnalysis.Testing.CodeFixTest`1.VerifyProjectAsync(ProjectState newState, Project project, IVerifier verifier, CancellationToken cancellationToken)
        /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.CodeFix.Testing/CodeFixTest`1.cs(509,0): at Microsoft.CodeAnalysis.Testing.CodeFixTest`1.VerifyFixAsync(String language, ImmutableArray`1 analyzers, ImmutableArray`1 codeFixProviders, SolutionState oldState, SolutionState newState, Int32 numberOfIterations, Func`10 getFixedProject, IVerifier verifier, CancellationToken cancellationToken)
        /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.CodeFix.Testing/CodeFixTest`1.cs(471,0): at Microsoft.CodeAnalysis.Testing.CodeFixTest`1.VerifyFixAsync(SolutionState testState, SolutionState fixedState, SolutionState batchFixedState, IVerifier verifier, CancellationToken cancellationToken)
        /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.CodeFix.Testing/CodeFixTest`1.cs(310,0): at Microsoft.CodeAnalysis.Testing.CodeFixTest`1.RunImplAsync(CancellationToken cancellationToken)
        /_/src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs(188,0): at Microsoft.CodeAnalysis.Testing.AnalyzerTest`1.RunAsync(CancellationToken cancellationToken)
        --- End of stack trace from previous location ---

I'm not sure what is happening. @sbomer @vitek-karas @agocke - any thoughts?

[eerhardt]

The failures may be: #88492

[sbomer]

Those failures are #88278, unrelated to this change.

The change here looks good to me as far as defaulting the feature switch goes. I think it's fine to test this in the SDK repo, which is the approach we currently use for testing MSBuild logic.

The discussion in dotnet/sdk#33757 mentioned different defaults for MAUI and wasm. Did we land on what their defaults should be? Will they need to update in response to this change (or has that already been done)?

[eiriktsarpalis]

The discussion in dotnet/sdk#33757 mentioned different defaults for MAUI and wasm. Did we land on what their defaults should be? Will they need to update in response to this change (or has that already been done)?

Yes, these targets should explicitly set the JsonSerializerIsReflectionEnabledByDefault property to true. This has already been done for blazor wasm via dotnet/sdk#31909, so would need to follow up with the MAUI folks about this.