Implement trigger payload type validation diagnostics (CSDK200-204)#61
Merged
Conversation
Closes #42 - CSDK200: Deserialize<T> type mismatch vs expected payload type - CSDK201: Deserialize<T> uses weak type when typed payload available - CSDK202: Generic argument type not found in SDK - CSDK203: Operation has no mapped payload type - CSDK204: Type is not a TriggerCallbackPayload subclass Co-authored-by: Dobby <dobby@microsoft.com>
…ge, extract shared helpers - Normalize operation name to canonical casing before payload lookup (CSDK203 fix) - Update CSDK204 message to reflect naming-convention check - Extract shared ValidatorHelpers for FindNamedArgument, ExtractStringValue, etc. Co-authored-by: Dobby <dobby@microsoft.com>
- Correct test comment: SharepointOnline has trigger ops but no typed payload in mock - Update CSDK204 summary to say TriggerPayload naming convention - Update test section header for CSDK204 to match naming-convention check Co-authored-by: Dobby <dobby@microsoft.com>
- Reorder CSDK203 before weak-type check so unmapped ops always surface - Cache TriggerMetadataInfo per method to avoid repeated attribute scanning - Pre-compute HashSet of type names for O(1) lookup in TypeExistsInIndex - Fix ExtractStringValue doc comment to clarify syntactic-only extraction - Update test to expect CSDK203 for weak type with unmapped operation Co-authored-by: Dobby <dobby@microsoft.com>
…ing test - Update CSDK201 XML summary to list all 6 weak types - Fix HashSet comment to reflect it stores both full and simple names - Add test for operation name casing normalization (onnewemail -> OnNewEmail) Co-authored-by: Dobby <dobby@microsoft.com>
…atching, tests - Move type name lookup into SdkIndex.TypeNameLookup (lazily-initialized, cached) - Use full syntax text for qualified type arguments in CSDK200/202 matching - Compare both fully-qualified and simple names for type existence and match - Add tests for fully-qualified correct type and fully-qualified weak type Co-authored-by: Dobby <dobby@microsoft.com>
- Mark typeNameLookupCache as volatile for safe lazy init - Return IReadOnlySet<string> to prevent external mutation Co-authored-by: Dobby <dobby@microsoft.com>
Co-authored-by: Dobby <dobby@microsoft.com>
- Split on both '.' and '+' for nested type names in TypeNameLookup - Handle MemberBindingExpressionSyntax for conditional access Deserialize calls - Add test for conditional-access deserialization pattern Co-authored-by: Dobby <dobby@microsoft.com>
…pe text in messages - Split on both '.' and '+' in ExtractSimpleNameFromFullName for nested types - Use typeArgument.ToString() in diagnostic messages for accurate rendering Co-authored-by: Dobby <dobby@microsoft.com>
…ck to known serializers - TypeNameLookup now uses FrozenSet<string> with Interlocked.CompareExchange for proper single-init - Add KnownSerializerTypeNames (JsonSerializer, JsonConvert) to reduce false positives - Check receiver type name for static member access deserialization calls - Skip receiver check for conditional access (instance-based) and direct calls Co-authored-by: Dobby <dobby@microsoft.com>
- Add ! operator to return since field is guaranteed non-null after Interlocked.CompareExchange - Keep camelCase field naming per SA1309 (no underscore prefix in this repo) Co-authored-by: Dobby <dobby@microsoft.com>
Co-authored-by: Dobby <dobby@microsoft.com>
…param - Extract receiver from ConditionalAccessExpressionSyntax for known-serializer check - Fix TypeNameLookup XML doc to say 'rightmost simple name segment' - Fix lambda parameter name in GetAllTriggerOperations Co-authored-by: Dobby <dobby@microsoft.com>
- Define local JsonSerializer class (non-static) for realistic conditional access pattern - Add comment explaining this is a syntax-level test, not semantic Co-authored-by: Dobby <dobby@microsoft.com>
…itives - Return null for GenericNameSyntax (direct calls) since there is no receiver to verify Co-authored-by: Dobby <dobby@microsoft.com>
… inconsistency - Guarantee single-build with LazyInitializer.EnsureInitialized + lock - Fix inconsistent comment: direct calls are skipped, not allowed Co-authored-by: Dobby <dobby@microsoft.com>
- Track isQualifiedType to distinguish qualified vs unqualified type arguments - For qualified types: only match against fully-qualified expected type (no simple-name fallback) - Prevents false suppression of CSDK200/202 for same-named types in different namespaces Co-authored-by: Dobby <dobby@microsoft.com>
…lified check - Strip global:: from AliasQualifiedNameSyntax in typeKey - Detect using static directives for known serializers to allow bare Deserialize<T>() calls - Remove redundant Contains check for unqualified types (typeKey == simpleTypeName) Co-authored-by: Dobby <dobby@microsoft.com>
…d test - Use IsKind(SyntaxKind.StaticKeyword) instead of Value is not null for static detection - Allow null receiverName through receiver check (direct calls with static import) - Add ValidateAsync_UsingStaticDirectCall_EmitsCSdk201 test Co-authored-by: Dobby <dobby@microsoft.com>
…meSyntax - Unwrap NullableTypeSyntax before qualified-type detection - Add NormalizeQualifiedName to strip global:: from QualifiedNameSyntax.Left Co-authored-by: Dobby <dobby@microsoft.com>
- NormalizeQualifiedName now uses string prefix strip for deeply nested global:: - Add ValidateAsync_GlobalQualifiedCorrectType_NoDiagnostic test Co-authored-by: Dobby <dobby@microsoft.com>
…nown limitations - Iterate MethodDeclarationSyntax first, filter by trigger attribute, then scan invocations - Pass TriggerMetadataInfo directly to ValidateDeserializeInvocation (no more ancestor walk) - Document syntax-only limitation for same-named types in different namespaces Co-authored-by: Dobby <dobby@microsoft.com>
- Require using System.Text.Json or using Newtonsoft.Json for simple-name receiver matching - Add KnownSerializerNamespaces set and IsKnownSerializerNamespace helper - Update conditional access test to include using System.Text.Json Co-authored-by: Dobby <dobby@microsoft.com>
- Changed return null to continue in ExtractTriggerMetadata for unparseable attributes - Allows finding valid attributes when multiple/malformed attributes exist Co-authored-by: Dobby <dobby@microsoft.com>
- Each method is visited once in the loop, so cache is redundant — removed Co-authored-by: Dobby <dobby@microsoft.com>
- Check full receiver expression text for System.Text.Json.JsonSerializer etc. - Allows diagnostics when using fully-qualified calls without a using directive Co-authored-by: Dobby <dobby@microsoft.com>
- Map each serializer type to its specific namespace (JsonSerializer->System.Text.Json) - Fix NormalizeQualifiedName doc: uses string prefix strip, not recursive walk - Acknowledge nested type '+' vs '.' and using static false positive as known limitations Co-authored-by: Dobby <dobby@microsoft.com>
- Use exact namespace match instead of StartsWith to prevent child namespace false positives - Strip global:: prefix from fully-qualified receiver text - Remove unused IsKnownSerializerNamespace and KnownSerializerNamespaces Co-authored-by: Dobby <dobby@microsoft.com>
- Check using static against KnownFullyQualifiedSerializerTypes instead of simple name - Add NormalizeUsingName to strip global:: from using name Co-authored-by: Dobby <dobby@microsoft.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Implements syntax-only trigger payload type validation diagnostics (CSDK200–CSDK204) by adding a new validator that scans trigger-attributed methods for Deserialize<T> usage and validates T against SDK-indexed payload types.
Changes:
- Added
TriggerPayloadValidatorto emit CSDK200–CSDK204 and wired it into server DI. - Added shared Roslyn/LSP helper utilities (
ValidatorHelpers) and refactoredAttributeValidatorto use them. - Extended
SdkIndexwith a lazily-builtFrozenSetlookup (TypeNameLookup) and enhanced test scaffolding + new validator test suite.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Server/SdkIndex.cs | Adds TypeNameLookup frozen set for fast type existence checks; extends test factory. |
| Server/Program.cs | Registers the new TriggerPayloadValidator in DI. |
| Server/Diagnostics/Validators/ValidatorHelpers.cs | Introduces shared syntax-tree validator helper utilities. |
| Server/Diagnostics/Validators/TriggerPayloadValidator.cs | Adds the trigger payload diagnostics implementation (CSDK200–CSDK204). |
| Server/Diagnostics/Validators/AttributeValidator.cs | Refactors to reuse ValidatorHelpers for shared parsing/range/diagnostic creation. |
| Server/Diagnostics/DiagnosticCodes.cs | Defines new diagnostic codes CSDK200–CSDK204. |
| Server.Tests/TriggerPayloadValidatorTests.cs | Adds extensive unit tests for the new validator and edge cases. |
| README.md | Updates repo structure/docs description to include design documents. |
1. Collect using directives from both root.Usings and namespace-scoped
declarations (NamespaceDeclarationSyntax, FileScopedNamespaceDeclarationSyntax)
to avoid false negatives when serializer imports are inside a namespace block.
2. Add dotted variant of nested type names ('+' -> '.') to TypeNameLookup
so fully-qualified source references like 'Outer.Inner' match SDK types
stored with '+' separator.
3. Added tests for namespace-scoped using and fully-qualified nested types.
1. Local variables named JsonSerializer/JsonConvert now suppress the known-serializer match (e.g., var JsonSerializer = new ...). 2. Added caveat comment about multi-namespace using aggregation being a simplification acceptable for Azure Functions projects. 3. Updated TypeNameLookup XML doc to accurately describe nested type entries (dotted variant + innermost simple name).
1. Expand local type name collection to include BaseTypeDeclarationSyntax (covers enums) and DelegateDeclarationSyntax, not just TypeDeclarationSyntax. 2. Reject instance member access (this.JsonSerializer.Deserialize<T>()) since known serializers are static types and cannot be instance members.
Only accept bare identifier receivers (e.g., JsonSerializer.Deserialize<T>) for the namespace-gated serializer check. Rejects SomeObject.JsonSerializer, this.JsonSerializer, and any nested member access that extracts the same simple name from deeper in the expression tree.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements all 5 trigger payload type validation diagnostics from #42.
Supersedes #49 (closed to reset review context).
Diagnostics
Key Design Decisions
Validation
Closes #42