Skip to content

Commit 55646f9

Browse files
fix(adapter): Add missing diagnostics, validation, and deterministic ordering (#111)
* Initial plan * fix(adapter): Address PR review feedback - diagnostics, validation, and docs Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: JerrettDavis <2610199+JerrettDavis@users.noreply.github.com>
1 parent 27cfb82 commit 55646f9

4 files changed

Lines changed: 142 additions & 6 deletions

File tree

docs/generators/adapter.md

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,47 @@ public static partial class Adapters
239239
| **PKADP006** | Error | Adapter type name conflicts with existing type |
240240
| **PKADP007** | Error | Adaptee must be a concrete class or struct |
241241
| **PKADP008** | Error | Mapping method must be static |
242+
| **PKADP009** | Error | Events are not supported |
243+
| **PKADP010** | Error | Generic methods are not supported |
244+
| **PKADP011** | Error | Overloaded methods are not supported |
245+
| **PKADP012** | Error | Abstract class target requires accessible parameterless constructor |
246+
| **PKADP013** | Error | Settable properties are not supported |
247+
| **PKADP014** | Error | Nested or generic host not supported |
248+
| **PKADP015** | Error | Mapping method must be accessible (public or internal) |
249+
| **PKADP016** | Error | Static members are not supported |
250+
| **PKADP017** | Error | Ref-return members are not supported |
251+
252+
## Limitations
253+
254+
### Multiple Adapters with Shared Adaptee
255+
256+
When defining multiple `[GenerateAdapter]` attributes within the same host class that share the same adaptee type, mapping ambiguity can occur. The generator matches `[AdapterMap]` methods to adapters solely by adaptee type and then by `TargetMember` name. If two target types have overlapping member names (both use `nameof(...)` resulting in the same string), mappings become ambiguous and may trigger false `PKADP004` duplicate mapping diagnostics.
257+
258+
**Workaround:** Define separate host classes for each adapter when they share the same adaptee type:
259+
260+
```csharp
261+
// ✅ Good: Separate hosts avoid ambiguity
262+
[GenerateAdapter(Target = typeof(IServiceA), Adaptee = typeof(LegacyService))]
263+
public static partial class ServiceAAdapters
264+
{
265+
[AdapterMap(TargetMember = nameof(IServiceA.DoWork))]
266+
public static void MapDoWork(LegacyService adaptee) => adaptee.Execute();
267+
}
268+
269+
[GenerateAdapter(Target = typeof(IServiceB), Adaptee = typeof(LegacyService))]
270+
public static partial class ServiceBAdapters
271+
{
272+
[AdapterMap(TargetMember = nameof(IServiceB.DoWork))]
273+
public static void MapDoWork(LegacyService adaptee) => adaptee.Run();
274+
}
275+
276+
// ⚠️ Problematic: Multiple adapters with same adaptee in one host
277+
public static partial class AllAdapters
278+
{
279+
// Both IServiceA and IServiceB have DoWork() members
280+
// The generator cannot distinguish which mapping is for which target
281+
}
282+
```
242283

243284
## Best Practices
244285

src/PatternKit.Generators/Adapter/AdapterGenerator.cs

Lines changed: 95 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
using Microsoft.CodeAnalysis;
22
using Microsoft.CodeAnalysis.CSharp;
33
using Microsoft.CodeAnalysis.CSharp.Syntax;
4-
using System.Collections.Immutable;
54
using System.Text;
65

76
namespace PatternKit.Generators.Adapter;
@@ -35,6 +34,10 @@ public sealed class AdapterGenerator : IIncrementalGenerator
3534
private const string DiagIdOverloadedMethodsNotSupported = "PKADP011";
3635
private const string DiagIdAbstractClassNoParameterlessCtor = "PKADP012";
3736
private const string DiagIdSettablePropertiesNotSupported = "PKADP013";
37+
private const string DiagIdNestedOrGenericHost = "PKADP014";
38+
private const string DiagIdMappingMethodNotAccessible = "PKADP015";
39+
private const string DiagIdStaticMembersNotSupported = "PKADP016";
40+
private const string DiagIdRefReturnNotSupported = "PKADP017";
3841

3942
private static readonly DiagnosticDescriptor HostNotStaticPartialDescriptor = new(
4043
id: DiagIdHostNotStaticPartial,
@@ -140,6 +143,38 @@ public sealed class AdapterGenerator : IIncrementalGenerator
140143
defaultSeverity: DiagnosticSeverity.Error,
141144
isEnabledByDefault: true);
142145

146+
private static readonly DiagnosticDescriptor NestedOrGenericHostDescriptor = new(
147+
id: DiagIdNestedOrGenericHost,
148+
title: "Nested or generic host not supported",
149+
messageFormat: "Adapter host '{0}' cannot be nested or generic",
150+
category: "PatternKit.Generators.Adapter",
151+
defaultSeverity: DiagnosticSeverity.Error,
152+
isEnabledByDefault: true);
153+
154+
private static readonly DiagnosticDescriptor MappingMethodNotAccessibleDescriptor = new(
155+
id: DiagIdMappingMethodNotAccessible,
156+
title: "Mapping method must be accessible",
157+
messageFormat: "Mapping method '{0}' must be public or internal to be accessible from generated adapter",
158+
category: "PatternKit.Generators.Adapter",
159+
defaultSeverity: DiagnosticSeverity.Error,
160+
isEnabledByDefault: true);
161+
162+
private static readonly DiagnosticDescriptor StaticMembersNotSupportedDescriptor = new(
163+
id: DiagIdStaticMembersNotSupported,
164+
title: "Static members are not supported",
165+
messageFormat: "Target type '{0}' contains static member '{1}' which is not supported by the adapter generator",
166+
category: "PatternKit.Generators.Adapter",
167+
defaultSeverity: DiagnosticSeverity.Error,
168+
isEnabledByDefault: true);
169+
170+
private static readonly DiagnosticDescriptor RefReturnNotSupportedDescriptor = new(
171+
id: DiagIdRefReturnNotSupported,
172+
title: "Ref-return members are not supported",
173+
messageFormat: "Target type '{0}' contains ref-return member '{1}' which is not supported by the adapter generator",
174+
category: "PatternKit.Generators.Adapter",
175+
defaultSeverity: DiagnosticSeverity.Error,
176+
isEnabledByDefault: true);
177+
143178
public void Initialize(IncrementalGeneratorInitializationContext context)
144179
{
145180
// Find all class declarations with [GenerateAdapter] attribute
@@ -183,6 +218,16 @@ private void GenerateAdapterForAttribute(
183218
return;
184219
}
185220

221+
// Validate host is not nested or generic
222+
if (hostSymbol.ContainingType is not null || hostSymbol.TypeParameters.Length > 0)
223+
{
224+
context.ReportDiagnostic(Diagnostic.Create(
225+
NestedOrGenericHostDescriptor,
226+
node.GetLocation(),
227+
hostSymbol.Name));
228+
return;
229+
}
230+
186231
// Parse attribute arguments
187232
var config = ParseAdapterConfig(attribute);
188233
if (config.TargetType is null || config.AdapteeType is null)
@@ -240,7 +285,7 @@ private void GenerateAdapterForAttribute(
240285
// Get all mapping methods from host
241286
var mappingMethods = GetMappingMethods(hostSymbol, config.AdapteeType);
242287

243-
// Validate mapping methods are static
288+
// Validate mapping methods are static and accessible
244289
foreach (var (method, _) in mappingMethods)
245290
{
246291
if (!method.IsStatic)
@@ -251,6 +296,17 @@ private void GenerateAdapterForAttribute(
251296
method.Name));
252297
return;
253298
}
299+
300+
// Validate method is accessible (public or internal)
301+
if (method.DeclaredAccessibility != Accessibility.Public &&
302+
method.DeclaredAccessibility != Accessibility.Internal)
303+
{
304+
context.ReportDiagnostic(Diagnostic.Create(
305+
MappingMethodNotAccessibleDescriptor,
306+
method.Locations.FirstOrDefault() ?? node.GetLocation(),
307+
method.Name));
308+
return;
309+
}
254310
}
255311

256312
// Get target members that need mapping
@@ -403,6 +459,16 @@ private List<Diagnostic> ValidateTargetMembers(INamedTypeSymbol targetType, Loca
403459

404460
foreach (var member in membersToCheck)
405461
{
462+
// Check for static members (not supported)
463+
if (member.IsStatic)
464+
{
465+
diagnostics.Add(Diagnostic.Create(
466+
StaticMembersNotSupportedDescriptor,
467+
location,
468+
targetType.Name,
469+
member.Name));
470+
}
471+
406472
// Check for events (not supported)
407473
if (member is IEventSymbol evt)
408474
{
@@ -423,6 +489,16 @@ private List<Diagnostic> ValidateTargetMembers(INamedTypeSymbol targetType, Loca
423489
prop.Name));
424490
}
425491

492+
// Check for ref-return properties (not supported)
493+
if (member is IPropertySymbol refProp && refProp.ReturnsByRef)
494+
{
495+
diagnostics.Add(Diagnostic.Create(
496+
RefReturnNotSupportedDescriptor,
497+
location,
498+
targetType.Name,
499+
refProp.Name));
500+
}
501+
426502
// Check for generic methods (not supported)
427503
if (member is IMethodSymbol method && method.MethodKind == MethodKind.Ordinary)
428504
{
@@ -435,6 +511,16 @@ private List<Diagnostic> ValidateTargetMembers(INamedTypeSymbol targetType, Loca
435511
method.Name));
436512
}
437513

514+
// Check for ref-return methods (not supported)
515+
if (method.ReturnsByRef || method.ReturnsByRefReadonly)
516+
{
517+
diagnostics.Add(Diagnostic.Create(
518+
RefReturnNotSupportedDescriptor,
519+
location,
520+
targetType.Name,
521+
method.Name));
522+
}
523+
438524
// Track full method signature for overload detection
439525
var sig = GetMemberSignature(method);
440526
if (!methodSignatures.TryGetValue(method.Name, out var sigs))
@@ -557,6 +643,7 @@ private static List<ISymbol> GetTargetMembers(INamedTypeSymbol targetType)
557643
continue;
558644

559645
var membersToProcess = type.GetMembers()
646+
.Where(m => !m.IsStatic) // Exclude static members
560647
.Where(m => !isAbstractClass || m.IsAbstract);
561648

562649
foreach (var member in membersToProcess)
@@ -592,8 +679,12 @@ private static List<ISymbol> GetTargetMembers(INamedTypeSymbol targetType)
592679
}
593680
}
594681

