Skip to content

Commit c4747d7

Browse files
C#: Fix typed capture constraints for generic interfaces and primitives (#7122)
Use Roslyn-resolved JavaType from the pattern scaffold for type comparison instead of the raw type string, fixing two issues: - Generic interface types (IDictionary<object, object>) now correctly match concrete implementations (Dictionary<string, int>) by comparing base FQNs from the resolved type hierarchy - Primitive types (int, bool, double, etc.) are now handled in IsAssignableTo via a PrimitiveKind-to-FQN mapping
1 parent d9c0ecf commit c4747d7

File tree

6 files changed

+238
-16
lines changed

6 files changed

+238
-16
lines changed

rewrite-csharp/csharp/OpenRewrite/CSharp/Template/Capture.cs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,16 @@ public bool EvaluateConstraint(object candidate, CaptureConstraintContext contex
117117
// attribution, verify the candidate's semantic type is assignable to it.
118118
// If the candidate has no type info (expr.Type == null), skip the check —
119119
// type attribution may be unavailable when reference assemblies aren't provided.
120-
if (Type != null && candidate is Expression { Type: not null } expr
121-
&& !TypeUtils.IsAssignableTo(expr.Type, ResolveCSharpAlias(Type)))
122-
return false;
120+
if (Type != null && candidate is Expression { Type: not null } expr)
121+
{
122+
// Prefer the Roslyn-resolved type from the pattern scaffold when available.
123+
// This handles generics (IDictionary<object, object> resolves to its FQN)
124+
// and primitives (int resolves to System.Int32) correctly without string parsing.
125+
var matched = context.PatternType != null
126+
? TypeUtils.IsAssignableTo(expr.Type, context.PatternType)
127+
: TypeUtils.IsAssignableTo(expr.Type, ResolveCSharpAlias(Type));
128+
if (!matched) return false;
129+
}
123130

124131
if (Constraint == null) return true;
125132
// The `is T` check acts as an implicit type guard: if the candidate is not

rewrite-csharp/csharp/OpenRewrite/CSharp/Template/CaptureConstraintContext.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
* limitations under the License.
1515
*/
1616
using OpenRewrite.Core;
17+
using OpenRewrite.Java;
1718

1819
namespace OpenRewrite.CSharp.Template;
1920

@@ -26,7 +27,11 @@ namespace OpenRewrite.CSharp.Template;
2627
/// <param name="Captures">Read-only snapshot of captures already bound at the point this
2728
/// constraint is evaluated. Enables dependent constraints where one capture's validity
2829
/// depends on another's value.</param>
30+
/// <param name="PatternType">The Roslyn-resolved <see cref="JavaType"/> of the pattern
31+
/// placeholder, when available. Used by typed captures to compare against the candidate's
32+
/// type using the fully-resolved type from the scaffold rather than the raw type string.</param>
2933
public sealed record CaptureConstraintContext(
3034
Cursor Cursor,
31-
IReadOnlyDictionary<string, object> Captures
35+
IReadOnlyDictionary<string, object> Captures,
36+
JavaType? PatternType = null
3237
);

rewrite-csharp/csharp/OpenRewrite/CSharp/Template/PatternMatchingComparator.cs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,9 @@ private bool MatchNode(J pattern, J candidate, Cursor cursor)
6060
// Already bound — check consistency
6161
return MatchValue(existing, candidate, cursor);
6262
}
63-
// Evaluate constraint before binding
64-
if (!EvaluateConstraint(_captures[captureName], candidate, cursor))
63+
// Evaluate constraint before binding — pass the pattern placeholder's
64+
// resolved type so typed captures can compare JavaType-to-JavaType
65+
if (!EvaluateConstraint(_captures[captureName], candidate, cursor, patternId.Type))
6566
return false;
6667
_bindings[captureName] = candidate;
6768
return true;
@@ -559,11 +560,11 @@ private static bool IsStatic(JavaType.Method method) =>
559560
private static bool IsStatic(JavaType.Variable variable) =>
560561
(variable.FlagsBitMap & FlagStatic) != 0;
561562

