search

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

Analyzer: warn when marking a route parameter as optional if it isn't at the end of a route #39486

Open Jcparkyn opened 2 years ago

Jcparkyn commented 2 years ago

Is your feature request related to a problem? Please describe.

Route parameters in an ASP.NET Core Web API can be marked as optional with a ?, e.g.,

app.MapGet("/test1/{foo?}", (string? foo) => foo ?? "none"); // "/test1" returns "none"

If a route parameter marked as optional is followed by a non-optional route segment, the optional annotation is ignored. E.g.,

app.MapGet("/test2/{foo?}/bar", (string? foo) => foo ?? "none"); // "/test2//bar" returns 404

It is not immediately obvious (to me at least) that it would not be possible to have an optional parameter within a route, and there is no run-time error or warning when doing this.

The behavior appears to be the same for nullable value types (e.g. int?).

Describe the solution you'd like

An analyzer that checks for routes containing optional parameters followed by non-optional route segments, and provides an appropriate warning.

I also couldn't find any mention in the documentation stating that this is not possible.

Additional context

Optional route parameters are allowed to be followed by other optional parameters, so the analyzer should consider this case. E.g.,

app.MapGet("/test3/{foo?}/{bar?}", (string? foo, string? bar) => foo ?? bar ?? "none"); // "/test3" returns "none"

Related: #36637

ghost commented 2 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

rjgotten commented 2 years ago

I don't really (or rather: really don't) understand why optional segments are not allowed in the middle of a pattern to begin with.

Regular optional segments, i.e. that are not marked catch-all, have a {0,1} cardinality creating a well-defined upper bound. A pattern that has N consecutive optional segments in the middle, followed by some required (incl. literal) segments, should always be able to be matched using an Nth degree lookahead.

It's completely silly that the framework implementation handling the routing DFA can't handle this and ruins simple solutions with a lot of practical gain, like multilingual routing using an BCP-47 tag as an optional first path segment to force selection of a certain language, overruling e.g. culture-selection based on the Accept-Language header.