-
-
Notifications
You must be signed in to change notification settings - Fork 349
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
Create CtTypeAccess/CtTypeReference for ArrayReferences in noclasspath mode and static context. #811
Conversation
Once #809 is merged I can extend the provided example checking if |
I believe github now allows pull requests against other branches than master. I don't know if this might help here.
|
throw new SpoonException(); | ||
} | ||
ref.setDeclaringType(typeAccess.getAccessedType()); | ||
} catch (final SpoonException e) { /* ignore */ } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
really all SpoonException?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since it is noclasspath mode any of these function may throw a SpoonException
if they were not able to create a type access. Do you have a better idea?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could replace the SpoonException
with a null
return. It's up to you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An ignored exception is always a threat to debugging. Is it necessary? What if you don't catch anything in this method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I will remove the exception handling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, thanks.
do you have a test case? |
Is comming |
Added an example that shows that even multi dimensional arrays are processed. |
Big thanks for this PR @ReTuXx! I'm happy to see that someone else can contribute to these new JDT builders. (: |
This PR need a rebase. |
I still need to remove the catch block and fix some other issues. I will try to fix it the next days, currently I'm in a rush. Sorry for that. |
@@ -392,6 +395,28 @@ public boolean onAccess(char[][] tokens, int index) { | |||
return typeAccess; | |||
} | |||
|
|||
<T> CtTypeAccess<T> createTypeAccessNoClasspath(ArrayReference arrayReference) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add javadoc to explain what do you do your new method? (:
final CodeFactory codeFactory = jdtTreeBuilder.getFactory().Code(); | ||
|
||
final char[] readableName; | ||
if (arrayReference.receiver instanceof ArrayReference) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
arrayReference.receiver
can't never be null
in noclasspath mode? If yes, you'll go here.
try { | ||
final JDTTreeBuilderHelper helper = jdtTreeBuilder.getHelper(); | ||
final CtTypeAccess<T> typeAccess; | ||
if (messageSend.receiver instanceof SingleNameReference) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, messageSend.receiver
can't never be null in noclasspath mode?
@ReTuXx , should we close this old one? |
I'm still working on it but it is hard to test. If I remove the catch blocks I get another error which only appears when using |
#884 may help the mvn test problem. will be merged soon. |
The problem is, that when selecting one of the tests failing with |
|
I tried to run a particular test from withing the IDE in order to have breakpoints. |
OK, so the difference is between MVN and IDE, not between all-tests and
single test.
Did you rebase against master recently?
|
I don't thing so. I'm using IntelliJ which simply runs maven without further processing. Running
Not yet. |
closing this old inactive PR. Don't hesitate to re-open it if required. |
No description provided.