Skip to content

feat: Add SDK anti-pattern diagnostics (#44)#81

Merged
daviburg merged 9 commits into
Azure:mainfrom
daviburg:feature/sdk-anti-patterns
May 12, 2026
Merged

feat: Add SDK anti-pattern diagnostics (#44)#81
daviburg merged 9 commits into
Azure:mainfrom
daviburg:feature/sdk-anti-patterns

Conversation

@daviburg
Copy link
Copy Markdown
Member

@daviburg daviburg commented May 12, 2026

Summary

Extends the LSP server with five new SDK usage anti-pattern diagnostics (CSDK401-CSDK405), with codes CSDK406-CSDK407 reserved for future use.

Diagnostics Added

Code Severity Description
CSDK401 Warning [ConnectorOperation]\ operation value doesn't match any known operation (only when ConnectorName is absent; ConnectorName-present case is handled by CSDK009 in AttributeValidator)
CSDK402 Info *Input\ type used where *Output\ is expected (wrong payload direction)
CSDK403 Warning Catching \ConnectorException\ without checking \StatusCode\
CSDK404 Warning Async connector method called without \�wait\
CSDK405 Warning \CancellationToken\ available but not passed to connector API call

Implementation

  • *\SdkAntiPatternValidator* - new validator registered in DI alongside existing validators
  • CSDK401-403 use syntax-only analysis (fast, no compilation)
  • CSDK404-405 use semantic analysis via \CompilationService\
  • CSDK401 defers to AttributeValidator (CSDK009) when ConnectorName is present to avoid duplicates

Testing

  • 23+ test cases in \SdkAntiPatternValidatorTests.cs\
  • Build: 0 errors
  • Tests: 233 passed, 15 skipped, 0 failed

Closes #44

@daviburg daviburg self-assigned this May 12, 2026
@daviburg daviburg marked this pull request as ready for review May 12, 2026 19:20
@daviburg daviburg requested a review from a team as a code owner May 12, 2026 19:20
Copilot AI review requested due to automatic review settings May 12, 2026 19:20
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new Roslyn-based diagnostic validator to the LSP server to detect common Azure Connectors SDK usage anti-patterns, expanding the existing diagnostics suite.

Changes:

  • Introduces SdkAntiPatternValidator emitting diagnostics CSDK401–CSDK405 (with CSDK406–CSDK407 reserved).
  • Registers the new validator in server DI so it participates in diagnostic publishing.
  • Adds unit tests covering positive/negative cases and edge conditions for the new diagnostics.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
Server/Program.cs Registers the new SdkAntiPatternValidator in DI so it runs with other validators.
Server/Diagnostics/Validators/SdkAntiPatternValidator.cs Implements syntax + semantic checks for CSDK401–CSDK405.
Server/Diagnostics/DiagnosticCodes.cs Adds new diagnostic code constants CSDK401–CSDK407.
Server.Tests/SdkAntiPatternValidatorTests.cs Adds test coverage for the new validator behaviors.

Comment thread Server/Diagnostics/Validators/SdkAntiPatternValidator.cs Outdated
Comment thread Server/Diagnostics/Validators/SdkAntiPatternValidator.cs Outdated
Comment thread Server/Diagnostics/Validators/SdkAntiPatternValidator.cs
Comment thread Server/Diagnostics/Validators/SdkAntiPatternValidator.cs Outdated
Comment thread Server/Diagnostics/Validators/SdkAntiPatternValidator.cs Outdated
- CSDK401: connector-scoped validation + positional arg range placement
- CSDK402: unwrap nullable/qualified types before Input/Output check
- CSDK403: handle conditional access (ex?.StatusCode)
- CSDK405: use semantic model for CancellationToken parameter detection

Co-authored-by: Dobby <dobby@microsoft.com>
@daviburg daviburg requested a review from Copilot May 12, 2026 19:37
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comment thread Server/Diagnostics/Validators/SdkAntiPatternValidator.cs
Comment thread Server/Diagnostics/Validators/SdkAntiPatternValidator.cs
Comment thread Server/Diagnostics/Validators/SdkAntiPatternValidator.cs Outdated
…pace check

- Thread cancellationToken into syntax-only helpers (CSDK401-403)
- Unwrap chained invocations for CSDK404 (e.g. .ConfigureAwait())
- Add System.Threading namespace check for CancellationToken in CSDK405

Co-authored-by: Dobby <dobby@microsoft.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comment thread Server/Diagnostics/Validators/SdkAntiPatternValidator.cs
Comment thread Server/Diagnostics/Validators/SdkAntiPatternValidator.cs Outdated
Comment thread Server.Tests/SdkAntiPatternValidatorTests.cs
…, chained invocation tests

- CSDK402: break after first awaited initializer to avoid duplicate diagnostics
- Fix ConfigureAwait comments to use named parameter convention
- Add tests for chained ConfigureAwait fire-and-forget detection (CSDK404)

Co-authored-by: Dobby <dobby@microsoft.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment thread Server/Diagnostics/Validators/SdkAntiPatternValidator.cs
Comment thread Server/Diagnostics/Validators/SdkAntiPatternValidator.cs
- CSDK403: scan catch filter expression (when clause) for StatusCode
- CSDK405: add ThrowIfCancellationRequested in inner invocation loop
- New test for catch filter pattern

Co-authored-by: Dobby <dobby@microsoft.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment thread Server/Diagnostics/Validators/SdkAntiPatternValidator.cs Outdated
Comment thread Server/Diagnostics/Validators/SdkAntiPatternValidator.cs
…-qualified catch type

- CSDK401: resolve constant-style ConnectorName (FieldName) to canonical value via ConnectorNameConstants
- CSDK403: use ExtractRightmostIdentifier for alias-qualified types (global::)
- New tests for both patterns

Co-authored-by: Dobby <dobby@microsoft.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread Server/Program.cs
- Skip CSDK401 when ConnectorName is present (AttributeValidator CSDK009 handles it)
- CSDK401 now only fires when ConnectorName is absent (unique value-add)
- Updated tests to match new behavior

Co-authored-by: Dobby <dobby@microsoft.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

Server/Diagnostics/Validators/SdkAntiPatternValidator.cs:141

  • In CSDK401, ConnectorName is mapped via sdkIndex.ConnectorNameConstants (lines 127–141), but the value is never used for any validation because the code immediately skips whenever ConnectorName is non-null. This makes the mapping logic and its associated comments/tests misleading; consider either using the resolved ConnectorName to drive connector-scoped validation / resolution checks, or removing this unused resolution block if CSDK401 intentionally never runs when ConnectorName is provided.
                    // Try connector-scoped validation first when ConnectorName is present.
                    string? connectorName = SdkAntiPatternValidator.GetConnectorNameFromAttribute(attribute);

                    // Resolve constant-style ConnectorName (e.g., "Office365" from
                    // ConnectorNames.Office365) to the canonical value ("office365")
                    // using the SDK index's ConnectorNameConstants.
                    if (connectorName is not null)
                    {
                        SdkConstant? matchedConnector = sdkIndex.ConnectorNameConstants
                            .FirstOrDefault(connector =>
                                string.Equals(connector.FieldName, connectorName, StringComparison.OrdinalIgnoreCase) ||
                                string.Equals(connector.Value, connectorName, StringComparison.OrdinalIgnoreCase));

                        if (matchedConnector is not null)
                        {
                            connectorName = matchedConnector.Value;
                        }
                    }

Comment thread Server/Diagnostics/Validators/SdkAntiPatternValidator.cs Outdated
Comment thread Server/Diagnostics/Validators/SdkAntiPatternValidator.cs
Comment thread Server/Diagnostics/DiagnosticCodes.cs Outdated
Comment thread Server.Tests/SdkAntiPatternValidatorTests.cs
- CSDK401 doc: clarify defers to AttributeValidator when ConnectorName present
- CSDK402 doc: remove 'or vice versa' (only Input->Output direction implemented)
- Update test comment for constant-reference case

Co-authored-by: Dobby <dobby@microsoft.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comment thread Server/Diagnostics/Validators/SdkAntiPatternValidator.cs Outdated
Comment thread Server/Diagnostics/Validators/SdkAntiPatternValidator.cs Outdated
Comment thread Server/Diagnostics/Validators/SdkAntiPatternValidator.cs
…oken check

- Remove dead ConnectorName normalization code (unconditionally skipped)
- CSDK405: use semantic GetTypeInfo for CancellationToken argument detection
  instead of identifier text matching (handles cts.Token, default, etc.)
- Update PR description to clarify CSDK401 scope

Co-authored-by: Dobby <dobby@microsoft.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@daviburg daviburg merged commit 20fe02f into Azure:main May 12, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Diagnostics: SDK usage patterns and anti-pattern detection

2 participants