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
959 changes: 620 additions & 339 deletions .editorconfig

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,12 @@ dotnet run --project "tests/Refit.GeneratorTests/Refit.GeneratorTests.csproj" -f
- `RefitEmitGeneratedCodeMarkers=false` is used by generated-code compliance tests so analyzers treat generator output as normal source.
- Keep generated source compatible with the repository `.editorconfig`; avoid broad `#pragma warning disable`.

### Generator performance

Settle generator perf with benchmarks and EventPipe traces, never by inspection or a hunch that "it's already optimal" — build overall (whole-generator) and micro (per-component) benchmarks and let the traces show where the real cost is before optimizing. Context, not a verdict that nothing can improve: the generator already uses `ForAttributeWithMetadataName`, value-equatable `readonly record struct` models + `ImmutableEquatableArray` (incremental caching depends on this), `PooledStringBuilder`, multi-slot Roslyn constants (`#if ROSLYN_5_OR_GREATER` for 5.0-only APIs), and incremental-cache regression tests.

Generator / incremental-pipeline benchmarks go in their own BenchmarkDotNet project (`src/benchmarks/Refit.Generator.Benchmarks`), separate from the runtime `Refit.Benchmarks`, built on the existing BenchmarkDotNet setup — never a bespoke driver. Profile with BenchmarkDotNet's native `[EventPipeProfiler(EventPipeProfile.GcVerbose|CpuSampling)]` diagnoser — EventPipe is the right lens for a Roslyn generator, whereas `[MemoryDiagnoser]` is a weak signal for whole-generator runs. Widening a generator member `private` -> `internal` (with `InternalsVisibleTo`) to micro-benchmark it is fine. Name benchmark classes for the component under test (parser, emitter, query/path building), not the measurement type.

Useful validation:

```bash
Expand Down
1 change: 1 addition & 0 deletions src/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@
<PackageReference Include="MinVer" PrivateAssets="all"/>
<PackageReference Include="StyleSharp.Analyzers" PrivateAssets="all"/>
<PackageReference Include="PerformanceSharp.Analyzers" PrivateAssets="all"/>
<PackageReference Include="SecuritySharp.Analyzers" PrivateAssets="all"/>
<PackageReference Include="Roslynator.Analyzers" PrivateAssets="All"/>
<PackageReference Include="SonarAnalyzer.CSharp" PrivateAssets="all"/>
</ItemGroup>
Expand Down
9 changes: 5 additions & 4 deletions src/Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@

<PropertyGroup Label="Shared Version Variables">
<PrimitivesVersion>7.0.0</PrimitivesVersion>
<TUnitVersion>1.59.0</TUnitVersion>
<TUnitVersion>1.61.38</TUnitVersion>

<!-- StyleSharp and PerformanceSharp ship from the same repo on a single version line. -->
<SharpAnalyzersVersion>3.33.0</SharpAnalyzersVersion>
<SharpAnalyzersVersion>3.38.1</SharpAnalyzersVersion>
</PropertyGroup>

<PropertyGroup Label="Framework-Aligned Versions">
<AspNetVersion>8.0.28</AspNetVersion>
<AspNetVersion>8.0.29</AspNetVersion>
<AspNetVersion Condition="$(TargetFramework.StartsWith('net9'))">9.0.18</AspNetVersion>
<AspNetVersion Condition="$(TargetFramework.StartsWith('net10'))">10.0.10</AspNetVersion>

Expand Down Expand Up @@ -46,8 +46,9 @@
<ItemGroup Label="Analyzers">
<PackageVersion Include="StyleSharp.Analyzers" Version="$(SharpAnalyzersVersion)"/>
<PackageVersion Include="PerformanceSharp.Analyzers" Version="$(SharpAnalyzersVersion)"/>
<PackageVersion Include="SecuritySharp.Analyzers" Version="$(SharpAnalyzersVersion)"/>
<PackageVersion Include="Roslynator.Analyzers" Version="4.15.0"/>
<PackageVersion Include="SonarAnalyzer.CSharp" Version="10.29.0.143774"/>
<PackageVersion Include="SonarAnalyzer.CSharp" Version="10.30.0.144632"/>
<PackageVersion Include="Microsoft.CodeAnalysis.Analyzers" Version="$(MicrosoftCodeAnalysisAnalyzersVersion)"/>
<PackageVersion Include="Microsoft.CodeAnalysis.PublicApiAnalyzers" Version="5.6.0"/>
</ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion src/InterfaceStubGenerator.Shared/Emitter.Inline.Method.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ internal static InlineMethodFragments BuildInlineMethodFragments(
: string.Empty;
var opening = BuildMethodOpening(methodModel, isExplicit, isExplicit, interfaceModel.SupportsNullable);

return new InlineMethodFragments(
return new(
requestPrologueSource,
httpMethodFieldSource,
httpMethodExpression,
Expand Down
15 changes: 8 additions & 7 deletions src/InterfaceStubGenerator.Shared/Emitter.Inline.Multipart.cs
Original file line number Diff line number Diff line change
Expand Up @@ -214,13 +214,14 @@ internal static void AppendMultipartAddArguments(
/// <param name="settingsLocal">The generated settings local name.</param>
/// <param name="value">The value expression (a parameter accessor or a foreach element local).</param>
/// <param name="fieldName">The C# string literal for the part's field name.</param>
internal static void AppendSerializedMultipartArgument(PooledStringBuilder sb, string settingsLocal, string value, string fieldName)
{
// A sealed/value part is JSON-serialized under its field name, matching AddSerializedMultipartItem's
// serializer fallback. The declared type drives ToHttpContent<T>, so the serialized form matches; a
// serialization failure is wrapped in the same descriptive ArgumentException the reflection builder raises.
_ = sb.Append("global::Refit.GeneratedRequestRunner.SerializeMultipartPart(").Append(settingsLocal)
/// <remarks>A sealed/value part is JSON-serialized under its field name, matching AddSerializedMultipartItem's
/// serializer fallback. The declared type drives ToHttpContent&lt;T&gt;, so the serialized form matches; a
/// serialization failure is wrapped in the same descriptive ArgumentException the reflection builder raises.</remarks>
internal static void AppendSerializedMultipartArgument(
PooledStringBuilder sb,
string settingsLocal,
string value,
string fieldName) => _ = sb.Append("global::Refit.GeneratedRequestRunner.SerializeMultipartPart(").Append(settingsLocal)
.Append(", ").Append(value).Append(", ").Append(fieldName).Append("), ")
.Append(fieldName).AppendLine(");");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,15 @@ internal static string GetParameterInfoFieldName(string parameterName, UniqueNam
/// <param name="sb">The target builder.</param>
/// <param name="separator">The separator to append.</param>
/// <returns>The same builder for chaining.</returns>
internal static PooledStringBuilder AppendSeparator(int i, PooledStringBuilder sb, string separator = ", ")
{
return i <= 0 ? sb : sb.Append(separator);
}
internal static PooledStringBuilder AppendSeparator(int i, PooledStringBuilder sb, string separator = ", ") => i <= 0 ? sb : sb.Append(separator);

/// <summary>Appends a value, prefixed by a separator for all but the first element.</summary>
/// <param name="value">The value to append.</param>
/// <param name="i">The zero-based element index.</param>
/// <param name="sb">The target builder.</param>
/// <param name="separator">The separator to append before the value.</param>
/// <returns>The same builder for chaining.</returns>
internal static PooledStringBuilder AppendJoining(string value, int i, PooledStringBuilder sb, string separator = ", ")
{
return AppendSeparator(i, sb, separator).Append(value);
}
internal static PooledStringBuilder AppendJoining(string value, int i, PooledStringBuilder sb, string separator = ", ") => AppendSeparator(i, sb, separator).Append(value);

/// <summary>Appends a C# attribute construction expression to the builder.</summary>
/// <param name="attribute">The attribute model to render.</param>
Expand Down
2 changes: 1 addition & 1 deletion src/InterfaceStubGenerator.Shared/Emitter.Testing.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ internal static string BuildRefitMethodForTesting(
isTopLevel,
interfaceModel,
uniqueNames,
new GeneratedFieldNames(requestBuilderFieldName, settingsFieldName),
new(requestBuilderFieldName, settingsFieldName),
new(uniqueNames));
return builder.ToString();
}
Expand Down
12 changes: 6 additions & 6 deletions src/InterfaceStubGenerator.Shared/InterfaceStubGeneratorV2.cs
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,8 @@ internal static bool IsInterfaceMethodDeclaration(SyntaxNode syntax) =>
/// <returns>The combined candidates.</returns>
internal static ImmutableArray<MethodDeclarationSyntax> CombineStandardHttpMethodCandidates(
in StandardHttpMethodCandidates candidates)
{
#if ROSLYN_5
return
=>
[
..candidates.DeleteMethods,
..candidates.GetMethods,
Expand All @@ -227,6 +226,7 @@ internal static ImmutableArray<MethodDeclarationSyntax> CombineStandardHttpMetho
..candidates.PutMethods
];
#else
{
var count =
candidates.DeleteMethods.Length
+ candidates.GetMethods.Length
Expand All @@ -244,19 +244,19 @@ internal static ImmutableArray<MethodDeclarationSyntax> CombineStandardHttpMetho
builder.AddRange(candidates.PostMethods);
builder.AddRange(candidates.PutMethods);
return builder.MoveToImmutable();
#endif
}
#endif

/// <summary>Combines built-in and potential custom HTTP method candidates.</summary>
/// <param name="combined">The built-in and custom candidate arrays.</param>
/// <returns>The combined candidates.</returns>
internal static ImmutableArray<MethodDeclarationSyntax> CombineCandidateMethods(
(ImmutableArray<MethodDeclarationSyntax> StandardMethods,
ImmutableArray<MethodDeclarationSyntax> CustomMethods) combined)
{
#if ROSLYN_5
return [.. combined.StandardMethods, .. combined.CustomMethods];
=> [.. combined.StandardMethods, .. combined.CustomMethods];
#else
{
if (combined.StandardMethods.IsEmpty)
{
return combined.CustomMethods;
Expand All @@ -272,8 +272,8 @@ internal static ImmutableArray<MethodDeclarationSyntax> CombineCandidateMethods(
builder.AddRange(combined.StandardMethods);
builder.AddRange(combined.CustomMethods);
return builder.MoveToImmutable();
#endif
}
#endif

/// <summary>Determines whether syntax might be a method using a custom Refit HTTP method attribute.</summary>
/// <param name="syntax">The syntax node to inspect.</param>
Expand Down
6 changes: 2 additions & 4 deletions src/InterfaceStubGenerator.Shared/Parser.InlineEligibility.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,8 @@ internal static bool CanBuildRequestInline(
/// <summary>Classifies a return type into the shape buckets inline eligibility distinguishes.</summary>
/// <param name="returnType">The declared return type.</param>
/// <returns>The return shape; unsupported shapes map to <see cref="ReturnTypeInfo.Return"/>.</returns>
internal static ReturnTypeInfo ClassifyInlineReturnShape(ITypeSymbol returnType)
{
return returnType is not INamedTypeSymbol namedType
internal static ReturnTypeInfo ClassifyInlineReturnShape(ITypeSymbol returnType) =>
returnType is not INamedTypeSymbol namedType
? ReturnTypeInfo.Return
: namedType.MetadataName switch
{
Expand All @@ -94,7 +93,6 @@ internal static ReturnTypeInfo ClassifyInlineReturnShape(ITypeSymbol returnType)
"IObservable`1" when IsInNamespace(namedType, "System") => ReturnTypeInfo.Observable,
_ => ReturnTypeInfo.Return
};
}

