Skip to content

Commit 317e4b9

Browse files
authored
C#: Pass classpath via environment variable instead of -cp argument (#7345)
* C#: Pass classpath via environment variable instead of -cp argument Avoids command-line length limits on Windows by setting the CLASSPATH environment variable instead of passing it as a -cp argument to the Java RPC test server process. * C#: Fix PreconditionsCheckTest flaky failure from RPC static state leak PreconditionsCheckTest expects LocalUsesType (local fallback) but when RPC tests run first, RewriteRpcServer.Current is non-null, causing UsesType to return an RpcVisitor that fails silently. Save/restore the static in the test fixture to ensure isolation. * C#: Remove LocalUsesType fallback, always delegate preconditions to Java via RPC UsesType/UsesMethod now require an RPC connection and delegate to Java's HasType/HasMethod. This eliminates the static state leak that caused flaky test failures — tests using preconditions are inherently RPC-bound and must run sequentially via RpcRewriteTest/[Collection("RPC")]. Also fix rpcTestClasspath to include processResources output directories so Java's ServiceLoader discovers CSharpRpcCodec when the C# test fixture spawns a Java RPC process.
1 parent 955a86b commit 317e4b9

5 files changed

Lines changed: 45 additions & 179 deletions

File tree

rewrite-csharp/build.gradle.kts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,14 +98,18 @@ val rpcTestClasspath by tasks.registering {
9898
.withNormalizer(ClasspathNormalizer::class)
9999
inputs.files(tasks.named<JavaCompile>("compileJava").flatMap { it.destinationDirectory })
100100
inputs.files(tasks.named<JavaCompile>("compileTestJava").flatMap { it.destinationDirectory })
101+
inputs.files(tasks.named("processResources"))
102+
inputs.files(tasks.named("processTestResources"))
101103

102104
val classpathFile = layout.buildDirectory.file("rpc-test-server-classpath.txt")
103105
outputs.file(classpathFile)
104106

105107
doLast {
106108
val cp = configurations["testRuntimeClasspath"].resolve().joinToString(File.pathSeparator) +
107109
File.pathSeparator + tasks.named<JavaCompile>("compileJava").get().destinationDirectory.get().asFile +
108-
File.pathSeparator + tasks.named<JavaCompile>("compileTestJava").get().destinationDirectory.get().asFile
110+
File.pathSeparator + tasks.named<JavaCompile>("compileTestJava").get().destinationDirectory.get().asFile +
111+
File.pathSeparator + tasks.named("processResources").get().outputs.files.singleFile +
112+
File.pathSeparator + tasks.named("processTestResources").get().outputs.files.singleFile
109113
classpathFile.get().asFile.writeText(cp)
110114
}
111115
}

rewrite-csharp/csharp/OpenRewrite/Java/Search/LocalUsesType.cs

Lines changed: 0 additions & 69 deletions
This file was deleted.

rewrite-csharp/csharp/OpenRewrite/Java/Search/Preconditions.cs

Lines changed: 30 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -21,55 +21,50 @@
2121
namespace OpenRewrite.Java.Search;
2222

