Add cop static-analysis rules for the C# emitter

packages/http-client-csharp/cop-checks/braces.cop

@@ -0,0 +1,49 @@
+# Rule: enforce braces on control-flow statements in the C# emitter.
+#
+# Flags `if` / `else` / `else if` / `for` / `foreach` / `while` whose body is
+# written inline on the same line without a `{ ... }` block, e.g.:
+#
+#     if (a) DoX();              -> violation
+#     if (x is null) throw ...;  -> violation (guard clause without braces)
+#     for (...) Step();          -> violation
+#     while (a) Tick();          -> violation
+#     else Two();                -> violation
+#
+# These are allowed (body is a brace-delimited block, Allman or K&R):
+#
+#     if (a) { ... }
+#     if (a)                     # Allman: brace on the next line
+#     {
+#         ...
+#     }
+#
+# A line is reported when it is a control-flow header that also contains an
+# inline body terminated by `;` and introduces no `{` block. Keying on a
+# trailing `;` avoids false positives on multi-line conditions whose first
+# line ends in an operator (e.g. `if (a &&`), which are not complete headers.
+#
+# Known limitation: an unbraced body written on a *separate* line (Allman
+# header followed by a single unbraced statement) is not detected, because the
+# header line is indistinguishable from a valid Allman braced header.
+#
+# Predicates only; the runnable command/test live in main.cop.
+
+import code
+
+# A control-flow header whose condition closes on this line.
+predicate isControlHeader(Line) => Line.Text:matches('^\s*(else\s+)?(if|for|foreach|while)\s*\(.*\)')
+
+# An `else` that is not `else if`.
+predicate isElseHeader(Line) => Line.Text:matches('^\s*else\b') && !Line.Text:matches('^\s*else\s+if\b')
+
+# The line introduces a `{ ... }` block.
+predicate hasBrace(Line) => Line.Text:matches('\{')
+
+# The line ends with a `;`-terminated statement (the inline body), optionally
+# followed by a trailing line comment.
+predicate hasInlineBody(Line) => Line.Text:matches(';\s*(//.*)?$')
+
+predicate missingBraces(Line) =>
+    !Line:hasBrace
+    && Line:hasInlineBody
+    && (Line:isControlHeader || Line:isElseHeader)

packages/http-client-csharp/cop-checks/main.cop

@@ -0,0 +1,28 @@
+# Entry point for the C# emitter cop checks.
+#
+# Each rule's detection logic (predicates) lives in its own file:
+#
+#     braces.cop          -> control-flow statements must use braces
+#     snippet-factory.cop -> expressions must be created via the Snippet factory
+#
+# This file wires those predicates to the C# codebase and exposes one runnable
+# command (and assertion) per rule. Run everything with:
+#
+#     cop cop-checks/main.cop -t .
+#
+# Or a single rule with `-c CHECK-BRACES` / `-c CHECK-SNIPPETS`.
+
+import code
+import csharp
+
+let cb = code.codebase('csharp')
+
+# Rule: braces.cop
+command CHECK-BRACES = foreach cb.Lines:missingBraces => '{error:@red} {item.File.Path}:{item.Number}: missing braces -> {item.Text}'
+
+test no-missing-braces = assert(cb.Lines:missingBraces.Count == 0, 'control-flow statements must use braces')
+
+# Rule: snippet-factory.cop
+command CHECK-SNIPPETS = foreach cb.Lines:usesCtorOverSnippet => '{error:@red} {item.File.Path}:{item.Number}: use the Snippet factory instead of constructing the expression directly -> {item.Text}'
+
+test no-direct-expression-construction = assert(cb.Lines:usesCtorOverSnippet.Count == 0, 'expression types should be created via the Snippet factory')

packages/http-client-csharp/cop-checks/snippet-factory.cop

