Skip to content

Commit

Permalink
Fix UI freeze due to extract to local computations (#1186)
Browse files Browse the repository at this point in the history 
* Fix UI freeze due to extract to local computations

- fixes #1185
- don't run checkFinalConditions() from QuickAssistProcessor to see if
  extract to local variable proposals are valid
- override the preview for extract to local variable and extract to
  local variable (replace all occurrences) so they just display a
  message instead of creating the change
- fix ExtractTempRefactoring.checkFinalConditions() to recognize an
  invalid change in which case add a fatal error status with a code from
  RefactoringStatusCodes
- modify RefactoringCorrectionProposalCore to recognize the
  error and create a dummy edit plus a log entry to note that
  the extraction will not occur
- fix AssistQuickFixTest tests proposal counts
  • Loading branch information
@jjohnstn
jjohnstn authored Feb 10, 2024
1 parent 8133c17 commit fe167c1
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 34 deletions.
4 changes: 3 additions & 1 deletion ...e.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2023 IBM Corporation and others.
* Copyright (c) 2000, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -439,7 +439,9 @@ private CorrectionMessages() {
public static String ReorgCorrectionsSubProcessor_configure_buildpath_label;
public static String ReorgCorrectionsSubProcessor_configure_buildpath_description;
public static String QuickAssistProcessor_extract_to_local_all_description;
public static String QuickAssistProcessor_extract_to_local_all_preview;
public static String QuickAssistProcessor_extract_to_local_description;
public static String QuickAssistProcessor_extract_to_local_preview;
public static String QuickAssistProcessor_extractmethod_description;
public static String QuickAssistProcessor_extractmethod_from_lambda_description;
public static String QuickAssistProcessor_move_exception_to_separate_catch_block;
Expand Down
4 changes: 3 additions & 1 deletion ...pulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.properties
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
###############################################################################
# Copyright (c) 2000, 2023 IBM Corporation and others.
# Copyright (c) 2000, 2024 IBM Corporation and others.
#
# This program and the accompanying materials
# are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -394,7 +394,9 @@ QuickAssistProcessor_split_case_labels=Split case labels
QuickAssistProcessor_splitdeclaration_description=Split variable declaration
QuickAssistProcessor_exceptiontothrows_description=Replace exception with throws
QuickAssistProcessor_extract_to_local_all_description=Extract to local variable (replace all occurrences)
QuickAssistProcessor_extract_to_local_all_preview=Extract to local variable (replace all occurrences) if possible
QuickAssistProcessor_extract_to_local_description=Extract to local variable
QuickAssistProcessor_extract_to_local_preview=Extract to local variable if possible
QuickAssistProcessor_extractmethod_description=Extract to method
QuickAssistProcessor_extractmethod_from_lambda_description=Extract lambda body to method
QuickAssistProcessor_extract_to_constant_description=Extract to constant
Expand Down
3 changes: 2 additions & 1 deletion ...re extension/org/eclipse/jdt/internal/corext/refactoring/base/RefactoringStatusCodes.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2008 IBM Corporation and others.
* Copyright (c) 2000, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand All @@ -24,6 +24,7 @@ private RefactoringStatusCodes() {
public static final int EXPRESSION_NOT_RVALUE= 64;
public static final int EXPRESSION_NOT_RVALUE_VOID= 65;
public static final int EXTRANEOUS_TEXT= 66;
public static final int EXPRESSION_MAY_CAUSE_SIDE_EFFECTS= 67;

public static final int NOT_STATIC_FINAL_SELECTED= 128;
public static final int SYNTAX_ERRORS= 129;
Expand Down
10 changes: 9 additions & 1 deletion .../eclipse/jdt/internal/ui/text/correction/proposals/RefactoringCorrectionProposalCore.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2023 IBM Corporation and others.
* Copyright (c) 2023, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand All @@ -16,7 +16,9 @@

import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.core.runtime.Status;

import org.eclipse.core.resources.IFile;

Expand All @@ -31,6 +33,9 @@

import org.eclipse.jdt.core.ICompilationUnit;

import org.eclipse.jdt.internal.core.manipulation.JavaManipulationPlugin;
import org.eclipse.jdt.internal.corext.refactoring.base.RefactoringStatusCodes;

public class RefactoringCorrectionProposalCore extends LinkedCorrectionProposalCore {

private final Refactoring fRefactoring;
Expand Down Expand Up @@ -62,6 +67,9 @@ public TextChange createTextChange() throws CoreException {
if (fRefactoringStatus.hasFatalError()) {
TextFileChange dummyChange= new TextFileChange("fatal error", (IFile) getCompilationUnit().getResource()); //$NON-NLS-1$
dummyChange.setEdit(new InsertEdit(0, "")); //$NON-NLS-1$
if (fRefactoringStatus.getEntryAt(0).getCode() == RefactoringStatusCodes.EXPRESSION_MAY_CAUSE_SIDE_EFFECTS) {
JavaManipulationPlugin.log(new Status(IStatus.INFO, JavaManipulationPlugin.getPluginId(), fRefactoringStatus.getEntryAt(0).getMessage()));
}
return dummyChange;
}
Change o = fRefactoring.createChange(new NullProgressMonitor());
Expand Down
7 changes: 6 additions & 1 deletion .../refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractTempRefactoring.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,12 @@ public void preVisit(TextEdit edit) {
TextEdit[] children= topEdit.getChildren();
if (children.length == 1 && children[0] instanceof MultiTextEdit childEdit) {
if (childEdit.hasChildren() == false) {
result.addInfo(RefactoringCoreMessages.ExtractTempRefactoring_side_effects_possible);
fCURewrite.clearASTAndImportRewrites();
if (fLinkedProposalModel != null) {
fLinkedProposalModel.clear();
}
fLinkedProposalModel= null;
result.addEntry(RefactoringStatus.FATAL, RefactoringCoreMessages.ExtractTempRefactoring_side_effects_possible, null, JavaManipulationPlugin.getPluginId(), RefactoringStatusCodes.EXPRESSION_MAY_CAUSE_SIDE_EFFECTS, null);
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions ...i.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ExtractTempTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1358,19 +1358,19 @@ public void testFail40() throws Exception {
@Test
public void testFail41() throws Exception {
//test for https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/39
failHelper1(5, 28, 5, 38, true, false, "length", RefactoringStatus.INFO);
failHelper1(5, 28, 5, 38, true, false, "length", RefactoringStatus.FATAL);
}

@Test
public void testFail42() throws Exception {
//test for https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/39
failHelper1(5, 68, 5, 94, true, false, "intValue", RefactoringStatus.INFO);
failHelper1(5, 68, 5, 94, true, false, "intValue", RefactoringStatus.FATAL);
}

@Test
public void testFail43() throws Exception {
//test for https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/1176
failHelper1(19, 9, 19, 88, true, false, "intValue", RefactoringStatus.INFO);
failHelper1(19, 9, 19, 88, true, false, "intValue", RefactoringStatus.FATAL);
}

}
8 changes: 3 additions & 5 deletions org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2104,9 +2104,7 @@ public void testExtractToLocalVariable5() throws Exception { //https://github.co
AssistContext context= getCorrectionContext(cu, offset, selection.length());
List<IJavaCompletionProposal> proposals= collectAssists(context, false);

assertNumberOfProposals(proposals, 3);
assertProposalDoesNotExist(proposals, EXTRACT_TO_LOCAL);
assertProposalDoesNotExist(proposals, EXTRACT_TO_LOCAL_REPLACE);
assertNumberOfProposals(proposals, 5);
assertProposalDoesNotExist(proposals, EXTRACT_TO_CONSTANT);
}

Expand Down Expand Up @@ -3446,7 +3444,7 @@ public void testJoinDeclaration2() throws Exception {
AssistContext context= getCorrectionContext(cu, buf.toString().indexOf(str), 0);
List<IJavaCompletionProposal> proposals= collectAssists(context, false);

assertNumberOfProposals(proposals, 2);
assertNumberOfProposals(proposals, 4);
assertCorrectLabels(proposals);

buf= new StringBuilder();
Expand Down Expand Up @@ -3622,7 +3620,7 @@ public void testJoinDeclaration5() throws Exception {
AssistContext context= getCorrectionContext(cu, buf.toString().indexOf(str), 0);
List<IJavaCompletionProposal> proposals= collectAssists(context, false);

assertNumberOfProposals(proposals, 2);
assertNumberOfProposals(proposals, 4);
assertCorrectLabels(proposals);

buf= new StringBuilder();
Expand Down
45 changes: 24 additions & 21 deletions org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickAssistProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.eclipse.swt.graphics.Image;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.core.runtime.preferences.InstanceScope;

Expand All @@ -50,7 +51,6 @@
import org.eclipse.ltk.core.refactoring.Change;
import org.eclipse.ltk.core.refactoring.NullChange;
import org.eclipse.ltk.core.refactoring.Refactoring;
import org.eclipse.ltk.core.refactoring.RefactoringStatus;

import org.eclipse.jdt.core.Flags;
import org.eclipse.jdt.core.IBuffer;
Expand Down Expand Up @@ -537,6 +537,7 @@ private static boolean getExtractVariableProposal(IInvocationContext context, bo
extractTempRefactoringSelectedOnly.setCheckResultForCompileProblems(false);

String label= CorrectionMessages.QuickAssistProcessor_extract_to_local_description;
String preview= CorrectionMessages.QuickAssistProcessor_extract_to_local_preview;
Image image= JavaPluginImages.get(JavaPluginImages.IMG_CORRECTION_LOCAL);
int relevance;
if (context.getSelectionLength() == 0) {
Expand All @@ -546,16 +547,12 @@ private static boolean getExtractVariableProposal(IInvocationContext context, bo
} else {
relevance= IProposalRelevance.EXTRACT_LOCAL;
}
ExtractTempRefactoringProposalCore core= new ExtractTempRefactoringProposalCore(label, cu, extractTempRefactoringSelectedOnly, relevance);
core.init(extractTempRefactoringSelectedOnly);
RefactoringStatus status= extractTempRefactoringSelectedOnly.checkFinalConditions(new NullProgressMonitor());
if (status.isOK()) {
RefactoringCorrectionProposal proposal= new RefactoringCorrectionProposalExtension(label, cu, relevance, image, core);
ExtractTempRefactoringProposalCore core= new ExtractTempRefactoringProposalCore(label, cu, extractTempRefactoringSelectedOnly, relevance, preview);
RefactoringCorrectionProposal proposal= new RefactoringCorrectionProposalExtension(label, cu, relevance, image, core);

proposal.setCommandId(EXTRACT_LOCAL_NOT_REPLACE_ID);
proposal.setLinkedProposalModel(linkedProposalModel);
proposals.add(proposal);
}
proposal.setCommandId(EXTRACT_LOCAL_NOT_REPLACE_ID);
proposal.setLinkedProposalModel(linkedProposalModel);
proposals.add(proposal);
}

ExtractTempRefactoring extractTempRefactoring= new ExtractTempRefactoring(context.getASTRoot(), context.getSelectionOffset(), context.getSelectionLength());
Expand All @@ -566,25 +563,23 @@ private static boolean getExtractVariableProposal(IInvocationContext context, bo
extractTempRefactoring.setCheckResultForCompileProblems(false);

String label= CorrectionMessages.QuickAssistProcessor_extract_to_local_all_description;
String preview= CorrectionMessages.QuickAssistProcessor_extract_to_local_all_preview;
Image image= JavaPluginImages.get(JavaPluginImages.IMG_CORRECTION_LOCAL);
int relevance;
if (context.getSelectionLength() == 0) {
relevance= IProposalRelevance.EXTRACT_LOCAL_ALL_ZERO_SELECTION;
} else if (problemsAtLocation) {
relevance= IProposalRelevance.EXTRACT_LOCAL_ALL_ERROR;
} else {
relevance= IProposalRelevance.EXTRACT_LOCAL;
relevance= IProposalRelevance.EXTRACT_LOCAL_ALL;
}
ExtractTempRefactoringProposalCore core= new ExtractTempRefactoringProposalCore(label, cu, extractTempRefactoring, relevance);
core.init(extractTempRefactoring);
RefactoringStatus status= extractTempRefactoring.checkFinalConditions(new NullProgressMonitor());
if (status.isOK()) {
RefactoringCorrectionProposal proposal= new RefactoringCorrectionProposalExtension(label, cu, relevance, image, core);
ExtractTempRefactoringProposalCore core= new ExtractTempRefactoringProposalCore(label, cu,
extractTempRefactoring, relevance, preview);
RefactoringCorrectionProposal proposal= new RefactoringCorrectionProposalExtension(label, cu, relevance, image, core);

proposal.setCommandId(EXTRACT_LOCAL_ID);
proposal.setLinkedProposalModel(linkedProposalModel);
proposals.add(proposal);
}
proposal.setCommandId(EXTRACT_LOCAL_ID);
proposal.setLinkedProposalModel(linkedProposalModel);
proposals.add(proposal);
}

ExtractConstantRefactoring extractConstRefactoring= new ExtractConstantRefactoring(context.getASTRoot(), context.getSelectionOffset(), context.getSelectionLength());
Expand Down Expand Up @@ -614,15 +609,23 @@ private static boolean getExtractVariableProposal(IInvocationContext context, bo
}

private static class ExtractTempRefactoringProposalCore extends RefactoringCorrectionProposalCore {
public ExtractTempRefactoringProposalCore(String name, ICompilationUnit cu, Refactoring refactoring, int relevance) {
private final String fPreview;

public ExtractTempRefactoringProposalCore(String name, ICompilationUnit cu, Refactoring refactoring, int relevance, String preview) {
super(name, cu, refactoring, relevance);
fPreview= preview;
}

@Override
protected void init(Refactoring refactoring) throws CoreException {
ExtractTempRefactoring etr= (ExtractTempRefactoring) refactoring;
etr.setTempName(etr.guessTempName()); // expensive
}

@Override
public Object getAdditionalProposalInfo(IProgressMonitor monitor) {
return fPreview;
}
}


Expand Down

0 comments on commit fe167c1

Please sign in to comment.