Don't offer extract to constant unless expression is final and static (…

…#1182) * Don't offer extract to constant unless expression is final and static - fixes #1180 - add new logic to QuickAssistProcessor.getExtractVariableProposal() - add new tests to AssistQuickFixTest
eclipse-jdt · Feb 9, 2024 · dc8daea · dc8daea
1 parent c61554b
commit dc8daea
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 6 deletions.
diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest.java
@@ -80,6 +80,7 @@ public class AssistQuickFixTest extends QuickFixTest {
 	private static final String CHANGE_MODIFIER_TO_FINAL= FixMessages.VariableDeclarationFix_changeModifierOfUnknownToFinal_description;
 	private static final String EXTRACT_TO_LOCAL= CorrectionMessages.QuickAssistProcessor_extract_to_local_description;
 	private static final String EXTRACT_TO_LOCAL_REPLACE= CorrectionMessages.QuickAssistProcessor_extract_to_local_all_description;
+	private static final String EXTRACT_TO_CONSTANT= CorrectionMessages.QuickAssistProcessor_extract_to_constant_description;
 
 	private IJavaProject fJProject1;
 	private IPackageFragmentRoot fSourceFolder;
@@ -2103,9 +2104,79 @@ public void testExtractToLocalVariable5() throws Exception { //https://github.co
 		AssistContext context= getCorrectionContext(cu, offset, selection.length());
 		List<IJavaCompletionProposal> proposals= collectAssists(context, false);
 
-		assertNumberOfProposals(proposals, 4);
+		assertNumberOfProposals(proposals, 3);
 		assertProposalDoesNotExist(proposals, EXTRACT_TO_LOCAL);
 		assertProposalDoesNotExist(proposals, EXTRACT_TO_LOCAL_REPLACE);
+		assertProposalDoesNotExist(proposals, EXTRACT_TO_CONSTANT);
+	}
+
+	@Test
+	public void testExtractToConstant1() throws Exception {
+		IPackageFragment pack1= fSourceFolder.createPackageFragment("test", false, null);
+		StringBuilder buf= new StringBuilder();
+		buf.append("package test;\n");
+		buf.append("\n");
+		buf.append("class E {\n");
+		buf.append("    public final static E instance= new E();\n");
+		buf.append("    \n");
+		buf.append("    int s;\n");
+		buf.append("\n");
+		buf.append("    final static int f() {\n");
+		buf.append("        System.out.println(E.instance.s + 1);\n");
+		buf.append("        return 1;\n");
+		buf.append("    }\n");
+		buf.append("}\n");
+		ICompilationUnit cu= pack1.createCompilationUnit("E.java", buf.toString(), false, null);
+
+		String selection= "E.instance.s + 1";
+		int offset= buf.toString().indexOf(selection);
+		AssistContext context= getCorrectionContext(cu, offset, selection.length());
+		List<IJavaCompletionProposal> proposals= collectAssists(context, false);
+
+		assertProposalDoesNotExist(proposals, EXTRACT_TO_CONSTANT);
+	}
+
+	@Test
+	public void testExtractToConstant2() throws Exception {
+		IPackageFragment pack1= fSourceFolder.createPackageFragment("test", false, null);
+		StringBuilder buf= new StringBuilder();
+		buf.append("package test;\n");
+		buf.append("\n");
+		buf.append("class E {\n");
+		buf.append("    public final static E instance= new E();\n");
+		buf.append("\n");
+		buf.append("    static final int t = 5;\n");
+		buf.append("\n");
+		buf.append("    int f1() {\n");
+		buf.append("        return 23 * E.t;  \n");
+		buf.append("    }\n");
+		buf.append("}\n");
+		ICompilationUnit cu= pack1.createCompilationUnit("E.java", buf.toString(), false, null);
+
+		String selection= "23 * E.t";
+		int offset= buf.toString().indexOf(selection);
+		AssistContext context= getCorrectionContext(cu, offset, selection.length());
+		List<IJavaCompletionProposal> proposals= collectAssists(context, false);
+
+		assertCorrectLabels(proposals);
+
+		buf= new StringBuilder();
+		buf.append("package test;\n");
+		buf.append("\n");
+		buf.append("class E {\n");
+		buf.append("    public final static E instance= new E();\n");
+		buf.append("\n");
+		buf.append("    static final int t = 5;\n");
+		buf.append("\n");
+		buf.append("    private static final int INT = 23 * E.t;\n");
+		buf.append("\n");
+		buf.append("    int f1() {\n");
+		buf.append("        return INT;  \n");
+		buf.append("    }\n");
+		buf.append("}\n");
+		String ex1= buf.toString();
+
+		assertExpectedExistInProposals(proposals, new String[] { ex1 });
 	}
 
 	@Test

diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest1d8.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest1d8.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2013, 2023 IBM Corporation and others.
+ * Copyright (c) 2013, 2024 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -1666,7 +1666,7 @@ public void testConvertToAnonymousClassCreation1() throws Exception {
 		assertNoErrors(context);
 		List<IJavaCompletionProposal> proposals= collectAssists(context, false);
 
-		assertNumberOfProposals(proposals, 4);
+		assertNumberOfProposals(proposals, 3);
 		assertCorrectLabels(proposals);
 
 		buf= new StringBuilder();
@@ -1765,7 +1765,7 @@ public void testConvertToAnonymousClassCreation3() throws Exception {
 		assertNoErrors(context);
 		List<IJavaCompletionProposal> proposals= collectAssists(context, false);
 
-		assertNumberOfProposals(proposals, 6);
+		assertNumberOfProposals(proposals, 5);
 		assertCorrectLabels(proposals);
 
 		buf= new StringBuilder();
@@ -5023,7 +5023,7 @@ public void testFixParenthesesInLambdaExpressionCannotRemove1() throws Exception
 		AssistContext context= getCorrectionContext(cu, offset, 0);
 		assertNoErrors(context);
 		List<IJavaCompletionProposal> proposals= collectAssists(context, false);
-		assertNumberOfProposals(proposals, 5);
+		assertNumberOfProposals(proposals, 4);
 		assertCorrectLabels(proposals);
 		assertProposalDoesNotExist(proposals, CorrectionMessages.QuickAssistProcessor_removeParenthesesInLambda);
 	}

diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickAssistProcessor.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickAssistProcessor.java
@@ -588,7 +588,7 @@ private static boolean getExtractVariableProposal(IInvocationContext context, bo
 		}
 
 		ExtractConstantRefactoring extractConstRefactoring= new ExtractConstantRefactoring(context.getASTRoot(), context.getSelectionOffset(), context.getSelectionLength());
-		if (extractConstRefactoring.checkInitialConditions(new NullProgressMonitor()).isOK()) {
+		if (extractConstRefactoring.checkInitialConditions(new NullProgressMonitor()).isOK() && extractConstRefactoring.selectionAllStaticFinal()) {
 			LinkedProposalModelCore linkedProposalModel= createProposalModel();
 			extractConstRefactoring.setLinkedProposalModel(linkedProposalModel);
 			extractConstRefactoring.setCheckResultForCompileProblems(false);