@@ -0,0 +1,58 @@
+# Rule: prefer Snippet factory helpers over constructing expressions directly.
+#
+# Expression types should be created through the `Snippet` factory, not via
+# their constructors, so call sites read intentionally and stay consistent
+# (see https://github.com/microsoft/typespec/issues/3724). For example:
+#
+#     Static<File>().Invoke(...)          // good
+#     new InvokeMethodExpression(...)     // bad
+#
+# Only expression types that have a clear Snippet equivalent are flagged:
+#
+#     new LiteralExpression(...)          -> Literal(...)
+#     new TypeOfExpression(...)           -> TypeOf(...)
+#     new TypeReferenceExpression(...)    -> Static(...)
+#     new NewInstanceExpression(...)      -> New.Instance(...)
+#     new InvokeMethodExpression(recv,..) -> recv.Invoke(...) / Static(t).Invoke(...)
+#     new KeywordExpression("null"|"this"|"default", ...) -> Null / This / Default
+#     new BinaryOperatorExpression("||"|"&&"|"or", ...) -> .Or(...) / .And(...) / .OrPattern(...)
+#
+# Precision notes:
+#   * KeywordExpression is only flagged for the `null` / `this` / `default`
+#     keywords, since those are the only ones with Snippet equivalents
+#     (Null / This / Default). Keywords like `yield` / `break` have no
+#     equivalent and are not reported.
+#   * InvokeMethodExpression is only flagged when it has a non-null instance
+#     receiver (something to hang `.Invoke(...)` off of). An unqualified call
+#     `new InvokeMethodExpression(null, ...)` has no Snippet equivalent and is
+#     not reported.
+#
+# The Snippet factory layer (Snippets/) and the expression type definitions
+# (Expressions/) legitimately use these constructors and are excluded, as are
+# tests and TestData.
+#
+# Predicates only; the runnable command/test live in main.cop.
+
+import code
+
+predicate constructsViaCtor(Line) =>
+    Line.Text:matches('\bnew\s+(LiteralExpression|TypeOfExpression|TypeReferenceExpression|NewInstanceExpression)\s*\(')
+    || Line.Text:matches('\bnew\s+KeywordExpression\s*\(\s*"(null|this|default)"')
+    || Line.Text:matches('\bnew\s+InvokeMethodExpression\s*\(\s*(?!null\b)[A-Za-z_]')
+    || Line.Text:matches('\bnew\s+BinaryOperatorExpression\s*\(\s*"(\|\||&&|or)"')
+
+predicate isComment(Line) => Line.Text:matches('^\s*(//|\*|///)')
+
+# The Snippet factory layer and the expression type definitions are allowed to
+# construct expressions directly.
+predicate inFactoryOrDefs(Line) =>
+    Line.File.Path:contains('/Snippets/') || Line.File.Path:contains('/Expressions/')
+
+predicate isTestCode(Line) =>
+    Line.File.Path:contains('/test/') || Line.File.Path:contains('TestData')
+
+predicate usesCtorOverSnippet(Line) =>
+    Line:constructsViaCtor
+    && !Line:isComment
+    && !Line:inFactoryOrDefs
+    && !Line:isTestCode

packages/http-client-csharp/eng/scripts/Invoke-Cop.ps1