/// <summary>Determines whether a type is <c>System.Net.Http.HttpRequestMessage</c>, the type argument of the
/// build-and-return <c>Task&lt;HttpRequestMessage&gt;</c> shape.</summary>
Expand Down
5 changes: 1 addition & 4 deletions src/InterfaceStubGenerator.Shared/Parser.Request.Helpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,10 @@ internal static bool IsInNamespace(ISymbol? symbol, string dottedNamespace)
/// <returns><see langword="true"/> when the path is supported.</returns>
/// <remarks>A no-leading-slash path is supported: it resolves against the base address under RFC 3986 and throws
/// under legacy resolution at request time, exactly as the reflection request builder validates it.</remarks>
internal static bool IsPathSupported(string path)
{
return IsPathTemplateValid(path)
internal static bool IsPathSupported(string path) => IsPathTemplateValid(path)
&& path.IndexOf('\\') < 0
&& path.IndexOf('\r') < 0
&& path.IndexOf('\n') < 0;
}

/// <summary>Prepends the client interface's shared route prefix to a method's relative path.</summary>
/// <param name="prefix">The shared route prefix, or an empty/whitespace string for a no-op.</param>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@
/// <returns>The source expression, or <c>"null"</c> when the value is null.</returns>
internal static string ConstantValueToString(TypedConstant argument, in InterfaceGenerationContext context)
{
var result = string.Empty;
string? result = string.Empty;

if (!argument.IsNull)
{
Expand All @@ -541,11 +541,11 @@
TypedConstantKind.Enum => $"({QualifyType(argument.Type!, context)}){argument.Value!}",
TypedConstantKind.Type => $"typeof({QualifyType((ITypeSymbol)argument.Value!, context)})",
TypedConstantKind.Array => RenderConstantArray(argument, context),
_ => SymbolDisplay.FormatPrimitive(argument.Value!, true, false)!

Check warning on line 544 in src/InterfaceStubGenerator.Shared/Parser.Request.ParameterKinds.cs

View workflow job for this annotation

GitHub Actions / sonarcloud / sonarcloud

Remove this null-forgiving operator; the compiler already knows this expression is not null here.
};
}

