Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 32 additions & 30 deletions packages/http-client-csharp/cop-checks/braces.cop
Original file line number Diff line number Diff line change
@@ -1,49 +1,51 @@
# Rule: enforce braces on control-flow statements in the C# emitter.
# Rule: enforce braces on every control-flow body 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.:
# Flags `if` / `else` / `else if` / `for` / `foreach` / `while` whose body is not
# wrapped in a `{ ... }` block, whether the body is written inline or on its own
# line, e.g.:
#
# if (a) DoX(); -> violation
# if (x is null) throw ...; -> violation (guard clause without braces)
# if (a) DoX(); -> violation (inline body)
# if (x is null) throw ...; -> violation (inline guard clause)
# if (x is null) -> violation (Allman guard clause)
# return;
# for (...) Step(); -> violation
# while (a) Tick(); -> violation
# else Two(); -> violation
# else if (a) DoY(); -> violation (inner `if` body is unbraced)
#
# These are allowed (body is a brace-delimited block, Allman or K&R):
# These are allowed (body is a brace-delimited block):
#
# if (a) { ... }
# if (a) # Allman: brace on the next line
# if (a)
# {
# ...
# }
# else if (a) { ... } # `else if` chains are idiomatic when braced
#
# 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.
# The rule is driven by the code provider's structural parse rather than line
# text: a control-flow statement exposes `Braced` (true only when its body is a
# `{ ... }` block) and `Children` (its body statements). A violation is any
# unbraced control-flow statement. Keying on the structural model removes the
# false positives on multi-line conditions and recognizes `else if` chains
# correctly.
#
# 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.
# An `else if` is modeled as an `else` whose body is the inner (braced or
# unbraced) `if`, so the `else` itself reports `Braced == false`. That is
# idiomatic C#, so the `else` is excluded; the inner `if` is still checked on its
# own, so `else if (a) DoY();` is reported via that inner `if`.
#
# 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*\(.*\)')
# True when the statement has a direct child `if` -- i.e. it is the `else` of an
# `else if` chain. `where` is used instead of `any` so a child that carries no
# statement kind does not poison the result with a null.
predicate hasChildIf(Statement) => Statement.Children:where((c) => c.Kind == 'if').Count > 0

# 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)
# A control-flow statement (if / else / for / foreach / while) whose body is not a
# brace-delimited block, excluding the idiomatic `else if` chain header.
predicate missingBraces(Statement) =>
Statement:isControlFlow
&& !Statement.Braced
&& !(Statement.Kind == 'else' && Statement:hasChildIf)
4 changes: 2 additions & 2 deletions packages/http-client-csharp/cop-checks/main.cop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ import csharp
let cb : Codebase = provider('csharp')

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

test no-missing-braces = assert(cb.Lines:missingBraces.Count == 0, 'control-flow statements must use braces')
test no-missing-braces = assert(cb.Statements: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}'
Expand Down
2 changes: 1 addition & 1 deletion packages/http-client-csharp/eng/scripts/Invoke-Cop.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ param(
# Pinned cop release. Bump deliberately after verifying the cop-checks/ rules
# still run against the new release (cop can make breaking API changes
# between releases). Pass "latest" to test against the newest release.
[string] $Version = "v2026.6.10.1"
[string] $Version = "v2026.6.23.2"
)

$ErrorActionPreference = 'Stop'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ internal class StubLibraryVisitor : ScmLibraryVisitor
if (!type.DeclarationModifiers.HasFlag(TypeSignatureModifiers.Public) &&
!type.Name.StartsWith("Unknown", StringComparison.Ordinal) &&
!type.Name.Equals("MultiPartFormDataBinaryContent", StringComparison.Ordinal))
{
return null;
}

type.Update(xmlDocs: XmlDocProvider.Empty);
return type;
Expand All @@ -48,7 +50,9 @@ internal class StubLibraryVisitor : ScmLibraryVisitor
!IsParameterlessInternalCtorOnMrwSerializationType(constructor) &&
!IsInternalClientConstructor(constructor) &&
(constructor.EnclosingType is not ModelProvider model || model.DerivedModels.Count == 0))
{
return null;
}

