Skip to content

Commit 8e4237b

Browse files
jgarciadelanocedaJavier García de la Noceda Argüelles
jgarciadelanoceda
and
Javier García de la Noceda Argüelles
authored
fix:Analyzer false positive on Indexed CollectionFormat (#2274)
* fix:Analyzer false positive on Indexed CollectionFormat * Fix analyzer warning * fix reindent * Fix analyzer warning --------- Co-authored-by: Javier García de la Noceda Argüelles <jgarcian@riamoneytransfer.com>
1 parent eb92914 commit 8e4237b

8 files changed

Lines changed: 79 additions & 15 deletions

File tree

‎src/InterfaceStubGenerator.Shared/Emitter.Inline.Query.Object.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -671,7 +671,7 @@ internal static void AppendIndexedCollectionQueryStatements(
671671
{
672672
var bodyIndent = Indent(MethodBodyIndentation);
673673
var guarded = parameter.CanBeNull;
674-
var outerIndent = guarded ? bodyIndent + " " : bodyIndent;
674+
var outerIndent = guarded ? $"{bodyIndent} " : bodyIndent;
675675

676676
if (guarded)
677677
{
@@ -680,9 +680,9 @@ internal static void AppendIndexedCollectionQueryStatements(
680680
}
681681

682682
// Wrap in a block so the index local is scoped to this parameter and cannot clash with other parameters.
683-
var idxLocal = emission.QueryValueLocal + "_idx";
684-
var blockIndent = outerIndent + " ";
685-
var foreachIndent = blockIndent + " ";
683+
var idxLocal = $"{emission.QueryValueLocal}_idx";
684+
var blockIndent = $"{outerIndent} ";
685+
var foreachIndent = $"{blockIndent} ";
686686

687687
_ = sb.Append(outerIndent).AppendLine("{")
688688
.Append(blockIndent).Append("var ").Append(idxLocal).AppendLine(" = 0;")
@@ -732,14 +732,14 @@ internal static void AppendIndexedElement(
732732
{
733733
_ = sb.Append(foreachIndent).Append("if (").Append(emission.QueryValueLocal).AppendLine(NotNullCheckSuffix)
734734
.Append(foreachIndent).AppendLine("{");
735-
itemIndent = foreachIndent + " ";
735+
itemIndent = $"{foreachIndent} ";
736736
}
737737

738738
// Flatten the element's properties under the indexed key; pass null as collection format so nested
739739
// collection properties fall back to settings default instead of inheriting Indexed.
740740
// PreEscapedKeys tells every leaf property to use AddPreEscapedKey so the brackets in the key are not re-encoded.
741741
var context = new QueryObjectContext(parameter, providerField, null, ToLowerInvariantString(query.PreEncoded), true);
742-
var scope = new ObjectFlattenScope(emission.QueryValueLocal, keyLocal, query.NestingDelimiter, "_" + parameter.Name, itemIndent);
742+
var scope = new ObjectFlattenScope(emission.QueryValueLocal, keyLocal, query.NestingDelimiter, $"_{parameter.Name}", itemIndent);
743743
AppendObjectPropertyList(sb, context, query.ObjectProperties!.Value, scope, emission);
744744

745745
if (!query.ElementCanBeNull)

‎src/InterfaceStubGenerator.Shared/Parser.InlineEligibility.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,16 @@ internal static bool CanBuildRequestInline(
3535
/// <param name="returnTypeAdapterInterface">The resolved <c>Refit.IReturnTypeAdapter`2</c> symbol, or null.</param>
3636
/// <param name="returnTypeAdapters">The discovered <c>IReturnTypeAdapter</c> implementations, so the analyzer agrees
3737
/// with the generator that an adapter-backed return type is inline-eligible.</param>
38+
/// <param name="indexedCollectionFormatValue">The underlying integer value of <c>CollectionFormat.Indexed</c> resolved once from the
39+
/// compilation, or <see langword="null"/> when the <c>Refit.CollectionFormat</c> type cannot be found.</param>
3840
/// <returns><see langword="true"/> when the method's request is inline-eligible.</returns>
3941
internal static bool CanBuildRequestInline(
4042
IMethodSymbol methodSymbol,
4143
INamedTypeSymbol httpMethodBaseAttributeSymbol,
4244
INamedTypeSymbol? formattableSymbol,
4345
INamedTypeSymbol? returnTypeAdapterInterface,
44-
INamedTypeSymbol[] returnTypeAdapters)
46+
INamedTypeSymbol[] returnTypeAdapters,
47+
int? indexedCollectionFormatValue = null)
4548
{
4649
if (FindHttpMethodAttribute(methodSymbol, httpMethodBaseAttributeSymbol) is null)
4750
{
@@ -69,7 +72,8 @@ internal static bool CanBuildRequestInline(
6972
ExternAliases: [],
7073
AssemblyAliasCache: new Dictionary<ISymbol, string?>(SymbolEqualityComparer.Default),
7174
QualifiedTypeCache: new Dictionary<ISymbol, string>(SymbolEqualityComparer.Default),
72-
FormattableClassificationCache: new Dictionary<ISymbol, (bool Formattable, bool SpanFormattable)>(SymbolEqualityComparer.Default));
75+
FormattableClassificationCache: new Dictionary<ISymbol, (bool Formattable, bool SpanFormattable)>(SymbolEqualityComparer.Default),
76+
IndexedCollectionFormatValue: indexedCollectionFormatValue);
7377
return ParseRequest(methodSymbol, ClassifyInlineReturnShape(methodSymbol.ReturnType), context)
7478
.CanGenerateInline;
7579
}

‎src/InterfaceStubGenerator.Shared/Parser.Request.Query.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,7 @@ internal static bool TryBuildQueryModel(
293293
/// per generation pass, so per-parameter checks are a direct integer comparison rather than a symbol lookup.</summary>
294294
/// <param name="compilation">The compilation to resolve against.</param>
295295
/// <returns>The integer value, or <see langword="null"/> when the type cannot be found.</returns>
296+
[ExcludeFromCodeCoverage]
296297
internal static int? ResolveIndexedCollectionFormatValue(Compilation compilation)
297298
{
298299
var collectionFormatType = compilation.GetTypeByMetadataName(CollectionFormatTypeName);

‎src/Refit.Analyzers.Shared/RefitInterfaceAnalyzer.cs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// ReactiveUI and Contributors licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for full license information.
44
using System.Collections.Immutable;
5-
using System.Linq;
65
using Microsoft.CodeAnalysis;
76
using Microsoft.CodeAnalysis.Diagnostics;
87

@@ -113,7 +112,8 @@ private static void RegisterCompilationAnalysis(CompilationStartAnalysisContext
113112
formattableInterface,
114113
returnTypeAdapterInterface,
115114
returnTypeAdapters,
116-
reportGeneratedRequestBuildingFallback),
115+
reportGeneratedRequestBuildingFallback,
116+
Refit.Generator.Parser.ResolveIndexedCollectionFormatValue(context.Compilation)),
117117
symbolContext.ReportDiagnostic,
118118
symbolContext.CancellationToken),
119119
SymbolKind.NamedType);
@@ -230,7 +230,8 @@ private static void AnalyzeRefitMethod(
230230
httpMethodAttribute,
231231
analysis.FormattableInterface,
232232
analysis.ReturnTypeAdapterInterface,
233-
analysis.ReturnTypeAdapters))
233+
analysis.ReturnTypeAdapters,
234+
analysis.IndexedCollectionFormatValue))
234235
{
235236
return;
236237
}
@@ -618,11 +619,14 @@ private static bool IsSupportedHeaderCollectionType(ITypeSymbol type) =>
618619
/// <param name="ReturnTypeAdapterInterface">The <c>Refit.IReturnTypeAdapter`2</c> symbol, if available.</param>
619620
/// <param name="ReturnTypeAdapters">The discovered <c>IReturnTypeAdapter</c> implementations, kept in lockstep with the generator.</param>
620621
/// <param name="ReportGeneratedRequestBuildingFallback">Whether the RF006 fallback diagnostic is enabled.</param>
622+
/// /// <param name="IndexedCollectionFormatValue">The underlying integer value of <c>CollectionFormat.Indexed</c> resolved once from the
623+
/// compilation, or <see langword="null"/> when the <c>Refit.CollectionFormat</c> type cannot be found.</param>
621624
private readonly record struct CompilationAnalysisState(
622625
INamedTypeSymbol HttpMethodAttribute,
623626
INamedTypeSymbol DisposableInterface,
624627
INamedTypeSymbol? FormattableInterface,
625628
INamedTypeSymbol? ReturnTypeAdapterInterface,
626629
INamedTypeSymbol[] ReturnTypeAdapters,
627-
bool ReportGeneratedRequestBuildingFallback);
630+
bool ReportGeneratedRequestBuildingFallback,
631+
int? IndexedCollectionFormatValue = null);
628632
}

