Handle PR comments

Author: Javier García de la Noceda Argüelles

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

@@ -17,8 +17,11 @@ internal static partial class Parser
     /// <summary>The maximum nested-object depth flattened inline before the whole parameter falls back to reflection.</summary>
     private const int MaxNestingDepth = 32;
 
-    /// <summary>The underlying integer value of <c>CollectionFormat.Indexed</c>, used by the parser without a direct reference to the Refit assembly.</summary>
-    private const int IndexedCollectionFormatValue = 6;
+    /// <summary>The metadata name of <c>Refit.CollectionFormat</c>.</summary>
+    private const string CollectionFormatTypeName = "Refit.CollectionFormat";
+
+    /// <summary>The member name of <c>CollectionFormat.Indexed</c>.</summary>
+    private const string IndexedMemberName = "Indexed";
 
     /// <summary>The metadata name of <c>Refit.QueryAttribute</c>.</summary>
     private const string QueryAttributeDisplayName = "QueryAttribute";
@@ -286,6 +289,38 @@ internal static bool TryBuildQueryModel(
                 BuildValueFormat(elementType!, format, formattableSymbol, context))
             : null;
 
+    /// <summary>Determines whether <paramref name="collectionFormatValue"/> is the underlying integer value of
+    /// <c>CollectionFormat.Indexed</c> by resolving the enum member from the compilation rather than comparing
+    /// against a hardcoded literal, so the check stays correct if the enum is ever reordered.</summary>
+    /// <param name="collectionFormatValue">The collection format value from the attribute, or null.</param>
+    /// <param name="context">The generation context supplying the compilation.</param>
+    /// <returns><see langword="true"/> when the value matches <c>CollectionFormat.Indexed</c>.</returns>
+    internal static bool IsIndexedCollectionFormat(int? collectionFormatValue, in InterfaceGenerationContext context)
+    {
+        if (collectionFormatValue is null)
+        {
+            return false;
+        }
+
+        // Resolve the CollectionFormat enum type from the compilation and find the Indexed member's constant value.
+        // This avoids hardcoding the integer and stays correct if the enum is ever reordered.
+        var collectionFormatType = context.Compilation?.GetTypeByMetadataName(CollectionFormatTypeName);
+        if (collectionFormatType is null)
+        {
+            return false;
+        }
+
+        foreach (var member in collectionFormatType.GetMembers())
+        {
+            if (member is IFieldSymbol { HasConstantValue: true, Name: IndexedMemberName } field)
+            {
+                return collectionFormatValue == (int)field.ConstantValue!;
+            }
+        }
+
+        return false;
+    }
+
     /// <summary>Builds the query model for a <c>[Query(CollectionFormat.Indexed)]</c> parameter whose element type
     /// is a complex object that can be flattened inline, or null when the parameter does not match.</summary>
     /// <param name="parameter">The parameter to classify.</param>
