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

ExplicitPluginGroupId wrongly matches on maven enforcer rule configuration #4875

Closed
Bananeweizen opened this issue Jan 9, 2025 · 1 comment
Labels
bug Something isn't working

Comments

@Bananeweizen
Copy link
Contributor

What version of OpenRewrite are you using?

maven plugin 5.47.3

What is the smallest, simplest way to reproduce the problem?

Have this maven enforcer configuration in a pom.xml.

<plugin>
	<groupId>org.apache.maven.plugins</groupId>
	<artifactId>maven-enforcer-plugin</artifactId>
	<configuration>
		<rules>
			<requireSameVersions>
				<plugins>
					<plugin>org.eclipse.tycho:*</plugin>
				</plugins>
			</requireSameVersions>
		</rules>
	</configuration>
</plugin>

Run org.openrewrite.maven.cleanup.ExplicitPluginGroupId.

What did you expect to see?

No change.

What did you see instead?

The content of the most inner nested plugin tag has been changed to this:

<plugin>
	<groupId>org.apache.maven.plugins</groupId>org.eclipse.tycho:*
</plugin>

The recipe wrongly assumes the plugin tag to be one of those tags that contain groupid, artifactid, etc., but that's not the case here. It might be useful to require a potential plugin tag to contain a nested artifactId tag, that field should be mandatory everywhere, right?

Are you interested in contributing a fix to OpenRewrite?

@Bananeweizen Bananeweizen added the bug Something isn't working label Jan 9, 2025
@timtebeek timtebeek moved this to Backlog in OpenRewrite Jan 9, 2025
@timtebeek
Copy link
Contributor

Thanks for logging an issue @Bananeweizen ! We can either fix this in the recipe itself, or in the isPluginTag() it uses.

if (isPluginTag() && !t.getChild("groupId").isPresent()) {

I'm leaning towards adding a && t.getChild("artifactId").isPresent() in the recipe, as that would have the least performance impact, and is unlikely to negatively affect other use cases.

@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenRewrite Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

2 participants