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.

Sign up for GitHub

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

Jump to bottom

[WIP] fix: Exception CtTryWithResourceImpl does not have CtRole.TYPE #4705

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[WIP] fix: Exception CtTryWithResourceImpl does not have CtRole.TYPE #4705

wants to merge 1 commit into from

Conversation

danglotb
Copy link
Member

Hello,

I encountered the following exception:

spoon.SpoonException: The element of class class spoon.support.reflect.code.CtTryWithResourceImpl does not have CtRole.TYPE 

	at spoon.reflect.meta.impl.RoleHandlerHelper.getRoleHandler(RoleHandlerHelper.java:54)
	at spoon.support.sniper.internal.ElementSourceFragment.getRoleHandler(ElementSourceFragment.java:270)
	at spoon.support.sniper.internal.ElementSourceFragment.addChild(ElementSourceFragment.java:243)
	at spoon.support.sniper.internal.ElementSourceFragment$1.enter(ElementSourceFragment.java:201)
	at spoon.reflect.visitor.CtScanner.visitCtTypeReference(CtScanner.java:809)
	at spoon.support.reflect.reference.CtTypeReferenceImpl.accept(CtTypeReferenceImpl.java:78)
	at spoon.reflect.visitor.EarlyTerminatingScanner.doScan(EarlyTerminatingScanner.java:145)
	at spoon.reflect.visitor.EarlyTerminatingScanner.scan(EarlyTerminatingScanner.java:121)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:184)
	at spoon.reflect.visitor.EarlyTerminatingScanner.scan(EarlyTerminatingScanner.java:106)
	at spoon.reflect.visitor.CtScanner.visitCtLocalVariable(CtScanner.java:552)
	at spoon.support.sniper.internal.ElementSourceFragment$1.visitCtLocalVariable(ElementSourceFragment.java:182)
	at spoon.support.reflect.code.CtLocalVariableImpl.accept(CtLocalVariableImpl.java:51)
	at spoon.reflect.visitor.EarlyTerminatingScanner.doScan(EarlyTerminatingScanner.java:145)
	at spoon.reflect.visitor.EarlyTerminatingScanner.scan(EarlyTerminatingScanner.java:121)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:184)
	at spoon.reflect.visitor.EarlyTerminatingScanner.scan(EarlyTerminatingScanner.java:106)
	at spoon.reflect.visitor.EarlyTerminatingScanner.scan(EarlyTerminatingScanner.java:83)
	at spoon.reflect.visitor.CtScanner.visitCtTryWithResource(CtScanner.java:770)
	at spoon.support.reflect.code.CtTryWithResourceImpl.accept(CtTryWithResourceImpl.java:31)
	at spoon.reflect.visitor.EarlyTerminatingScanner.doScan(EarlyTerminatingScanner.java:145)
	at spoon.reflect.visitor.EarlyTerminatingScanner.scan(EarlyTerminatingScanner.java:121)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:184)
	at spoon.reflect.visitor.EarlyTerminatingScanner.scan(EarlyTerminatingScanner.java:106)
	at spoon.reflect.visitor.EarlyTerminatingScanner.scan(EarlyTerminatingScanner.java:83)
	at spoon.reflect.visitor.CtScanner.visitCtBlock(CtScanner.java:321)
	at spoon.support.reflect.code.CtBlockImpl.accept(CtBlockImpl.java:58)
	at spoon.reflect.visitor.EarlyTerminatingScanner.doScan(EarlyTerminatingScanner.java:145)
	at spoon.reflect.visitor.EarlyTerminatingScanner.scan(EarlyTerminatingScanner.java:121)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:184)
	at spoon.reflect.visitor.EarlyTerminatingScanner.scan(EarlyTerminatingScanner.java:106)
	at spoon.reflect.visitor.CtScanner.visitCtMethod(CtScanner.java:587)
	at spoon.support.reflect.declaration.CtMethodImpl.accept(CtMethodImpl.java:58)
	at spoon.reflect.visitor.EarlyTerminatingScanner.doScan(EarlyTerminatingScanner.java:145)
	at spoon.reflect.visitor.EarlyTerminatingScanner.scan(EarlyTerminatingScanner.java:121)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:184)
	at spoon.reflect.visitor.EarlyTerminatingScanner.scan(EarlyTerminatingScanner.java:106)
	at spoon.reflect.visitor.EarlyTerminatingScanner.scan(EarlyTerminatingScanner.java:83)
	at spoon.reflect.visitor.CtScanner.visitCtClass(CtScanner.java:357)
	at spoon.support.reflect.declaration.CtClassImpl.accept(CtClassImpl.java:58)
	at spoon.reflect.visitor.EarlyTerminatingScanner.doScan(EarlyTerminatingScanner.java:145)
	at spoon.reflect.visitor.EarlyTerminatingScanner.scan(EarlyTerminatingScanner.java:121)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:184)
	at spoon.reflect.visitor.EarlyTerminatingScanner.scan(EarlyTerminatingScanner.java:106)
	at spoon.reflect.visitor.EarlyTerminatingScanner.scan(EarlyTerminatingScanner.java:83)
	at spoon.reflect.visitor.EarlyTerminatingScanner.visitCtCompilationUnit(EarlyTerminatingScanner.java:160)
	at spoon.support.reflect.declaration.CtCompilationUnitImpl.accept(CtCompilationUnitImpl.java:408)
	at spoon.reflect.visitor.EarlyTerminatingScanner.doScan(EarlyTerminatingScanner.java:145)
	at spoon.reflect.visitor.EarlyTerminatingScanner.scan(EarlyTerminatingScanner.java:121)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:184)
	at spoon.reflect.visitor.EarlyTerminatingScanner.scan(EarlyTerminatingScanner.java:106)
	at spoon.support.sniper.internal.ElementSourceFragment.createSourceFragmentsFrom(ElementSourceFragment.java:229)
	at spoon.support.reflect.declaration.CtCompilationUnitImpl.getOriginalSourceFragment(CtCompilationUnitImpl.java:360)
	at spoon.support.modelobs.SourceFragmentCreator.onChange(SourceFragmentCreator.java:32)
	at spoon.support.modelobs.ChangeCollector$ChangeListener.onListAdd(ChangeCollector.java:190)
	at spoon.support.util.ModelList.add(ModelList.java:170)
	at spoon.support.reflect.code.CtBlockImpl.insertBegin(CtBlockImpl.java:110)
	at spoon.test.prettyprinter.SniperTryCatchWithResourceTest$1.process(SniperTryCatchWithResourceTest.java:34)
	at spoon.test.prettyprinter.SniperTryCatchWithResourceTest$1.process(SniperTryCatchWithResourceTest.java:31)
	at spoon.support.visitor.ProcessingVisitor.scan(ProcessingVisitor.java:72)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:184)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:149)
	at spoon.reflect.visitor.CtScanner.visitCtClass(CtScanner.java:357)
	at spoon.support.reflect.declaration.CtClassImpl.accept(CtClassImpl.java:58)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:194)
	at spoon.support.visitor.ProcessingVisitor.scan(ProcessingVisitor.java:68)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:184)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:149)
	at spoon.reflect.visitor.CtScanner.visitCtPackage(CtScanner.java:677)
	at spoon.support.reflect.declaration.CtPackageImpl.accept(CtPackageImpl.java:88)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:194)
	at spoon.support.visitor.ProcessingVisitor.scan(ProcessingVisitor.java:68)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:184)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:149)
	at spoon.reflect.visitor.CtScanner.visitCtPackage(CtScanner.java:676)
	at spoon.support.reflect.declaration.CtPackageImpl.accept(CtPackageImpl.java:88)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:194)
	at spoon.support.visitor.ProcessingVisitor.scan(ProcessingVisitor.java:68)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:184)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:149)
	at spoon.reflect.visitor.CtScanner.visitCtPackage(CtScanner.java:676)
	at spoon.support.reflect.declaration.CtPackageImpl.accept(CtPackageImpl.java:88)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:194)
	at spoon.support.visitor.ProcessingVisitor.scan(ProcessingVisitor.java:68)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:184)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:149)
	at spoon.reflect.visitor.CtScanner.visitCtPackage(CtScanner.java:676)
	at spoon.support.reflect.declaration.CtPackageImpl.accept(CtPackageImpl.java:88)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:194)
	at spoon.support.visitor.ProcessingVisitor.scan(ProcessingVisitor.java:68)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:184)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:149)
	at spoon.reflect.visitor.CtScanner.visitCtPackage(CtScanner.java:676)
	at spoon.support.reflect.declaration.CtPackageImpl.accept(CtPackageImpl.java:88)
	at spoon.reflect.visitor.CtScanner.scan(CtScanner.java:194)
	at spoon.support.visitor.ProcessingVisitor.scan(ProcessingVisitor.java:68)
	at spoon.support.QueueProcessingManager.process(QueueProcessingManager.java:118)
	at spoon.support.QueueProcessingManager.process(QueueProcessingManager.java:132)
	at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.process(JDTBasedSpoonCompiler.java:196)
	at spoon.Launcher.process(Launcher.java:791)
	at spoon.Launcher.run(Launcher.java:735)
	at spoon.test.prettyprinter.SniperTryCatchWithResourceTest.test(SniperTryCatchWithResourceTest.java:41)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:725)
	at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
	at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:84)
	at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$7(TestMethodTestDescriptor.java:214)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:210)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:135)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:66)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:107)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:88)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:67)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:52)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:86)
	at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:86)
	at org.junit.platform.launcher.core.SessionPerRequestLauncher.execute(SessionPerRequestLauncher.java:53)
	at com.intellij.junit5.JUnit5IdeaTestRunner.startRunnerWithArgs(JUnit5IdeaTestRunner.java:71)
	at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
	at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:235)
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:54)