2323
/// <summary>
24-
/// Search-based precondition visitors.
25-
/// When connected to Java via RPC, delegates to Java's implementations.
26-
/// Otherwise falls back to local implementations.
24+
/// Search-based precondition visitors that delegate to Java's implementations via RPC.
25+
/// Requires an active RPC connection to a Java process.
2726
/// </summary>
2827
public static class Preconditions
2928
{
3029
/// <summary>
31-
/// Creates a UsesType precondition. If connected to Java via RPC, delegates to
32-
/// Java's org.openrewrite.java.search.HasType. Otherwise falls back to local implementation.
30+
/// Creates a UsesType precondition that delegates to Java's
31+
/// org.openrewrite.java.search.HasType via RPC.
3332
/// </summary>
33+
/// <exception cref="InvalidOperationException">Thrown when no RPC connection is available.</exception>
3434
public static ITreeVisitor<ExecutionContext> UsesType(string fullyQualifiedTypeName)
3535
{
36-
var rpc = RewriteRpcServer.Current;
37-
if (rpc != null)
38-
{
39-
var response = rpc.PrepareRecipeOnRemote(
40-
"org.openrewrite.java.search.HasType",
41-
new Dictionary<string, object?>
42-
{
43-
["fullyQualifiedTypeName"] = fullyQualifiedTypeName,
44-
["checkAssignability"] = false
45-
}
46-
);
47-
return new RpcVisitor(rpc, response.EditVisitor);
48-
}
36+
var rpc = RewriteRpcServer.Current
37+
?? throw new InvalidOperationException("UsesType requires an RPC connection to Java");
4938

50-
return new LocalUsesType<ExecutionContext>(fullyQualifiedTypeName);
39+
var response = rpc.PrepareRecipeOnRemote(
40+
"org.openrewrite.java.search.HasType",
41+
new Dictionary<string, object?>
42+
{
43+
["fullyQualifiedTypeName"] = fullyQualifiedTypeName,
44+
["checkAssignability"] = false
45+
}
46+
);
47+
return new RpcVisitor(rpc, response.EditVisitor);
5148
}
5249

5350
/// <summary>
54-
/// Creates a UsesMethod precondition. If connected to Java via RPC, delegates to
55-
/// Java's org.openrewrite.java.search.HasMethod.
51+
/// Creates a UsesMethod precondition that delegates to Java's
52+
/// org.openrewrite.java.search.HasMethod via RPC.
5653
/// </summary>
54+
/// <exception cref="InvalidOperationException">Thrown when no RPC connection is available.</exception>
5755
public static ITreeVisitor<ExecutionContext> UsesMethod(string methodPattern)
5856
{
59-
var rpc = RewriteRpcServer.Current;
60-
if (rpc != null)
61-
{
62-
var response = rpc.PrepareRecipeOnRemote(
63-
"org.openrewrite.java.search.HasMethod",
64-
new Dictionary<string, object?>
65-
{
66-
["methodPattern"] = methodPattern,
67-
["matchOverrides"] = false
68-
}
69-
);
70-
return new RpcVisitor(rpc, response.EditVisitor);
71-
}
57+
var rpc = RewriteRpcServer.Current
58+
?? throw new InvalidOperationException("UsesMethod requires an RPC connection to Java");
7259

73-
throw new InvalidOperationException("UsesMethod requires an RPC connection to Java");
60+
var response = rpc.PrepareRecipeOnRemote(
61+
"org.openrewrite.java.search.HasMethod",
62+
new Dictionary<string, object?>
63+
{
64+
["methodPattern"] = methodPattern,
65+
["matchOverrides"] = false
66+
}
67+
);
68+
return new RpcVisitor(rpc, response.EditVisitor);
7469
}
7570
}

rewrite-csharp/csharp/OpenRewrite/Test/RpcFixture.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,16 @@ private static ProcessStartInfo CreateJavaProcessStartInfo()
8080
"Run './gradlew :rewrite-csharp:rpcTestClasspath' to generate the classpath file.");
8181

8282
var classpath = File.ReadAllText(cpFile).Trim();
83-
return new ProcessStartInfo("java",
84-
$"-cp \"{classpath}\" org.openrewrite.maven.rpc.JavaRewriteRpc")
83+
var psi = new ProcessStartInfo("java", "org.openrewrite.maven.rpc.JavaRewriteRpc")
8584
{
8685
RedirectStandardInput = true,
8786
RedirectStandardOutput = true,
8887
RedirectStandardError = true,
8988
UseShellExecute = false,
9089
CreateNoWindow = true
9190
};
91+
psi.Environment["CLASSPATH"] = classpath;
92+
return psi;
9293
}
9394

9495
public void Dispose()

rewrite-csharp/csharp/OpenRewrite/Tests/Recipe/PreconditionsCheckTest.cs

Lines changed: 7 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -24,46 +24,12 @@
2424

2525
namespace OpenRewrite.Tests.Recipe;
2626

