feat: generate code for derived non refit methods and update tests. (#1875)

TimothyMakkison · web-flow · commit 56d7bcde9bde · 2024-10-22T23:26:02.000+01:00
diff --git a/InterfaceStubGenerator.Shared/Parser.cs b/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();
 
diff --git a/Refit.GeneratorTests/Incremental/InheritanceTest.cs b/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);
     }
 }
diff --git a/Refit.GeneratorTests/InterfaceTests.cs b/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
diff --git a/Refit.GeneratorTests/_snapshots/InterfaceTests.RefitInterfaceDerivedFromBaseTest#IGeneratedInterface.g.verified.cs b/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.");
+        }
     }
     }
 }
diff --git a/Refit.GeneratorTests/_snapshots/InterfaceTests.RefitInterfaceDerivedFromBaseTest.verified.txt b/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
+      }
+    }
+  ]
+}
diff --git a/Refit.Tests/InheritedInterfacesApi.cs b/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("/")]
diff --git a/Refit.Tests/RestService.cs b/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();
 

Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,6 @@ public interface IGitHubApi : IDisposable`
`74`	`74`	`[Fact]`
`75`	`75`	`public void InheritFromInterfaceDoesRegenerate()`
`76`	`76`	`{`
`77`		`- // TODO: this currently generates invalid code see issue #1801 for more information`
`78`	`77`	`var syntaxTree = CSharpSyntaxTree.ParseText(TwoInterface, CSharpParseOptions.Default);`
`79`	`78`	`var compilation1 = Fixture.CreateLibrary(syntaxTree);`
`80`	`79`
`@@ -94,6 +93,6 @@ public interface IGitHubApi : IBaseInterface`
`94`	`93`	`"""`
`95`	`94`	`);`
`96`	`95`	`var driver2 = driver1.RunGenerators(compilation2);`
`97`		`- TestHelper.AssertRunReasons(driver2, IncrementalGeneratorRunReasons.Cached);`
	`96`	`+ TestHelper.AssertRunReasons(driver2, IncrementalGeneratorRunReasons.Modified);`
`98`	`97`	`}`
`99`	`98`	`}`
Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,6 @@ public interface IBaseInterface`
`40`	`40`	`[Fact]`
`41`	`41`	`public Task RefitInterfaceDerivedFromBaseTest()`
`42`	`42`	`{`
`43`		`- // TODO: this currently generates invalid code see issue #1801 for more information`
`44`	`43`	`return Fixture.VerifyForType(`
`45`	`44`	`"""`
`46`	`45`	`public interface IGeneratedInterface : IBaseInterface`
Original file line number	Diff line number	Diff line change
`@@ -46,6 +46,12 @@ public RefitGeneratorTestIGeneratedInterface(global::System.Net.Http.HttpClient`
`46`	`46`
`47`	`47`	`return await ((global::System.Threading.Tasks.Task<string>)______func(this.Client, ______arguments)).ConfigureAwait(false);`
`48`	`48`	`}`
	`49`	`+`
	`50`	`+ /// <inheritdoc />`
	`51`	`+ void global::RefitGeneratorTest.IBaseInterface.NonRefitMethod()`
	`52`	`+ {`
	`53`	`+ 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.");`
	`54`	`+ }`
`49`	`55`	`}`
`50`	`56`	`}`
`51`	`57`	`}`