Merge branch 'main' into reflection_test

InterfaceStubGenerator.Shared/Parser.cs

@@ -300,7 +300,7 @@ bool nullableEnabled
             .Where(m => IsRefitMethod(m, httpMethodBaseAttributeSymbol))
             .ToArray();
         var derivedNonRefitMethods = derivedMethods
-            .Except(derivedMethods, SymbolEqualityComparer.Default)
+            .Except(derivedRefitMethods, SymbolEqualityComparer.Default)
             .Cast<IMethodSymbol>()
             .ToArray();
 

Refit.GeneratorTests/Incremental/InheritanceTest.cs

@@ -74,7 +74,6 @@ public interface IGitHubApi : IDisposable
     [Fact]
     public void InheritFromInterfaceDoesRegenerate()
     {
-        // TODO: this currently generates invalid code see issue #1801 for more information
         var syntaxTree = CSharpSyntaxTree.ParseText(TwoInterface, CSharpParseOptions.Default);
         var compilation1 = Fixture.CreateLibrary(syntaxTree);
 
@@ -94,6 +93,6 @@ public interface IGitHubApi : IBaseInterface
             """
         );
         var driver2 = driver1.RunGenerators(compilation2);
-        TestHelper.AssertRunReasons(driver2, IncrementalGeneratorRunReasons.Cached);
+        TestHelper.AssertRunReasons(driver2, IncrementalGeneratorRunReasons.Modified);
     }
 }

Refit.GeneratorTests/InterfaceTests.cs

@@ -40,7 +40,6 @@ public interface IBaseInterface
     [Fact]
     public Task RefitInterfaceDerivedFromBaseTest()
     {
-        // TODO: this currently generates invalid code see issue #1801 for more information
         return Fixture.VerifyForType(
             """
             public interface IGeneratedInterface : IBaseInterface

Refit.GeneratorTests/_snapshots/InterfaceTests.RefitInterfaceDerivedFromBaseTest#IGeneratedInterface.g.verified.cs

@@ -46,6 +46,12 @@ public RefitGeneratorTestIGeneratedInterface(global::System.Net.Http.HttpClient
 
             return await ((global::System.Threading.Tasks.Task<string>)______func(this.Client, ______arguments)).ConfigureAwait(false);
         }
+
+        /// <inheritdoc />
+        void global::RefitGeneratorTest.IBaseInterface.NonRefitMethod()
+        {
+                throw new global::System.NotImplementedException("Either this method has no Refit HTTP method attribute or you've used something other than a string literal for the 'path' argument.");
+        }
     }
     }
 }

Refit.GeneratorTests/_snapshots/InterfaceTests.RefitInterfaceDerivedFromBaseTest.verified.txt

@@ -0,0 +1,24 @@
+{
+  Diagnostics: [
+    {
+      Location: /*
+{
+    void NonRefitMethod();
+         ^^^^^^^^^^^^^^
+}
+*/
+ : (19,9)-(19,23),
+      Message: Method IBaseInterface.NonRefitMethod either has no Refit HTTP method attribute or you've used something other than a string literal for the 'path' argument,
+      Severity: Warning,
+      WarningLevel: 1,
+      Descriptor: {
+        Id: RF001,
+        Title: Refit types must have Refit HTTP method attributes,
+        MessageFormat: Method {0}.{1} either has no Refit HTTP method attribute or you've used something other than a string literal for the 'path' argument,
+        Category: Refit,
+        DefaultSeverity: Warning,
+        IsEnabledByDefault: true
+      }
+    }
+  ]
+}

Refit.Tests/InheritedInterfacesApi.cs

@@ -66,6 +66,14 @@ public interface IImplementTheInterfaceAndUseRefit : IAmInterfaceEWithNoRefit<in
         [Get("/DoSomethingElse")]
         public new Task DoSomethingElse();
     }