return result.Length > 0 ? result : "null";
return result is { Length: > 0 } ? result : "null";
}

/// <summary>Renders an array-valued attribute argument as a C# array-creation expression.</summary>
Expand Down
2 changes: 1 addition & 1 deletion src/InterfaceStubGenerator.Shared/Parser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ internal static InterfaceGenerationContext CreateGenerationContext(

var indexedCollectionFormatValue = ResolveIndexedCollectionFormatValue(compilation);

return new InterfaceGenerationContext(
return new(
diagnostics,
preserveAttributeDisplayName,
generatedClassName,
Expand Down
4 changes: 2 additions & 2 deletions src/Refit.HttpClientFactory/HttpClientFactoryCore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ internal static IHttpClientBuilder AddRefitClientCore(
_ = services.AddSingleton(
settingsType,
provider => Activator.CreateInstance(
typeof(SettingsFor<>).MakeGenericType(refitInterfaceType)!,
typeof(SettingsFor<>).MakeGenericType(refitInterfaceType),
settings?.Invoke(provider))!);

// register RequestBuilder
Expand Down Expand Up @@ -165,7 +165,7 @@ internal static IHttpClientBuilder AddKeyedRefitClientCore(
settingsType,
serviceKey,
(provider, _) => Activator.CreateInstance(
typeof(SettingsFor<>).MakeGenericType(refitInterfaceType)!,
typeof(SettingsFor<>).MakeGenericType(refitInterfaceType),
settings?.Invoke(provider))!);

// register RequestBuilder
Expand Down
1 change: 1 addition & 0 deletions src/Refit.HttpClientFactory/Refit.HttpClientFactory.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
<Compile Include="..\Shared\AuthenticatedHttpClientHandler.cs" Link="Shared\AuthenticatedHttpClientHandler.cs" />
</ItemGroup>
<ItemGroup>
<InternalsVisibleTo Include="Refit.HttpClientFactory.Tests" />
<ProjectReference Include="..\Refit\Refit.csproj" PrivateAssets="Analyzers" />
<PackageReference Include="Microsoft.Extensions.Http" />
</ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion src/Refit.NativeAotSmoke/SmokeApiFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ internal static class SmokeApiFactory
internal static INativeAotApi Create(HttpClient client, JsonSerializerOptions jsonOptions) =>
RestService.ForGenerated<INativeAotApi>(
client,
new RefitSettings(new SystemTextJsonContentSerializer(jsonOptions)));
new(new SystemTextJsonContentSerializer(jsonOptions)));
}
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ internal void AddFlattenedFormObject(MultipartFormDataContent multiPartContent,
{
// A field with no resolvable name cannot be a valid form-data part (the framework rejects an empty
// content-disposition name), so it is skipped rather than allowed to throw mid-request.
if (string.IsNullOrWhiteSpace(field.Key))
if (field.Key is null || string.IsNullOrWhiteSpace(field.Key))
{
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ internal static void AssignAbsoluteRequestUri(
/// blank key is skipped, and keys that match case-insensitively are joined with commas under the first-seen key.</remarks>
internal static void ParseQueryStringInto(string? queryString, ref List<QueryParameterEntry>? queryParamsToAdd)
{
if (string.IsNullOrEmpty(queryString))
if (queryString is not { Length: > 0 })
{
return;
}
Expand Down Expand Up @@ -485,7 +485,7 @@ internal List<QueryMapEntry> BuildQueryMap(
var keyType = key.GetType();
var formattedKey = GeneratedRequestRunner.FormatUrlParameter(_settings, key, GetCachedAttributeProvider(keyType), keyType);

if (string.IsNullOrWhiteSpace(formattedKey)) // blank keys can't be put in the query string
if (formattedKey is null || string.IsNullOrWhiteSpace(formattedKey)) // blank keys can't be put in the query string
{
continue;
}
Expand Down
37 changes: 33 additions & 4 deletions src/Refit.Testing/StubHttp.Matching.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
using System.Collections.Generic;
using System.Net.Http;
using System.Net.Http.Headers;
#if NET6_0_OR_GREATER
using System.Security.Cryptography;
using System.Text;
#endif
using System.Threading;
using System.Threading.Tasks;

Expand Down Expand Up @@ -68,7 +72,7 @@ private static bool SegmentsMatch(string expected, string actual)
continue;
}

if (!string.Equals(expectedSegment, actualSegments[i], StringComparison.Ordinal))
if (!FixedTimeEquals(expectedSegment, actualSegments[i]))
{
return false;
}
Expand Down Expand Up @@ -153,23 +157,48 @@ private static bool HeaderMatches(HttpRequestMessage request, string name, strin
{
var present = TryGetHeader(request.Headers, name, out var actual) ||
(request.Content is not null && TryGetHeader(request.Content.Headers, name, out actual));
return present && string.Equals(actual, value, StringComparison.Ordinal);
return present && FixedTimeEquals(actual, value);
}

/// <summary>Compares non-secret route data without timing-dependent equality.</summary>
/// <param name="left">The optional left value.</param>
/// <param name="right">The right value.</param>
/// <returns><see langword="true"/> when the UTF-8 representations match.</returns>
#if NET6_0_OR_GREATER
private static bool FixedTimeEquals(string left, string right)
{
var leftBytes = Encoding.UTF8.GetBytes(left);
var rightBytes = Encoding.UTF8.GetBytes(right);
return CryptographicOperations.FixedTimeEquals(leftBytes, rightBytes);
}
#else
private static bool FixedTimeEquals(string left, string right)
{
var difference = left.Length ^ right.Length;
var length = Math.Min(left.Length, right.Length);
for (var i = 0; i < length; i++)
{
difference |= left[i] ^ right[i];
}

return difference == 0;
}
#endif

/// <summary>Gets the combined value of a named header, if present.</summary>
/// <param name="headers">The header collection to search.</param>
/// <param name="name">The header name.</param>
/// <param name="value">The comma-joined header value when found.</param>
/// <returns><see langword="true"/> when the header exists.</returns>
private static bool TryGetHeader(HttpHeaders headers, string name, out string? value)
private static bool TryGetHeader(HttpHeaders headers, string name, out string value)
{
if (headers.TryGetValues(name, out var values))
{
value = string.Join(", ", values);
return true;
}

value = null;
value = string.Empty;
return false;
}

Expand Down
Loading
Loading