Community
Participate
Working Groups
Create a clean up / multi fix / quick assist that migrates anonymous class creations to lambda expressions. The result must be semantically equivalent to the original code. The migration can only work if the type is a "Functional Interface", and if the anonymous class body only contains a single method.
Markus, I pushed branch the following branch with an initial implementation of the quick assist - http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/log/?h=dazad/BETA_JAVA8 We need an API from JDT Core to check if an anonymous class creation corresponds to a Functional interface. For now I implemented a very simple check in our code. Note: the branch has SHARED_AST_LEVEL= AST.JLS8 otherwise the quick assist will not work.
(In reply to comment #1) > We need an API from JDT Core to check if an anonymous class creation > corresponds to a Functional interface. For now I implemented a very simple > check in our code. Opened Bug 412726
Clean up implementation - first draft http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?h=dazad/BETA_JAVA8&id=00ea70edad7c0cafb22fe394db2115b0a0aa2e66 For now, the preference for the new clean up is on 'Code Style' page. But I am wondering if there should be a new tab specifically for Java 8 features, Markus?
http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?h=dazad/BETA_JAVA8&id=deee37e65cea10dafad06e45fe1979afcbc7b7af Incorporated the new API from Bug 412726.
Thanks Deepak, looks like we're on a good track here. I didn't do a thorough review yet, but I just noticed one UI glitch in the Clean Up edit dialog: The "Use lambda where possible" option does not update the enabled clean ups counter, and hence it can't be run as the sole clean up. Please also start thinking about the inverse operation. For the "Enhanced for loop", it turned out that users also need to go back to the verbose form. Initially, to convert 1.5 code to 1.4, but also on a case-by-case basis if more control over the iteration is necessary. I think we should at least offer a quick assist to convert a lambda to an anonymous class, but maybe we also want this as a clean up?
(In reply to comment #5) > I think we should at least offer a quick assist to convert a lambda to an > anonymous class, but maybe we also want this as a clean up? Fair point. I will add the reverse quick assist. If it would be useful, we can also add the clean up since it is not that much more work. I will also take a look at the UI glitch.
If the method body just contains a single return statement, then the lambda should use the short form with just an expression body, e.g.: Callable<String> c = new Callable<String>() { @Override public String call() { return "OK"; } }; => Callable<String> c = () -> "OK";
- I created the reverse quick assist. But I was not sure that we need the reverse clean up as well, hence I just implemented the quick assist as a multi-fix so that if we need the cleanup it will be a trivial task. http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?h=dazad/BETA_JAVA8&id=5fe26f2d0a91a33b0f75c6538337ab3494489948 - Fixed the UI glitch from comment 5 - Fixed the use case from comment 7 To ensure that everything works without throwing an exception I had to fix a few usages of MethodDeclaration#thrownExceptions(). For this I have followed the following code pattern... int apiLevel= ast.apiLevel(); List<ASTNode> thrownExceptions= apiLevel < AST.JLS8 ? decl.thrownExceptions() : decl.thrownExceptionTypes(); - There are a few TODOs in LambdaExpressionsFix which relate to minor (style) points. Awaiting review...
*** Bug 417795 has been marked as a duplicate of this bug. ***
*** Bug 417796 has been marked as a duplicate of this bug. ***
We are hoping this will be in reasonable shape for us to include it in a demo in ECE Oct 30th. Any help needed from JDT/Core, let us know, we will pull out all stops to make it happen. Thank you.
(In reply to Srikanth Sankaran from comment #11) > We are hoping this will be in reasonable shape for us to include it in > a demo in ECE Oct 30th. Any help needed from JDT/Core, let us know, we > will pull out all stops to make it happen. Thank you. This can be demoed. The code is in dazad/BETA_JAVA8 branch (and not in the common BETA_JAVA8 branch), but that should not be a blocker for it to be demoed.
(In reply to Deepak Azad from comment #12) > (In reply to Srikanth Sankaran from comment #11) > > We are hoping this will be in reasonable shape for us to include it in > > a demo in ECE Oct 30th. Any help needed from JDT/Core, let us know, we > > will pull out all stops to make it happen. Thank you. > > This can be demoed. The code is in dazad/BETA_JAVA8 branch (and not in the > common BETA_JAVA8 branch), but that should not be a blocker for it to be > demoed. 4.3.1 + bundles from http://dist.springsource.com/snapshot/TOOLS/java8/e43 as the IDE, I launched on a clean workspace, clone the UI repo, imported the projects and switched to your branch. Launched an inner instance on a clean workspace, created a 1.8 project and pasted the code below. I get the offer of assist to transform anonymous class to lambda and it works fine for the case below. Thanks. However any attempt to even ever so lightly modify the file results in numerous exceptions being thrown about use of unsupported operations in JLS8 (multiple errors have occurred, operation supported only in JLS2,3, 4. So while this feature looks ready, other foundational pieces don't. We are evaluating our options. // --- import java.util.EventListener; interface IResourceChangeEvent { int CHEESE_MOVE = 0; Object getDelta(); } interface IResourceChangeListener extends EventListener { // ... public void handleResourceChange(IResourceChangeEvent event); // ... } interface IWorkspace { // ... public void addResourceChangeListener(IResourceChangeListener listener, int eventMask); // ... } public class X { void foo(IWorkspace workspace) { workspace.addResourceChangeListener(new IResourceChangeListener() { @Override public void handleResourceChange(IResourceChangeEvent event) { handleCheeseMove(event.getDelta()); } }, IResourceChangeEvent.CHEESE_MOVE); } private void handleCheeseMove(Object delta) { // ... } } // --
From playing with this for the demo: - the quickfix doesn't seem to respect the formatter settings, notably: I get no linebreaks within the lambda, although it exceeds the Maximum line width by many (> 50) characters Is this expected? Deselecting the lambda and hitting Ctrl-Shift-F produces a somewhat useful layout (we probably don't yet have formatter settings for really smart formatting of lambdas, right?). - are there any interesting variations where the quickfix behaves differently, apart from just not being available if, e.g., not having a functional interface type?
Deepak, now that https://bugs.eclipse.org/bugs/show_bug.cgi?id=403927 is resolved, can you pull the changes and get your branch up to date so we can test it and use it for a demo on 30th Oct if possible ? Please also see unanswered questions in comment#14 Thanks in advance.
Markus, since it appears Deepak may be done with this work and it is waiting for review from your side, if you can give it a look through and if is ready to be promoted to branch head, go ahead and release it that would help. TIA.
I've already merged dazad/BETA_JAVA8 into BETA_JAVA8 in my workspace and I'll push it to BETA_JAVA8 after I'm done with my review.
I'm not yet done with the full review, but a fairly-well-working preview is now in branch mkeller/BETA_JAVA8: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?h=mkeller/BETA_JAVA8&id=455ffd012f312dd5147ffb01ee159912d5c26317 This can be used to prepare a demo, but don't base other work on it, since I'll massage the commits a bit before it goes into BETA_JAVA8.
(In reply to Stephan Herrmann from comment #14) > From playing with this for the demo: > > - the quickfix doesn't seem to respect the formatter settings, > notably: I get no linebreaks within the lambda, although it > exceeds the Maximum line width by many (> 50) characters > Is this expected? > Deselecting the lambda and hitting Ctrl-Shift-F produces a > somewhat useful layout (we probably don't yet have formatter > settings for really smart formatting of lambdas, right?). Umm.. I think the formatter usually handles this by default. We don't really do anything special in UI to format the code, ASTRewrite is supposed to take care of things. > - are there any interesting variations where the quickfix behaves > differently, apart from just not being available if, e.g., > not having a functional interface type? Not really, the only non-trivial case where the quick assist is not offered was when the anonymous class had fields or say implemented methods from the Object class. There could be other minor variations which are currently TODOs in code // TODO: minor: no parentheses for single inferred-type parameter? // TODO: minor: do we want to create VaribaleDeclarationFragments or inferred-type parameter - never? But I doubt if any of these are too interesting to be worth mentioning in a demo.
(In reply to Markus Keller from comment #18) > I'm not yet done with the full review, but a fairly-well-working preview is > now in branch mkeller/BETA_JAVA8: > http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?h=mkeller/ > BETA_JAVA8&id=455ffd012f312dd5147ffb01ee159912d5c26317 > > This can be used to prepare a demo, but don't base other work on it, since > I'll massage the commits a bit before it goes into BETA_JAVA8. This works pretty well, I tested it a bit by editing a file with the sample code from comment#13 and by adding default methods, static methods, redeclaring java.lang.Object's methods, converting interface to abstract class, adding a second abstract method etc. I get the quick assist offer where I should and don't where I shouldn't. This is great news, Thanks Markus & Deepak. Is there any chance this will be to get promoted to BETA_JAVA8 after review with follow up bugs raised for open items found during code review which can be addressed in due course ? It would be nice to give a demo right from BETA_JAVA8 without having to launch an inner IDE, so the audience know this is real & live - but only if it makes sense from your team's pov.
Fixed with http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=21b51b36c3333f310d1cb3db44748ee5ee28f721 LambdaExpressionsFix has a few TODOs that can be handles in bug 421479.
See that this transformation has the potential to break programs: e.g: interface I { int foo(String s); } interface J { Integer foo(String s); } public class X { static void goo(I i) { System.out.println("goo(I)"); } static void goo(J j) { System.out.println("goo(J)"); } public static void main(String[] args) { goo(new I() { @Override public int foo(String s) { return 0; }}); } } The transformed code won't compile any more. There could be scenarios where the call would compile, but bind to a different method altogether. If you need some help from Core to figure out whether the overload semantics would alter the resolution, please file an ER with a proposed API. Thanks.
(In reply to Srikanth Sankaran from comment #22) > See that this transformation has the potential to break programs: [...] > If you need some help from Core to figure out whether the overload semantics > would alter the resolution, please file an ER with a proposed API. Thanks. In the interim, if you wish, you can stick in a cast to the functional type. A casted poly expression ceases to be a poly expression and cannot be influenced by the context or influence the context.
(In reply to Srikanth Sankaran from comment #22 and comment #23) Noopur is working on this in 408966. Filed bug 423439 to fix the cleanup/quickassist, since bug 421479 is about something else.
