Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite sources added through build-helper-maven-plugin #755

Closed
Rapster opened this issue Mar 14, 2024 · 8 comments · Fixed by #888
Closed

Rewrite sources added through build-helper-maven-plugin #755

Rapster opened this issue Mar 14, 2024 · 8 comments · Fixed by #888
Assignees
Labels
enhancement New feature or request

Comments

@Rapster
Copy link

Rapster commented Mar 14, 2024

Based on https://stackoverflow.com/q/77927116/4605161

What problem are you trying to solve?

In our codebase, we had to copy an entire class by hand just because some methods are private. So instead of being able to overwrite a few methods, we end up maintining a class of thousands LOC.

Describe the solution you'd like

After copying the entire source code with build-helper-plugin, I'd love to apply a few transformations with recipes (essentially private to protected). Execute the recipe maven plugin in process-sources phase to execute a recipe

Have you considered any alternatives or workarounds?

Spoon

Additional context

Are you interested in contributing this feature to OpenRewrite?

@Rapster Rapster added the enhancement New feature or request label Mar 14, 2024
@timtebeek
Copy link
Contributor

Thanks for logging an issue @Rapster ! We'd up to now been tracking this internally, but good to have a public facing counterpart as well for the wider community to see, track progress and potentially help out.

I haven't looked at the specifics yet, but it's likely to involve changes to our MavenMojoProjectParser, likely around here.

sourceFiles = Stream.concat(sourceFiles, processMainSources(mavenProject, javaParserBuilder.clone(), kotlinParserBuilder.clone(), rp, projectProvenance, alreadyParsed, ctx));
sourceFiles = Stream.concat(sourceFiles, processTestSources(mavenProject, javaParserBuilder.clone(), kotlinParserBuilder.clone(), rp, projectProvenance, alreadyParsed, ctx));

You'll notice we have access to both a MavenProject and Xml.Document maven there, which should I think give access to any configured build-helper-maven-plugin in the same project. From there it's likely a case of parse those additional sources with the correct classpath, and we should then be set to make code changes.

Any help appreciated!

@timtebeek timtebeek moved this to Backlog in OpenRewrite Mar 14, 2024
@Rapster
Copy link
Author

Rapster commented Mar 14, 2024

To realize what I want to do here, I need the goal to be executed in process-sources phase, is it feasible? AFAICS, rewrite goals are bound to process-test-sources

@timtebeek
Copy link
Contributor

To realize what I want to do here, I need the goal to be executed in process-sources phase, is it feasible? AFAICS, rewrite goals are bound to process-test-sources

Not sure if that's feasible as a single build step, as indeed we tie to process-test-classes;

@Mojo(name = "run", requiresDependencyResolution = ResolutionScope.TEST, threadSafe = true,
defaultPhase = LifecyclePhase.PROCESS_TEST_CLASSES)
@Execute(phase = LifecyclePhase.PROCESS_TEST_CLASSES)
public class RewriteRunMojo extends AbstractRewriteRunMojo {

Would having to run multiple commands rule out your use case?

@Rapster
Copy link
Author

Rapster commented Mar 15, 2024

I guess it should be fine, depends what those steps would be. But maybe instead of relying on maven, I could do it programmatically (using rewrite API) but can't find much doc there. Any idea?

@timtebeek
Copy link
Contributor

Using just the recipes as an API isn't recommended in most cases, as that might then fail to set up the class path correctly, miss markers, cycles and other complications. For that reason we removed the doc , as some folks went down that path and ran into issues. The historic page is still available in the history though; it's just not been kept up to date with the move to rewrite v8.

Your use case seems limited enough where it could be an option; you might want to look at how the OSS plugins run recipes as an example.

@Rapster
Copy link
Author

Rapster commented Mar 17, 2024

Thank you @timtebeek I'll definitely give it a try

@timtebeek timtebeek changed the title Rewrite third party project using rewrite-maven-plugin Rewrite sources added through build-helper-maven-plugin May 24, 2024
@timtebeek
Copy link
Contributor

This issue came up again both on StackOverflow and in our Slack: https://stackoverflow.com/questions/78493917/openrewrite-is-not-visiting-files-from-src-integration-tests-java-directory?noredirect=1#comment138383161_78493917

In short the recommended approach is not to use separate source sets for integration tests where possible, but to instead use a naming convention where the Maven surefire plugin then runs any unit tests named *Test.java and the Maven failsafe plugin runs integration tests named *IT.java.

Any other uses of build-helper-maven-plugin could still benefit from an addition to the rewrite-maven-plugin that parses those additional source sets.

@timtebeek
Copy link
Contributor

hi @Rapster ; We've just put out a new version of the Maven plugin containing a fix #888

Could you let us know how that works out for you?

Notably we for now only handle additional test source roots; do let us know if that's enough for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants