Skip to content

Commit c5d3e9f

Browse files
Enforce braces on all control-flow bodies in the C# emitter (cop) (#11062)
Upgrades the C# emitter `braces.cop` cop rule to use the cop library's new structural braces support, and extends it to enforce braces on **every** control-flow body. ## Changes - **`cop-checks/braces.cop`** — replaced the previous line-text regex heuristic with the code provider's structural parse (`Statement.Braced` / `Statement.Children` / `isControlFlow`). Flags any unbraced `if` / `else` / `for` / `foreach` / `while` body, whether written inline or on its own line (Allman). Idiomatic `else if` chains (an unbraced `else` whose child is an `if`) are excluded, while the inner `if` is still checked. - **`cop-checks/main.cop`** — wires the predicate to `cb.Statements` with an updated violation message. - **`eng/scripts/Invoke-Cop.ps1`** — bumped the pinned cop release `v2026.6.10.1` → `v2026.6.23.2` (structural braces support). - **179 existing violations fixed** across 77 `.cs` files by adding braces via the Roslyn IDE0011 add-braces fixer (`dotnet format style`). ## Validation - `npm run cop` reports `cop checks passed.` (0 violations). - `dotnet build Microsoft.TypeSpec.Generator.sln -c Release` succeeds with 0 warnings / 0 errors (StyleCop clean). --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 85ff828 commit c5d3e9f

80 files changed

Lines changed: 476 additions & 87 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 32 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,51 @@
1-
# Rule: enforce braces on control-flow statements in the C# emitter.
1+
# Rule: enforce braces on every control-flow body in the C# emitter.
22
#
3-
# Flags `if` / `else` / `else if` / `for` / `foreach` / `while` whose body is
4-
# written inline on the same line without a `{ ... }` block, e.g.:
3+
# Flags `if` / `else` / `else if` / `for` / `foreach` / `while` whose body is not
4+
# wrapped in a `{ ... }` block, whether the body is written inline or on its own
5+
# line, e.g.:
56
#
6-
# if (a) DoX(); -> violation
7-
# if (x is null) throw ...; -> violation (guard clause without braces)
7+
# if (a) DoX(); -> violation (inline body)
8+
# if (x is null) throw ...; -> violation (inline guard clause)
9+
# if (x is null) -> violation (Allman guard clause)
10+
# return;
811
# for (...) Step(); -> violation
912
# while (a) Tick(); -> violation
1013
# else Two(); -> violation
14+
# else if (a) DoY(); -> violation (inner `if` body is unbraced)
1115
#
12-
# These are allowed (body is a brace-delimited block, Allman or K&R):
16+
# These are allowed (body is a brace-delimited block):
1317
#
1418
# if (a) { ... }
15-
# if (a) # Allman: brace on the next line
19+
# if (a)
1620
# {
1721
# ...
1822
# }
23+
# else if (a) { ... } # `else if` chains are idiomatic when braced
1924
#
20-
# A line is reported when it is a control-flow header that also contains an
21-
# inline body terminated by `;` and introduces no `{` block. Keying on a
22-
# trailing `;` avoids false positives on multi-line conditions whose first
23-
# line ends in an operator (e.g. `if (a &&`), which are not complete headers.
25+
# The rule is driven by the code provider's structural parse rather than line
26+
# text: a control-flow statement exposes `Braced` (true only when its body is a
27+
# `{ ... }` block) and `Children` (its body statements). A violation is any
28+
# unbraced control-flow statement. Keying on the structural model removes the
29+
# false positives on multi-line conditions and recognizes `else if` chains
30+
# correctly.
2431
#
25-
# Known limitation: an unbraced body written on a *separate* line (Allman
26-
# header followed by a single unbraced statement) is not detected, because the
27-
# header line is indistinguishable from a valid Allman braced header.
32+
# An `else if` is modeled as an `else` whose body is the inner (braced or
33+
# unbraced) `if`, so the `else` itself reports `Braced == false`. That is
34+
# idiomatic C#, so the `else` is excluded; the inner `if` is still checked on its
35+
# own, so `else if (a) DoY();` is reported via that inner `if`.
2836
#
2937
# Predicates only; the runnable command/test live in main.cop.
3038

3139
import code
3240

33-
# A control-flow header whose condition closes on this line.
34-
predicate isControlHeader(Line) => Line.Text:matches('^\s*(else\s+)?(if|for|foreach|while)\s*\(.*\)')
41+
# True when the statement has a direct child `if` -- i.e. it is the `else` of an
42+
# `else if` chain. `where` is used instead of `any` so a child that carries no
43+
# statement kind does not poison the result with a null.
44+
predicate hasChildIf(Statement) => Statement.Children:where((c) => c.Kind == 'if').Count > 0
3545

36-
# An `else` that is not `else if`.
37-
predicate isElseHeader(Line) => Line.Text:matches('^\s*else\b') && !Line.Text:matches('^\s*else\s+if\b')
38-
39-
# The line introduces a `{ ... }` block.
40-
predicate hasBrace(Line) => Line.Text:matches('\{')
41-
42-
# The line ends with a `;`-terminated statement (the inline body), optionally
43-
# followed by a trailing line comment.
44-
predicate hasInlineBody(Line) => Line.Text:matches(';\s*(//.*)?$')
45-
46-
predicate missingBraces(Line) =>
47-
!Line:hasBrace
48-
&& Line:hasInlineBody
49-
&& (Line:isControlHeader || Line:isElseHeader)
46+
# A control-flow statement (if / else / for / foreach / while) whose body is not a
47+
# brace-delimited block, excluding the idiomatic `else if` chain header.
48+
predicate missingBraces(Statement) =>
49+
Statement:isControlFlow
50+
&& !Statement.Braced
51+
&& !(Statement.Kind == 'else' && Statement:hasChildIf)

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ import csharp
1818
let cb : Codebase = provider('csharp')
1919

2020
# Rule: braces.cop
21-
command CHECK-BRACES = foreach cb.Lines:missingBraces => '{error:@red} {item.File.Path}:{item.Number}: missing braces -> {item.Text}'
21+
command CHECK-BRACES = foreach cb.Statements:missingBraces => '{error:@red} {item.File.Path}:{item.Line}: missing braces on {item.Kind} statement'
2222

23-
test no-missing-braces = assert(cb.Lines:missingBraces.Count == 0, 'control-flow statements must use braces')
23+
test no-missing-braces = assert(cb.Statements:missingBraces.Count == 0, 'control-flow statements must use braces')
2424

2525
# Rule: snippet-factory.cop
2626
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}'

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ param(
3838
# Pinned cop release. Bump deliberately after verifying the cop-checks/ rules
3939
# still run against the new release (cop can make breaking API changes
4040
# between releases). Pass "latest" to test against the newest release.
41-
[string] $Version = "v2026.6.10.1"
41+
[string] $Version = "v2026.6.23.2"
4242
)
4343

4444
$ErrorActionPreference = 'Stop'

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel.StubLibrary/src/StubLibraryVisitor.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ internal class StubLibraryVisitor : ScmLibraryVisitor
2121
if (!type.DeclarationModifiers.HasFlag(TypeSignatureModifiers.Public) &&
2222
!type.Name.StartsWith("Unknown", StringComparison.Ordinal) &&
2323
!type.Name.Equals("MultiPartFormDataBinaryContent", StringComparison.Ordinal))
24+
{
2425
return null;
26+
}
2527

2628
type.Update(xmlDocs: XmlDocProvider.Empty);
2729
return type;
@@ -48,7 +50,9 @@ internal class StubLibraryVisitor : ScmLibraryVisitor
4850
!IsParameterlessInternalCtorOnMrwSerializationType(constructor) &&
4951
!IsInternalClientConstructor(constructor) &&
5052
(constructor.EnclosingType is not ModelProvider model || model.DerivedModels.Count == 0))
53+
{
5154
return null;
55+
}
5256