562-
private CaptureConstraintContext BuildConstraintContext(Cursor cursor) =>
563-
new(cursor, new Dictionary<string, object>(_bindings));
563+
private CaptureConstraintContext BuildConstraintContext(Cursor cursor, JavaType? patternType = null) =>
564+
new(cursor, new Dictionary<string, object>(_bindings), patternType);
564565

565-
private bool EvaluateConstraint(object captureObj, object candidate, Cursor cursor) =>
566-
captureObj is not ICapture capture || capture.EvaluateConstraint(candidate, BuildConstraintContext(cursor));
566+
private bool EvaluateConstraint(object captureObj, object candidate, Cursor cursor, JavaType? patternType = null) =>
567+
captureObj is not ICapture capture || capture.EvaluateConstraint(candidate, BuildConstraintContext(cursor, patternType));
567568

568569
private bool EvaluateVariadicConstraint(object captureObj, IReadOnlyList<object> captured, Cursor cursor) =>
569570
captureObj is not ICapture capture || capture.EvaluateVariadicConstraint(captured, BuildConstraintContext(cursor));

rewrite-csharp/csharp/OpenRewrite/Java/TypeUtils.cs

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,26 +38,55 @@ public static bool IsAssignableTo(JavaType? type, string fullyQualifiedName)
3838
{
3939
if (type == null) return false;
4040

41-
// Primitive(String) is assignable to "System.String" but has no Class representation
42-
if (type is JavaType.Primitive { Kind: JavaType.Primitive.PrimitiveKind.String })
43-
return string.Equals("System.String", fullyQualifiedName, StringComparison.Ordinal);
41+
// Primitives (int, bool, string, etc.) have no Class representation —
42+
// map to their .NET FQN and compare directly
43+
if (type is JavaType.Primitive prim)
44+
{
45+
var primFqn = PrimitiveToFqn(prim.Kind);
46+
return primFqn != null && string.Equals(primFqn, fullyQualifiedName, StringComparison.Ordinal);
47+
}
4448

4549
var cls = AsClass(type);
4650
if (cls == null) return false;
4751

4852
return IsAssignableToInternal(cls, fullyQualifiedName, new HashSet<string>());
4953
}
5054

