dotnet / razor

Compiler and tooling experience for Razor ASP.NET Core apps in Visual Studio, Visual Studio for Mac, and VS Code.
https://asp.net
MIT License
502 stars 191 forks source link

@model should warn when used with @inherits without `<TModel>` in MVC #10987

Open chsienki opened 4 weeks ago

chsienki commented 4 weeks ago

When using the @model directive in MVC, it changes the code generation of the page base type.

Default generation:

public class TestPage : global::Microsoft.AspNetCore.Mvc.Razor.RazorPage<dynamic>

With @model PageModel:

public class TestPage : global::Microsoft.AspNetCore.Mvc.Razor.RazorPage<PageModel>

However, its also possible to control the base type of the generated page via the @inherits directive.

public class MyBase : global::Microsoft.AspNetCore.Mvc.Razor.RazorPage<dynamic>

@inherits MyBase generates:

public class TestPage : MyBase

In the following case, the @model directive is silenty ignored:

@inherits MyBase
@model PageModel

Generates:

public class TestPage : MyBase

Meaning the underlying page model is still dynamic not the strongly typed PageModel

It is possible to pass the model parameter through by having the inherited type take a single type argument:

public class MyBase<TModel> : global::Microsoft.AspNetCore.Mvc.Razor.RazorPage<TModel>
@inherits MyBase<TModel>
@model PageModel

Generates:

public class TestPage : MyBase<PageModel>

Note that in the base declaration the name of the type parameter can be anything:

public class MyBase<TSomething> : global::Microsoft.AspNetCore.Mvc.Razor.RazorPage<TSomething>
@inherits MyBase<TModel>
@model PageModel

Generates:

public class TestPage : MyBase<PageModel>

However the type parameter used in @inherits must be called TModel

@inherits MyBase<TSomething>
@model PageModel

Generates:

public class TestPage : MyBase<TSomething>

With errors about an unresolved type parameter of TSomething.

The razor compiler actually does a string match specifically for <TModel> when the model parameter is used, so any other value is not replaced.

While we could make the compiler more intelligent in what it accepts, it's been this way for a long time, so it's probably not worth changing it, or inventing a new syntax.

We should, however, at least issue a warning in this case to make sure that the user is aware of it.

chsienki commented 3 weeks ago

Note we also won't emit anything for IntelliSense to work in these cases.