595-
// Return in declaration order (members already added in traversal order)
596-
return members;
682+
// Ensure stable, deterministic ordering by kind+name+signature
683+
// This provides a predictable output order even if member traversal is non-deterministic
684+
return members.OrderBy(m => m.Kind)
685+
.ThenBy(m => m.Name)
686+
.ThenBy(m => m.ToDisplayString(FullyQualifiedFormat))
687+
.ToList();
597688
}
598689

599690
private static string GetMemberSignature(IMethodSymbol method)

src/PatternKit.Generators/AnalyzerReleases.Unshipped.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,3 +111,7 @@ PKADP010 | PatternKit.Generators.Adapter | Error | Generic methods are not suppo
111111
PKADP011 | PatternKit.Generators.Adapter | Error | Overloaded methods are not supported
112112
PKADP012 | PatternKit.Generators.Adapter | Error | Abstract class target requires accessible parameterless constructor
113113
PKADP013 | PatternKit.Generators.Adapter | Error | Settable properties are not supported
114+
PKADP014 | PatternKit.Generators.Adapter | Error | Nested or generic host not supported
115+
PKADP015 | PatternKit.Generators.Adapter | Error | Mapping method must be accessible
116+
PKADP016 | PatternKit.Generators.Adapter | Error | Static members are not supported
117+
PKADP017 | PatternKit.Generators.Adapter | Error | Ref-return members are not supported

test/PatternKit.Generators.Tests/AdapterGeneratorTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ public class NotStaticPartial
184184
}
185185

186186
[Fact]
187-
public void ErrorWhenTargetNotInterface()
187+
public void ErrorWhenTargetNotInterfaceOrAbstract()
188188
{
189189
const string source = """
190190
using PatternKit.Generators.Adapter;
@@ -202,7 +202,7 @@ public class LegacyClock { }
202202
public static partial class Adapters { }
203203
""";
204204

205-
var comp = RoslynTestHelpers.CreateCompilation(source, nameof(ErrorWhenTargetNotInterface));
205+
var comp = RoslynTestHelpers.CreateCompilation(source, nameof(ErrorWhenTargetNotInterfaceOrAbstract));
206206
var gen = new AdapterGenerator();
207207
_ = RoslynTestHelpers.Run(comp, gen, out var result, out _);
208208

0 commit comments

Comments
 (0)