Community
Participate
Working Groups
With the following snippet and the given selection we do not offer the 'Extract to method' quick assist. However, the refactoring proceeds after duly warning the user with this message - "Selected statements contain a return statement but not all possible execution flows end in a return. Semantics may not be preserved if you proceed." ------------------------------------------------------------ int foo(int a) { /*[*/if (a > 10) { return 10; } else { System.out.println(a); }/*]*/ return a; } ------------------------------------------------------------ I am inclined to offer the quick assist even in this case, we can show the warning/error msg in the status line once the user applies the quick assist. And maybe also use an indicator as discussed in bug 353759. Implementation details: In QuickAssistProcessor.getExtractMethodProposal(...) we can change the check from "extractMethodRefactoring.checkInitialConditions(new NullProgressMonitor()).isOK()" to "!extractMethodRefactoring.checkInitialConditions(new NullProgressMonitor()).hasFatalError()"
Quick Assist is a handy way of invoking refactorings (See http://codingspectator.cs.illinois.edu/Blog/ProgrammersPreferContextAwareAndLightWeightMethodsOfInvokingAutomatedRefactorings) and programmers usually perform refactorings even when the tool reports some error (See http://codingspectator.cs.illinois.edu/Blog/ProgrammersUseAutomatedRefactoringsEvenWhenTheyMayBreakTheCode). Therefore, I think it'd be great for Quick Assist to propose refactorings even when they may not be behavior-preserving. Currently, if the programmer invokes a refactoring via Quick Assist and the refactoring tool reports an error, it will open a dialog. As Deepak said, I think we should explore alternative ways of communicating the error message in such cases because there are several problems with dialogs, e.g: 1. Dialogs disrupt the workflow of programmers. Programmers do not want to context switch to a dialog. 2. Dialogs are not well-integrated into the source editor and cannot easily refer to code snippets related to the problem. While I support the idea of proposing Quick Assist in more cases, I suggest that we split this issue into the following two: 1. Propose Quick Assist for Extract Method even when not all execution flows end in a return and show the error message in a dialog as usual. 2. Explore better ways of communicating the error messages of Quick Assist.
(In reply to comment #1) > Currently, if the programmer invokes a refactoring via Quick Assist and the > refactoring tool reports an error, it will open a dialog. Nope. No dialog comes up in the case of 'Extract to method'.
(In reply to comment #2) > (In reply to comment #1) > > Currently, if the programmer invokes a refactoring via Quick Assist and the > > refactoring tool reports an error, it will open a dialog. > Nope. No dialog comes up in the case of 'Extract to method'. I believe the dialog is shown for some refactorings invoked via Quick Assist, e.g. Rename. Eclipse should take a consistent behavior or it will confuse users. Presenting the error messages of Quick Assist using a dialog works but is not ideal. It is also possible to present the error messages in the status line. But, there are several problems with using the status line for this purpose, e.g.: 1. The error message might not fit in the status line. 2. Status line is easy to miss. Programmers are used to seeing the problems in the "Problems" view. 3. Usually, the text in the status line is not linked to the source editor. So, I wonder if programmers can click on the error message in the status line to navigate to the relevant piece of code. I'd prefer to present the error messages of Quick Assist in the "Problems" view because that's the standard place for presenting error messages in Eclipse. However, the only issue is how long should the error message persist in the "Problems" view? Perhaps Eclipse can remove the error message in the "Problems" view when the programmer removes the corresponding piece of code or explicitly marks the message as resolved or uses Quick Assist again. What do you think about reporting the error message of Quick Assist in the "Problems" view?
(In reply to comment #3) > I believe the dialog is shown for some refactorings invoked via Quick Assist, > e.g. Rename. Eclipse should take a consistent behavior or it will confuse > users. I think the only quick assist which results in a dialog is 'Create getter and setter'. Rename will not show a dialog by default as 'Preferences > Java > Rename in editor without dialog' is enabled by default. > What do you think about reporting the error message of Quick Assist in the > "Problems" view? Problems view is for compilation errors/warnings.
