Skip to content

Commit 5c4736e

Browse files
C#: Fix typed capture type resolution and improve type system (#7126)
* C#: Remove RewriteRule.Rewrite(TemplateStringHandler) shorthand The `Rewrite(TemplateStringHandler, TemplateStringHandler)` overload created patterns without `usings`, so typed captures with generic types (e.g., `IDictionary<TKey, TValue>`) silently failed — the scaffold had no semantic model and type constraints fell back to string matching. Removing the shorthand forces callers through the explicit `CSharpPattern`/`CSharpTemplate` API where `usings` is visible. Also adds two new tests exercising typed captures with `typeParameters` through `ToVisitor()`: `UseContainsKeyRecipe` (IDictionary) and `UseElementAtRecipe` (IEnumerable). * C#: Add implicit usings to scaffold compilations DependencyWorkspace.CreateSemanticModel now includes a synthetic GlobalUsings source file mirroring the .NET 6+ implicit usings. Without this, types like Dictionary<,> didn't resolve in scaffold compilations unless the caller explicitly added using directives. Also adds a test verifying Dictionary<string, int> resolves without an explicit using System.Collections.Generic directive. * C#: Map System.String as JavaType.Class instead of JavaType.Primitive System.String is a reference type with interfaces (IEnumerable<char>, IComparable<string>, IEquatable<string>, etc.) that TypeUtils.IsAssignableTo needs to walk. Mapping it as Primitive lost this hierarchy information. Now MapPrimitive skips System_String, so the type mapper builds a full JavaType.Class with supertypes and interfaces. J.Literal and J.Primitive (the syntax node for the `string` keyword) still use JavaType.Primitive(String) directly and are unaffected. * C#: Add test that string type argument is mapped as JavaType.Class Verifies that List<string>'s type parameter is JavaType.Class("System.String"), not JavaType.Primitive(String), confirming the type mapper preserves the full type hierarchy for string. * C#: Handle array type assignability in TypeUtils T[] implements a known set of non-generic (System.Array, IList, ICloneable, etc.) and generic (IEnumerable<T>, IList<T>, IReadOnlyList<T>, etc.) supertypes. TypeUtils.IsAssignableTo now recognizes these by synthesizing the parameterized interfaces on the fly when encountering a JavaType.Array. * C#: Move TypeUtils and MethodMatcher to OpenRewrite.CSharp namespace Both contain .NET-specific logic (array interface knowledge, string alias resolution, C# method matching) rather than Java concepts. * C#: Always create semantic model for scaffold compilations Previously the semantic model was only created when explicit usings or dependencies were provided. This meant scaffolds without usings got no type attribution at all, even though DependencyWorkspace now includes implicit usings. Always creating the semantic model ensures types resolve consistently. Reference resolution is cached so the cost is negligible. * C#: Fix MethodMatcher short name matching after String type change MethodMatcher patterns like "Aes Create(String)" failed to match when the parameter type is JavaType.Class("System.String") because the regex only matched the full FQN or keyword aliases. Now TypeArgMatcher also tries matching the simple name (last segment of the FQN), so "String" matches "System.String".
1 parent 84de4cf commit 5c4736e

File tree

10 files changed

+459
-52
lines changed

10 files changed

+459
-52
lines changed