@@ -0,0 +1,199 @@
+#Requires -Version 7.0
+
+<#
+.SYNOPSIS
+Runs the Agent Cop (https://github.com/KrzysztofCwalina/cop) static-analysis
+rules under cop-checks/ against the C# generator sources.
+
+.DESCRIPTION
+Downloads a cop release for the current platform and runs the checks defined in
+cop-checks/main.cop against the generator. Exits with cop's exit code, so any
+rule violation fails the build (cop returns 1 when violations are found, 0 when
+clean).
+
+The checks are executed from a throwaway working directory that contains only
+the rule files. This keeps cop targeted at the generator (via -t) and avoids
+scanning node_modules or the rest of the repository.
+
+.PARAMETER Version
+The cop release tag to use, or "latest" (default) to track the newest release.
+
+NOTE: cop auto-restores its provider packages (code, csharp) from the GitHub
+feed, and the feed always serves the *latest* provider build. Those providers
+are version-locked to the cop runtime assembly, so the binary must match the
+provider version the feed currently serves -- otherwise the provider fails to
+load ("Could not load file or assembly 'cop, Version=...'"). Because the feed
+moves with upstream, we default to "latest" so the binary stays in lockstep with
+the providers. Pass an explicit tag only to reproduce a historical run.
+#>
+
+[CmdletBinding()]
+param(
+    [string] $Version = "latest"
+)
+
+$ErrorActionPreference = 'Stop'
+Set-StrictMode -Version 3.0
+
+$packageRoot = (Resolve-Path "$PSScriptRoot/../..").Path.Replace('\', '/')
+$copChecks = "$packageRoot/cop-checks"
+$generator = "$packageRoot/generator"
+
+# Map the current platform to a release asset and executable name.
+$arch = [System.Runtime.InteropServices.RuntimeInformation]::OSArchitecture.ToString().ToLowerInvariant()
+if ($arch -notin @('x64', 'arm64')) {
+    throw "Unsupported architecture for cop: $arch"
+}
+
+if ($IsWindows) {
+    $os = 'win'
+    $exe = 'cop.exe'
+}
+elseif ($IsLinux) {
+    $os = 'linux'
+    $exe = 'cop'
+}
+elseif ($IsMacOS) {
+    $os = 'osx'
+    $exe = 'cop'
+}
+else {
+    throw "Unsupported operating system for cop."
+}
+
+$asset = "cop-$os-$arch.zip"
+if ($Version -eq 'latest') {
+    # The /releases/latest/download/ redirect always resolves to the newest
+    # release asset without an API call (so no unauthenticated rate-limit risk).
+    $url = "https://github.com/KrzysztofCwalina/cop/releases/latest/download/$asset"
+}
+else {
+    $url = "https://github.com/KrzysztofCwalina/cop/releases/download/$Version/$asset"
+}
+
+# Cache the downloaded tool per version + platform. When tracking "latest" the
+# tag is not fixed, so always re-download to stay in lockstep with the feed's
+# provider packages.
+$toolDir = Join-Path ([System.IO.Path]::GetTempPath()) "cop-$Version-$os-$arch"
+$copPath = Join-Path $toolDir $exe
+
+if ($Version -eq 'latest' -or -not (Test-Path $copPath)) {
+    Write-Host "Downloading Agent Cop $Version ($asset)..." -ForegroundColor Cyan
+    Remove-Item -Recurse -Force $toolDir -ErrorAction SilentlyContinue
+    New-Item -ItemType Directory -Force $toolDir | Out-Null
+    $zipPath = Join-Path $toolDir $asset
+    Invoke-WebRequest -Uri $url -OutFile $zipPath
+    Expand-Archive -Path $zipPath -DestinationPath $toolDir -Force
+    if (-not $IsWindows) {
+        chmod +x $copPath
+    }
+}
+
+# Seed cop's provider package cache from the cop repo instead of relying on its
+# auto-restore feed. cop resolves feed packages via the GitHub REST API
+# (api.github.com), which is rate-limited to 60 requests/hour for unauthenticated
+# callers. On shared CI agents that budget is routinely exhausted, so the restore
+# fails with "Package 'code'/'csharp' not found in any configured feed" and the
+# csharp provider never loads. Fork PRs have no secrets, so we cannot supply a
+# GITHUB_TOKEN to raise the limit.
+#
+# Instead we vendor the two packages our checks import (code, csharp) directly
+# from the cop repo at the tag matching the downloaded binary, using a sparse,
+# blob-filtered git clone (git protocol, not the REST API; fetches only those
+# two directories). cop is version-locked between its runtime and providers, so
+# the package version must match the binary -- we read it from `cop -v`.
+$pkgCache = Join-Path $HOME ".cop/packages"
+$copVersion = (& $copPath -v) -split '\+' | Select-Object -First 1
+$copTag = "v$copVersion"
+# Map: cache package name -> path within the cop repo.
+$vendoredPackages = @{
+    'code'   = 'packages/code'
+    'csharp' = 'packages/dotnet/csharp'
+}
+$cloneDir = Join-Path ([System.IO.Path]::GetTempPath()) "cop-pkgsrc-$([System.Guid]::NewGuid().ToString('N'))"
+try {
+    Write-Host "Seeding cop provider packages from cop repo @ $copTag..." -ForegroundColor Cyan
+    git clone --quiet --no-checkout --depth 1 --branch $copTag --filter=blob:none `
+        https://github.com/KrzysztofCwalina/cop.git $cloneDir
+    if ($LASTEXITCODE -ne 0) {
+        throw "Failed to clone cop repo at tag $copTag for provider packages."
+    }
+    Push-Location $cloneDir
+    try {
+        git sparse-checkout set --no-cone @($vendoredPackages.Values) | Out-Null
+        git checkout --quiet $copTag
+        if ($LASTEXITCODE -ne 0) {
+            throw "Failed to check out provider package sources at $copTag."
+        }
+    }
+    finally {
+        Pop-Location
+    }
+    New-Item -ItemType Directory -Force $pkgCache | Out-Null
+    foreach ($name in $vendoredPackages.Keys) {
+        $src = Join-Path $cloneDir $vendoredPackages[$name]
+        $dest = Join-Path $pkgCache $name
+        if (-not (Test-Path $src)) {
+            throw "Expected package '$name' at '$($vendoredPackages[$name])' in the cop repo, but it was not found."
+        }
+        Remove-Item -Recurse -Force $dest -ErrorAction SilentlyContinue
+        Copy-Item -Recurse -Force $src $dest
+    }
+}
+finally {
+    Remove-Item -Recurse -Force $cloneDir -ErrorAction SilentlyContinue
+}
+
+# Run cop from a clean directory that contains only the rule files so it
+# analyzes the generator and nothing else.
+$runDir = Join-Path ([System.IO.Path]::GetTempPath()) "cop-run-$([System.Guid]::NewGuid().ToString('N'))"
+New-Item -ItemType Directory -Force $runDir | Out-Null
+try {
+    Copy-Item "$copChecks/*.cop" $runDir
+    Push-Location $runDir
+    try {
+        Write-Host "Running cop checks against $generator" -ForegroundColor Cyan
+
+        # cop auto-restores its provider packages (code, csharp, ...) from the
+        # GitHub feed on first run. That download can fail transiently on CI
+        # agents (rate limiting / network blips), surfacing as "not found in any
+        # configured feed" and a non-zero exit. Retry the run with backoff only
+        # for that transient case. A provider *load* failure ("is not loaded" /
+        # "Could not load file or assembly") is a version mismatch between the
+        # cop binary and the feed's provider build -- not transient -- which the
+        # default "latest" $Version avoids; so fail fast instead of retrying.
+        $maxAttempts = 5
+        $transientPattern = 'not found in any configured feed'
+        for ($attempt = 1; $attempt -le $maxAttempts; $attempt++) {
+            $output = & $copPath main.cop -t $generator 2>&1 | Out-String
+            Write-Host $output
+            $exit = $LASTEXITCODE
+
+            if ($exit -eq 0 -or $output -notmatch $transientPattern) {
+                break
+            }
+
+            if ($attempt -lt $maxAttempts) {
+                $delay = [Math]::Min(30, [Math]::Pow(2, $attempt))
+                Write-Host "Package restore failed (transient). Retrying in $delay s..." -ForegroundColor Yellow
+                Start-Sleep -Seconds $delay
+            }
+            else {
+                throw "Failed to restore cop provider packages after $maxAttempts attempts."
+            }
+        }
+    }
+    finally {
+        Pop-Location
+    }
+}
+finally {
+    Remove-Item -Recurse -Force $runDir -ErrorAction SilentlyContinue
+}
+
+if ($exit -ne 0) {
+    Write-Error "cop reported violations (exit code $exit). Fix the reported issues before merging."
+    exit $exit
+}
+
+Write-Host "cop checks passed." -ForegroundColor Green

packages/http-client-csharp/eng/scripts/Test-Packages.ps1

@@ -21,6 +21,9 @@ try {
             Invoke-LoggedCommand "npm run build" -GroupOutput
             Invoke-LoggedCommand "npm run test:emitter" -GroupOutput
 
+            # enforce cop static-analysis rules on the generator sources
+            Invoke-LoggedCommand "./eng/scripts/Invoke-Cop.ps1" -GroupOutput
+
             # test the generator
             Invoke-LoggedCommand "dotnet test ./generator" -GroupOutput
 

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientOptionsProvider.cs

@@ -342,7 +342,7 @@ protected override ConstructorProvider[] BuildConstructors()
                     // ServiceVersion.Version => "version"
                     switchCases.Add(new SwitchCaseExpression(
                         Static(serviceVersionEnum.Type).Property(serviceVersionMember.Name),
-                        new LiteralExpression(serviceVersionMember.Value)));
+                        Literal(serviceVersionMember.Value)));
                 }
 
                 switchCases.Add(SwitchCaseExpression.Default(ThrowExpression(New.NotSupportedException(ValueExpression.Empty))));

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientPipelineExtensionsDefinition.cs

@@ -92,7 +92,7 @@ private MethodBodyStatement GetProcessHeadAsBoolMessageBody(HttpResponseApi resp
                     }),
                     new SwitchCaseStatement(Array.Empty<ValueExpression>(), new MethodBodyStatement[]
                     {
-                        Return(new NewInstanceExpression(ErrorResultSnippets.ErrorResultType.MakeGenericType([typeof(bool)]), [response, new NewInstanceExpression(ScmCodeModelGenerator.Instance.TypeFactory.ClientResponseApi.ClientResponseExceptionType, [response])]))
+                        Return(New.Instance(ErrorResultSnippets.ErrorResultType.MakeGenericType([typeof(bool)]), [response, New.Instance(ScmCodeModelGenerator.Instance.TypeFactory.ClientResponseApi.ClientResponseExceptionType, [response])]))
                     })
                 ]),
             };

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs

@@ -748,7 +748,7 @@ private IEnumerable<ConstructorProvider> BuildSettingsConstructors()
                 var propAccess = new MemberExpression(new NullConditionalExpression(settingsParam), propName);
                 // Value types (enums, primitives) need ?? default since null-conditional returns T?
                 ValueExpression arg = param.Type.IsValueType
-                    ? propAccess.NullCoalesce(new KeywordExpression("default", null))
+                    ? propAccess.NullCoalesce(Default)
                     : propAccess;
                 args.Add(arg);
             }

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ModelSerializationExtensionsDefinition.Xml.cs

@@ -254,7 +254,7 @@ private MethodProvider BuildXmlWriteObjectValueMethodProvider()
                 });
 
             var defaultCase = SwitchCaseStatement.Default(
-                Throw(New.NotSupportedException(new FormattableStringExpression("Not supported type {0}", [new TypeOfExpression(_t)]))));
+                Throw(New.NotSupportedException(new FormattableStringExpression("Not supported type {0}", [TypeOf(_t)]))));
 
             var body = new SwitchStatement(value, [persistableModelCase, defaultCase]);
 

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.cs

@@ -2382,7 +2382,7 @@ private static bool ValueTypeIsNumber(Type valueType) =>
         {
             // when `@encode(string)`, the type is serialized as string, so we need to deserialize it from string
             // sbyte.Parse(element.GetString())
-            SerializationFormat.Int_String => new InvokeMethodExpression(type, nameof(int.Parse), [element.GetString()]),
+            SerializationFormat.Int_String => Static(type).Invoke(nameof(int.Parse), [element.GetString()]),
             _ => type switch
             {
                 Type t when t == typeof(long) => element.GetInt64(),