I do not have a lot of experience about CtRole, do you have any advice to fix this?

Best.

@SirYwell
Copy link
Collaborator

It looks like spoon wrongly tries to find the TYPE role in CtTryWithResources, while it should look in CtLocalVariable (or its super types). I'm not sure why that happens as I never really touched the sniper printer, but maybe it's helpful for someone else.

It would also probably make sense to minimize the test code, I don't think the second resource with the lambda is actually needed to trigger the issue.

@SirYwell
Copy link
Collaborator

SirYwell commented Jul 5, 2022

I started looking into this, to the point where I know where the issue comes from: Spoon fails to set the source position of the variable because it fails to set the source position of the final modifier. The start and end position given by JDT are totally off for the local variable, they even include the comment outside of the try. Spoon will simply skip the comment and then reads "tokens" separated by whitespaces (it tries to find a modifier for try and for (final in that case). You can simply debug this by placing a breakpoint here

spoon/src/main/java/spoon/support/compiler/jdt/PositionBuilder.java

Line 594 in 454dada

String modifierName = String.valueOf(contents, o1, o2 - o1);
and step through your test code.

I'm not sure what's an appropriate fix here as

  1. JDT delivers weird source position results.
  2. The PositionBuilder is fundamentally flawed: Whitespaces are considered as the only delimiters. This is a wrong assumption, and therefore a ( without a space afterwards will cause issues, similar as final@NonNull String s; will crash the printer due to the exact same problem you found.

@slarse
Copy link
Collaborator

slarse commented Jul 9, 2022

JDT delivers weird source position results.

In general, I'm of the opinion that we shouldn't try too hard to work around flaws in JDT. Such workarounds make Spoon less maintainable, and depend on implementation details of JDT that can change unexpectedly.

The PositionBuilder is fundamentally flawed: Whitespaces are considered as the only delimiters

This seems like something we'll want to address, though.

@SirYwell
Copy link
Collaborator

SirYwell commented Jul 9, 2022

In general, I'm of the opinion that we shouldn't try too hard to work around flaws in JDT. Such workarounds make Spoon less maintainable, and depend on implementation details of JDT that can change unexpectedly.

I agree, however the sniper printer heavily relies on source positions and those are currently provided by JDT, with a lot of best effort approaches as it seems when looking into PositionBuilder. I just don't know if there's anything we can do better.

This seems like something we'll want to address, though.

Yes. I started writing down a simple lexer, and it might be good enough for our use case, but I'll need some more time to evaluate that (and I don't know when I'll find that time).

@slarse
Copy link
Collaborator

slarse commented Jul 9, 2022

I agree, however the sniper printer heavily relies on source positions and those are currently provided by JDT, with a lot of best effort approaches as it seems when looking into PositionBuilder. I just don't know if there's anything we can do better.

It is indeed quite the conundrum.

Yes. I started writing down a simple lexer, and it might be good enough for our use case, but I'll need some more time to evaluate that (and I don't know when I'll find that time).

If you come up with a solution we'll be thankful for it, but there's of course no obligation. We're all volunteering time here after all :)

@SirYwell SirYwell mentioned this pull request Apr 8, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@danglotb @SirYwell @slarse