‎src/tests/Refit.Analyzers.Tests/RefitInterfaceAnalyzerTests.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,12 @@ public async Task DoesNotReportInlineSupportedMethods()
255255
{
256256
var diagnostics = await AnalyzerFixture.RunForBody(
257257
"""
258+
public sealed class Filter
259+
{
260+
[AliasAs("n")] public string? Name { get; set; }
261+
public int Count { get; set; }
262+
}
263+
258264
[Get("/users")]
259265
Task<string> List();
260266
@@ -285,6 +291,9 @@ public async Task DoesNotReportInlineSupportedMethods()
285291
286292
[Get("/stream")]
287293
IObservable<string> Observe();
294+
295+
[Get("/filters")]
296+
Task<string> Search([Query(CollectionFormat.Indexed)] List<Filter>? filters);
288297
""");
289298

290299
await Assert.That(diagnostics.Select(static diagnostic => diagnostic.Id))

‎src/tests/Refit.GeneratorTests/IndexedCollectionGenerationTests.cs‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,4 +196,44 @@ public interface IGeneratedClient
196196
// A scalar element has no object properties to flatten — handled as regular Collection inline generation.
197197
await Assert.That(result.GeneratedSources[Hint]).DoesNotContain(ReflectiveFallback);
198198
}
199+
200+
/// <summary>Verifies a Indexed parameter whose element type is a collection with a complex element type falls back to reflective generation.</summary>
201+
/// <returns>A task representing the asynchronous test.</returns>
202+
[Test]
203+
public async Task IndexedWithComplexIndexElementFallsBackToReflective()
204+
{
205+
const string source =
206+
"""
207+
#nullable enable
208+
using System.Collections.Generic;
209+
using System.Threading.Tasks;
210+
using Refit;
211+
212+
namespace RefitGeneratorTest;
213+
214+
public sealed class Car
215+
{
216+
public int Id { get; set; }
217+
public string? Name { get; set; }
218+
}
219+
220+
public sealed class ComplexElement
221+
{
222+
public List<Car>? Cars { get; set; }
223+
}
224+
225+
public interface IGeneratedClient
226+
{
227+
[Get("/v")]
228+
Task<string> Values([Query(CollectionFormat.Indexed)] List<ComplexElement> values);
229+
}
230+
""";
231+
232+
var result = Fixture.RunGenerator(source, generatedRequestBuilding: true);
233+
234+
await Assert.That(result.CompilesWithoutErrors).IsTrue();
235+
236+
// A scalar element has no object properties to flatten — handled as regular Collection inline generation.
237+
await Assert.That(result.GeneratedSources[Hint]).Contains(ReflectiveFallback);
238+
}
199239
}