rewrite-csharp/csharp/OpenRewrite/CSharp/CSharpTypeMapping.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,11 @@ private JavaType.Variable MapVariable(ISymbol symbol, string name, JavaType? own
321321

322322
private static JavaType.Primitive? MapPrimitive(INamedTypeSymbol symbol)
323323
{
324+
// System.String is intentionally omitted: it is a reference type with interfaces
325+
// (IEnumerable<char>, IComparable<string>, etc.) that TypeUtils.IsAssignableTo needs
326+
// to walk. Mapping it as JavaType.Class preserves the full type hierarchy.
327+
// J.Literal and J.Primitive (the syntax node for the `string` keyword) still use
328+
// JavaType.Primitive(String) directly — they don't go through this method.
324329
return symbol.SpecialType switch
325330
{
326331
SpecialType.System_Boolean => JavaType.Primitive.Of(JavaType.PrimitiveKind.Boolean),
@@ -332,7 +337,6 @@ private JavaType.Variable MapVariable(ISymbol symbol, string name, JavaType? own
332337
SpecialType.System_Int64 => JavaType.Primitive.Of(JavaType.PrimitiveKind.Long),
333338
SpecialType.System_Int16 => JavaType.Primitive.Of(JavaType.PrimitiveKind.Short),
334339
SpecialType.System_Void => JavaType.Primitive.Of(JavaType.PrimitiveKind.Void),
335-
SpecialType.System_String => JavaType.Primitive.Of(JavaType.PrimitiveKind.String),
336340
_ => null
337341
};
338342
}

rewrite-csharp/csharp/OpenRewrite/Java/MethodMatcher.cs renamed to rewrite-csharp/csharp/OpenRewrite/CSharp/MethodMatcher.cs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@
1414
* limitations under the License.
1515
*/
1616
using System.Text.RegularExpressions;
17+
using OpenRewrite.Java;
1718

18-
namespace OpenRewrite.Java;
19+
namespace OpenRewrite.CSharp;
1920

2021
/// <summary>
2122
/// Matches method invocations and declarations against AspectJ-style method patterns.
@@ -323,6 +324,15 @@ public override bool Matches(JavaType paramType)
323324
{
324325
if (_pattern.IsMatch(fqn)) return true;
325326
if (_fqnPattern != null && _fqnPattern.IsMatch(fqn)) return true;
327+
328+
// Match the simple name (last segment) against the pattern.
329+
// This allows patterns like "String" to match "System.String".
330+
var lastDot = fqn.LastIndexOf('.');
331+
if (lastDot >= 0)
332+
{
333+
var simpleName = fqn[(lastDot + 1)..];
334+
if (_pattern.IsMatch(simpleName)) return true;
335+
}
326336
}
327337

328338
// Also match simple name for primitives/keywords

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

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,24 @@ internal static class DependencyWorkspace
3232
{
3333
private static readonly ConcurrentDictionary<string, ImmutableArray<MetadataReference>> ReferencesCache = new();
3434

35+
/// <summary>
36+
/// Synthetic source file mirroring the <c>GlobalUsings.g.cs</c> that MSBuild generates
37+
/// when <c>&lt;ImplicitUsings&gt;enable&lt;/ImplicitUsings&gt;</c> is set (.NET 6+ default).
38+
/// Added to scaffold compilations so types like <c>Dictionary&lt;,&gt;</c> resolve
39+
/// without explicit <c>using</c> directives.
40+
/// </summary>
41+
private static readonly SyntaxTree ImplicitUsingsSyntaxTree = CSharpSyntaxTree.ParseText(
42+
"""
43+
global using System;
44+
global using System.Collections.Generic;
45+
global using System.IO;
46+
global using System.Linq;
47+
global using System.Net.Http;
48+
global using System.Threading;
49+
global using System.Threading.Tasks;
50+
""",
51+
path: "__GlobalUsings__.g.cs");
52+
3553
/// <summary>
3654
/// Resolve NuGet dependencies into metadata references.
3755
/// Results are cached by the sorted dependency set.
@@ -46,6 +64,8 @@ internal static ImmutableArray<MetadataReference> ResolveReferences(
4664
/// <summary>
4765
/// Create a Roslyn <see cref="SemanticModel"/> for the given source code,
4866
/// with NuGet package references resolved for type attribution.
67+
/// Includes .NET 6+ implicit usings so types like <c>Dictionary&lt;,&gt;</c>
68+
/// resolve without explicit <c>using</c> directives in the scaffold.
4969
/// </summary>
5070
internal static SemanticModel CreateSemanticModel(string source,
5171
IReadOnlyDictionary<string, string> dependencies)
@@ -55,7 +75,7 @@ internal static SemanticModel CreateSemanticModel(string source,
5575
var compilation = CSharpCompilation.Create("TemplateCompilation")
5676
.WithOptions(new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary))
5777
.AddReferences(references)
58-
.AddSyntaxTrees(syntaxTree);
78+
.AddSyntaxTrees(ImplicitUsingsSyntaxTree, syntaxTree);
5979
return compilation.GetSemanticModel(syntaxTree);
6080
}
6181

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

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,14 @@ namespace OpenRewrite.CSharp.Template;
2121

2222
/// <summary>
2323
/// A declarative rewrite rule that pairs structural pattern matching with template application.
24-
/// Create rules with <see cref="RewriteRule.Rewrite(TemplateStringHandler, TemplateStringHandler)"/>
25-
/// or <see cref="RewriteRule.Rewrite(CSharpPattern, CSharpTemplate)"/>.
24+
/// Create rules with <see cref="RewriteRule.Rewrite(CSharpPattern, CSharpTemplate)"/>.
2625
/// </summary>
2726
/// <example>
2827
/// <code>
2928
/// var expr = Capture.Expression("expr");
30-
/// var rule = RewriteRule.Rewrite($"Console.Write({expr})", $"Console.WriteLine({expr})");
29+
/// var rule = RewriteRule.Rewrite(
30+
/// CSharpPattern.Expression($"Console.Write({expr})"),
31+
/// CSharpTemplate.Expression($"Console.WriteLine({expr})"));
3132
///
3233
/// // In a visitor method:
3334
/// return rule.TryOn(Cursor, node) ?? node;
@@ -59,7 +60,9 @@ public interface IRewriteRule
5960
/// public override ITreeVisitor&lt;ExecutionContext&gt; GetVisitor()
6061
/// {
6162
/// var x = Capture.Expression("x");
62-
/// return RewriteRule.Rewrite($"Console.Write({x})", $"Console.WriteLine({x})")
63+
/// return RewriteRule.Rewrite(
64+
/// CSharpPattern.Expression($"Console.Write({x})"),
65+
/// CSharpTemplate.Expression($"Console.WriteLine({x})"))
6366
/// .ToVisitor();
6467
/// }
6568
/// </code>
@@ -72,25 +75,15 @@ public interface IRewriteRule
7275
/// </summary>
7376
public static class RewriteRule
7477
{
75-
/// <summary>
76-
/// Create a rewrite rule from interpolated strings. Uses <see cref="ScaffoldKind.Expression"/>
77-
/// scaffolding — suitable for matching within expression-level visitor methods.
78-
/// </summary>
79-
/// <example>
80-
/// <code>
81-
/// using static OpenRewrite.CSharp.Template.RewriteRule;
82-
///
83-
/// var expr = Capture.Expression("expr");
84-
/// var rule = Rewrite($"Console.Write({expr})", $"Console.WriteLine({expr})");
85-
/// return rule.TryOn(cursor, node) ?? node;
86-
/// </code>
87-
/// </example>
88-
public static IRewriteRule Rewrite(TemplateStringHandler before, TemplateStringHandler after) =>
89-
Rewrite(CSharpPattern.Expression(before), CSharpTemplate.Expression(after));
90-
9178
/// <summary>
9279
/// Create a rewrite rule from a <see cref="CSharpPattern"/> and <see cref="CSharpTemplate"/>.
93-
/// Use this when you need explicit scaffold control (e.g., <see cref="CSharpPattern.Statement"/>).
80+
/// <para>
81+
/// When using typed captures with <c>typeParameters</c>, the pattern's <c>usings</c> must
82+
/// include the namespaces needed for the capture type to resolve (e.g.,
83+
/// <c>"System.Collections.Generic"</c> for <c>IDictionary&lt;TKey, TValue&gt;</c>).
84+
/// Without proper usings, the scaffold has no semantic model and type constraints
85+
/// fall back to string matching, which cannot handle generic types.
86+
/// </para>
9487
/// </summary>
9588
public static IRewriteRule Rewrite(CSharpPattern before, CSharpTemplate after) =>
9689
new RewriteRuleImpl(before, after);

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

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -100,18 +100,13 @@ private static J ParseInternal(string code, ScaffoldPreamble preamble,
100100
var scaffold = BuildScaffold(code, preamble, usings, context, scaffoldKind);
101101
var parser = new CSharpParser();
102102

103-
CompilationUnit cu;
104-
if (dependencies.Count > 0 || usings.Count > 0)
105-
{
106-
// When usings or dependencies are provided, create a semantic model
107-
// so the scaffold gets type attribution (needed for semantic matching).
108-
var semanticModel = DependencyWorkspace.CreateSemanticModel(scaffold, dependencies);
109-
cu = parser.Parse(scaffold, "__template__.cs", semanticModel);
110-
}
111-
else
112-
{
113-
cu = parser.Parse(scaffold, "__template__.cs");
114-
}
103+
// Always create a semantic model so the scaffold gets type attribution.
104+
// DependencyWorkspace includes .NET 6+ implicit usings (System,
105+
// System.Collections.Generic, etc.) via a synthetic global-using source file,
106+
// so common types resolve even without explicit usings in the pattern.
107+
// Reference resolution is cached, so repeated calls are cheap.
108+
var semanticModel = DependencyWorkspace.CreateSemanticModel(scaffold, dependencies);
109+
var cu = parser.Parse(scaffold, "__template__.cs", semanticModel);
115110

116111
var result = ExtractTemplateNode(cu, code, scaffoldKind);
117112

rewrite-csharp/csharp/OpenRewrite/Java/TypeUtils.cs renamed to rewrite-csharp/csharp/OpenRewrite/CSharp/TypeUtils.cs

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16-
namespace OpenRewrite.Java;
16+
using OpenRewrite.Java;
17+
18+
namespace OpenRewrite.CSharp;
1719

1820
/// <summary>
1921
/// Utility methods for working with <see cref="JavaType"/> instances.
@@ -46,6 +48,10 @@ public static bool IsAssignableTo(JavaType? type, string fullyQualifiedName)
4648
return primFqn != null && string.Equals(primFqn, fullyQualifiedName, StringComparison.Ordinal);
4749
}
4850

51+
// Arrays implement a known set of non-generic interfaces
52+
if (type is JavaType.Array)
53+
return IsArrayAssignableTo(fullyQualifiedName);
54+
4955
var cls = AsClass(type);
5056
if (cls == null) return false;
5157

@@ -354,6 +360,12 @@ private static void CollectParameterizedMatches(JavaType? type, string targetFqn
354360
}
355361
}
356362
}
363+
else if (type is JavaType.Array arr)
364+
{
365+
// T[] implements IEnumerable<T>, IList<T>, etc. — synthesize matches
366+
if (arr.ElemType != null)
367+
CollectArrayParameterizedMatches(arr.ElemType, targetFqn, results);
368+
}
357369
else if (type is JavaType.Class cls)
358370
{
359371
if (!seen.Add(cls.FullyQualifiedName)) return;
@@ -501,6 +513,62 @@ private static bool TypeParametersMatch(IList<JavaType>? candidateParams, IList<
501513
_ => null
502514
};
503515

516+
// ===============================================================
517+
// Array type support
518+
// ===============================================================
519+
520+
/// <summary>
521+
/// Non-generic supertypes of single-dimensional .NET arrays.
522+
/// </summary>
523+
private static readonly HashSet<string> ArrayNonGenericSupertypes =
524+
[
525+
"System.Array",
526+
"System.Object",
527+
"System.ICloneable",
528+
"System.Collections.IList",
529+
"System.Collections.ICollection",
530+
"System.Collections.IEnumerable",
531+
"System.Collections.IStructuralComparable",
532+
"System.Collections.IStructuralEquatable"
533+
];
534+
535+
/// <summary>
536+
/// FQNs of generic interfaces that single-dimensional .NET arrays implement,
537+
/// parameterized by the element type.
538+
/// </summary>
539+
private static readonly string[] ArrayGenericInterfaceFqns =
540+
[
541+
"System.Collections.Generic.IEnumerable",
542+
"System.Collections.Generic.ICollection",
543+
"System.Collections.Generic.IList",
544+
"System.Collections.Generic.IReadOnlyCollection",
545+
"System.Collections.Generic.IReadOnlyList"
546+
];
547+
548+
/// <summary>
549+
/// Check if a <see cref="JavaType.Array"/> is assignable to a non-generic target FQN.
550+
/// </summary>
551+
private static bool IsArrayAssignableTo(string fqn) => ArrayNonGenericSupertypes.Contains(fqn);
552+
553+
/// <summary>
554+
/// Synthesize the generic interfaces that <c>T[]</c> implements as
555+
/// <see cref="JavaType.Parameterized"/> instances with the element type substituted.
556+
/// Used by <see cref="CollectParameterizedMatches"/> to handle array assignability.
557+
/// </summary>
558+
private static void CollectArrayParameterizedMatches(JavaType elemType, string targetFqn,
559+
List<JavaType.Parameterized> results)
560+
{
561+
foreach (var ifaceFqn in ArrayGenericInterfaceFqns)
562+
{
563+
if (string.Equals(ifaceFqn, targetFqn, StringComparison.Ordinal))
564+
{
565+
// Synthesize Parameterized(IFoo<elemType>)
566+
var rawClass = new JavaType.Class { FullyQualifiedName = ifaceFqn };
567+
results.Add(new JavaType.Parameterized(rawClass, [elemType]));
568+
}
569+
}
570+
}
571+
504572
private static JavaType? TryGetTypeDynamic(Expression expr)
505573
{
506574
try

rewrite-csharp/csharp/OpenRewrite/Tests/CSharp/CSharpTypeMappingTests.cs

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,18 @@ public class CSharpTypeMappingTests : RewriteTest
3030
/// <summary>
3131
/// Parse source code with reference assemblies and return the CompilationUnit.
3232
/// </summary>
33+
private static readonly SyntaxTree ImplicitUsingsSyntaxTree = CSharpSyntaxTree.ParseText(
34+
"""
35+
global using System;
36+
global using System.Collections.Generic;
37+
global using System.IO;
38+
global using System.Linq;
39+
global using System.Net.Http;
40+
global using System.Threading;
41+
global using System.Threading.Tasks;
42+
""",
43+
path: "__GlobalUsings__.g.cs");
44+
3345
private static CompilationUnit ParseWithSemanticModel(string code)
3446
{
3547
var refs = Assemblies.Net90.ResolveAsync(LanguageNames.CSharp, CancellationToken.None)
@@ -38,7 +50,7 @@ private static CompilationUnit ParseWithSemanticModel(string code)
3850
var compilation = CSharpCompilation.Create("TestCompilation")
3951
.WithOptions(new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary))
4052
.AddReferences(refs)
41-
.AddSyntaxTrees(syntaxTree);
53+
.AddSyntaxTrees(ImplicitUsingsSyntaxTree, syntaxTree);
4254
var semanticModel = compilation.GetSemanticModel(syntaxTree);
4355

4456
var parser = new CSharpParser();
@@ -194,6 +206,32 @@ void M()
194206
}
195207
}
196208

209+
[Fact]
210+
public void ImplicitUsings_ShortNameResolvesWithoutExplicitUsing()
211+
{
212+
// In .NET 6+, System.Collections.Generic is an implicit using.
213+
// Test whether Dictionary (short name, no using directive) gets
214+
// type attribution. This will fail if implicit usings are not
215+
// configured in the compilation.
216+
var cu = ParseWithSemanticModel("""
217+
class Test
218+
{
219+
void M()
220+
{
221+
Dictionary<string, int> dict = new Dictionary<string, int>();
222+
}
223+
}
224+
""");
225+
226+
var varDecl = FindVariableDeclaration(cu, "dict");
227+
Assert.NotNull(varDecl);
228+
var declType = varDecl!.TypeExpression?.Type;
229+
// If implicit usings work, this should be Parameterized.
230+
// If not, it will be null or Unknown (unresolved).
231+
Assert.NotNull(declType);
232+
Assert.IsType<JavaType.Parameterized>(declType);
233+
}
234+
197235
[Fact]
198236
public void DictionaryClass_HasFormalTypeParameters()
199237
{
@@ -227,4 +265,29 @@ void M()
227265
var formal1 = Assert.IsType<JavaType.GenericTypeVariable>(dictClass.TypeParameters[1]);
228266
Assert.Equal("TValue", formal1.Name);
229267
}
268+
269+
[Fact]
270+
public void StringTypeArgument_IsMappedAsClass()
271+
{
272+
// System.String should be mapped as JavaType.Class (not Primitive) when used
273+
// as a type argument, so TypeUtils.IsAssignableTo can walk its interface chain.
274+
var cu = ParseWithSemanticModel("""
275+
using System.Collections.Generic;
276+
class Test
277+
{
278+
void M()
279+
{
280+
List<string> items = new List<string>();
281+
}
282+
}
283+
""");
284+
285+
var varDecl = FindVariableDeclaration(cu, "items");
286+
Assert.NotNull(varDecl);
287+
288+
var paramType = Assert.IsType<JavaType.Parameterized>(varDecl!.TypeExpression?.Type);
289+
Assert.NotNull(paramType.TypeParameters);
290+
var stringArg = Assert.IsType<JavaType.Class>(Assert.Single(paramType.TypeParameters!));
291+
Assert.Equal("System.String", stringArg.FullyQualifiedName);
292+
}
230293
}

0 commit comments

Comments
 (0)