+
+    public interface IImplementTheInterfaceAndDontUseRefit : IAmInterfaceD
+    {
+#pragma warning disable CS0108 // Member hides inherited member; missing new keyword
+        Task<string> Test();
+#pragma warning restore CS0108 // Member hides inherited member; missing new keyword
+    }
+
     public interface IMyClient
     {
         [Get("/")]

Refit.Tests/RestService.cs

@@ -1974,19 +1974,43 @@ public async Task InheritedInterfaceWithoutRefitInBaseMethodsTest()
         await fixture.DoSomethingElse();
         mockHttp.VerifyNoOutstandingExpectation();
 
-        mockHttp
-            .Expect(HttpMethod.Get, "https://httpbin.org/DoSomethingElse")
-            .Respond("application/json", nameof(IImplementTheInterfaceAndUseRefit.DoSomethingElse));
-        await ((IAmInterfaceEWithNoRefit<int>)fixture).DoSomethingElse();
+        // base non refit method should throw NotImplementedException
+        await Assert.ThrowsAsync<NotImplementedException>(
+            () => ((IAmInterfaceEWithNoRefit<int>)fixture).DoSomethingElse()
+        );
         mockHttp.VerifyNoOutstandingExpectation();
+    }
+
+    [Fact]
+    public async Task InheritedInterfaceWithoutRefitMethodsOverrideBaseTest()
+    {
+        var mockHttp = new MockHttpMessageHandler();
+
+        var settings = new RefitSettings { HttpMessageHandlerFactory = () => mockHttp };
 
-        Assert.Throws<InvalidOperationException>(
-            () => RestService.For<IAmInterfaceEWithNoRefit<int>>("https://httpbin.org")
+        var fixture = RestService.For<IImplementTheInterfaceAndDontUseRefit>(
+            "https://httpbin.org",
+            settings
         );
+
+        // inherited non refit method should throw NotImplementedException
+        await Assert.ThrowsAsync<NotImplementedException>(
+            () => fixture.Test()
+        );
+        mockHttp.VerifyNoOutstandingExpectation();
+
+        // base Refit method should respond
+        mockHttp
+            .Expect(HttpMethod.Get, "https://httpbin.org/get")
+            .WithQueryString("result", "Test")
+            .Respond("application/json", nameof(IAmInterfaceD.Test));
+
+        await ((IAmInterfaceD)fixture).Test();
+        mockHttp.VerifyNoOutstandingExpectation();
     }
 
     [Fact]
-    public async Task DictionaryDynamicQueryparametersTest()
+    public async Task DictionaryDynamicQueryParametersTest()
     {
         var mockHttp = new MockHttpMessageHandler();
 

Refit.Tests/RestServiceExceptions.cs

@@ -0,0 +1,174 @@
+using Xunit;
+// ReSharper disable InconsistentNaming
+
+namespace Refit.Tests;
+
+public interface IManyCancellationTokens
+{
+    [Get("/")]
+    Task<string> GetValue(CancellationToken token0, CancellationToken token1);
+}
+
+public interface IManyHeaderCollections
+{
+    [Get("/")]
+    Task<string> GetValue([HeaderCollection] IDictionary<string, string> collection0, [HeaderCollection] IDictionary<string, string> collection1);
+}
+
+public interface IHeaderCollectionWrongType
+{
+    [Get("/")]
+    Task<string> GetValue([HeaderCollection] IDictionary<string, object> collection);
+}
+
+public interface IDoesNotStartSlash
+{
+    [Get("users")]
+    Task<string> GetValue();
+}
+
+public interface IUrlContainsCRLF
+{
+    [Get("/\r")]
+    Task<string> GetValue();
+}
+
+public interface IRoundTripNotString
+{
+    [Get("/{**value}")]
+    Task<string> GetValue(int value);
+}
+
+public interface IUrlNoMatchingParameters
+{
+    [Get("/{value}")]
+    Task<string> GetValue();
+}
+
+public interface IMultipartAndBody
+{
+    [Get("/}")]
+    [Multipart]
+    Task<string> GetValue([Body] string body);
+}
+
+public interface IManyBody
+{
+    [Get("/")]
+    Task<string> GetValue([Body] string body0, [Body] string body1);
+}
+
+public class UserBody
+{
+    public string Value { get; set; }
+}
+
+public interface IManyComplexTypes
+{
+    [Post("/")]
+    Task<string> PostValue(UserBody body0, UserBody body1);
+}
+
+public interface IManyAuthorize
+{
+    [Get("/")]
+    Task<string> GetValue([Authorize("Bearer")] string token0, [Authorize("Bearer")] string token1);
+}
+
+public interface IInvalidReturnType
+{
+    [Get("/")]
+    string GetValue();
+}
+
+public class RestServiceExceptionTests
+{
+    [Fact]
+    public void ManyCancellationTokensShouldThrow()
+    {
+        var exception = Assert.Throws<ArgumentException>(() => RestService.For<IManyCancellationTokens>("https://api.github.com"));
+        AssertExceptionContains("only contain a single CancellationToken", exception);
+    }
+
+    [Fact]
+    public void ManyHeaderCollectionShouldThrow()
+    {
+        var exception = Assert.Throws<ArgumentException>(() => RestService.For<IManyHeaderCollections>("https://api.github.com"));
+        AssertExceptionContains("Only one parameter can be a HeaderCollection parameter", exception);
+    }
+
+    [Fact]
+    public void InvalidHeaderCollectionTypeShouldThrow()
+    {
+        var exception = Assert.Throws<ArgumentException>(() => RestService.For<IHeaderCollectionWrongType>("https://api.github.com"));
+        AssertExceptionContains("HeaderCollection parameter of type", exception);
+    }
+
+    [Fact]
+    public void UrlDoesntStartWithSlashShouldThrow()
+    {
+        var exception = Assert.Throws<ArgumentException>(() => RestService.For<IDoesNotStartSlash>("https://api.github.com"));
+        AssertExceptionContains("must start with '/' and be of the form", exception);
+    }
+
+    [Fact]
+    public void UrlContainsCRLFShouldThrow()
+    {
+        var exception = Assert.Throws<ArgumentException>(() => RestService.For<IUrlContainsCRLF>("https://api.github.com"));
+        AssertExceptionContains("must not contain CR or LF characters", exception);
+    }
+
+    [Fact]
+    public void RoundTripParameterNotStringShouldThrow()
+    {
+        var exception = Assert.Throws<ArgumentException>(() => RestService.For<IRoundTripNotString>("https://api.github.com"));
+        AssertExceptionContains("has round-tripping parameter", exception);
+    }
+
+    [Fact]
+    public void UrlNoMatchingParameterShouldThrow()
+    {
+        var exception = Assert.Throws<ArgumentException>(() => RestService.For<IUrlNoMatchingParameters>("https://api.github.com"));
+        AssertExceptionContains("but no method parameter matches", exception);
+    }
+
+    [Fact]
+    public void MultipartAndBodyShouldThrow()
+    {
+        var exception = Assert.Throws<ArgumentException>(() => RestService.For<IMultipartAndBody>("https://api.github.com"));
+        AssertExceptionContains("Multipart requests may not contain a Body parameter", exception);
+    }
+
+    [Fact]
+    public void ManyBodyShouldThrow()
+    {
+        var exception = Assert.Throws<ArgumentException>(() => RestService.For<IManyBody>("https://api.github.com"));
+        AssertExceptionContains("Only one parameter can be a Body parameter", exception);
+    }
+
+    [Fact]
+    public void ManyComplexTypesShouldThrow()
+    {
+        var exception = Assert.Throws<ArgumentException>(() => RestService.For<IManyComplexTypes>("https://api.github.com"));
+        AssertExceptionContains("Multiple complex types found. Specify one parameter as the body using BodyAttribute", exception);
+    }
+
+    [Fact]
+    public void ManyAuthorizeAttributesShouldThrow()
+    {
+        var exception = Assert.Throws<ArgumentException>(() => RestService.For<IManyAuthorize>("https://api.github.com"));
+        AssertExceptionContains("Only one parameter can be an Authorize parameter", exception);
+    }
+
+    [Fact]
+    public void InvalidReturnTypeShouldThrow()
+    {
+        var exception = Assert.Throws<ArgumentException>(() => RestService.For<IInvalidReturnType>("https://api.github.com"));
+        AssertExceptionContains("is invalid. All REST Methods must return either Task<T> or ValueTask<T> or IObservable<T>", exception);
+    }
+
+    private static void AssertExceptionContains(string expectedSubstring, Exception exception)
+    {
+        Assert.Contains(expectedSubstring, exception.Message!, StringComparison.Ordinal);
+    }
+}