Enable CA1062 (validate arguments of public methods) to catch missing null-argument guards at build time

Open
#199 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
52/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Quiet
Tech stack
csharp

Research direction

Start with .editorconfig and eng/build/Engineering.props, then inspect ConnectorClientBase for the reported CallConnectorAsync violations. Run a build with CA1062 enabled to identify handwritten and generated-code diagnostics. Done means the rule is configured, violations are fixed or justified at the generator, and the build remains green.

Written by the indexing model from the issue text.

Description

.NET enhancement

Summary

Enable CA1062 ("Validate arguments of externally visible methods") as a build-enforced analyzer rule so missing null-argument guards on public/protected APIs are caught deterministically at build time instead of ad hoc during code review.

Motivation

Missing ArgumentNullException guards on public entry points are a recurring manual review nit (including from the Copilot PR reviewer). Each occurrence currently costs a review round-trip: reviewer spots it, author adds ArgumentNullException.ThrowIfNull(...), re-review. CA1062 catches the entire class at compile time, consistently, before a PR is ever opened.

This rule is disabled by default in the SDK's current analysis mode (AnalysisMode=Default), so it never fires today even though EnableNETAnalyzers=true and TreatWarningsAsErrors=true are set in eng/build/Engineering.props.

Evidence (from a local AnalysisMode=All build)

CA1062 fires on hand-written production code that skips argument validation, e.g. in ConnectorClientBase:

  • CallConnectorAsync<TResponse>(HttpMethod method, ...)method not validated before use.
  • CallConnectorAsync(HttpMethod method, ...) (no-body overload) — same.

New code such as ConnectorTriggerPayload already validates via ArgumentNullException.ThrowIfNull, so it is clean — which shows the rule aligns with the direction the codebase is already moving.

Scope / considerations

  • Generated code is in scope. The generated clients are produced by our own CodefulSdkGenerator (BPM repo) and are a core part of the value proposition; they must not ship missing guards. Enabling CA1062 will surface any generated entry points lacking validation — if it flags generated code, the fix belongs in the generator's emit templates, not a per-file suppression.
  • Severity: start as warning (which TreatWarningsAsErrors escalates to an error) or suggestion for a soft rollout, then promote.
  • Rollout: enable via .editorconfig (dotnet_diagnostic.CA1062.severity = warning) rather than raising the whole AnalysisMode, to keep the change targeted and avoid the ~17k unrelated diagnostics that AnalysisMode=All produces.
  • Note: CA1062 has known limitations (it may not track validation performed in a called helper), so expect a few false positives to suppress with justification.

Acceptance criteria

  • dotnet_diagnostic.CA1062.severity set in .editorconfig.
  • Existing hand-written violations fixed (add ArgumentNullException.ThrowIfNull).
  • Generated-code violations (if any) fixed at the generator, or a documented, justified suppression policy agreed.
  • Build stays green with the rule enforced.

Filed from analysis on PR #196; not part of that PR's scope.

Dominant language
C#
Stars
3
Forks
5
Avg merge
2d 56m
Merged PRs (30d)
8

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from Azure/Connectors-NET-SDK

All issues in Azure/Connectors-NET-SDK

Similar issues

More C# issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.