cincheo / jsweet

A Java to JavaScript transpiler.
http://www.jsweet.org
Other
1.45k stars 160 forks source link

transation of lombok generated equals method fails #581

Closed mueller-jens closed 4 years ago

mueller-jens commented 4 years ago

As described in this issue the equals method generated by lombok was translated without parenthesis. The pull request should prevent the generation without a blank after if and without parenthesis.

Example:

Before fix:

ifo === this return true;

Now

if (o === this) return true;
lgrignon commented 4 years ago

Hello @mueller-jens I am sorry to answer this late, I've been quite busy lately.

Would it be possible to add the Lombok generated Java code instead of adding a dependency to Lombok in pom.xml.

I don't know Lombok well but I guess you can access some way the intermediate Java code.

I will try to answer quickly

mueller-jens commented 4 years ago

Hi @lgrignon, i have tried to figure out what is happening. But I can't. I see that the generated methods by lombok looks like:

   @java.lang.Override()
    @java.lang.SuppressWarnings(value = "all")
    public boolean equals(final java.lang.Object o) {
        if (o == this) return true;
        if (!(o instanceof EqualsAndHashCodeExample)) return false;
        final EqualsAndHashCodeExample other = (EqualsAndHashCodeExample)o;
        if (!other.canEqual((java.lang.Object)this)) return false;
        final java.lang.Object this$property = this.getProperty();
        final java.lang.Object other$property = other.getProperty();
        if (this$property == null ? other$property != null : !this$property.equals(other$property)) return false;
        return true;
    }

    @java.lang.SuppressWarnings(value = "all")
    protected boolean canEqual(final java.lang.Object other) {
        return other instanceof EqualsAndHashCodeExample;
    }

    @java.lang.Override()
    @java.lang.SuppressWarnings(value = "all")
    public int hashCode() {
        final int PRIME = 59;
        int result = 1;
        final java.lang.Object $property = this.getProperty();
        result = result * PRIME + ($property == null ? 43 : $property.hashCode());
        return result;
    }

If i copy it into the test code, everything works file.

I have debuged the code a little bit and see if I use the lombok annotation ifStatement.getCondition() return o == this, while using the code it returns (o == this).

in my opinon there are two possibilities. A bug in the JCTree (which i can't debug at the moment) or while creating these class.

So it is not possible to reproduce my error without the lombok annotation. But if i have more time i will try to go into a deeper level.

BTW: While debugging i see that lombok is a compiler extention.

Best regards Jens

lgrignon commented 4 years ago

@mueller-jens thanks, that is really interesting. If you have more time to spend on it in order to have a plain java unit test, it would be great, if not, please update your PR to remove unit test and Lombok dependency, and we will simply merge it as is :)

mueller-jens commented 4 years ago

@lgrignon I am thinking about Open a bugreport at the lombok project. They override some com.sun classes in there jar. Is it possible to extract a small parser for making a sample project for the bug Report? I am not so good with parsing java classes?

Rawi01 commented 4 years ago

The generated internal structure generated by lombok is different, thats why this one fails. Instead of simply adding parentheses I would recommend to add an additional if condition as it was done in delombok (https://github.com/rzwitserloot/lombok/blob/master/src/delombok/lombok/delombok/PrettyPrinter.java#L1231)

mueller-jens commented 4 years ago

@lgrignon I have discussed the problem with @Rawi01 and looks like it is not a bug in lombok project and it is not reproduceable in plain java. So I have removed the lombok dependency and the test case and have changed the implementation to the suggestion of @Rawi01. I hope it is ok for you now.

Thanks and best regards to both of you. Jens

lgrignon commented 4 years ago

Thanks @mueller-jens for all of this (and @Rawi01 of course to point at those precisions) I am just waiting for the build to finish, and then we are all set to merge :)

mueller-jens commented 4 years ago

@lgrignon you are welcome. When do you plan to release these changes?

lgrignon commented 4 years ago

I will do it today, thanks again and sorry for the delay

lgrignon commented 4 years ago

3.0.0-RC3 is deployed (and maven plugin as well).