Add additional Dapr Actors Analyzers#1715
Conversation
…(DAPR1405-DAPR1414) Co-authored-by: moonolgerd <3743184+moonolgerd@users.noreply.github.com>
Add ActorSerializationAnalyzer with DAPR1405-DAPR1414 diagnostic rules
There was a problem hiding this comment.
Pull request overview
Adds a new Roslyn analyzer + code fix provider intended to catch common Dapr Actors serialization “gotchas” (interfaces inheriting IActor, enum member attributes, validation of types used in actor method signatures), along with initial unit tests and analyzer release notes.
Changes:
- Introduces
ActorSerializationAnalyzerwith new rule IDsDAPR1405–DAPR1414. - Adds
ActorSerializationCodeFixProviderwith fixes for a subset of the new diagnostics. - Adds
ActorSerializationAnalyzerTestsand updatesAnalyzerReleases.Unshipped.mdto document the rules.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| test/Dapr.Actors.Analyzers.Test/ActorSerializationAnalyzerTests.cs | Adds analyzer tests for several new serialization-related diagnostics. |
| src/Dapr.Actors.Analyzers/Dapr.Actors.Analyzers/AnalyzerReleases.Unshipped.md | Documents newly introduced analyzer rules (unshipped). |
| src/Dapr.Actors.Analyzers/Dapr.Actors.Analyzers/ActorSerializationCodeFixProvider.cs | Adds code fixes for several new serialization diagnostics. |
| src/Dapr.Actors.Analyzers/Dapr.Actors.Analyzers/ActorSerializationAnalyzer.cs | Implements new actor serialization analyzer rules and diagnostic descriptors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/Dapr.Actors.Analyzers/Dapr.Actors.Analyzers/ActorSerializationAnalyzer.cs
Show resolved
Hide resolved
src/Dapr.Actors.Analyzers/Dapr.Actors.Analyzers/ActorSerializationAnalyzer.cs
Outdated
Show resolved
Hide resolved
src/Dapr.Actors.Analyzers/Dapr.Actors.Analyzers/ActorSerializationCodeFixProvider.cs
Outdated
Show resolved
Hide resolved
| [Fact] | ||
| public async Task ActorClassWithoutIActorInterface_ShouldReportDAPR1413() | ||
| { | ||
| var context = CreateTest(); | ||
| context.TestCode = """ |
There was a problem hiding this comment.
There are no tests covering DAPR1411 (collection element validation) or DAPR1414 (parameterless ctor / [DataContract]) even though ActorSerializationAnalyzer introduces both rules. Adding targeted analyzer tests (and code-fix tests for ActorSerializationCodeFixProvider) would help prevent regressions and validate diagnostic locations.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
src/Dapr.Actors.Analyzers/Dapr.Actors.Analyzers/ActorSerializationAnalyzer.cs
Outdated
Show resolved
Hide resolved
src/Dapr.Actors.Analyzers/Dapr.Actors.Analyzers/ActorSerializationAnalyzer.cs
Outdated
Show resolved
Hide resolved
| private static void AnalyzeEnumDeclaration(SyntaxNodeAnalysisContext context) | ||
| { | ||
| var enumDeclaration = (EnumDeclarationSyntax)context.Node; | ||
| var semanticModel = context.SemanticModel; | ||
| var enumSymbol = semanticModel.GetDeclaredSymbol(enumDeclaration); |
There was a problem hiding this comment.
AnalyzeEnumDeclaration currently enforces DAPR1406 for every enum in the compilation regardless of whether the enum is related to Dapr actors. If the intent is "Actor types" only (per title/notes), the analyzer should scope this rule to enums reachable from Actor method signatures or Actor-related types, or adjust the wording to indicate a global rule.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
src/Dapr.Actors.Analyzers/Dapr.Actors.Analyzers/ActorSerializationCodeFixProvider.cs
Show resolved
Hide resolved
| private static void RegisterAddRecordDataContractFix(CodeFixContext context, SyntaxNode root, SyntaxNode node, Diagnostic diagnostic) | ||
| { | ||
| if (node is RecordDeclarationSyntax recordDeclaration) | ||
| { | ||
| var action = CodeAction.Create( |
There was a problem hiding this comment.
DAPR1412 is reported at the record usage location (e.g., a method signature), but this code fix only registers when the diagnostic node is a RecordDeclarationSyntax. In most cases FindNode(span) will return an IdentifierName/TypeSyntax, so the fix won't appear; resolve the record symbol via SemanticModel and apply the fix at its declaring syntax reference(s).
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| DAPR1405 | Usage | Error | Actor interface should inherit from IActor. | ||
| DAPR1406 | Usage | Warning | Enum members in Actor types should use EnumMember attribute. | ||
| DAPR1407 | Usage | Info | Consider using JsonPropertyName for property name consistency. | ||
| DAPR1408 | Usage | Warning | Complex types used in Actor methods need serialization attributes. | ||
| DAPR1409 | Usage | Warning | Actor method parameter needs proper serialization attributes. |
There was a problem hiding this comment.
This release note lists DAPR1407 and DAPR1408 as new rules, but ActorSerializationAnalyzer currently never reports either diagnostic. Either implement them or remove them from the release list so the documented rule set matches actual behavior.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
|
@copilot open a new pull request to apply changes based on the comments in this thread |
WhitWaldo
left a comment
There was a problem hiding this comment.
Thank you for submitting this PR. I've added a few review notes and if you could also address what Copilot has raised, I think that'll put this PR in a good place to merge it.
src/Dapr.Actors.Analyzers/Dapr.Actors.Analyzers/ActorSerializationAnalyzer.cs
Show resolved
Hide resolved
src/Dapr.Actors.Analyzers/Dapr.Actors.Analyzers/ActorSerializationAnalyzer.cs
Show resolved
Hide resolved
src/Dapr.Actors.Analyzers/Dapr.Actors.Analyzers/ActorSerializationAnalyzer.cs
Outdated
Show resolved
Hide resolved
…tionAnalyzer.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Oleg Merkulov <moonolgerd@gmail.com>
…tionCodeFixProvider.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Oleg Merkulov <moonolgerd@gmail.com>
|
Something about your analyzer is blocking the build altogether - could you please take a look at that? |
…tionAnalyzer.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Oleg Merkulov <moonolgerd@gmail.com>
…tionAnalyzer.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Oleg Merkulov <moonolgerd@gmail.com>
…tionCodeFixProvider.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Oleg Merkulov <moonolgerd@gmail.com>
Description
Add additional Dapr Actors Analyzers
Issue reference
Add analyzers that help prevent developer gotchas
Please reference the issue this PR will close: ##1426
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: