Skip to content

csharp: implement TreeVisitor.Adapt() on the .NET side#7591

Merged
jkschneider merged 1 commit intomainfrom
worktree-adaptation
May 7, 2026
Merged

csharp: implement TreeVisitor.Adapt() on the .NET side#7591
jkschneider merged 1 commit intomainfrom
worktree-adaptation

Conversation

@jkschneider
Copy link
Copy Markdown
Member

@jkschneider jkschneider commented May 7, 2026

Summary

  1. Silent no-op: A bare TreeVisitor<J, P> (or any subclass that overrides only PreVisit / PostVisit — a common pattern for cross-language collectors) traversing a parsed C# source would silently no-op past the root. The default Accept is identity, so traversal never descended into children. Quiet, but every per-node hook missed every node.

  2. Hard throw: A plain JavaVisitor<P> traversing a Cs source would throw InvalidOperationException("Unknown J tree type: ...") on the first Cs.UsingDirective / Cs.PropertyDeclaration / etc. its switch doesn't recognize.

Unlike the JVM side (which inherits a real TreeVisitor.adapt() from rewrite-core backed by Quarkus Gizmo bytecode generation) and unlike the TypeScript side (which sidesteps the problem with a kind-keyed registry consulted only inside JavaVisitor.accept), the C# port had no adaptation mechanism at all.

Approach

  • TreeVisitorAdapterRegistry — a non-generic, lock-protected registry of (treeType, openLangVisitorType, openAdapterType) triples kept sorted with the most specific tree type first, so a Cs node hits the CSharpVisitor adapter before falling through to the JavaVisitor one.

  • TreeVisitor<T, P>.Adapt(tree) — looks up the matching language adapter, closes the open generic with P, and instantiates via Activator. Result is cached per visitor instance keyed by the closed language-visitor type, so the wrapper allocation happens at most once per language per visitor lifetime.

  • Visit() now dispatches via Adapt(t).Accept(t, p) instead of Accept(t, p). Cursor is made virtual so the adapter can forward it to the wrapped visitor.

  • TreeVisitorAsJavaVisitor<P> / TreeVisitorAsCSharpVisitor<P> — thin adapter classes that IS-A JavaVisitor / CSharpVisitor (so the language switch and child-traversal defaults are inherited), but override Visit to forward to the wrapped visitor (so its lifecycle — _visitCount, _afterVisit, _stopAfterPreVisit, PreVisit, PostVisit, DefaultValue — runs unchanged) and override Cursor to forward to wrapped state. Each language registers its triple via a [ModuleInitializer] that fires on assembly load, so registration is guaranteed before any TreeVisitor<J, P>.Visit() call regardless of which generic instantiations the user code happens to reference.

Test plan

  • New TreeVisitorAdaptTest covers both regression modes:
    • BareTreeVisitorTraversesCSharpSource: a TreeVisitor<J, ExecutionContext> overriding only PreVisit descends into both Cs- and J-typed nodes of a parsed file.
    • JavaVisitorTraversesCSharpSourceWithoutThrowing: a JavaVisitor<ExecutionContext> traverses the same source without throwing on Cs nodes.
  • Full C# suite green: 1917 / 1917 passing locally with RPC_TEST_SERVER_CLASSPATH wired via ./gradlew :rewrite-csharp:rpcTestClasspath (covers JVM↔.NET RPC integration tests too).
  • No changes to public API surface beyond making Cursor virtual and adding Adapt (protected virtual). Existing JavaVisitor / CSharpVisitor / XmlVisitor overrides of Accept continue to compile unchanged.

…isitors can visit Cs / J nodes

Mirrors the Python fix from c89d246 (#7590), but for the parallel .NET
implementation in `rewrite-csharp/csharp/OpenRewrite/`. Two distinct failure
modes existed before this change:

1. A bare `TreeVisitor<J, P>` (or any subclass that overrides only
   `PreVisit` / `PostVisit` — a common pattern for cross-language
   collectors) traversing a parsed C# source would silently no-op past the
   root: the default `Accept` is identity, so traversal never descended
   into children. Quiet, but every per-node hook missed every node.

2. A plain `JavaVisitor<P>` traversing a Cs source would throw
   `InvalidOperationException("Unknown J tree type: ...")` on the first
   `Cs.UsingDirective` / `Cs.PropertyDeclaration` / etc. its switch
   doesn't recognize.

Unlike the JVM side (which inherits a real `TreeVisitor.adapt()` from
`rewrite-core` backed by Quarkus Gizmo bytecode generation), and unlike
the TypeScript side (which sidesteps the problem with a kind-keyed
registry consulted only inside `JavaVisitor.accept`), the C# port had no
adaptation mechanism at all.

This change introduces one:

* `TreeVisitorAdapterRegistry` — a non-generic, lock-protected registry
  of `(treeType, openLangVisitorType, openAdapterType)` triples kept
  sorted with the most specific tree type first, so a Cs node hits the
  `CSharpVisitor` adapter before falling through to the `JavaVisitor`
  one.

* `TreeVisitor<T, P>.Adapt(tree)` — looks up the matching language
  adapter, closes the open generic with `P`, and instantiates via
  `Activator`. Result is cached per visitor instance keyed by the closed
  language-visitor type, so the wrapper allocation happens at most once
  per language per visitor lifetime.

* `Visit()` now dispatches via `Adapt(t).Accept(t, p)` instead of
  `Accept(t, p)`. `Cursor` is made virtual so the adapter can forward it
  to the wrapped visitor.

* `TreeVisitorAsJavaVisitor<P>` and `TreeVisitorAsCSharpVisitor<P>` —
  thin adapter classes that IS-A `JavaVisitor` / `CSharpVisitor` (so the
  language switch and child-traversal defaults are inherited), but
  override `Visit` to forward to the wrapped visitor (so its lifecycle —
  `_visitCount`, `_afterVisit`, `_stopAfterPreVisit`, `PreVisit`,
  `PostVisit`, `DefaultValue` — runs unchanged) and override `Cursor` to
  forward to wrapped state. Each language registers its triple via a
  `[ModuleInitializer]` that fires on assembly load, so registration is
  guaranteed before any `TreeVisitor<J, P>.Visit()` call regardless of
  which generic instantiations the user code happens to reference.

Tests: `TreeVisitorAdaptTest` covers both regression modes — a
`CountingPreVisitor : TreeVisitor<J, ExecutionContext>` overriding only
`PreVisit` must descend into both Cs and J nodes of a parsed file, and a
`MethodCountingJavaVisitor : JavaVisitor<ExecutionContext>` must
traverse the same source without throwing. Full suite stays green
(1917/1917 with `RPC_TEST_SERVER_CLASSPATH` wired via
`./gradlew :rewrite-csharp:rpcTestClasspath`).
@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite May 7, 2026
@jkschneider jkschneider merged commit d3b13ec into main May 7, 2026
1 check passed
@jkschneider jkschneider deleted the worktree-adaptation branch May 7, 2026 02:44
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant