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

[BUG] Regression: #2006 breaks Delegate in Tycho #2633

Closed
rgra opened this issue Oct 29, 2020 · 4 comments · Fixed by #2742
Closed

[BUG] Regression: #2006 breaks Delegate in Tycho #2633

rgra opened this issue Oct 29, 2020 · 4 comments · Fixed by #2742

Comments

@rgra
Copy link
Contributor

rgra commented Oct 29, 2020

Describe the bug
Inclusion of the new method PatchDelegate.getChildren(IJavaElement[], SourceTypeElementInfo) from #2006 and according implementation with imports from org.eclipse.jdt.internal.core break compatibility with Tycho (which uses ECJ, didn't test with plain ECJ) because the packages are not available there.
Loading from lombok.eclipse.agent.PatchDelegatePortal.Reflection throws ClassNotFoundExceptions (with -Dlombok.debug.reflection=true).
Thus @DeleGate doesn't work at all with the latest Lombok on Tycho.

To Reproduce
Reproduction would need my changes for Tycho, but should be reproducable with ECJ only as well.

Expected behavior
Delegates should be working on Tycho/ECJ as well.

Version info (please complete the following information):

  • 1.18.16
  • Tycho/ECJ >=1.5.0

Additional context
@Rawi01 I think it should work if you can put the additions to a new class and load it seperately in PatchDelegatePortal.

Wouldn't it be good to show the Exceptions when Delegate is actually used. Swallowing makes sense when it's not used, but in this case it took me a long time to figure out what was wrong because I didn't see the exception without the lombok.debug.reflection switch.

@rgra rgra changed the title [BUG] Regression #2006 breaks Delegate in Tycho [BUG] Regression: #2006 breaks Delegate in Tycho Oct 29, 2020
rgra added a commit to rgra/lombok that referenced this issue Oct 29, 2020
@Rawi01
Copy link
Collaborator

Rawi01 commented Oct 29, 2020

@rgra I think the getChildren method was added in #2551. I can reproduce the problem using a modified version of test/ecj/SampleTest.java. There clearly is a class loading issue but I don't get why it tries to load org/eclipse/jdt/internal/core/JavaElement.

@rgra
Copy link
Contributor Author

rgra commented Oct 30, 2020

@Rawi01 lombok.eclipse.agent.PatchDelegateDelegateSourceMethod.forMethodDeclaration(JavaElement, MethodDeclaration)
is using JavaElement so it get's loaded on class load.

@Rawi01
Copy link
Collaborator

Rawi01 commented Oct 30, 2020

@rgra It clearly tries to use classes that are not there in ECJ and adding a check or a second class as you did in your PR should help here.
My problem was that I expected that it would fail at the actual method invocation as I expected lazy class loading as usual. Some further investigation showed that it does not fail loading PatchDelegate, it fails at the getMethod call. Lesson learned: Getting the methods of a class using reflection triggers class loading of all classes used as return type or method parameter.

@rzwitserloot
Copy link
Collaborator

@rgra Just checking, because I never quite got the tycho build to work - does the current lombok release (v1.18.20 from April 2nd, 2021) address this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants