Enable trim analyzer

[sbomer]

I think we should be able to remove this workaround now.
Fixes #41016.

[dougbu]

@sbomer this looks early given dotnet/linker#2718 and #41016 remain open. But I'm adding @JamesNK as a reviewer…

[sbomer]

Sorry, I forgot to close dotnet/linker#2718 - but the issues there have been fixed and I checked that the analyzer no longer throws on aspnetcore.

As @JamesNK noticed in #41021 (comment) there are some new analyzer warnings that ILLink didn't report. One reason this might be happening is for dead code that the linker removes (whereas the analyzer will report warnings for all code). But we need to go through these warnings and identify any that are unexpected.

[JamesNK]

I've fixed warnings in Kestrel, Http.Extensions and middleware. The remaining warnings are in components and JSInterop. I'm not familiar with that code or the existing trimming attributes.

@mkArtakMSFT Could someone on the Blazor team contribute to this PR and fix up these warnings? I suggest waiting until #41610 is merged. That PR could resolve some of these warnings. unrelated

Remaining warnings:

/home/vsts/work/1/s/src/Components/Components/src/CascadingParameterState.cs(93,30): error IL2067: 'targetType' argument does not satisfy 'DynamicallyAccessedMemberTypes.All' in call to 'Microsoft.AspNetCore.Components.Reflection.ComponentProperties.GetCandidateBindableProperties(Type)'. The parameter 'componentType' of method 'Microsoft.AspNetCore.Components.CascadingParameterState.CreateReflectedCascadingParameterInfos(Type)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [/home/vsts/work/1/s/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj]
/home/vsts/work/1/s/src/Components/Components/src/DynamicComponent.cs(70,17): error IL2072: 'value' argument does not satisfy 'DynamicallyAccessedMemberTypes.All' in call to 'Microsoft.AspNetCore.Components.DynamicComponent.Type.set'. The return value of method 'Microsoft.AspNetCore.Components.ParameterValue.Value.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [/home/vsts/work/1/s/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj]
/home/vsts/work/1/s/src/Components/Components/src/Routing/RouteTableFactory.cs(57,34): error IL2026: Using member 'System.Reflection.Assembly.ExportedTypes.get' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Types might be removed. [/home/vsts/work/1/s/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj]
/home/vsts/work/1/s/src/Components/Components/src/LayoutView.cs(59,24): error IL2072: 'layoutType' argument does not satisfy 'DynamicallyAccessedMemberTypes.All' in call to 'Microsoft.AspNetCore.Components.LayoutView.WrapInLayout(Type, RenderFragment)'. The return value of method 'Microsoft.AspNetCore.Components.LayoutView.GetParentLayoutType(Type)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [/home/vsts/work/1/s/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj]
/home/vsts/work/1/s/src/Components/Components/src/BindConverter.cs(2033,33): error IL2026: Using member 'System.ComponentModel.TypeDescriptor.GetConverter(Type)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Generic TypeConverters may require the generic types to be annotated. For example, NullableConverter requires the underlying type to be DynamicallyAccessedMembers All. [/home/vsts/work/1/s/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj]
/home/vsts/work/1/s/src/Components/Components/src/BindConverter.cs(1831,33): error IL2026: Using member 'System.ComponentModel.TypeDescriptor.GetConverter(Type)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Generic TypeConverters may require the generic types to be annotated. For example, NullableConverter requires the underlying type to be DynamicallyAccessedMembers All. [/home/vsts/work/1/s/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj]
/home/vsts/work/1/s/src/Components/Components/src/RouteView.cs(80,17): error IL2111: Method 'Microsoft.AspNetCore.Components.LayoutView.Layout.set' with parameters or return value with `DynamicallyAccessedMembersAttribute` is accessed via reflection. Trimmer can't guarantee availability of the requirements of the method. [/home/vsts/work/1/s/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj]
/home/vsts/work/1/s/src/Components/Components/src/RouteView.cs(80,17): error IL2111: Method 'Microsoft.AspNetCore.Components.LayoutView.Layout.set' with parameters or return value with `DynamicallyAccessedMembersAttribute` is accessed via reflection. Trimmer can't guarantee availability of the requirements of the method. [/home/vsts/work/1/s/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj]
/home/vsts/work/1/s/src/Components/Components/src/RouteView.cs(80,17): error IL2111: Method 'Microsoft.AspNetCore.Components.LayoutView.WrapInLayout(Type, RenderFragment)' with parameters or return value with `DynamicallyAccessedMembersAttribute` is accessed via reflection. Trimmer can't guarantee availability of the requirements of the method. [/home/vsts/work/1/s/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj]
/home/vsts/work/1/s/src/JSInterop/Microsoft.JSInterop/src/JSRuntime.cs(126,17): error IL2026: Using member 'System.Text.Json.JsonSerializer.Serialize<TValue>(TValue, JsonSerializerOptions)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. JSON serialization and deserialization might require types that cannot be statically analyzed. Use the overload that takes a JsonTypeInfo or JsonSerializerContext, or make sure all of the required types are preserved. [/home/vsts/work/1/s/src/JSInterop/Microsoft.JSInterop/src/Microsoft.JSInterop.csproj]
/home/vsts/work/1/s/src/JSInterop/Microsoft.JSInterop/src/JSRuntime.cs(245,30): error IL2026: Using member 'System.Text.Json.JsonSerializer.Deserialize(ref Utf8JsonReader, Type, JsonSerializerOptions)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. JSON serialization and deserialization might require types that cannot be statically analyzed. Use the overload that takes a JsonTypeInfo or JsonSerializerContext, or make sure all of the required types are preserved. [/home/vsts/work/1/s/src/JSInterop/Microsoft.JSInterop/src/Microsoft.JSInterop.csproj]
/home/vsts/work/1/s/src/JSInterop/Microsoft.JSInterop/src/Infrastructure/DotNetDispatcher.cs(63,16): error IL2026: Using member 'System.Text.Json.JsonSerializer.Serialize<TValue>(TValue, JsonSerializerOptions)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. JSON serialization and deserialization might require types that cannot be statically analyzed. Use the overload that takes a JsonTypeInfo or JsonSerializerContext, or make sure all of the required types are preserved. [/home/vsts/work/1/s/src/JSInterop/Microsoft.JSInterop/src/Microsoft.JSInterop.csproj]
/home/vsts/work/1/s/src/JSInterop/Microsoft.JSInterop/src/Infrastructure/DotNetDispatcher.cs(133,34): error IL2026: Using member 'System.Text.Json.JsonSerializer.Serialize<TValue>(TValue, JsonSerializerOptions)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. JSON serialization and deserialization might require types that cannot be statically analyzed. Use the overload that takes a JsonTypeInfo or JsonSerializerContext, or make sure all of the required types are preserved. [/home/vsts/work/1/s/src/JSInterop/Microsoft.JSInterop/src/Microsoft.JSInterop.csproj]
/home/vsts/work/1/s/src/JSInterop/Microsoft.JSInterop/src/Infrastructure/DotNetDispatcher.cs(149,26): error IL2026: Using member 'System.Text.Json.JsonSerializer.Serialize<TValue>(TValue, JsonSerializerOptions)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. JSON serialization and deserialization might require types that cannot be statically analyzed. Use the overload that takes a JsonTypeInfo or JsonSerializerContext, or make sure all of the required types are preserved. [/home/vsts/work/1/s/src/JSInterop/Microsoft.JSInterop/src/Microsoft.JSInterop.csproj]
/home/vsts/work/1/s/src/JSInterop/Microsoft.JSInterop/src/Infrastructure/DotNetDispatcher.cs(242,39): error IL2026: Using member 'System.Text.Json.JsonSerializer.Deserialize(ref Utf8JsonReader, Type, JsonSerializerOptions)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. JSON serialization and deserialization might require types that cannot be statically analyzed. Use the overload that takes a JsonTypeInfo or JsonSerializerContext, or make sure all of the required types are preserved. [/home/vsts/work/1/s/src/JSInterop/Microsoft.JSInterop/src/Microsoft.JSInterop.csproj]
/home/vsts/work/1/s/src/JSInterop/Microsoft.JSInterop/src/Infrastructure/DotNetDispatcher.cs(436,36): error IL2075: 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'System.Type.GetMethods(BindingFlags)'. The return value of method 'System.Collections.IEnumerator.Current.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [/home/vsts/work/1/s/src/JSInterop/Microsoft.JSInterop/src/Microsoft.JSInterop.csproj]