‎src/tests/Refit.GeneratorTests/QueryRequestBuildingLiveTests.Helpers.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,10 @@ public interface ILiveQueryApi
287287
Task<string> IndexedListSearch([Query(CollectionFormat.Indexed)] List<int>? items);
288288
289289
[Get("/indexedNameWithSerialized")]
290-
Task<string> indexedNameWithSerialized([Query(CollectionFormat.Indexed)] List<Name>? items);
290+
Task<string> indexedNameWithSerialized([Query(CollectionFormat.Indexed)] List<Name> items);
291+
292+
[Get("/indexedSimpleType")]
293+
Task<string> IndexedSimpleType([Query(CollectionFormat.Indexed)] Item item);
291294
}
292295
""";
293296

@@ -305,7 +308,7 @@ public static LiveQueryHarness Create(RefitSettings? settings = null)
305308
if (!result.CompilesWithoutErrors)
306309
{
307310
throw new InvalidOperationException(
308-
"Generated compilation failed: " + string.Join(Environment.NewLine, result.CompilationErrors));
311+
$"Generated compilation failed: {string.Join(Environment.NewLine, result.CompilationErrors)}");
309312
}
310313

311314
var (assembly, loadContext) = Fixture.EmitAndLoad(result);

‎src/tests/Refit.GeneratorTests/QueryRequestBuildingLiveTests.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ public async Task IndexedCollectionQueryParametersMatchReflection()
277277
const string typeName = "Refit.LiveQuery.Item";
278278
await harness.AssertParityAsync(indexedSearchMethodName, [null], "/base/indexed");
279279
var item0 = harness.CreateApiValue(typeName, (id, 1), (value, "a"));
280-
var indexedList = harness.CreateApiList(typeName, item0);
280+
var indexedList = harness.CreateApiList(typeName, item0, null);
281281
await harness.AssertParityAsync(indexedSearchMethodName, [indexedList], $"/base/indexed?{parameter}[0].{id}=1&{parameter}[0].{value}=a");
282282

283283
const string indexedSearchWithListIntMethodName = "IndexedListSearch";
@@ -293,6 +293,9 @@ public async Task IndexedCollectionQueryParametersMatchReflection()
293293
var nameSerialized = harness.CreateApiValue(typeNameSerialized, (name, "John"), (lastName, "Doe"));
294294
var indexedNameSerialized = harness.CreateApiList(typeNameSerialized, nameSerialized);
295295
await harness.AssertParityAsync(indexedSearchSerialized, [indexedNameSerialized], $"/base/indexedNameWithSerialized?{parameter}[0].{jsonPropertyName}=John&{parameter}[0].{lastName}=Doe");
296+
297+
const string indexedSimpleType = "IndexedSimpleType";
298+
await harness.AssertParityAsync(indexedSimpleType, [item0], $"/base/indexedSimpleType?{id}=1&{value}=a");
296299
}
297300

298301
/// <summary>Verifies a custom URL parameter formatter still runs for every generated value.</summary>

0 commit comments

Comments
 (0)