Skip to content

fix: Add StringComparison to all string operations and promote CA1307/CA1310 to warning (#34, #8)#84

Merged
daviburg merged 2 commits into
Azure:mainfrom
daviburg:fix/string-comparison
May 13, 2026
Merged

fix: Add StringComparison to all string operations and promote CA1307/CA1310 to warning (#34, #8)#84
daviburg merged 2 commits into
Azure:mainfrom
daviburg:fix/string-comparison

Conversation

@daviburg
Copy link
Copy Markdown
Member

Summary

Fixes #34 (CA1307/CA1310 StringComparison violations) and #8 (small code quality fixes).

CA1307/CA1310 — StringComparison (38 violations fixed)

Added explicit StringComparison.Ordinal to all .Contains(), .StartsWith(), .EndsWith(), .IndexOf(), and .Replace() calls across the server codebase. Promoted CA1307/CA1310 severity from suggestion to warning in .editorconfig so future violations are caught at build time (TreatWarningsAsErrors is enabled).

Issue #8 — Code quality fixes

  • BufferManager.cs — Added namespace SdkLspServer; and changed class to internal (was public in global namespace). Updated 5 consuming handler classes to internal to match.
  • DynamicValuesCacheEntry.cs — Removed unused using System.Collections.Concurrent;.
  • .editorconfig — Removed non-standard guidelines = 120 key.
  • SdkIndex.cs — Changed 2 bare catch (Exception e) blocks to catch (Exception ex) when (!ex.IsFatal()) matching the pattern used elsewhere in the file.

Verification

  • dotnet buildzero warnings (all 38 CA1307/CA1310 violations resolved)
  • dotnet test233 passed, 15 skipped, 0 failed (matches baseline)
  • No test files modified
  • No functional logic changes

Copilot AI review requested due to automatic review settings May 13, 2026 00:11
@daviburg daviburg requested a review from a team as a code owner May 13, 2026 00:11
@daviburg daviburg self-assigned this May 13, 2026
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

This PR addresses analyzer enforcement for explicit string comparison across the LSP server and applies small code quality cleanups from prior review feedback.

Changes:

  • Adds explicit StringComparison.Ordinal to string operations and promotes CA1307/CA1310 to warnings.
  • Moves BufferManager into the SdkLspServer namespace and reduces selected handler classes from public to internal.
  • Cleans up unused using/config entries and updates exception filters in SdkIndex.

Reviewed changes

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

Show a summary per file
File Description
.editorconfig Promotes CA1307/CA1310 severity and removes non-standard guidelines setting.
Server/BufferManager.cs Adds namespace and changes BufferManager to internal.
Server/SdkIndex.cs Adds ordinal comparison and fatal-exception filtering.
Server/Store/DynamicData/DynamicValuesCacheEntry.cs Removes unused using.
Server/Handlers/HoverHandler/HoverHandler.cs Adds explicit string comparisons and makes handler internal.
Server/Handlers/HoverHandler/SdkDynamicOperationsDiscovery.cs Adds explicit string comparisons.
Server/Handlers/CompletionHandler/CompletionHandler.cs Adds explicit string comparisons and makes handler internal.
Server/Handlers/CodeLensHandler.cs Makes handler internal.
Server/Handlers/CodeActionHandler/DynamicSchemaCodeActionHandler.cs Adds explicit string comparison and makes handler internal.
Server/Handlers/CodeActionHandler/GenerateDynamicSchemaCommandHandler.cs Makes command handler internal.

Comment thread .editorconfig
@daviburg daviburg force-pushed the fix/string-comparison branch from 137cea0 to ee3db31 Compare May 13, 2026 00:16
Copilot AI review requested due to automatic review settings May 13, 2026 00:18
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 13 out of 13 changed files in this pull request and generated no new comments.

@daviburg daviburg merged commit 7dd3680 into Azure:main May 13, 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.

Fix 40 pre-existing CA1307/CA1310 StringComparison violations and promote to warning

2 participants