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

Warn extension authors if build step and recorder are in the same package #34006

Merged
merged 1 commit into from
Jun 20, 2023

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Jun 13, 2023

This is meant to guard against cases where a call to a recorder method cannot be proxied (due to packaging rules) leading to a real call to the recorder method being performed in the build step method.
For public recorder methods, the method call is always proxyable, thus by warning about same package use, we nudge extension authors towards using public methods in their recorder classes.

Note that we cannot simply reject package private
methods in recorder since these methods could very well be used in code that is not called at build time.

Relates to: #33957

…kage

This is meant to guard against cases where a call to a
recorder method cannot be proxied (due to packaging rules)
leading to a real call to the recorder method being performed
in the build step method.
For public recorder methods, the method call is always proxyable, thus by warning about same package use,
we nudge extension authors towards using public methods
in their recorder classes.

Note that we cannot simply reject package private
methods in recorder since these methods could very well
be used in code that is not called at build time.

Relates to: quarkusio#33957
@quarkus-bot
Copy link

quarkus-bot bot commented Jun 13, 2023

Failing Jobs - Building bfc0904

Status Name Step Failures Logs Raw logs
✔️ Maven Tests - JDK 11
Maven Tests - JDK 11 Windows Build ⚠️ Check → Logs Raw logs
Native Tests - Data2 Build ⚠️ Check → Logs Raw logs

@geoand
Copy link
Contributor Author

geoand commented Jun 15, 2023

@gsmet I really think we should include this as it can save extension hours of wasted time (and us from debugging such issues)

@geoand geoand requested a review from gastaldi June 20, 2023 11:26
@@ -390,6 +396,10 @@ private void validateRecordBuildSteps(TypeElement clazz) {
}
}

private Name getPackageName(TypeElement clazz) {
return processingEnv.getElementUtils().getPackageOf(clazz).getQualifiedName();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is returned if a class is in the default package?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It returns PackageElement that represents the default package.

@geoand geoand merged commit e6f35ed into quarkusio:main Jun 20, 2023
@quarkus-bot quarkus-bot bot added this to the 3.2 - main milestone Jun 20, 2023
@geoand geoand deleted the #33957-2 branch June 20, 2023 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants