search

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
492 stars 190 forks source link

Strict parameter passing mode #7660

Open sylvainonweb opened 4 years ago

sylvainonweb commented 4 years ago

Update by @SteveSandersonMS: The request here is compile-time checking that you're not passing parameters that a component doesn't actually accept. The proposed design is something like a "strictness" mode that a component could opt into, with options controlling:

Ideally this would be settable globally via _Imports.razor too, so that new projects could default to a fairly strict choice, so that it becomes normal that parameter checking is on and you have to explicitly opt out of it in .razor files that do unusually dynamic things.

Describe the bug

I have a component Called DlmComboBox containing a Label property. This component is used in a lot of places. I decided to removed this Label property of the DlmComboBox component but after recompiling Visual Studio didn't show me warnings or errors at all.

Further technical details

.NET Core SDK (reflecting any global.json): Version: 3.1.101 Commit: b377529961

Runtime Environment: OS Name: Windows OS Version: 10.0.18362 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\3.1.101\

Host (useful for support): Version: 3.1.1 Commit: a1388f194c

.NET Core SDKs installed: 1.0.0-preview2-003131 [C:\Program Files\dotnet\sdk] 1.0.1 [C:\Program Files\dotnet\sdk] 2.0.2 [C:\Program Files\dotnet\sdk] 2.0.3 [C:\Program Files\dotnet\sdk] 2.1.4 [C:\Program Files\dotnet\sdk] 2.1.104 [C:\Program Files\dotnet\sdk] 2.1.200 [C:\Program Files\dotnet\sdk] 2.1.202 [C:\Program Files\dotnet\sdk] 2.1.300-rc1-008673 [C:\Program Files\dotnet\sdk] 2.1.300 [C:\Program Files\dotnet\sdk] 2.1.302 [C:\Program Files\dotnet\sdk] 2.1.504 [C:\Program Files\dotnet\sdk] 2.1.600 [C:\Program Files\dotnet\sdk] 2.1.601 [C:\Program Files\dotnet\sdk] 2.1.602 [C:\Program Files\dotnet\sdk] 2.1.604 [C:\Program Files\dotnet\sdk] 2.1.700 [C:\Program Files\dotnet\sdk] 2.1.701 [C:\Program Files\dotnet\sdk] 2.1.800-preview-009696 [C:\Program Files\dotnet\sdk] 2.1.800 [C:\Program Files\dotnet\sdk] 2.1.801 [C:\Program Files\dotnet\sdk] 2.2.202 [C:\Program Files\dotnet\sdk] 2.2.204 [C:\Program Files\dotnet\sdk] 2.2.300 [C:\Program Files\dotnet\sdk] 2.2.301 [C:\Program Files\dotnet\sdk] 2.2.401 [C:\Program Files\dotnet\sdk] 3.0.100-preview3-010431 [C:\Program Files\dotnet\sdk] 3.0.100-preview7-012821 [C:\Program Files\dotnet\sdk] 3.0.100-preview8-013656 [C:\Program Files\dotnet\sdk] 3.0.100-preview9-014004 [C:\Program Files\dotnet\sdk] 3.1.100 [C:\Program Files\dotnet\sdk] 3.1.101 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed: Microsoft.AspNetCore.All 2.1.0-rc1-final [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.1.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.1.14 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.1.15 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.2.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.2.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.App 2.1.0-rc1-final [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.1.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.1.14 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.1.15 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.2.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.2.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.0.0-preview8.19405.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.0.0-preview9.19424.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 1.0.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 1.0.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 1.1.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.0.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.0.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.0.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.0-rc1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.14 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.15 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.2.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.2.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.0.0-preview8-28405-07 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.0.0-preview9-19423-09 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.WindowsDesktop.App 3.0.0-preview7-27912-14 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 3.0.0-preview8-28405-07 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 3.0.0-preview9-19423-09 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

javiercn commented 4 years ago

@sylvainonweb thanks for contacting us.

@NTaylorMullen @ajaybhargavb @ryanbrandenburg can you follow up here?

sylvainonweb commented 4 years ago

I add an image as an attachment to let you see that no error occurs 1

SteveSandersonMS commented 4 years ago

Thanks for reporting this, @sylvainonweb! However, this is by design.

Components are allowed to accept arbitrary "catch-all" parameters (e.g., when passing through arbitrary sets of HTML attributes), and can even decide at runtime whether or not to accept a given parameter. So there's no way to say at compile time whether it's a problem that you're passing an unknown parameter name (and by default, they trigger a runtime error if they don't accept the parameter).

The IDE does provide useful hints about this through colorization. Depending on whether or not the parameter name you're using matches a statically-known parameter, we use different colorization. But it's still up to the developer to know whether or not it makes sense to pass through additional parameters to a given component.

Theoretically we could change this by adding further static annotations to components to say explicitly that they won't accept any other parameters. In this case we could raise a compile-time error. However there hasn't been much demand for such a feature.

sylvainonweb commented 4 years ago

Hi @SteveSandersonMS,

Wouldn't it be possible to distinguish parameters according to the fact that the first letter is an upper character or not ? The idea would be to say that :

Note : for me, it's really important to detect this kind of errors as soon as possible.

SteveSandersonMS commented 4 years ago

"Normal" parameters must be in upper case

We can't really have a rule like that, because it's common to pass regular HTML attributes but then give them special behaviors (by accepting them as parameters) in components.

sylvainonweb commented 4 years ago

it's common to pass regular HTML attributes but then give them special behaviors (by accepting them as parameters) in components

@SteveSandersonMS Sorry, it's a bit hard to understand, can you give me an example ?

sylvainonweb commented 4 years ago

I'm a little disappointed because in my point of view, this is a "real" issue. Actually, renaming a property of a component without renaming its usage will not throw compiler error. For a C# guy like me, it's really a problem. If you can't do anything, I will let you close this issue.

SteveSandersonMS commented 4 years ago

Sorry, it's a bit hard to understand, can you give me an example ?

Sure:

<MySpecialLinkTag href="some-page" title="Click this link" />

In this example:

The compiler can't know the difference based on casing.

However, if we wanted, we could have a rule that "parameters with PascalCase names must be defined explicitly" (and not the converse, i.e., parameters with non-PascalCase names might but also might not be defined explicitly). I think that would give you what you're looking for, right?

sylvainonweb commented 4 years ago

@SteveSandersonMS

However, if we wanted, we could have a rule that "parameters with PascalCase names must be defined explicitly"

For sure, it's exactly what I would like 👍

sylvainonweb commented 4 years ago

When do you think it could be added? This morning, I had the same kind of matter removing a property in a component used in probably one hundred pages, I had to search for each instead of let the compiler tells me where the matters are.

sylvainonweb commented 4 years ago

However, if we wanted, we could have a rule that "parameters with PascalCase names must be defined explicitly"

For sure, it's exactly what I would like 👍

@SteveSandersonMS @NTaylorMullen Is it normal that this issue is set as Resolved and ResolvedByDesign?

ghost commented 3 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.

NTaylorMullen commented 2 years ago

@allisonchou this would be a compiler issue.

lukblazewicz commented 2 years ago

We would like to bump up this issue a little. In our Blazor projects we encounter a scenario where rename of a child component's parameter is not reflected in the parent component and the only indication is changed font color inside VS editor. Without design-time error we need to fail at runtime to get aware of the issue - which definitely slows down development cycle. Delivering https://github.com/dotnet/razor-tooling/issues/4569 would partially do the trick (at least for the renaming scenario, not covering manual errors though).

Fabster1993 commented 1 year ago

This is in my opinion a big issue in Blazor. Working now for quite a while with Blazor, this design decision caused by far the most bugs in production.

Other frontend frameworks with typescript support (React, Angular) do throw a compiler error by default, if a unknown component parameter is passed. Therefore it is contrary to intuition that Blazor, using a typesafe language as well, handles this completely different.

philippdolder commented 3 months ago

We're also struggling with this behavior after simplifying a few components I literally have to retest the entire system to be sure nothing breaks.

I'm thinking this would be something to leverage Roslyn capabilities and build an analyzer that you can configure as opt-in for those people that want to rely on this behavior. Individual cases could always be suppressed with an attribute on the component I would think

@SteveSandersonMS before I even start on a prototype for this. Knowing you know a whole lot about the entire .NET universe. Would this be a job that can be achieved using Roslyn? Or does the model not provide the required information to detect such cases?

Fabster1993 commented 3 months ago

@philippdolder Should be possible with a roslyn analyzer see https://github.com/meziantou/Meziantou.Analyzer, MA0115

philippdolder commented 3 months ago

in the meantime I've found https://www.meziantou.net/new-rules-for-blazor-in-meziantou-analyzer.htm which works fine to detect properties used in markup that don't exist on a component. It follow the rule that properties are PascalCase and the attributes affected by CaptureUnmatchedValues should be camelCase.