5357
constructor.Update(
5458
bodyStatements: null,
@@ -61,18 +65,24 @@ internal class StubLibraryVisitor : ScmLibraryVisitor
6165
private static bool IsInternalClientConstructor(ConstructorProvider constructor)
6266
{
6367
if (!constructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Internal))
68+
{
6469
return false;
70+
}
6571

6672
return constructor.EnclosingType is ClientProvider;
6773
}
6874

6975
private static bool IsParameterlessInternalCtorOnMrwSerializationType(ConstructorProvider constructor)
7076
{
7177
if (constructor.Signature.Parameters.Count != 0)
78+
{
7279
return false;
80+
}
7381

7482
if (!constructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Internal))
83+
{
7584
return false;
85+
}
7686

7787
return constructor.EnclosingType is MrwSerializationTypeDefinition;
7888
}
@@ -101,7 +111,9 @@ private static bool IsCallingBaseCtor(ConstructorProvider constructor)
101111
protected override MethodProvider? VisitMethod(MethodProvider method)
102112
{
103113
if (method.Signature.ExplicitInterface is null && !IsEffectivelyPublic(method.Signature.Modifiers))
114+
{
104115
return null;
116+
}
105117

106118
method.Signature.Update(modifiers: method.Signature.Modifiers & ~MethodSignatureModifiers.Async);
107119

@@ -116,7 +128,9 @@ private static bool IsCallingBaseCtor(ConstructorProvider constructor)
116128
protected override PropertyProvider? VisitProperty(PropertyProvider property)
117129
{
118130
if (!property.IsDiscriminator && !IsEffectivelyPublic(property.Modifiers))
131+
{
119132
return null;
133+
}
120134

121135
var propertyBody = new ExpressionPropertyBody(_throwNull, property.Body.HasSetter ? _throwNull : null);
122136

@@ -130,10 +144,14 @@ private static bool IsCallingBaseCtor(ConstructorProvider constructor)
130144
private bool IsEffectivelyPublic(MethodSignatureModifiers modifiers)
131145
{
132146
if (modifiers.HasFlag(MethodSignatureModifiers.Public))
147+
{
133148
return true;
149+
}
134150

135151
if (modifiers.HasFlag(MethodSignatureModifiers.Protected) && !modifiers.HasFlag(MethodSignatureModifiers.Private))
152+
{
136153
return true;
154+
}
137155

138156
return false;
139157
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,9 @@ private bool NeedsSerializationMethod()
4949
// fixed enum with int based types, we do not write a method for serialization because it was embedded in the definition
5050
bool isIntValueType = _enumProvider.EnumUnderlyingType.Equals(typeof(int)) || _enumProvider.EnumUnderlyingType.Equals(typeof(long));
5151
if (!_enumType.IsExtensible && isIntValueType)
52+
{
5253
return false;
54+
}
5355

5456
// otherwise we need a serialization method with the name of `ToSerial{UnderlyingTypeName}`
5557
return true;

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,9 @@ private static bool ImplementsIPersistableModel(CSharpType? type, TypeProvider?
337337
}
338338

339339
if (!type.IsFrameworkType || type.IsEnum || type.IsLiteral)
340+
{
340341
return false;
342+
}
341343

342344
return type.FrameworkType.GetInterfaces().Any(i => i.Name == "IPersistableModel`1" || i.Name == "IJsonModel`1");
343345
}
@@ -356,15 +358,19 @@ protected override XmlDocProvider BuildXmlDocs()
356358
private static string RemovePeriods(string input)
357359
{
358360
if (string.IsNullOrEmpty(input))
361+
{
359362
return input;
363+
}
360364

361365
Span<char> buffer = stackalloc char[input.Length];
362366
int index = 0;
363367

364368
foreach (char c in input)
365369
{
366370
if (c != '.')
371+
{
367372
buffer[index++] = c;
373+
}
368374
}
369375

370376
return buffer.Slice(0, index).ToString();
@@ -373,7 +379,9 @@ private static string RemovePeriods(string input)
373379
private static bool ImplementsModelReaderWriter(Type type)
374380
{
375381
if (type.IsEnum || type.IsValueType)
382+
{
376383
return false;
384+
}
377385

378386
return type.GetInterfaces().Any(i => i.Name == "IPersistableModel`1" || i.Name == "IJsonModel`1");
379387
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,10 @@ [new VariableExpression(valueVar.Type, valueVar.Declaration, IsOut: true)]))
505505
private static bool IsDirectDynamicListProperty(PropertyProvider property)
506506
{
507507
if (!property.Type.IsList && !property.Type.IsArray)
508+
{
508509
return false;
510+
}
511+
509512
return ScmCodeModelGenerator.Instance.TypeFactory.CSharpTypeMap.TryGetValue(
510513
property.Type.ElementType,
511514
out var provider) &&

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1927,7 +1927,9 @@ private MethodBodyStatement WrapInIsDefined(
19271927
(IsNonNullableValueType(propertyType) || (propertyIsRequired && !propertyIsNullable)))
19281928
{
19291929
if (patchCheck == null)
1930+
{
19301931
return writePropertySerializationStatement;
1932+
}
19311933

19321934
return (propertyType.IsList || propertyType.IsArray)
19331935
? CreateConditionalPatchSerializationStatement(jsonSerializedName, null, writePropertySerializationStatement, writePropertySerializationStatement)
@@ -2254,26 +2256,34 @@ internal static MethodBodyStatement SerializeJsonValueCore(
22542256
if (valueType.IsStruct) // extensible enum
22552257
{
22562258
if (valueType.UnderlyingEnumType.Equals(typeof(string)))
2259+
{
22572260
return utf8JsonWriter.WriteStringValue(value.Invoke(nameof(ToString)));
2261+
}
22582262

22592263
return utf8JsonWriter.WriteNumberValue(value.Invoke($"ToSerial{valueType.UnderlyingEnumType.Name}"));
22602264
}
22612265
else // fixed enum
22622266
{
22632267
if (valueType.UnderlyingEnumType.Equals(typeof(int)))
2268+
{
22642269
// when the fixed enum is implemented as int, we cast to the value
22652270
return utf8JsonWriter.WriteNumberValue(value.CastTo(valueType.UnderlyingEnumType));
2271+
}
22662272

22672273
if (valueType.UnderlyingEnumType.Equals(typeof(string)))
2274+
{
22682275
return utf8JsonWriter.WriteStringValue(value.Invoke($"ToSerial{valueType.UnderlyingEnumType.Name}"));
2276+
}
22692277

22702278
return utf8JsonWriter.WriteNumberValue(value.Invoke($"ToSerial{valueType.UnderlyingEnumType.Name}"));
22712279
}
22722280
}
22732281

22742282
// Handle non-enum types
22752283
if (!valueType.IsFrameworkType)
2284+
{
22762285
return utf8JsonWriter.WriteObjectValue(value.As(valueType), options: mrwOptionsParameter);
2286+
}
22772287

22782288
// Handle framework types
22792289
var frameworkType = valueType.FrameworkType;

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,9 @@ private List<MethodBodyStatement> AppendQueryParameters(ScopedApi uri, InputOper
469469
foreach (var inputParameter in operation.Parameters)
470470
{
471471
if (inputParameter is not InputQueryParameter inputQueryParameter)
472+
{
472473
continue;
474+
}
473475

474476
var queryStatement = BuildQueryParameterStatement(uri, inputQueryParameter, paramMap, operation, isNextLinkRequest);
475477
if (queryStatement != null)
@@ -993,7 +995,9 @@ private static List<int> GetSuccessStatusCodes(InputOperation operation)
993995
foreach (var response in operation.Responses)
994996
{
995997
if (response.IsErrorResponse)
998+
{
996999
continue;
1000+
}
9971001

9981002
foreach (var statusCode in response.StatusCodes)
9991003
{

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,9 @@ private IEnumerable<MethodBodyStatement> GetStackVariablesForProtocolParamConver
278278
foreach (var parameter in convenienceMethodParameters)
279279
{
280280
if (parameter.SpreadSource != null)
281+
{
281282
continue;
283+
}
282284

283285
if (parameter.Location == ParameterLocation.Body)
284286
{

0 commit comments

Comments
 (0)