[mkArtakMSFT]

I've fixed warnings in Kestrel, Http.Extensions and middleware. The remaining warnings are in components and JSInterop. I'm not familiar with that code or the existing trimming attributes.

@mkArtakMSFT Could someone on the Blazor team contribute to this PR and fix up these warnings? I suggest waiting until #41610 is merged. That PR could resolve some of these warnings. unrelated

Thanks @JamesNK.
@MackinnonBuck can you please help with this some time this week? Thanks!

[MackinnonBuck]

Ok, I've fixed the warnings on justifiable cases, but I think the analyzer may have pointed out some real potential issues.

@SteveSandersonMS, would you be able to look over these remaining warning locations and confirm whether my concerns are valid? If so, I can do a follow-up commit with some fixes.

---

RouteTableFactory.cs(57,34): error IL2026: Using member 'System.Reflection.Assembly.ExportedTypes.get' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Types might be removed.

I suppose it's safe to assume that routeable component types won't get completely trimmed away, but what about their [Parameter] properties? Routable component types aren't passed directly into RenderTreeBuilder.OpenComponent() like other component types (and aren't preserved explicitly elsewhere, afaict). Do we have some other mechanism preserving routable component parameter properties that I'm not aware of? It seems if this were a real problem, we would have seen it by now.

---

