fix: do not emit RF001 for methods from plain non-refit base interfaces#2072
fix: do not emit RF001 for methods from plain non-refit base interfaces#2072reabr wants to merge 3 commits intoreactiveui:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes an incorrect RF001 diagnostic/stub generation scenario when a Refit interface inherits from a “plain” (non-Refit) base interface, aiming to ignore those base members as non-endpoints.
Changes:
- Updates generator parsing logic to filter inherited non-Refit methods based on whether their declaring interface contains any Refit HTTP methods.
- Adds a new snapshot-based test case and snapshots for an interface inheriting a non-Refit base interface.
- Updates existing generator test snapshots to reflect the new inherited-member handling.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| Refit.Tests/Refit.Tests.csproj | Excludes the new fixture file from test project compilation. |
| Refit.Tests/InterfaceStubGenerator.cs | Adds a new snapshot test for inheriting from a non-Refit base interface. |
| Refit.Tests/IInterfaceInheritingNonRefit.cs | Adds a new fixture interface pair (plain base + Refit-derived). |
| Refit.Tests/_snapshots/InterfaceStubGeneratorTests.InheritingFromNonRefitInterfaceDoesNotGenerateDiagnostic#PreserveAttribute.g.verified.cs | Adds snapshot for generated PreserveAttribute. |
| Refit.Tests/_snapshots/InterfaceStubGeneratorTests.InheritingFromNonRefitInterfaceDoesNotGenerateDiagnostic#IMyRefitApi.g.verified.cs | Adds snapshot for generated client for IMyRefitApi. |
| Refit.Tests/_snapshots/InterfaceStubGeneratorTests.InheritingFromNonRefitInterfaceDoesNotGenerateDiagnostic#Generated.g.verified.cs | Adds snapshot for generated Generated container. |
| Refit.GeneratorTests/_snapshots/InterfaceTests.RefitInterfaceDerivedFromBaseTest#IGeneratedInterface.g.verified.cs | Updates snapshot to remove the previously generated non-Refit base method stub. |
| InterfaceStubGenerator.Shared/Parser.cs | Changes how inherited non-Refit methods are selected for diagnostics/stub generation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var derivedNonRefitMethods = derivedMethods | ||
| .Except(derivedRefitMethods, SymbolEqualityComparer.Default) | ||
| .Cast<IMethodSymbol>() | ||
| .Where(m => | ||
| // Only flag methods from interfaces that have at least one Refit method. |
There was a problem hiding this comment.
The new filter on derivedNonRefitMethods removes inherited non‑Refit members from processing entirely. derivedNonRefitMethods is later used to generate explicit interface implementations (via ParseNonRefitMethod / WriteNonRefitMethod) that are required for the generated client class to compile when the Refit interface inherits a base interface containing non‑Refit members. If the intent is only to suppress RF001 for methods coming from “plain” base interfaces, keep generating the stub methods but conditionally suppress the diagnostic emission (e.g., add a flag to ParseNonRefitMethod to skip DiagnosticDescriptors.InvalidRefitMember for those members).
| public interface IMyRefitApi : IBaseApi | ||
| { | ||
| [Get("/users")] | ||
| Task<List<string>> GetUsers(); | ||
| } |
There was a problem hiding this comment.
This new fixture is not self-contained for the generator test compilation: Task and List are unqualified and there are no using System.Threading.Tasks; / using System.Collections.Generic; directives. In VerifyGenerator only this file is compiled, so these become error symbols and can cause the generator to produce invalid output (and can hide real regressions). Add the required using directives or fully-qualify the types so the compilation is successful and the snapshot represents real generated code.
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <Compile Remove="IInterfaceInheritingNonRefit.cs" /> |
There was a problem hiding this comment.
Excluding IInterfaceInheritingNonRefit.cs from compilation looks like a workaround for the fixture not compiling as-is. If you add the missing using directives / fully-qualified type names in the fixture, consider removing this <Compile Remove> (or instead include the fixture as <None> if the intent is to keep it out of the test assembly) so the project doesn’t silently diverge from the actual scenario being tested.
| <Compile Remove="IInterfaceInheritingNonRefit.cs" /> | |
| <None Include="IInterfaceInheritingNonRefit.cs" /> |
| } | ||
|
|
||
|
|
||
| /// <inheritdoc /> |
There was a problem hiding this comment.
This snapshot shows the generated implementation of IMyRefitApi does not provide any implementation for the inherited IBaseApi.GetBaseUri() member. If IBaseApi is part of the consumer compilation (real-world case), this would make the generated client fail to compile (or fail to implement the interface contract). If the intention is only to avoid RF001 for “plain” base interfaces, the generator should still emit an explicit stub for GetBaseUri() (without reporting RF001) so the generated type remains a valid IMyRefitApi implementation.
| /// <inheritdoc /> | |
| /// <inheritdoc /> | |
| public global::System.Uri GetBaseUri() | |
| { | |
| return this.Client.BaseAddress; | |
| } | |
| /// <inheritdoc /> |
| @@ -37,11 +37,6 @@ public RefitGeneratorTestIGeneratedInterface(global::System.Net.Http.HttpClient | |||
| return await ((global::System.Threading.Tasks.Task<string>)______func(this.Client, ______arguments)).ConfigureAwait(false); | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
The updated snapshot no longer includes an explicit implementation for IBaseInterface.NonRefitMethod(). Since IGeneratedInterface : IBaseInterface, the generated client must still implement that member (even if it just throws) to compile and be castable to IGeneratedInterface. If the goal is to stop emitting RF001 for non‑Refit base-interface members, suppress the diagnostic but still emit a stub implementation to satisfy the interface contract.
| void global::RefitGeneratorTest.IBaseInterface.NonRefitMethod() | |
| { | |
| throw new global::System.NotImplementedException(); | |
| } |
Fixes #2050
Problem
When a Refit interface inherited from a plain non-Refit interface (e.g. IBaseApi
with GetBaseUri()), the source generator would incorrectly emit RF001 diagnostics
for the base interface methods and attempt to generate stubs for them.
Solution
In
Parser.cs, added a filter onderivedNonRefitMethodsto only flag methodsfrom base interfaces that themselves contain at least one Refit HTTP method attribute.
Methods from plain non-Refit interfaces are now silently ignored since they are not
HTTP endpoints.
Testing
Added a fixture
IInterfaceInheritingNonRefit.cswith a plain base interface anda Refit interface inheriting from it. The snapshot test confirms that:
GetBaseUri()GetUsers()gets a generated stub