make code compiles with jdk 6 #64
Conversation
|
It compiles with the jdk 1.6 and every test passes. The test needs still the jdk 7. For now I removed the usage of the AutoCloseable interface. The documentations tells the user that it'a a resource from the try as resource functionality. So it should be clear. What do you think about that? |
| @@ -162,7 +162,7 @@ public A getActualAnnotation() { | |||
| } | |||
|
|
|||
| @SuppressWarnings("unchecked") | |||
| private <E extends Enum<E>> Object convertValue(Object value) { | |||
There was a problem hiding this comment.
Why do you remove this generic method type ?
There was a problem hiding this comment.
I removed it because I got errors with that generic method type. Every time the convert value was invoked was the following message:
"incompatible types; inferred type argument(s) java.lang.Object do not conform to bounds of type variable(s) E
[ERROR] found : java.lang.Object
[ERROR] required: java.lang.Object"
I solved it within the convertValue method by invoking the method this way:
this.<E>convertValue(field.getDefaultExpression());But I didn't find a way to solve it within the method getElementValue(String). Do you have any suggestions for solving that?
By the way it seems to work for you because you compile the project with the jdk 7, right? The error just appears while compiling it with the jdk 6
There was a problem hiding this comment.
Unfortunately, no. Java 1.6 don't seems to support generic type in methods and the only solution I have found is the same of yours.
By the way it seems to work for you because you compile the project with the jdk 7, right? The error just appears while compiling it with the jdk 6
Facepalm for me. :)
There was a problem hiding this comment.
Facepalm for me. :)
No problem, I did that mistake several times. The tests still needs the jdk 7. So one has to validate the code with the jdk 7. But before I always try to compile it with jdk 6.
One could solve it with restructuring the tests. It's possible to have JUnit tests at the same place but move the files which are scanned by spoon into the resource folder. Maven wouldn't compile them anymore and so the scanned files could use java 7 or 8.. But the used jdk can be still 6. One could give it a try with another pull request.
@monperrus @pschichtel Do you have any suggestions to solve the generic method type thing without removing it?
There was a problem hiding this comment.
Java 1.6 don't seems to support generic type in methods
Java 6 does support this, the compiler is just not good when it comes to type inference, so you often have to hint the type
|
Thanks @boeserwolf91 for this PR, that's seems nice. I pulled your branch in local and everything seems ok with me. I just have one question (see comment on your source code) because, for me, it is compatible java 1.6 (and without the modification, the project still compiles). For |
swolf91
changed the title
| default: | ||
| return null; | ||
| public static OutputType fromString(String string) | ||
| { |
There was a problem hiding this comment.
At the root of this project, you can find a formatter for spoon project (https://github.com/INRIA/spoon/blob/master/SpoonFormatterPrefs.xml). If everyone use his own formatter, source code will quickly become unreadable.
Can you use it ? Or put braces at the end of the signature of the method ?
Thanks in advance.
There was a problem hiding this comment.
"Unfortunately" (I hate eclipse) I can't use this file because I don't use eclipse. But I'll format it like you did. Give me a second
There was a problem hiding this comment.
I hate Eclipse too and latest versions of IntelliJ support formatters of Eclipse (if you use IntelliJ of course). :)
There was a problem hiding this comment.
Really IntelliJ supports that? Good to know. Thank you 👍
|
We are about to accept this PR. However, note that as soon as full support of analysis and transformation of Java 7 is added, we'll probably add a dependency to Java 7 again. |
|
why is that @monperrus ? |
|
Does the transformation need Java 7? I think the analysis doesn't need it. |
Best regards, --Martin |
|
I got it @monperrus |
|
OK, we're on the same wavelength now. |
|
Of course I could squash it. But I did several commits because every commit changes another thing. Are you really sure that I shall squash it into a single one? |
…est code still needs java 1.7
This pull request closes #58 it changes the code so that it compiles with jdk 6 instead of 7.