dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.23k stars 9.95k forks source link

ASP0018 analyzer does not respect complex model binding #54212

Open agc93 opened 7 months ago

agc93 commented 7 months ago

Is there an existing issue for this?

Describe the bug

When binding a route parameter to a complex type using the [FromRoute] attribute, the ASP0018 analyzer claims a parameter is unused.

Expected Behavior

The ASP0018 analyzer should only generate messages if a parameter is actually unused. If the analyzer is unable to determine if a parameter is bound to a complex type, it should either a) not generate the message on actions with complex type parameters, or b) be documented that this is unsupported.

Steps To Reproduce

Sample repro available here.

In short:

  1. Add an action method that accepts a parameter with a model type annotated with [FromRoute] attributes
  2. Add an appropriately named optional route parameter to the route template (to match the [FromRoute] attribute in the type)
  3. Build and run the project. ASP.NET will correctly bind the parameter from the route, but ASP0018 will still be generated on build

Exceptions (if any)

No response

.NET Version

8.0.200

Anything else?

ASP.NET Core 8 in Visual Studio 2022.

dotnet --info output ``` .NET SDK: Version: 8.0.200 Commit: 438cab6a9d Workload version: 8.0.200-manifests.e575128c Runtime Environment: OS Name: Windows OS Version: 10.0.19045 OS Platform: Windows RID: win-x64 Base Path: C:\Program Files\dotnet\sdk\8.0.200\ .NET workloads installed: There are no installed workloads to display. Host: Version: 8.0.2 Architecture: x64 Commit: 1381d5ebd2 .NET SDKs installed: [...] 8.0.100 [C:\Program Files\dotnet\sdk] 8.0.200 [C:\Program Files\dotnet\sdk] .NET runtimes installed: [...] Microsoft.AspNetCore.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 8.0.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] [...] Microsoft.NETCore.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 8.0.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] [...] Microsoft.WindowsDesktop.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 8.0.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Other architectures found: arm64 [C:\Program Files\dotnet] registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\arm64\InstallLocation] x86 [C:\Program Files (x86)\dotnet] registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation] Environment variables: Not set global.json file: C:\Users\\Source\Samples\analyzer-unused-parameter\global.json ```

I have checked that this same behaviour also applies when using other binding sources (I first observed this when using a CompositeBindingSource to bind to both the path and query).

captainsafia commented 6 months ago

@JamesNK I suspect that the issue here is that we are not analyzing the properties within the bound model for FromRoute attributes. I'm not sure how trivial the fix is but agree that at minimum we should document that it is safe to disable this warning under this case.

cc: @Rick-Anderson for updates to doc page on this

jgarciadelanoceda commented 4 months ago

I am having the same problem. I define all the classes with all the properties and bind the whole class directly. In fact I am using SuppressInferBindingSourcesForParameters, that was a thing needed in previous dotnet versions (I think that is not longer needed in dotnet 8).

This is the controller method that I have: [HttpGet("{Id}", Name = nameof(GetCustomer))] public async Task<ActionResult<Customer>> GetCustomer(GetCustomerRequest getCustomerRequest)

And this is the class: public class GetCustomerRequest{ [FromRoute, Required] public int Id { get; set; } }

The analyzer gives me a suggestion that the Id is not used, when it is being used