55+
/// <summary>
56+
/// Check if a type is assignable to the target type, where the target is specified
57+
/// as a <see cref="JavaType"/> rather than a string FQN. This is the preferred overload
58+
/// when the target type comes from a parsed AST (e.g., a typed capture's scaffold).
59+
/// For parameterized targets like <c>IDictionary&lt;object, object&gt;</c>, the generic
60+
/// type arguments are ignored — only the base type definition is checked.
61+
/// </summary>
62+
public static bool IsAssignableTo(JavaType? type, JavaType? targetType)
63+
{
64+
if (type == null || targetType == null) return false;
65+
66+
// Both primitives: same kind means match
67+
if (type is JavaType.Primitive candPrim && targetType is JavaType.Primitive targetPrim)
68+
return candPrim.Kind == targetPrim.Kind;
69+
70+
// Extract the base FQN from the target, stripping generic type parameters
71+
var targetFqn = GetFullyQualifiedName(targetType);
72+
if (targetFqn == null) return false;
73+
74+
return IsAssignableTo(type, targetFqn);
75+
}
76+
5177
/// <summary>
5278
/// Check if a type is assignable to any of the given fully-qualified type names.
5379
/// </summary>
5480
public static bool IsAssignableTo(JavaType? type, IReadOnlyCollection<string> fullyQualifiedNames)
5581
{
5682
if (type == null) return false;
5783

58-
// Primitive(String) is assignable to "System.String" but has no Class representation
59-
if (type is JavaType.Primitive { Kind: JavaType.Primitive.PrimitiveKind.String })
60-
return fullyQualifiedNames.Contains("System.String");
84+
// Primitives have no Class representation — map to FQN and check
85+
if (type is JavaType.Primitive prim)
86+
{
87+
var primFqn = PrimitiveToFqn(prim.Kind);
88+
return primFqn != null && fullyQualifiedNames.Contains(primFqn);
89+
}
6190

6291
var cls = AsClass(type);
6392
if (cls == null) return false;
@@ -251,6 +280,24 @@ private static bool HasMethodInternal(JavaType.Class cls, string methodName, Has
251280
return false;
252281
}
253282

283+
/// <summary>
284+
/// Map a <see cref="JavaType.PrimitiveKind"/> to its .NET fully-qualified type name.
285+
/// Returns null for non-value primitives (Null, None, Void).
286+
/// </summary>
287+
private static string? PrimitiveToFqn(JavaType.PrimitiveKind kind) => kind switch
288+
{
289+
JavaType.PrimitiveKind.Boolean => "System.Boolean",
290+
JavaType.PrimitiveKind.Byte => "System.Byte",
291+
JavaType.PrimitiveKind.Char => "System.Char",
292+
JavaType.PrimitiveKind.Double => "System.Double",
293+
JavaType.PrimitiveKind.Float => "System.Single",
294+
JavaType.PrimitiveKind.Int => "System.Int32",
295+
JavaType.PrimitiveKind.Long => "System.Int64",
296+
JavaType.PrimitiveKind.Short => "System.Int16",
297+
JavaType.PrimitiveKind.String => "System.String",
298+
_ => null
299+
};
300+
254301
private static JavaType? TryGetTypeDynamic(Expression expr)
255302
{
256303
try

rewrite-csharp/csharp/OpenRewrite/Tests/Java/TypeUtilsTests.cs

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,87 @@ public void AsClass_NullReturnsNull()
291291
Assert.Null(TypeUtils.AsClass(null));
292292
}
293293

294+
// =============================================================
295+
// IsAssignableTo — primitives beyond String
296+
// =============================================================
297+
298+
[Fact]
299+
public void IsAssignableTo_PrimitiveInt()
300+
{
301+
var prim = JavaType.Primitive.Of(JavaType.Primitive.PrimitiveKind.Int);
302+
Assert.True(TypeUtils.IsAssignableTo(prim, "System.Int32"));
303+
}
304+
305+
[Fact]
306+
public void IsAssignableTo_PrimitiveBool()
307+
{
308+
var prim = JavaType.Primitive.Of(JavaType.Primitive.PrimitiveKind.Boolean);
309+
Assert.True(TypeUtils.IsAssignableTo(prim, "System.Boolean"));
310+
}
311+
312+
[Fact]
313+
public void IsAssignableTo_PrimitiveDouble()
314+
{
315+
var prim = JavaType.Primitive.Of(JavaType.Primitive.PrimitiveKind.Double);
316+
Assert.True(TypeUtils.IsAssignableTo(prim, "System.Double"));
317+
}
318+
319+
[Fact]
320+
public void IsAssignableTo_PrimitiveInt_NotAssignableToOther()
321+
{
322+
var prim = JavaType.Primitive.Of(JavaType.Primitive.PrimitiveKind.Int);
323+
Assert.False(TypeUtils.IsAssignableTo(prim, "System.String"));
324+
}
325+
326+
// =============================================================
327+
// IsAssignableTo — JavaType target overload
328+
// =============================================================
329+
330+
[Fact]
331+
public void IsAssignableTo_JavaType_SamePrimitive()
332+
{
333+
var candidateType = JavaType.Primitive.Of(JavaType.Primitive.PrimitiveKind.Int);
334+
var targetType = JavaType.Primitive.Of(JavaType.Primitive.PrimitiveKind.Int);
335+
Assert.True(TypeUtils.IsAssignableTo(candidateType, targetType));
336+
}
337+
338+
[Fact]
339+
public void IsAssignableTo_JavaType_DifferentPrimitive()
340+
{
341+
var candidateType = JavaType.Primitive.Of(JavaType.Primitive.PrimitiveKind.Int);
342+
var targetType = JavaType.Primitive.Of(JavaType.Primitive.PrimitiveKind.String);
343+
Assert.False(TypeUtils.IsAssignableTo(candidateType, targetType));
344+
}
345+
346+
[Fact]
347+
public void IsAssignableTo_JavaType_ClassTarget()
348+
{
349+
var iface = MakeClass("System.IDisposable");
350+
var candidate = MakeClass("MyClass", interfaces: [iface]);
351+
var target = MakeClass("System.IDisposable");
352+
Assert.True(TypeUtils.IsAssignableTo(candidate, target));
353+
}
354+
355+
[Fact]
356+
public void IsAssignableTo_JavaType_ParameterizedTarget()
357+
{
358+
// Target is Parameterized(IDictionary) — should compare by base FQN
359+
var idict = MakeClass("System.Collections.Generic.IDictionary");
360+
var dict = MakeClass("System.Collections.Generic.Dictionary", interfaces: [idict]);
361+
var target = new JavaType.Parameterized
362+
{
363+
Type = MakeClass("System.Collections.Generic.IDictionary")
364+
};
365+
Assert.True(TypeUtils.IsAssignableTo(dict, target));
366+
}
367+
368+
[Fact]
369+
public void IsAssignableTo_JavaType_NullTarget()
370+
{
371+
var cls = MakeClass("System.String");
372+
Assert.False(TypeUtils.IsAssignableTo(cls, (JavaType?)null));
373+
}
374+
294375
// =============================================================
295376
// Cycle protection
296377
// =============================================================

rewrite-csharp/csharp/OpenRewrite/Tests/Template/TypedCaptureTests.cs

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,84 @@ public void TypedCaptureMatchesCompatibleType()
165165
);
166166
}
167167

168+
[Fact]
169+
public void TypedCaptureMatchesGenericInterfaceImplementation()
170+
{
171+
var dict = Capture.Expression("dict", type: "IDictionary<object, object>");
172+
var key = Capture.Expression("key");
173+
var pat = CSharpPattern.Expression($"{dict}.Keys.Contains({key})",
174+
usings: ["System.Collections.Generic"]);
175+
176+
RewriteRun(
177+
spec => spec.SetRecipe(FindMethodInvocation(pat))
178+
.SetReferenceAssemblies(Assemblies.Net90),
179+
CSharp(
180+
"""
181+
using System.Collections.Generic;
182+
class Test
183+
{
184+
void M()
185+
{
186+
var dict = new Dictionary<string, int>();
187+
bool has = dict.Keys.Contains("key");
188+
}
189+
}
190+
""",
191+
"""
192+
using System.Collections.Generic;
193+
class Test
194+
{
195+
void M()
196+
{
197+
var dict = new Dictionary<string, int>();
198+
bool has = /*~~>*/dict.Keys.Contains("key");
199+
}
200+
}
201+
"""
202+
)
203+
);
204+
}
205+
206+
[Fact]
207+
public void TypedCaptureMatchesPrimitiveInt()
208+
{
209+
var expr = Capture.Expression("expr");
210+
var idx = Capture.Expression("idx", type: "int");
211+
var pat = CSharpPattern.Expression($"{expr}.ElementAt({idx})",
212+
usings: ["System.Linq"]);
213+
214+
RewriteRun(
215+
spec => spec.SetRecipe(FindMethodInvocation(pat))
216+
.SetReferenceAssemblies(Assemblies.Net90),
217+
CSharp(
218+
"""
219+
using System.Linq;
220+
using System.Collections.Generic;
221+
class Test
222+
{
223+
void M()
224+
{
225+
var list = new List<string>();
226+
var item = list.ElementAt(0);
227+
}
228+
}
229+
""",
230+
"""
231+
using System.Linq;
232+
using System.Collections.Generic;
233+
class Test
234+
{
235+
void M()
236+
{
237+
var list = new List<string>();
238+
var item = /*~~>*/list.ElementAt(0);
239+
}
240+
}
241+
"""
242+
)
243+
);
244+
}
245+
168246
// ===============================================================
169247
// Recipe factories
170248
// ===============================================================
@@ -177,6 +255,9 @@ private static Core.Recipe FindExpression(string code)
177255

178256
private static Core.Recipe FindMethodInvocation(TemplateStringHandler handler, IReadOnlyList<string> usings)
179257
=> new MethodInvocationSearchRecipe(CSharpPattern.Expression(handler, usings: usings));
258+
259+
private static Core.Recipe FindMethodInvocation(CSharpPattern pat)
260+
=> new MethodInvocationSearchRecipe(pat);
180261
}
181262

182263
file class TypedPatternSearchRecipe(CSharpPattern pat) : Core.Recipe

0 commit comments

Comments
 (0)