constructor.Update(
bodyStatements: null,
Expand All @@ -61,18 +65,24 @@ internal class StubLibraryVisitor : ScmLibraryVisitor
private static bool IsInternalClientConstructor(ConstructorProvider constructor)
{
if (!constructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Internal))
{
return false;
}

return constructor.EnclosingType is ClientProvider;
}

private static bool IsParameterlessInternalCtorOnMrwSerializationType(ConstructorProvider constructor)
{
if (constructor.Signature.Parameters.Count != 0)
{
return false;
}

if (!constructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Internal))
{
return false;
}

return constructor.EnclosingType is MrwSerializationTypeDefinition;
}
Expand Down Expand Up @@ -101,7 +111,9 @@ private static bool IsCallingBaseCtor(ConstructorProvider constructor)
protected override MethodProvider? VisitMethod(MethodProvider method)
{
if (method.Signature.ExplicitInterface is null && !IsEffectivelyPublic(method.Signature.Modifiers))
{
return null;
}

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

Expand All @@ -116,7 +128,9 @@ private static bool IsCallingBaseCtor(ConstructorProvider constructor)
protected override PropertyProvider? VisitProperty(PropertyProvider property)
{
if (!property.IsDiscriminator && !IsEffectivelyPublic(property.Modifiers))
{
return null;
}

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

Expand All @@ -130,10 +144,14 @@ private static bool IsCallingBaseCtor(ConstructorProvider constructor)
private bool IsEffectivelyPublic(MethodSignatureModifiers modifiers)
{
if (modifiers.HasFlag(MethodSignatureModifiers.Public))
{
return true;
}

if (modifiers.HasFlag(MethodSignatureModifiers.Protected) && !modifiers.HasFlag(MethodSignatureModifiers.Private))
{
return true;
}

return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ private bool NeedsSerializationMethod()
// fixed enum with int based types, we do not write a method for serialization because it was embedded in the definition
bool isIntValueType = _enumProvider.EnumUnderlyingType.Equals(typeof(int)) || _enumProvider.EnumUnderlyingType.Equals(typeof(long));
if (!_enumType.IsExtensible && isIntValueType)
{
return false;
}

// otherwise we need a serialization method with the name of `ToSerial{UnderlyingTypeName}`
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,9 @@ private static bool ImplementsIPersistableModel(CSharpType? type, TypeProvider?
}

if (!type.IsFrameworkType || type.IsEnum || type.IsLiteral)
{
return false;
}

return type.FrameworkType.GetInterfaces().Any(i => i.Name == "IPersistableModel`1" || i.Name == "IJsonModel`1");
}
Expand All @@ -346,15 +348,19 @@ protected override XmlDocProvider BuildXmlDocs()
private static string RemovePeriods(string input)
{
if (string.IsNullOrEmpty(input))
{
return input;
}

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

foreach (char c in input)
{
if (c != '.')
{
buffer[index++] = c;
}
}

return buffer.Slice(0, index).ToString();
Expand All @@ -363,7 +369,9 @@ private static string RemovePeriods(string input)
private static bool ImplementsModelReaderWriter(Type type)
{
if (type.IsEnum || type.IsValueType)
{
return false;
}

return type.GetInterfaces().Any(i => i.Name == "IPersistableModel`1" || i.Name == "IJsonModel`1");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,10 @@ [new VariableExpression(valueVar.Type, valueVar.Declaration, IsOut: true)]))
private static bool IsDirectDynamicListProperty(PropertyProvider property)
{
if (!property.Type.IsList && !property.Type.IsArray)
{
return false;
}

return ScmCodeModelGenerator.Instance.TypeFactory.CSharpTypeMap.TryGetValue(
property.Type.ElementType,
out var provider) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1881,7 +1881,9 @@ private MethodBodyStatement WrapInIsDefined(
(IsNonNullableValueType(propertyType) || (propertyIsRequired && !propertyIsNullable)))
{
if (patchCheck == null)
{
return writePropertySerializationStatement;
}

return (propertyType.IsList || propertyType.IsArray)
? CreateConditionalPatchSerializationStatement(jsonSerializedName, null, writePropertySerializationStatement, writePropertySerializationStatement)
Expand Down Expand Up @@ -2208,26 +2210,34 @@ internal static MethodBodyStatement SerializeJsonValueCore(
if (valueType.IsStruct) // extensible enum
{
if (valueType.UnderlyingEnumType.Equals(typeof(string)))
{
return utf8JsonWriter.WriteStringValue(value.Invoke(nameof(ToString)));
}

return utf8JsonWriter.WriteNumberValue(value.Invoke($"ToSerial{valueType.UnderlyingEnumType.Name}"));
}
else // fixed enum
{
if (valueType.UnderlyingEnumType.Equals(typeof(int)))
{
// when the fixed enum is implemented as int, we cast to the value
return utf8JsonWriter.WriteNumberValue(value.CastTo(valueType.UnderlyingEnumType));
}

if (valueType.UnderlyingEnumType.Equals(typeof(string)))
{
return utf8JsonWriter.WriteStringValue(value.Invoke($"ToSerial{valueType.UnderlyingEnumType.Name}"));
}

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

// Handle non-enum types
if (!valueType.IsFrameworkType)
{
return utf8JsonWriter.WriteObjectValue(value.As(valueType), options: mrwOptionsParameter);
}

// Handle framework types
var frameworkType = valueType.FrameworkType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,9 @@ private List<MethodBodyStatement> AppendQueryParameters(ScopedApi uri, InputOper
foreach (var inputParameter in operation.Parameters)
{
if (inputParameter is not InputQueryParameter inputQueryParameter)
{
continue;
}

var queryStatement = BuildQueryParameterStatement(uri, inputQueryParameter, paramMap, operation, isNextLinkRequest);
if (queryStatement != null)
Expand Down Expand Up @@ -993,7 +995,9 @@ private static List<int> GetSuccessStatusCodes(InputOperation operation)
foreach (var response in operation.Responses)
{
if (response.IsErrorResponse)
{
continue;
}

foreach (var statusCode in response.StatusCodes)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,9 @@ private IEnumerable<MethodBodyStatement> GetStackVariablesForProtocolParamConver
foreach (var parameter in convenienceMethodParameters)
{
if (parameter.SpreadSource != null)
{
continue;
}

if (parameter.Location == ParameterLocation.Body)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,10 @@ public void ValidateFrameworkTypesWithMRWInterfacesAreIncluded()
{
// Map string property to a framework type that implements MRW
if (input == InputPrimitiveType.String)
{
return new CSharpType(typeof(FrameworkModelWithMRW));
}

return ScmCodeModelGenerator.Instance.TypeFactory.CreateCSharpType(input)!;
},
createCSharpTypeCoreFallback: input => input == InputPrimitiveType.String);
Expand Down Expand Up @@ -448,7 +451,10 @@ public void ValidateTypesWithoutMRWButWithMRWPropertiesAreTraversed()
createModelCore: input =>
{
if (input.Name == "NonMRWModel")
{
return new NonMRWModelProvider(input);
}

return new ModelProvider(input);
});

Expand Down Expand Up @@ -562,7 +568,10 @@ public void ValidateFrameworkTypePropertiesAreTraversedUsingReflection()
createCSharpTypeCore: input =>
{
if (input == InputPrimitiveType.String)
{
return new CSharpType(typeof(ComplexFrameworkType));
}

return ScmCodeModelGenerator.Instance.TypeFactory.CreateCSharpType(input)!;
},
createCSharpTypeCoreFallback: input => input == InputPrimitiveType.String);
Expand Down Expand Up @@ -684,7 +693,10 @@ public void ValidateNestedFrameworkTypeHierarchy()
createCSharpTypeCore: input =>
{
if (input == InputPrimitiveType.String)
{
return new CSharpType(typeof(NestedFrameworkType));
}

return ScmCodeModelGenerator.Instance.TypeFactory.CreateCSharpType(input)!;
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ private static (ModelProvider Model, MrwSerializationTypeDefinition Serializatio
createModelCore: (model) =>
{
if (model.Name == "Resource")
{
return systemBase;
}

return new ModelProvider(model);
},
createSerializationsCore: (inputType, typeProvider) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,10 @@ public void ValidateClientResponseClassifiers(InputClient inputClient)
foreach (var response in inputOperation.Responses)
{
if (response.IsErrorResponse)
{
continue;
}

expectedStatusCodes.AddRange(response.StatusCodes);
}

Expand Down
Loading
Loading