@@ -306,7 +341,7 @@ internal static bool TryBuildQueryModel(
         INamedTypeSymbol? formattableSymbol,
         in InterfaceGenerationContext context)
     {
-        if (data.CollectionFormatValue != IndexedCollectionFormatValue)
+        if (!IsIndexedCollectionFormat(data.CollectionFormatValue, context))
         {
             return null;
         }

src/Refit.Reflection/RequestBuilderImplementation.Payload.cs

@@ -43,41 +43,41 @@ internal void AddBodyToRequest(RestMethodInfoInternal restMethod, object param,
         switch (restMethod.BodyParameterInfo.Item1)
         {
             case BodySerializationMethod.UrlEncoded:
-                {
-                    ret.Content = param is string str
-                        ? new StringContent(
-                            StringHelpers.EscapeDataString(str),
-                            Encoding.UTF8,
-                            "application/x-www-form-urlencoded")
-                        : new FormUrlEncodedContent(new FormValueMultimap(param, _settings));
-                    break;
-                }
+            {
+                ret.Content = param is string str
+                    ? new StringContent(
+                        StringHelpers.EscapeDataString(str),
+                        Encoding.UTF8,
+                        "application/x-www-form-urlencoded")
+                    : new FormUrlEncodedContent(new FormValueMultimap(param, _settings));
+                break;
+            }
 
             case BodySerializationMethod.JsonLines:
-                {
-                    ret.Content = new JsonLinesContent(AsJsonLinesSequence(param), _serializer);
-                    break;
-                }
+            {
+                ret.Content = new JsonLinesContent(AsJsonLinesSequence(param), _serializer);
+                break;
+            }
 
             case BodySerializationMethod.Default or BodySerializationMethod.Serialized:
-                {
-                    AddSerializedBodyToRequest(restMethod, param, ret);
-                    break;
-                }
+            {
+                AddSerializedBodyToRequest(restMethod, param, ret);
+                break;
+            }
 
             default:
+            {
+                // The obsolete legacy JSON serialization method must still serialize, because
+                // already-compiled callers can pass it. Treating it as Default would incorrectly
+                // send string bodies as raw text. It is matched by value rather than by name so
+                // this file never references the obsolete member.
+                if (GeneratedRequestRunner.IsObsoleteJsonSerializationMethod(restMethod.BodyParameterInfo.Item1))
                 {
-                    // The obsolete legacy JSON serialization method must still serialize, because
-                    // already-compiled callers can pass it. Treating it as Default would incorrectly
-                    // send string bodies as raw text. It is matched by value rather than by name so
-                    // this file never references the obsolete member.
-                    if (GeneratedRequestRunner.IsObsoleteJsonSerializationMethod(restMethod.BodyParameterInfo.Item1))
-                    {
-                        AddSerializedBodyToRequest(restMethod, param, ret);
-                    }
-
-                    break;
+                    AddSerializedBodyToRequest(restMethod, param, ret);
                 }
+
+                break;
+            }
         }
     }
 
@@ -423,7 +423,8 @@ internal bool TryAppendIndexedCollectionQuery(
         if (parameterCollectionFormat != CollectionFormat.Indexed
             || param is string
             || param is IDictionary
-            || param is not IEnumerable collection)
+            || param is not IEnumerable collection
+            || DoNotConvertToQueryMap(param))
         {
             return false;
         }
@@ -454,23 +455,16 @@ internal void AppendIndexedCollectionParameters(
             }
 
             var indexedKey = $"{paramKey}[{index}]";
-            if (DoNotConvertToQueryMap(item))
-            {
-                var type = item.GetType();
-                queryParamsToAdd.Add(new(indexedKey, GeneratedRequestRunner.FormatUrlParameter(_settings, item, type, type), KeyPreEscaped: true));
-            }
-            else
+
+            var queryMap = BuildQueryMap(item, attr.Delimiter);
+            for (var j = 0; j < queryMap.Count; j++)
             {
-                var queryMap = BuildQueryMap(item, attr.Delimiter);
-                for (var j = 0; j < queryMap.Count; j++)
-                {
-                    var kvp = queryMap[j];
-                    var valueType = kvp.Value?.GetType() ?? typeof(object);
-                    queryParamsToAdd.Add(new(
-                        indexedKey + attr.Delimiter + kvp.Key,
-                        GeneratedRequestRunner.FormatUrlParameter(_settings, kvp.Value, valueType, valueType),
-                        KeyPreEscaped: true));
-                }
+                var kvp = queryMap[j];
+                var valueType = kvp.Value?.GetType() ?? typeof(object);
+                queryParamsToAdd.Add(new(
+                    indexedKey + attr.Delimiter + kvp.Key,
+                    GeneratedRequestRunner.FormatUrlParameter(_settings, kvp.Value, valueType, valueType),
+                    KeyPreEscaped: true));
             }
 
             index++;

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

@@ -83,6 +83,7 @@ private sealed class LiveQueryHarness(
             using System.Collections.Generic;
             using System.Runtime.Serialization;
             using System.Threading.Tasks;
+            using System.Text.Json.Serialization;
             using Refit;
 
             namespace Refit.LiveQuery;
@@ -173,6 +174,12 @@ public sealed class Item
 
                 public string? Value { get; set; }
             }
+            public sealed class Name
+            {
+                [JsonPropertyName("First Name")]
+                public string? FirstName { get; set; }
+                public string? LastName { get; set; }
+            }
 
             public interface ILiveQueryApi
             {
@@ -275,6 +282,12 @@ public interface ILiveQueryApi
 
                 [Get("/indexed")]
                 Task<string> IndexedSearch([Query(CollectionFormat.Indexed)] List<Item>? items);
+
+                [Get("/indexedListInt")]
+                Task<string> IndexedListSearch([Query(CollectionFormat.Indexed)] List<int>? items);
+
+                [Get("/indexedNameWithSerialized")]
+                Task<string> indexedNameWithSerialized([Query(CollectionFormat.Indexed)] List<Name>? items);
             }
             """;
 

src/tests/Refit.GeneratorTests/QueryRequestBuildingLiveTests.cs

@@ -278,7 +278,21 @@ public async Task IndexedCollectionQueryParametersMatchReflection()
         await harness.AssertParityAsync(indexedSearchMethodName, [null], "/base/indexed");
         var item0 = harness.CreateApiValue(typeName, (id, 1), (value, "a"));
         var indexedList = harness.CreateApiList(typeName, item0);
-        _ = await harness.AssertParityAsync(indexedSearchMethodName, [indexedList], $"/base/indexed?{parameter}[0].{id}=1&{parameter}[0].{value}=a");
+        await harness.AssertParityAsync(indexedSearchMethodName, [indexedList], $"/base/indexed?{parameter}[0].{id}=1&{parameter}[0].{value}=a");
+
+        const string indexedSearchWithListIntMethodName = "IndexedListSearch";
+        await harness.AssertParityAsync(indexedSearchWithListIntMethodName, [null], "/base/indexedListInt");
+        List<int> values = [1, 2];
+        await harness.AssertParityAsync(indexedSearchWithListIntMethodName, [values], $"/base/indexedListInt?{parameter}=1%2C2");
+
+        const string name = "FirstName";
+        const string jsonPropertyName = "First%20Name";
+        const string lastName = "LastName";
+        const string indexedSearchSerialized = "indexedNameWithSerialized";
+        const string typeNameSerialized = "Refit.LiveQuery.Name";
+        var nameSerialized = harness.CreateApiValue(typeNameSerialized, (name, "John"), (lastName, "Doe"));
+        var indexedNameSerialized = harness.CreateApiList(typeNameSerialized, nameSerialized);
+        await harness.AssertParityAsync(indexedSearchSerialized, [indexedNameSerialized], $"/base/indexedNameWithSerialized?{parameter}[0].{jsonPropertyName}=John&{parameter}[0].{lastName}=Doe");
     }
 
     /// <summary>Verifies a custom URL parameter formatter still runs for every generated value.</summary>