Conversation
…ibute in Blazor components Co-authored-by: meziantou <509220+meziantou@users.noreply.github.com>
|
|
||
| public static bool IsCSharp12OrAbove(this LanguageVersion languageVersion) | ||
| { | ||
| return languageVersion >= LanguageVersion.CSharp12; |
There was a problem hiding this comment.
@copilot Also, move it between IsCSharp11OrAbove and IsCSharp13OrAbove
| return languageVersion >= LanguageVersion.CSharp12; | |
| return languageVersion >= (LanguageVersion)1200; |
There was a problem hiding this comment.
Done in 9779b79. Moved IsCSharp12OrAbove between IsCSharp11OrAbove and IsCSharp13OrAbove, and updated to use (LanguageVersion)1200 for consistency.
|
|
||
| public void AnalyzeProperty(SymbolAnalysisContext context) | ||
| { | ||
| if (context.Compilation.GetCSharpLanguageVersion() < LanguageVersion.CSharp12) |
There was a problem hiding this comment.
@copilot Use the extension method
| if (context.Compilation.GetCSharpLanguageVersion() < LanguageVersion.CSharp12) | |
| if (!context.Compilation.GetCSharpLanguageVersion().IsCSharp12OrAbove()) |
| messageFormat: "Use constructor injection instead of [Inject] attribute", | ||
| RuleCategories.Design, | ||
| DiagnosticSeverity.Info, | ||
| isEnabledByDefault: true, |
… disable by default Co-authored-by: meziantou <509220+meziantou@users.noreply.github.com>
|
@copilot CI is failing |
…ion compatibility Co-authored-by: meziantou <509220+meziantou@users.noreply.github.com>
Fixed in 4143148. The CI was failing because |
|
@copilot CI is failing |
…reateRange Co-authored-by: meziantou <509220+meziantou@users.noreply.github.com>
IsCSharp12OrAbovetoLanguageVersionExtensions.csMA0187constant toRuleIdentifiers.csAspNetCore9_0toTargetFramework.csandProjectBuilder.Validation.csBlazorPropertyInjectionShouldUseConstructorInjectionAnalyzer.csBlazorPropertyInjectionShouldUseConstructorInjectionFixer.csBlazorPropertyInjectionFixAllProvider.csdocs/Rules/MA0187.md#if CSHARP12_OR_GREATER(TypeDeclarationSyntax.ParameterList requires Roslyn 4.8+)[.. documentDiagnostics]collection expression withImmutableArray.CreateRange(documentDiagnostics)to avoid CS9210Original prompt
This section details on the original issue you should resolve
<issue_title>Blazor (.NET9+): migrate property injection to constructor injection</issue_title>
<issue_description>Is your feature request related to a problem? Please describe.
Since .NET9, it is possible to have constructor injection in Blazor components, instead of property injection: https://learn.microsoft.com/en-us/aspnet/core/release-notes/aspnetcore-9.0?view=aspnetcore-10.0#constructor-injection / https://learn.microsoft.com/en-us/aspnet/core/blazor/fundamentals/dependency-injection?view=aspnetcore-9.0#request-a-service-in-a-component
It would be nice to have an analyzer and a fix that helps detect and migrate property injection to constructor injection in Blazor components.
Describe the solution you'd like
Considering this code:
the injected dependency should be marked as a suggestion of fix, with the fix that would change the property to a constructor parameter like this:
Describe alternatives you've considered
N/A
Additional context
I'm not sure if anyone would prefer the other way around? Detecting constructor injection to migrate to property injection? Constructor injection seems like an improvement (avoid
defaut!and in somes cases, avoid more complicated init inOnInitializedAsync).The presence on an existing constructor in the class could complicate the fix, especially if async is involved :
would need to migrate to:
Then IDE0290 could complete the job to migrate to primary constructor to get close to the simpler example from the doc.</issue_description>
<agent_instructions>Can you implement the logic to detect [Inject] properties in Razor component (class implementing
Microsoft.AspNetCore.Components.IComponent). Report them as suggestions. Only report if ASP.NET Core 9 or greater (check assembly version).Also, only report if primary constructor is supported by the current version of C#.
The fixer must create the primary constructor or update it if any and update all references to the property. Use Renamer.RenameSymbolAsync before removing the property, so it replaces all usages.
Be sure to add tests for the batch fixer with multiple injected properties.</agent_instructions>
Comments on the Issue (you are @copilot in this section)
@meziantou Do you think it should report @Inject directive? Maybe only when there is a partial class? @meziantou > AFAIK constructor injection requires itI think you can do whatever you want in the
@code {}section.I think it's mostly checking if the file is generated or user-defined. It's trivial to exclude generated files in analyzers.
Maybe we can start with user-defined code and skip the code generated by the source generator.</comment_new>
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.