Add additional jar names to `classpathFromResources` for types referenced from types used by timtebeek · Pull Request #160 · openrewrite/rewrite-templating

timtebeek · 2025-08-15T12:00:28Z

Right now we add the transitive jar names only when referred to from types we're using.

I've wondered whether we should unconditionally add say "opentest4j" whenever any method from JUnit is used, but figured that might quickly result in a lot of classpathFromResources entries that would then need to be added, so I've opted to for now only add those when referred by the types we use in the templates, as can be seen for the difference between the tests we now have for ClasspathJarNameDetector.

diff --git c/src/main/java/org/openrewrite/java/template/internal/TemplateCode.java i/src/main/java/org/openrewrite/java/template/internal/TemplateCode.java index a9a22d2..58f1dea 100644 --- c/src/main/java/org/openrewrite/java/template/internal/TemplateCode.java +++ i/src/main/java/org/openrewrite/java/template/internal/TemplateCode.java @@ -68,12 +68,9 @@ public class TemplateCode { jarNames.addAll(ClasspathJarNameDetector.classpathFor(parameter, ImportDetector.imports(parameter))); } if (!jarNames.isEmpty()) { - // It might be preferable to enumerate exactly the needed dependencies rather than the full classpath - // But this is expedient - // See #86 - // String classpath = jarNames.stream().map(jarName -> '"' + jarName + '"').sorted().collect(joining(", ")); - // builder.append("\n .javaParser(JavaParser.fromJavaVersion().classpath(").append(classpath).append("))"); - builder.append("\n .javaParser(JavaParser.fromJavaVersion().classpath(JavaParser.runtimeClasspath()))"); + builder.append("\n .javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "); + builder.append(jarNames.stream().map(jarName -> '"' + jarName + '"').collect(joining(", "))); + builder.append("))\n "); } return builder.toString(); } catch (IOException e) {

@OverRide

diff --git c/src/main/java/org/openrewrite/java/template/internal/ClasspathJarNameDetector.java i/src/main/java/org/openrewrite/java/template/internal/ClasspathJarNameDetector.java index ab59a8a..8a4b126 100644 --- c/src/main/java/org/openrewrite/java/template/internal/ClasspathJarNameDetector.java +++ i/src/main/java/org/openrewrite/java/template/internal/ClasspathJarNameDetector.java @@ -19,6 +19,7 @@ import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.tree.JCTree; import com.sun.tools.javac.tree.JCTree.JCFieldAccess; import com.sun.tools.javac.tree.TreeScanner; +import org.jspecify.annotations.Nullable; import javax.tools.JavaFileObject; import java.util.LinkedHashSet; @@ -38,7 +39,7 @@ public class ClasspathJarNameDetector { public static Set<String> classpathFor(JCTree input, List<Symbol> imports) { Set<String> jarNames = new LinkedHashSet<String>() { @OverRide - public boolean add(String s) { + public boolean add(@nullable String s) { return s != null && super.add(s); } }; @@ -64,7 +65,7 @@ public class ClasspathJarNameDetector { } - private static String jarNameFor(Symbol anImport) { + private static @nullable String jarNameFor(Symbol anImport) { Symbol.ClassSymbol enclClass = anImport instanceof Symbol.ClassSymbol ? (Symbol.ClassSymbol) anImport : anImport.enclClass(); while (enclClass.enclClass() != null && enclClass.enclClass() != enclClass) { enclClass = enclClass.enclClass(); diff --git c/src/main/java/org/openrewrite/java/template/internal/TemplateCode.java i/src/main/java/org/openrewrite/java/template/internal/TemplateCode.java index 58f1dea..337a65a 100644 --- c/src/main/java/org/openrewrite/java/template/internal/TemplateCode.java +++ i/src/main/java/org/openrewrite/java/template/internal/TemplateCode.java @@ -68,9 +68,11 @@ public class TemplateCode { jarNames.addAll(ClasspathJarNameDetector.classpathFor(parameter, ImportDetector.imports(parameter))); } if (!jarNames.isEmpty()) { - builder.append("\n .javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, "); - builder.append(jarNames.stream().map(jarName -> '"' + jarName + '"').collect(joining(", "))); - builder.append("))\n "); + String joinedJarNames = jarNames.stream().collect(joining(", ", "\"", "\"")); + builder.append("\n .javaParser(JavaParser.fromJavaVersion().classpathFromResources(ctx, ") + .append(joinedJarNames) + .append("))\n "); + } return builder.toString(); } catch (IOException e) {

…untimeclasspath-in-remaster-templates

src/test/resources/refaster/ClasspathFromResourcesTransitiveRecipe.java

knutwannheden · 2025-08-16T16:15:37Z

I think what is required is to include the jars of all types directly referenced as either supertypes or in method signatures of the types referenced by the template. I don't think the full transitive closure is required.

src/test/java/org/openrewrite/java/template/internal/ClasspathJarNameDetectorTest.java

src/main/java/org/openrewrite/java/template/internal/ClasspathJarNameDetector.java

github-actions

Some suggestions could not be made:

src/main/java/org/openrewrite/java/template/internal/ClasspathJarNameDetector.java
- lines 26-26

timtebeek · 2025-08-18T12:42:10Z

src/main/java/org/openrewrite/java/template/internal/ClasspathJarNameDetector.java

+                        // Retain major version number, to avoid `log4j` conflict between `log4j-1` and `log4j2-1`
+                        .replaceAll("(-\\d+).*?$", "$1");


it's subtle, but since we match on prefixes on the other side we need a way to tell these apart. I've deliberately not pinned to more specific versions, as we use latest.release often, and I don't see overlap yet beyond major versions even for rewrite-spring.

Alternatively we could on the other end match based on "jarname" + '-' to tell these apart without pinning to major versions.

I think this is good enough! There's always the option to handle this differently in another PR 😉.

Indeed looks good as a first step. Possibly we can restrict it a bit more (untested), but I am also fine with leaving it as-is:

Suggested change

// Retain major version number, to avoid `log4j` conflict between `log4j-1` and `log4j2-1`

.replaceAll("(-\\d+).*?$", "$1");

// Retain major version number, to avoid `log4j` conflict between `log4j-1` and `log4j2-1`

.replaceFirst("(-\\d+)", "$1");

Thanks! With that original suggestion the tests fail with the following:

value of : classpathForSource(...) missing (2) : junit-jupiter-api-5, opentest4j-1 unexpected (2): junit-jupiter-api-5.13.3, opentest4j-1.3.0

Note that my intention was to only retain the major version, if present, and discard the rest, hence why I've included the non greedy match with end of string: .*?$. I did apply your suggestion to use replaceFirst, even if the inclusion of $ would result in the same.

knutwannheden · 2025-08-19T08:15:04Z

src/main/java/org/openrewrite/java/template/internal/ClasspathJarNameDetector.java

+                        // Retain major version number, to avoid `log4j` conflict between `log4j-1` and `log4j2-1`
+                        .replaceAll("(-\\d+).*?$", "$1");


Indeed looks good as a first step. Possibly we can restrict it a bit more (untested), but I am also fine with leaving it as-is:

Suggested change

// Retain major version number, to avoid `log4j` conflict between `log4j-1` and `log4j2-1`

.replaceAll("(-\\d+).*?$", "$1");

// Retain major version number, to avoid `log4j` conflict between `log4j-1` and `log4j2-1`

.replaceFirst("(-\\d+)", "$1");

JohannisK and others added 12 commits August 14, 2025 23:02

Update expectations

fc56e32

Do not use classpathFromResources for TemplateProcessor just yet

b44be87

Merge branch 'main' into 1136-use-classpathfromresources-instead-of-r…

026d86d

…untimeclasspath-in-remaster-templates

Update expectations for NoGuavaRefasterRecipes.java

979e22c

Add an option to generate classpathFromResources

06e4ab1

Adapt Semantics, PatternBuilder and TemplateProcessor

9d7ca4d

Apply formatter

013c36d

Determine classpathFromResources from first argument

979aedc

Correctly concatenate more than one jar name

ff581ba

Add transitive jars to classpathFromResources too

77f2aa6

timtebeek self-assigned this Aug 15, 2025

moderne-meeseeks bot added this to OpenRewrite Aug 15, 2025

github-project-automation bot moved this to In Progress in OpenRewrite Aug 15, 2025

timtebeek commented Aug 15, 2025

View reviewed changes

src/test/resources/refaster/ClasspathFromResourcesTransitiveRecipe.java Outdated Show resolved Hide resolved

Run CI for pull request targeted at any branch

867a659

Base automatically changed from 1136-use-classpathfromresources-instead-of-runtimeclasspath-in-remaster-templates to main August 15, 2025 13:47

Merge branch 'main' into transitive-jarnames

436f996

timtebeek added bug Something isn't working enhancement New feature or request labels Aug 15, 2025

timtebeek added 2 commits August 15, 2025 15:59

Update ci.yml

f561654

Merge branch 'main' into transitive-jarnames

73e8f11

This comment was marked as outdated.

Sign in to view

Expand to detect more types

cbc503f

This comment was marked as outdated.

Sign in to view

timtebeek added 2 commits August 16, 2025 12:17

Annotated types come with a classpath dependency

854d798

Annotated types come with a classpath dependency

c2526b4

This comment was marked as outdated.

Sign in to view

Merge branch 'main' into transitive-jarnames

8d76c92

timtebeek added 2 commits August 17, 2025 14:54

Add exports for ClasspathJarNameDetectorTest compilation

b784da7

Slight polish

1acb5c4

github-actions bot reviewed Aug 17, 2025

View reviewed changes

src/test/java/org/openrewrite/java/template/internal/ClasspathJarNameDetectorTest.java Outdated Show resolved Hide resolved

src/test/java/org/openrewrite/java/template/internal/ClasspathJarNameDetectorTest.java Outdated Show resolved Hide resolved

timtebeek added 6 commits August 17, 2025 15:06

Add a test showing we need transitive dependencies too

fc7b9ba

Also look at super class and interfaces

a241c1b

Fix execution in IntelliJ

d938e9d

Add another test for extraction based on first statement

693fb9c

Show extraction works even without imports

e013a4a

Start anew

b5c2e7c

github-actions bot reviewed Aug 17, 2025

View reviewed changes

timtebeek added 3 commits August 17, 2025 22:38

Apply formatter

bc02ba4

Use a private static class to minimize nullable warnings

3da3a53

Move where ImportsDetector is invoked

84fe2ec

github-actions bot reviewed Aug 17, 2025

View reviewed changes

timtebeek added 7 commits August 17, 2025 23:05

Remove the need for ImportDetector in ClasspathJarNameDetector

e5ce73a

Organize imports

e702d41

Collapse loops and visit in logical order

50dd501

Have ClasspathJarNameDetector directly extend TreeScanner

a5c394b

Inline the static jarNameFor

3071974

Add major version to classpath entries to avoid conflicts

34b646f

Also suffix JSpecify versions

6bfcdb8

jevanlingen mentioned this pull request Aug 18, 2025

Use classpath from resource for Refaster openrewrite/rewrite-logging-frameworks#248

Merged

3 tasks

timtebeek commented Aug 18, 2025

View reviewed changes

knutwannheden approved these changes Aug 19, 2025

View reviewed changes

Use replaceFirst as suggested

d670df4

timtebeek merged commit 28c1ded into main Aug 19, 2025
2 checks passed

timtebeek deleted the transitive-jarnames branch August 19, 2025 09:53

github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Aug 19, 2025

timtebeek mentioned this pull request Oct 13, 2025

Generate OpenRewrite recipes with JavaParser.Builder#classpathFromResources PicnicSupermarket/error-prone-support#1918

Merged

		// Retain major version number, to avoid `log4j` conflict between `log4j-1` and `log4j2-1`
		.replaceAll("(-\\d+).*?$", "$1");

Conversation

timtebeek commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

knutwannheden commented Aug 16, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

timtebeek Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

jevanlingen Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

knutwannheden Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

timtebeek Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

knutwannheden Aug 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

timtebeek commented Aug 15, 2025 •

edited

Loading