More precise diagnostics for methods attributes with wrong types#4514
More precise diagnostics for methods attributes with wrong types#4514
Conversation
|
Lightbeam link |
DiasAtBeamable
left a comment
DiasAtBeamable
left a comment
There was a problem hiding this comment.
LGTM, good addition to the Diagnostics. Thanks!
There was a problem hiding this comment.
Pull request overview
This PR improves diagnostic messages for ClientCallable methods with unsupported argument types by providing more precise error messages that include the specific parameter name causing the issue.
Changes:
- Added a new diagnostic descriptor
MissingSerializableAttributeForArgumentthat provides parameter-specific error messages - Modified
ValidateSerializableAttributeOnSymbolto accept an optionalparameterNameparameter and use the new diagnostic when a parameter name is provided - Updated the test to validate the new parameter-aware diagnostic
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| templates/MicroserviceSourceGen/MicroserviceSourceGen/Diagnostics.cs | Added new MissingSerializableAttributeForArgument diagnostic descriptor and registered it in the analyzer |
| templates/MicroserviceSourceGen/MicroserviceSourceGen/Analyzers/ServicesAnalyzer.cs | Updated method signature to accept parameter name and added logic to use the new diagnostic when validating method parameters |
| templates/SourceGen/MicroserviceSourceGen.Tests/BeamableSourceGeneratorTests.Srv.cs | Added test case for new parameter-aware diagnostic with wrongArgument parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public static readonly DiagnosticDescriptor MissingSerializableAttributeForArgument | ||
| = new(MISSING_SERIALIZABLE_ATTRIBUTE_ON_TYPE_ID, |
There was a problem hiding this comment.
The new diagnostic descriptor MissingSerializableAttributeForArgument is reusing the same diagnostic ID MISSING_SERIALIZABLE_ATTRIBUTE_ON_TYPE_ID as MissingSerializableAttributeOnType. This is problematic because diagnostic IDs should be unique to allow proper identification and filtering of specific diagnostic types. When two diagnostics share the same ID, it becomes impossible to distinguish between them programmatically, which can cause issues with diagnostic suppression, IDE filtering, and diagnostic tracking.
Consider creating a new unique diagnostic ID constant (e.g., MISSING_SERIALIZABLE_ATTRIBUTE_FOR_ARGUMENT_ID = "BEAM_SRV_0024") and using it for this diagnostic descriptor instead of reusing the existing ID.
| if (!string.IsNullOrWhiteSpace(parameterName)) | ||
| { | ||
| diagnostic = Diagnostic.Create(Diagnostics.Srv.MissingSerializableAttributeForArgument, location, typeSymbol.Name, parameterName); | ||
| } | ||
| else | ||
| { | ||
| diagnostic = Diagnostic.Create(Diagnostics.Srv.MissingSerializableAttributeOnType, location, typeSymbol.Name); | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
|
@Leinnan , I was experimenting with copilot. Your PR just had the luck of being recent. |
3 participants
It gives a more precise information when game maker is using unsupported type as argument in `ClientCallable. By providing the exact name of the argument being invalid it helps figuring out issue faster.