DynamicComponent.cs(70,17): error IL2072: 'value' argument does not satisfy 'DynamicallyAccessedMemberTypes.All' in call to 'Microsoft.AspNetCore.Components.DynamicComponent.Type.set'. The return value of method 'Microsoft.AspNetCore.Components.ParameterValue.Value.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

I see we have a [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] on the Type property, but I don't think this helps because it's being assigned from the ParameterView entry anyway. Maybe this could be fixed by adding a RenderTreeBuilder.AddAttribute() overload that accepts a correctly-annotated Type argument, rather than relying on the overload accepting an object?

---

LayoutView.cs(59,24): error IL2072: 'layoutType' argument does not satisfy 'DynamicallyAccessedMemberTypes.All' in call to 'Microsoft.AspNetCore.Components.LayoutView.WrapInLayout(Type, RenderFragment)'. The return value of method 'Microsoft.AspNetCore.Components.LayoutView.GetParentLayoutType(Type)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

The Layout property is set using reflection, so it seems like this could be problematic. A new RenderTreeBuilder.AddAttribute() overload might fix this one too.

---

RouteView.cs(80,17): error IL2111: Method 'Microsoft.AspNetCore.Components.LayoutView.Layout.set' with parameters or return value with DynamicallyAccessedMembersAttribute is accessed via reflection. Trimmer can't guarantee availability of the requirements of the method.

It seems like there could be a legitimate issue here that could be fixed by annotating the LayoutAttribute constructor parameter with [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)].

[SteveSandersonMS]

[RouteTableFactory.cs(57,34)]

You raise a good point about the [Parameter] properties, plus I guess any other framework-invoked methods on the component types. I think that so far, this hasn't been an issue because the framework itself doesn't contain routable components, and application code isn't normally trimmed at all. It would only be an issue for RCLs that somebody marks as trimmable (rare).

If there is some way for the [Route] attribute (Microsoft.AspNetCore.Components.RouteAttribute, not the one in MVC of course) to cause the trimmer to preserve that type, including all public API, then that would be ideal but I'm not sure the trimmer has a mechanism for this.

[DynamicComponent.cs(70,17)]

I suspect you're right that the existing annotation is ineffective. It was put there in reaction to trimmer warnings (IIRC).

Regarding the AddAttribute overload proposal, I'm not certain how/if it would work. The point of DynamicComponent is to use it with type values that vary at runtime, not just with fixed values. That is, people don't commonly write:

<DynamicComponent Type="typeof(SomeComponent)" />

... they commonly write:

<DynamicComponent Type="@chosenComponentType" />

@code {
    Type chosenComponentType;
}

So isn't this still an unknowable type as far as the trimmer is concerned? It's not performing some more sophisticated static analysis to work out the possible range of values for chosenComponentType based on assignments elsewhere, is it?

If I'm understanding, it seems like:

• In general we'll continue to expect people not to use it with framework components
• Most uses with application components are fine because application assemblies aren't trimmed
• In the edge case of a trim-enabled RCL, people will have to write some other annotation either in their RCL or their application code to say they want to preserve the types they know they'll be using dynamically

If I'm wrong about this, please let me know as that would be great news :)

[LayoutView.cs(59,24):]

The trimmer's concern (based on the warning) seems not to be about line 59, but about line 60, because the layoutType variable gets populated by GetParentLayoutType which doesn't declare anything about its return value being preserved. It seems like we could annotate GetParentLayoutType, but TBH I don't really follow why this would be the right place to annotate this.

I would have thought we should be annotating LayoutAttribute so that it knows that its constructor parameter (Type layoutType) indicates that we need to preserve the whole of layoutType as a component. If that was done, then it seems like LayoutView doesn't really need to trigger any preservation of anything, since we already know that all components used as layouts are already preserved. We'd just need to suppress any warnings that emerge from LayoutView. This seems like it should work since [Layout] is always set statically with a fixed literal type value for the constructor.

Please let me know if I'm misinterpreting all of this!

[RouteView.cs(80,17):] It seems like there could be a legitimate issue here that could be fixed by annotating the LayoutAttribute constructor parameter with [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)].

Right, yes! It seems like you're stating the same thing I was suggesting above. So maybe the constructor parameter annotation could solve both of these last two cases. As long as the routable page component itself is preserved, then [Layout] should cause its layout component to be preserved, and in turn any chain of ancestor layouts.

[MackinnonBuck]

Thanks for taking a look, @SteveSandersonMS!

Most uses with application components are fine because application assemblies aren't trimmed

Aha! This is something I wasn't aware of, and it drove a lot of the concerns I brought up in my last comment, so this is great to hear.

[RouteTableFactory.cs(57,34)]
If there is some way for the [Route] attribute to cause the trimmer to preserve that type, including all public API, then that would be ideal but I'm not sure the trimmer has a mechanism for this.

I'll go ahead and dismiss the warnings then since the framework doesn't contain routable components, as you stated.

[DynamicComponent.cs(70,17)]
The point of DynamicComponent is to use it with type values that vary at runtime, not just with fixed values.

Yeah, the AddAttribute overload wouldn't handle this case. And again, since application code doesn't get trimmed, we're probably safe here.

[LayoutView.cs(59,24)], [RouteView.cs(80,17)]

Everything you said about these seems right to me. Annotating the LayoutAttribute constructor is probably the right move then.

I'll make these final changes and push a commit with them soon, possibly tomorrow.

[JamesNK]

@MackinnonBuck Please rebase this before merging. A few more projects support trimming since this PR was started. Hopefully, they don't introduce any new warnings, but we don't want to break the build if they do.

I'll handle any new trimming errors that show up.

[JamesNK]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[MackinnonBuck]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[MackinnonBuck]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[MackinnonBuck]

@JamesNK @SteveSandersonMS Are we good to merge this now?

[JamesNK]

@SteveSandersonMS Could you review the components changes?

[SteveSandersonMS]

Looks great!