27-
public class PreconditionsCheckTest : RewriteTest
27+
/// <summary>
28+
/// Tests for Preconditions.Check with RPC-backed precondition visitors.
29+
/// These tests require a Java RPC connection for UsesType/UsesMethod.
30+
/// </summary>
31+
public class PreconditionsCheckTest(RpcFixture fixture) : RpcRewriteTest(fixture)
2832
{
29-
/// <summary>
30-
/// Directly test that LocalUsesType finds the type and Check delegates to visitor.
31-
/// </summary>
32-
[Fact]
33-
public async Task CheckDelegatesToVisitorWhenPreconditionMatches()
34-
{
35-
var parser = new CSharpParser();
36-
var syntaxTree = Microsoft.CodeAnalysis.CSharp.CSharpSyntaxTree.ParseText(
37-
"using System; class T { void M() { Console.WriteLine(\"hi\"); } }", path: "source.cs");
38-
var refs = await Assemblies.Net90
39-
.ResolveAsync(Microsoft.CodeAnalysis.LanguageNames.CSharp, System.Threading.CancellationToken.None);
40-
var compilation = Microsoft.CodeAnalysis.CSharp.CSharpCompilation.Create("Test")
41-
.WithOptions(new Microsoft.CodeAnalysis.CSharp.CSharpCompilationOptions(Microsoft.CodeAnalysis.OutputKind.DynamicallyLinkedLibrary))
42-
.AddReferences(refs)
43-
.AddSyntaxTrees(syntaxTree);
44-
var semanticModel = compilation.GetSemanticModel(syntaxTree);
45-
var source = parser.Parse(
46-
"using System; class T { void M() { Console.WriteLine(\"hi\"); } }",
47-
semanticModel: semanticModel);
48-
49-
// Step 1: Verify LocalUsesType finds System.Console
50-
var precondition = UsesType("System.Console");
51-
var precondResult = precondition.Visit(source, new ExecutionContext());
52-
Assert.NotSame(source, precondResult); // Should be different (marked with SearchResult)
53-
54-
// Step 2: Verify Check delegates to inner visitor
55-
var check = Check(precondition, new NoOpCSharpVisitor());
56-
var checkResult = check.Visit(source, new ExecutionContext());
57-
// If precondition matched, inner visitor ran (even if no-op)
58-
}
59-
60-
private class NoOpCSharpVisitor : CSharpVisitor<ExecutionContext> { }
61-
62-
/// <summary>
63-
/// Verifies that Preconditions.Check works with CSharpVisitor recipes.
64-
/// The precondition (LocalUsesType) must be able to traverse C# trees
65-
/// to find the type, and the inner CSharpVisitor must run when matched.
66-
/// </summary>
6733
[Fact]
6834
public void PreconditionMatchesAndVisitorRuns()
6935
{
@@ -171,40 +137,9 @@ void M()
171137
}
172138

173139
/// <summary>
174-
/// A test recipe that uses Preconditions.Check with a CSharpVisitor.
175-
/// Renames Console.WriteLine to Console.Write in files that use System.Console.
176-
/// </summary>
177-
/// <summary>
178-
/// Test recipe that removes Console.WriteLine by returning null from VisitMethodInvocation.
179-
/// This does NOT work — returning null from VisitMethodInvocation removes the invocation
180-
/// but not the enclosing ExpressionStatement, so the tree appears unchanged.
140+
/// Test recipe that renames Console.WriteLine to Console.Write in files that use System.Console.
141+
/// Uses Preconditions.Check with an RPC-backed UsesType precondition.
181142
/// </summary>
182-
class RemoveConsoleWriteLineRecipe : OpenRewrite.Core.Recipe
183-
{
184-
public override string DisplayName => "Remove Console.WriteLine (broken)";
185-
public override string Description => "Attempts to remove Console.WriteLine by returning null from VisitMethodInvocation.";
186-
187-
public override JavaVisitor<ExecutionContext> GetVisitor() => new Visitor();
188-
189-
private class Visitor : CSharpVisitor<ExecutionContext>
190-
{
191-
public override J VisitMethodInvocation(MethodInvocation mi, ExecutionContext ctx)
192-
{
193-
mi = (MethodInvocation)base.VisitMethodInvocation(mi, ctx);
194-
195-
if (mi.Name.SimpleName == "WriteLine" &&
196-
mi.Select?.Element is Identifier id &&
197-
id.SimpleName == "Console")
198-
{
199-
//noinspection DataFlowIssue
200-
return null!;
201-
}
202-
203-
return mi;
204-
}
205-
}
206-
}
207-
208143
class RenameWriteLineRecipe : OpenRewrite.Core.Recipe
209144
{
210145
public override string DisplayName => "Rename Console.WriteLine to Write";

0 commit comments

Comments
 (0)