groovy
  1. groovy
  2. GROOVY-833

Groovy-core compile errors in eclipse with 5.0 compiler enabled

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.0-JSR-2
    • Fix Version/s: 1.0-JSR-5
    • Component/s: groovy-jdk
    • Labels:
      None
    • Environment:
      Eclipse 3.1M5, JDK 5.0
    • Number of attachments :
      2

      Description

      Using eclipse with the 5.0 compiler (instead of the 1.4 compiler) causes the following ERRORs:

      org/codehaus/groovy/runtime/DefaultGroovyMethods.java (line 3355)
      The method compareTo(BigInteger) in the type BigInteger is not applicable for the arguments (BigDecimal)

      org/codehaus/groovy/runtime/WriteFile.java (line 102)
      Name clash: The method compareTo(Object) of type WritableFile has the same erasure as compareTo(T) of type Comparable<T> but does not override it

      org/codehaus/groovy/runtime/WriteFile.java (line 103)
      The method compareTo(File) in the type File is not applicable for the arguments (Object)

      1. groovy.java5.patch
        7 kB
        Marc Guillemot
      2. WritableFile.java
        3 kB
        Marc Guillemot

        Issue Links

          Activity

          Hide
          Christian Stein added a comment -

          I can confirm those 3 errors w/ JDK5 and Eclipse 3.1M6.

          My solution was to create an project specific 1.4 compiler target, which is stored by Eclipse under "groovy-core/.settings/". Yesterday, I added the ".settings" pattern to the local ".cvsignore" file and did not submit those working compiler settings.

          I upload those ".settings" now as a work around – but the compile error should not happen and get fixed soon!

          Show
          Christian Stein added a comment - I can confirm those 3 errors w/ JDK5 and Eclipse 3.1M6. My solution was to create an project specific 1.4 compiler target, which is stored by Eclipse under "groovy-core/.settings/". Yesterday, I added the ".settings" pattern to the local ".cvsignore" file and did not submit those working compiler settings. I upload those ".settings" now as a work around – but the compile error should not happen and get fixed soon!
          Hide
          Christian Stein added a comment -
              • Resolves when creating a new BD called "selfDec"

          org/codehaus/groovy/runtime/DefaultGroovyMethods.java (line 3355)
          The method compareTo(BigInteger) in the type BigInteger is not applicable for the arguments (BigDecimal)

          BigDecimal to1 = (BigDecimal) to;
          BigDecimal selfDec = new BigDecimal(self);
          if (selfDec.compareTo(to1) >= 0) {
          for (BigDecimal i = new BigDecimal(self); i.compareTo(to1) >= 0; i = i.subtract(one))

          { closure.call(i); }

          }

              • Cannot resolve under Java 5 - because any solution yields a duplicate method (line 98) or violates the Comparable<File> contract.

          org/codehaus/groovy/runtime/WriteFile.java (line 102)
          org/codehaus/groovy/runtime/WriteFile.java (line 103)

          public int compareTo(Object arg0)

          { // Replacing Object with File yields duplicated method return delegate.compareTo((File) arg0); // The cast fixes line 105 }

          The root of the problem is that WritableFile extends File, which in turn implements Comparable<File>. My proposal is to remove the "compareTo(Object)" method entirely – which breaks 1.4 Comparable. Humm...

          Show
          Christian Stein added a comment - Resolves when creating a new BD called "selfDec" org/codehaus/groovy/runtime/DefaultGroovyMethods.java (line 3355) The method compareTo(BigInteger) in the type BigInteger is not applicable for the arguments (BigDecimal) BigDecimal to1 = (BigDecimal) to; BigDecimal selfDec = new BigDecimal(self); if (selfDec.compareTo(to1) >= 0) { for (BigDecimal i = new BigDecimal(self); i.compareTo(to1) >= 0; i = i.subtract(one)) { closure.call(i); } } Cannot resolve under Java 5 - because any solution yields a duplicate method (line 98) or violates the Comparable<File> contract. org/codehaus/groovy/runtime/WriteFile.java (line 102) org/codehaus/groovy/runtime/WriteFile.java (line 103) public int compareTo(Object arg0) { // Replacing Object with File yields duplicated method return delegate.compareTo((File) arg0); // The cast fixes line 105 } The root of the problem is that WritableFile extends File, which in turn implements Comparable<File>. My proposal is to remove the "compareTo(Object)" method entirely – which breaks 1.4 Comparable. Humm...
          Hide
          Dave Lin added a comment -

          For the compareTo(Ojbect) of File, I think this is a core incompitable between Java 1.4 and 5.0. Is branching this file 1.4 and 5.0 a good solution? If not, remove the compareTo(Ojbect) is a better choose in my opinion since compareTo(Ojbect) is not common used when compareTo(File) is more likely used.

          Show
          Dave Lin added a comment - For the compareTo(Ojbect) of File, I think this is a core incompitable between Java 1.4 and 5.0. Is branching this file 1.4 and 5.0 a good solution? If not, remove the compareTo(Ojbect) is a better choose in my opinion since compareTo(Ojbect) is not common used when compareTo(File) is more likely used.
          Hide
          Dave Lin added a comment -

          Another possible solution: In WriteFIle constructor, doing

          public WritableFile(File delegate, String encoding)

          { super(delegate.toURI()); // instead of super("") this.delegate = delegate; this.encoding = encoding; }

          And remove both compareTo() methods (Even all methods except WriteTo()). Though some overhead, this can solve the compitability problem.

          Is this okay?

          Show
          Dave Lin added a comment - Another possible solution: In WriteFIle constructor, doing public WritableFile(File delegate, String encoding) { super(delegate.toURI()); // instead of super("") this.delegate = delegate; this.encoding = encoding; } And remove both compareTo() methods (Even all methods except WriteTo()). Though some overhead, this can solve the compitability problem. Is this okay?
          Hide
          Marc Guillemot added a comment -

          This patch fixes the compilation errors under Java 5 and fixes wrong behaviour of downTo (unit test included)

          There is absolute no need here for distinct code for Java 1.4 and 5 because of File.compareTo(Object): Java 1.4 File.compareTo(Object) throws a ClassCastException if the Object isn't a File. We just need to do the cast explicitely by ourself to produce the ClassCastException earlier if needed and to make Java 5 happy.

          The downTo compilaton errors allowed to find a bug in donwTo.

          Show
          Marc Guillemot added a comment - This patch fixes the compilation errors under Java 5 and fixes wrong behaviour of downTo (unit test included) There is absolute no need here for distinct code for Java 1.4 and 5 because of File.compareTo(Object): Java 1.4 File.compareTo(Object) throws a ClassCastException if the Object isn't a File. We just need to do the cast explicitely by ourself to produce the ClassCastException earlier if needed and to make Java 5 happy. The downTo compilaton errors allowed to find a bug in donwTo.
          Hide
          Dave Lin added a comment -

          For the Writable file, I fould this error message in Eclipse. Marc, did you get this message, too?

          Name clash: The method compareTo(Object) of type WritableFile has the same erasure as compareTo(T) of type Comparable<T> but does not override it

          I am not quite familiar with Java 5, but in the javadoc I found that the compareTo() in Java 5 now using Generic type, which may cause this problem

          Show
          Dave Lin added a comment - For the Writable file, I fould this error message in Eclipse. Marc, did you get this message, too? Name clash: The method compareTo(Object) of type WritableFile has the same erasure as compareTo(T) of type Comparable<T> but does not override it I am not quite familiar with Java 5, but in the javadoc I found that the compareTo() in Java 5 now using Generic type, which may cause this problem
          Hide
          Dave Lin added a comment -

          I think a possible solution is

          public WritableFile(File delegate, String encoding)

          { super(delegate.toURI()); // instead of super("") this.delegate = delegate; this.encoding = encoding; }

          And remove both compareTo() methods.

          The compareTo() compares absolute path of file, so compare a WritableFile with a File should be fine.

          Show
          Dave Lin added a comment - I think a possible solution is public WritableFile(File delegate, String encoding) { super(delegate.toURI()); // instead of super("") this.delegate = delegate; this.encoding = encoding; } And remove both compareTo() methods. The compareTo() compares absolute path of file, so compare a WritableFile with a File should be fine.
          Hide
          Marc Guillemot added a comment -

          Dave,
          yes I've checked out the project in Eclipse and got these errors. You're right concerning the cause of the compareTo() problem. It's possible that a refactoring like the one you propose would work and make sense, but in a first time I wanted to do the minimal changes.

          File.compareTo(File) is ok at compile time both for Java 1.4 and 5

          File.compareTo(Object) is ok at compile time for Java 1.4 but will throw a ClassCastException at runtime if the provided Object isn't a File. It is not ok at compile time for Java 5

          Casting the Object to File makes compilation ok for both Java 1.4 and 5 and the only consequence is that when the method gets called with a non-File parameter, the ClassCastException is thrown by the Groovy Class rather than by the File class.

          Show
          Marc Guillemot added a comment - Dave, yes I've checked out the project in Eclipse and got these errors. You're right concerning the cause of the compareTo() problem. It's possible that a refactoring like the one you propose would work and make sense, but in a first time I wanted to do the minimal changes. File.compareTo(File) is ok at compile time both for Java 1.4 and 5 File.compareTo(Object) is ok at compile time for Java 1.4 but will throw a ClassCastException at runtime if the provided Object isn't a File. It is not ok at compile time for Java 5 Casting the Object to File makes compilation ok for both Java 1.4 and 5 and the only consequence is that when the method gets called with a non-File parameter, the ClassCastException is thrown by the Groovy Class rather than by the File class.
          Hide
          Dierk König added a comment -

          applied patch of Marc Guillemot (thanks a lot)

          compiled and tested under JDKs 1.4 and 1.5

          Hein: please close the issue if that solves the problem for you

          Show
          Dierk König added a comment - applied patch of Marc Guillemot (thanks a lot) compiled and tested under JDKs 1.4 and 1.5 Hein: please close the issue if that solves the problem for you
          Hide
          Marc Guillemot added a comment -

          Ups, I now see that the errors I had (and that are fixed by my patch) were with Eclipse using 1.4 compiler compliance level BUT JRE 5. The type erasure problem remains.

          I think that your proposition to use

          super(delegate.toURI()); // instead of super("")

          would be a good solution and it would allow to remove nearly all overriden methods: they would be just useless.

          Show
          Marc Guillemot added a comment - Ups, I now see that the errors I had (and that are fixed by my patch) were with Eclipse using 1.4 compiler compliance level BUT JRE 5. The type erasure problem remains. I think that your proposition to use super(delegate.toURI()); // instead of super("") would be a good solution and it would allow to remove nearly all overriden methods: they would be just useless.
          Hide
          Christian Stein added a comment -

          I'm with Dave solution ... as the proposed and committed patch does not resolve under Java 5 - because this solution yields a duplicate method or violates the generic Comparable<File> contract.

          Show
          Christian Stein added a comment - I'm with Dave solution ... as the proposed and committed patch does not resolve under Java 5 - because this solution yields a duplicate method or violates the generic Comparable<File> contract.
          Hide
          Marc Guillemot added a comment -

          New version of WritableFile adapted like proposed by Dave what allows to remove completely the delegate.

          This allows the project to compile with java 1.4 as well as 5.0 (Dierk, I guess that you've started maven after having changed your JAVA_HOME and path but forgot to adapt the project.properties to java 5).

          Some of the unit tests fail with Java 5 (Dierk, this is the problem I already told you). I will try to have a look at them to see if it's possible to make them working both when sources are considered as 1.4 and 5. Nevertheless I think that it is a different issue and that this issue can be (re-)closed after fixing WritableFile.

          Show
          Marc Guillemot added a comment - New version of WritableFile adapted like proposed by Dave what allows to remove completely the delegate. This allows the project to compile with java 1.4 as well as 5.0 (Dierk, I guess that you've started maven after having changed your JAVA_HOME and path but forgot to adapt the project.properties to java 5). Some of the unit tests fail with Java 5 (Dierk, this is the problem I already told you). I will try to have a look at them to see if it's possible to make them working both when sources are considered as 1.4 and 5. Nevertheless I think that it is a different issue and that this issue can be (re-)closed after fixing WritableFile.
          Hide
          Dierk König added a comment -

          applied WritableFile.java.

          please test and close issue if ok.

          Show
          Dierk König added a comment - applied WritableFile.java. please test and close issue if ok.
          Hide
          blackdrag blackdrag added a comment -

          you people where a bit fast here...

          it is good you corrected WritableFile, because I think the compareTo method in the first patch would have produced a stack overflow when compiled with 1.4. I am not lucky about the changed tests

          +import java.math.*
          +

          public class DownUpStepTest extends GroovyTestCase {

          void testDownto() {

          • def z = 0.0
          • (10.5).downto(5.9) { z += it }
            - assert z == 10.5 + 9.5 + 8.5 + 7.5 + 6.5
            - assert z == 42.5

            + def z = []
            + (10.5).downto(5.9) { z << it }
            + assertEquals( [10.5, 9.5, 8.5, 7.5, 6.5], z)
            + }
            +
            + void testBigIntegerDowntoBigDecimal() {
            + def z = []
            + (new BigInteger("10")).downto(new BigDecimal("5.9")) { z << it }
            + assertEquals( [new BigInteger("10"), new BigInteger("9"), new BigInteger("8"),
            + new BigInteger("7"), new BigInteger("6")], z)

            }

            first... this test isn't groovy. There is no need to write new Biginteger("10"), we can write 10G. Second, why is the list used? I strongly recomend to revert the changes in testDownto(). And testBigIntegerDowntoBigDecimal could be also more groovy:

            void testBigIntegerDowntoBigDecimal() {
            def z = 0G
            10G.downto(5.9) { z += it }

            assert z == 40G
            assert z.class == BigInteger
            }

          using assertEquals(List,Object) won't work, as this merhod is not available.. then the testDownto in DefaultGroovyMethodsTest , why do we create a String here and compare that? I mean

          assertEquals("[1, 0]", DefaultGroovyMethods.toString(li));

          we could simply count how often the closure is called:

          public void testDownto() {
          final List li = new ArrayList();
          final Closure closure = new Closure(null) {
          int count = 0;
          public Object doCall(final Object params)

          { count++; return null; }

          };

          DefaultGroovyMethods.downto(new BigInteger("1"), new BigDecimal("0"), closure);
          assertEquals(closure.count, 2);

          closure.count=0;

          DefaultGroovyMethods.downto(new BigInteger("1"), new BigDecimal("0.123"), closure);
          assertEquals(closure.count,1);
          }

          btw, java.math isn't needed in groovy for BigInteger and BigDecimal in the CVS version.. so I think that's all

          Show
          blackdrag blackdrag added a comment - you people where a bit fast here... it is good you corrected WritableFile, because I think the compareTo method in the first patch would have produced a stack overflow when compiled with 1.4. I am not lucky about the changed tests +import java.math.* + public class DownUpStepTest extends GroovyTestCase { void testDownto() { def z = 0.0 (10.5).downto(5.9) { z += it } - assert z == 10.5 + 9.5 + 8.5 + 7.5 + 6.5 - assert z == 42.5 + def z = [] + (10.5).downto(5.9) { z << it } + assertEquals( [10.5, 9.5, 8.5, 7.5, 6.5] , z) + } + + void testBigIntegerDowntoBigDecimal() { + def z = [] + (new BigInteger("10")).downto(new BigDecimal("5.9")) { z << it } + assertEquals( [new BigInteger("10"), new BigInteger("9"), new BigInteger("8"), + new BigInteger("7"), new BigInteger("6")], z) } first... this test isn't groovy. There is no need to write new Biginteger("10"), we can write 10G. Second, why is the list used? I strongly recomend to revert the changes in testDownto(). And testBigIntegerDowntoBigDecimal could be also more groovy: void testBigIntegerDowntoBigDecimal() { def z = 0G 10G.downto(5.9) { z += it } assert z == 40G assert z.class == BigInteger } using assertEquals(List,Object) won't work, as this merhod is not available.. then the testDownto in DefaultGroovyMethodsTest , why do we create a String here and compare that? I mean assertEquals(" [1, 0] ", DefaultGroovyMethods.toString(li)); we could simply count how often the closure is called: public void testDownto() { final List li = new ArrayList(); final Closure closure = new Closure(null) { int count = 0; public Object doCall(final Object params) { count++; return null; } }; DefaultGroovyMethods.downto(new BigInteger("1"), new BigDecimal("0"), closure); assertEquals(closure.count, 2); closure.count=0; DefaultGroovyMethods.downto(new BigInteger("1"), new BigDecimal("0.123"), closure); assertEquals(closure.count,1); } btw, java.math isn't needed in groovy for BigInteger and BigDecimal in the CVS version.. so I think that's all
          Hide
          blackdrag blackdrag added a comment -

          sorry, the comment about assertEquals was stupid, of course you compare lists

          Show
          blackdrag blackdrag added a comment - sorry, the comment about assertEquals was stupid, of course you compare lists
          Hide
          Marc Guillemot added a comment -

          10G, 9G, ... should be used. Sorry, I'm not yet confident enough will all Groovy shortcuts.

          Otherwise the test has to compare lists because it is far more precise than comparing counts or sums (like what was previously made) and it is really more readable.

          Show
          Marc Guillemot added a comment - 10G, 9G, ... should be used. Sorry, I'm not yet confident enough will all Groovy shortcuts. Otherwise the test has to compare lists because it is far more precise than comparing counts or sums (like what was previously made) and it is really more readable.
          Hide
          blackdrag blackdrag added a comment -

          yes, I already said that my comment about assertEquals was stupid. I still don't like the thing with toString in DefaultGroovyMethodsTest, but as long as the build works I can life with it

          Show
          blackdrag blackdrag added a comment - yes, I already said that my comment about assertEquals was stupid. I still don't like the thing with toString in DefaultGroovyMethodsTest, but as long as the build works I can life with it

            People

            • Assignee:
              Dierk König
              Reporter:
              Hein Meling
            • Votes:
              1 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: