Skip to content

Remove past-due @ToBeRemoved code and enable RemoveToBeRemoved in rewrite.yml - #8598

Open
timtebeek wants to merge 2 commits into
mainfrom
tim/enable-remove-tobe-removed
Open

Remove past-due @ToBeRemoved code and enable RemoveToBeRemoved in rewrite.yml#8598
timtebeek wants to merge 2 commits into
mainfrom
tim/enable-remove-tobe-removed

Conversation

@timtebeek

Copy link
Copy Markdown
Member

Ran org.openrewrite.java.recipes.RemoveToBeRemoved and enabled it in rewrite.yml; the first commit is the recipe's raw output, the second fixes what it leaves behind (it deletes declarations but explicitly not their references).

Most past-due annotations sat on private shims for older parent runtimes (getStyle, addStyleMarker, evaluate, shouldKnowInflowStyle), so their callers now go to org.openrewrite.style.StyleHelper directly — with an explicit (Supplier<X>) cast, because StyleHelper.getStyle(Class, List, Supplier) and getStyle(Class, List, T extends SourceFile) are ambiguous to javac for a lambda or method reference. Removing J.VariableDeclarations' deprecated dimensionsBeforeName constructor means the Java 8/11 parser visitors, GroovyParserVisitor and one test drop that always-empty argument, and removing JavaParser.Builder.classpath(byte[]...) takes JavaParserTest#parseFromByteArray, its .class fixtures and the now-unused classgraph test dependency with it.

Two things left alone and worth a reviewer's call: protected Collection<byte[]> classBytesClasspath now has no in-repo writer, so the ByteArrayCapableJavacFileManager path is reachable only by external subclasses — ripping the plumbing out would change protected API across five parser modules; and the six // TO-BE-REMOVED(date) comments (five past due) are out of the recipe's reach, since its precondition is UsesType(ToBeRemoved) and none of those files import the annotation.

Verified with compileJava compileTestJava across every module, the rewrite-java/-test/-groovy/-kotlin/-yaml/-hcl test tasks, and compatibilityTest (TCK) on all five parser modules; the only remaining @ToBeRemoved is J.MethodDeclaration's, dated 2026-09-17, so the newly enabled recipe is a no-op until then.

Raw output of org.openrewrite.java.recipes.RemoveToBeRemoved; follow-up
commits adjust the call sites the recipe leaves behind.
RemoveToBeRemoved deletes declarations but not their references, so the
recipe output on its own does not compile:

- The `getStyle`/`addStyleMarker`/`evaluate`/`shouldKnowInflowStyle`
  helpers existed only to shim older parent runtimes. Their callers now
  go to `StyleHelper` directly, or to the style call the shim guarded.
  `StyleHelper.getStyle(Class, List, Supplier)` and
  `getStyle(Class, List, SourceFile)` are ambiguous for a lambda or
  method reference, so those arguments carry an explicit `Supplier` cast.
- `J.VariableDeclarations`' deprecated `dimensionsBeforeName` constructor
  is gone, so the Java 8/11 parser visitors, the Groovy parser visitor
  and one test drop that always-empty argument.
- `JavaParser.Builder.classpath(byte[]...)` is gone, taking
  `JavaParserTest#parseFromByteArray` and its class fixtures with it.
@timtebeek

timtebeek commented Aug 21, 2026

Copy link
Copy Markdown
Member Author

Serialized-LST compatibility review

@ToBeRemoved's own javadoc says removals "should, where applicable, also account for LST models that were serialized in the past to not cause deserialization errors", so I went back over this diff for that specifically. Only two of the removals touch LST types at all; the rest are format-visitor shims and a parser-builder method.

Hcl.TemplateInterpolation's @JsonCreator constructor

J.VariableDeclarations' dimensionsBeforeName members

I believe these are safe to remove, but flagging the reasoning so someone closer to the serializer can check me.

getDimensionsBeforeName() returned emptyList(), withDimensionsBeforeName(...) returned this, and the 9-arg constructor ignored the argument. Nothing readable is lost, an old payload's dimensionsBeforeName is both ignorable and always empty, and the explicit @JsonCreator sits on the 8-arg @RequiredArgsConstructor, so creator selection doesn't shift. There are no references to the name anywhere else in the repo, including the RPC codecs.

Not LST-related, but still a break

J.VariableDeclarations' 9-arg constructor, getDimensionsBeforeName/withDimensionsBeforeName, and JavaParser.Builder.classpath(byte[]...) are all public API, so external callers compiled against them get a NoSuchMethodError. That's the intended effect of a past-due @ToBeRemoved rather than a surprise, but worth naming explicitly since it's a different kind of risk from the deserialization one above.

@timtebeek
timtebeek force-pushed the tim/enable-remove-tobe-removed branch from 0c0727b to c232dac Compare August 21, 2026 22:38
@timtebeek
timtebeek marked this pull request as ready for review August 21, 2026 22:41
@timtebeek timtebeek changed the title Remove past-due @ToBeRemoved code and enable RemoveToBeRemoved in rewrite.yml Remove past-due @ToBeRemoved code and enable RemoveToBeRemoved in rewrite.yml Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

1 participant