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

delombok generates incorrect code from try-with-resources blocks #532

Closed
lombokissues opened this Issue Jul 14, 2015 · 4 comments

Comments

Projects
None yet
2 participants
@lombokissues
Collaborator

lombokissues commented Jul 14, 2015

Migrated from Google Code (issue 459)

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 steven676   🕗 Mar 14, 2013 at 19:35 UTC

Specifically, resources statements appear to be ignored and stripped completely from the delomboked code, resulting in code that doesn't compile.

For example, the following class:

@ RequiredArgsConstructor
public class Test {
private final String filename;

public void read() {
    try (BufferedReader r = new BufferedReader(new FileReader(filename))) {
        System.out.println(r.readLine());
    } catch (IOException e) {
        e.printStackTrace();
    }
}

}

when delomboked produces this:

public class Test {
private final String filename;

    public void read() {
            try {
                    System.out.println(r.readLine());
            } catch (IOException e) {
                    e.printStackTrace();
            }
    }

    [...]

}

The class appears to compile correctly with javac. Tested with Lombok 0.11.0, 0.11.2, 0.11.6, and master (cb9b907) with IcedTea 2.3.8 and Oracle JDK 7u17.

This would be a mere curiosity for me, except that when the try-with-resources statement has no associated catch block, Javadoc will choke on the resulting output, complaining of a "'try' without 'catch', 'finally' or resource declarations".

Collaborator

lombokissues commented Jul 14, 2015

👤 steven676   🕗 Mar 14, 2013 at 19:35 UTC

Specifically, resources statements appear to be ignored and stripped completely from the delomboked code, resulting in code that doesn't compile.

For example, the following class:

@ RequiredArgsConstructor
public class Test {
private final String filename;

public void read() {
    try (BufferedReader r = new BufferedReader(new FileReader(filename))) {
        System.out.println(r.readLine());
    } catch (IOException e) {
        e.printStackTrace();
    }
}

}

when delomboked produces this:

public class Test {
private final String filename;

    public void read() {
            try {
                    System.out.println(r.readLine());
            } catch (IOException e) {
                    e.printStackTrace();
            }
    }

    [...]

}

The class appears to compile correctly with javac. Tested with Lombok 0.11.0, 0.11.2, 0.11.6, and master (cb9b907) with IcedTea 2.3.8 and Oracle JDK 7u17.

This would be a mere curiosity for me, except that when the try-with-resources statement has no associated catch block, Javadoc will choke on the resulting output, complaining of a "'try' without 'catch', 'finally' or resource declarations".

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 reinierz   🕗 Mar 18, 2013 at 21:19 UTC

Yup, that makes sense. Fixed in about 3 minutes.

Of course, now we need to write a test case for it, which means we need to update our entire test infrastructure to allow for running certain tests ONLY if we're at least on something that understands java7 or higher, sigh. The things we do for our users :P

Hopefully we finish all that today in which case in a few hours this will be followed up by a Status: Verified update ;)

Collaborator

lombokissues commented Jul 14, 2015

👤 reinierz   🕗 Mar 18, 2013 at 21:19 UTC

Yup, that makes sense. Fixed in about 3 minutes.

Of course, now we need to write a test case for it, which means we need to update our entire test infrastructure to allow for running certain tests ONLY if we're at least on something that understands java7 or higher, sigh. The things we do for our users :P

Hopefully we finish all that today in which case in a few hours this will be followed up by a Status: Verified update ;)

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

👤 reinierz   🕗 Mar 18, 2013 at 23:00 UTC

Well, look at that - only an hour and a half. Go us :)

All fixed; will be in the next release. We'll try and get an official release out the door this week.

Until then, the current edge release has this bugfix: https://projectlombok.org/download-edge.html

Collaborator

lombokissues commented Jul 14, 2015

👤 reinierz   🕗 Mar 18, 2013 at 23:00 UTC

Well, look at that - only an hour and a half. Go us :)

All fixed; will be in the next release. We'll try and get an official release out the door this week.

Until then, the current edge release has this bugfix: https://projectlombok.org/download-edge.html

@lombokissues

This comment has been minimized.

Show comment
Hide comment
@lombokissues

lombokissues Jul 14, 2015

Collaborator

End of migration

Collaborator

lombokissues commented Jul 14, 2015

End of migration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment