Skip to main content
summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Corbat2016-06-09 11:41:25 -0400
committerGerrit Code Review @ Eclipse.org2016-06-14 03:26:18 -0400
commitf31d960271ee94d994d0b68b26b6d7bd4409e3e0 (patch)
treee145237a5fa904c637ac009b741d7c07f04dc176
parent5d2cbaaa1c59b45ad44e486d2f876dd8100b1045 (diff)
downloadorg.eclipse.cdt-f31d960271ee94d994d0b68b26b6d7bd4409e3e0.tar.gz
org.eclipse.cdt-f31d960271ee94d994d0b68b26b6d7bd4409e3e0.tar.xz
org.eclipse.cdt-f31d960271ee94d994d0b68b26b6d7bd4409e3e0.zip
Bug 488109 Refactoring of Extract Constant Implementation
Renewed extract constant implementation, including some improvements to its functionality: - Extraction of unary/binary expression trees with literals as leafs - Improved name suggestion for integers - Improved existing name detection - Selection is more forgiving (caret in literal is accepted as selection) Patchset 2: Fixed trailing whitespace Patchset 3: Improved progress implementation, removed unnecessary generic arguments and changed legacy implementation of IndentifierHelper Patchset 4: Position of split call & line wrapping Change-Id: I49ddb8355217e82d56728cd2abe253a63937f379 Signed-off-by: Thomas Corbat <tcorbat@hsr.ch>
-rw-r--r--core/org.eclipse.cdt.ui.tests/.classpath2
-rw-r--r--core/org.eclipse.cdt.ui.tests/.settings/org.eclipse.jdt.core.prefs8
-rw-r--r--core/org.eclipse.cdt.ui.tests/META-INF/MANIFEST.MF2
-rw-r--r--core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractconstant/ExtractConstantRefactoringTest.java227
-rw-r--r--core/org.eclipse.cdt.ui/.classpath2
-rw-r--r--core/org.eclipse.cdt.ui/.settings/org.eclipse.jdt.core.prefs6
-rw-r--r--core/org.eclipse.cdt.ui/META-INF/MANIFEST.MF2
-rw-r--r--core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/MethodContext.java4
-rw-r--r--core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantInfo.java23
-rw-r--r--core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoring.java466
-rw-r--r--core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/Messages.java3
-rw-r--r--core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/Messages.properties3
-rw-r--r--core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/IdentifierHelper.java34
-rwxr-xr-xcore/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/NodeHelper.java4
-rw-r--r--core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/SelectionHelper.java11
15 files changed, 514 insertions, 283 deletions
diff --git a/core/org.eclipse.cdt.ui.tests/.classpath b/core/org.eclipse.cdt.ui.tests/.classpath
index 918a14ebbc..b4784f4c94 100644
--- a/core/org.eclipse.cdt.ui.tests/.classpath
+++ b/core/org.eclipse.cdt.ui.tests/.classpath
@@ -2,7 +2,7 @@
<classpath>
<classpathentry kind="src" path="src"/>
<classpathentry kind="src" path="ui"/>
- <classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.7"/>
+ <classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.8"/>
<classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/>
<classpathentry kind="output" path="bin"/>
</classpath>
diff --git a/core/org.eclipse.cdt.ui.tests/.settings/org.eclipse.jdt.core.prefs b/core/org.eclipse.cdt.ui.tests/.settings/org.eclipse.jdt.core.prefs
index 4af03ef8a8..e5c000a63d 100644
--- a/core/org.eclipse.cdt.ui.tests/.settings/org.eclipse.jdt.core.prefs
+++ b/core/org.eclipse.cdt.ui.tests/.settings/org.eclipse.jdt.core.prefs
@@ -1,8 +1,8 @@
eclipse.preferences.version=1
-org.eclipse.jdt.core.compiler.codegen.inlineJsrBytecode=disabled
-org.eclipse.jdt.core.compiler.codegen.targetPlatform=1.7
+org.eclipse.jdt.core.compiler.codegen.inlineJsrBytecode=enabled
+org.eclipse.jdt.core.compiler.codegen.targetPlatform=1.8
org.eclipse.jdt.core.compiler.codegen.unusedLocal=preserve
-org.eclipse.jdt.core.compiler.compliance=1.7
+org.eclipse.jdt.core.compiler.compliance=1.8
org.eclipse.jdt.core.compiler.debug.lineNumber=generate
org.eclipse.jdt.core.compiler.debug.localVariable=generate
org.eclipse.jdt.core.compiler.debug.sourceFile=generate
@@ -78,7 +78,7 @@ org.eclipse.jdt.core.compiler.problem.unusedParameterWhenOverridingConcrete=disa
org.eclipse.jdt.core.compiler.problem.unusedPrivateMember=ignore
org.eclipse.jdt.core.compiler.problem.unusedWarningToken=warning
org.eclipse.jdt.core.compiler.problem.varargsArgumentNeedCast=warning
-org.eclipse.jdt.core.compiler.source=1.7
+org.eclipse.jdt.core.compiler.source=1.8
org.eclipse.jdt.core.formatter.align_fields_grouping_blank_lines=2147483647
org.eclipse.jdt.core.formatter.align_type_members_on_columns=false
org.eclipse.jdt.core.formatter.alignment_for_arguments_in_allocation_expression=16
diff --git a/core/org.eclipse.cdt.ui.tests/META-INF/MANIFEST.MF b/core/org.eclipse.cdt.ui.tests/META-INF/MANIFEST.MF
index 894da93baf..934830e40d 100644
--- a/core/org.eclipse.cdt.ui.tests/META-INF/MANIFEST.MF
+++ b/core/org.eclipse.cdt.ui.tests/META-INF/MANIFEST.MF
@@ -37,4 +37,4 @@ Require-Bundle: org.eclipse.jface.text,
org.eclipse.core.filesystem;bundle-version="1.2.0"
Bundle-ActivationPolicy: lazy
Bundle-Vendor: Eclipse CDT
-Bundle-RequiredExecutionEnvironment: JavaSE-1.7
+Bundle-RequiredExecutionEnvironment: JavaSE-1.8
diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractconstant/ExtractConstantRefactoringTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractconstant/ExtractConstantRefactoringTest.java
index 3606b7524b..d99cc6daf1 100644
--- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractconstant/ExtractConstantRefactoringTest.java
+++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractconstant/ExtractConstantRefactoringTest.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2008, 2012 Institute for Software, HSR Hochschule fuer Technik
+ * Copyright (c) 2008, 2016 Institute for Software, HSR Hochschule fuer Technik
* Rapperswil, University of applied sciences and others
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
@@ -9,14 +9,21 @@
* Contributors:
* Institute for Software - initial API and implementation
* Sergey Prigogin (Google)
+ * Thomas Corbat (IFS)
*******************************************************************************/
package org.eclipse.cdt.ui.tests.refactoring.extractconstant;
import junit.framework.Test;
+import java.util.Arrays;
+import java.util.function.Predicate;
+import org.eclipse.core.runtime.CoreException;
+import org.junit.Before;
+
import org.eclipse.cdt.ui.tests.refactoring.RefactoringTestBase;
import org.eclipse.cdt.internal.ui.refactoring.CRefactoring;
+import org.eclipse.cdt.internal.ui.refactoring.CRefactoringContext;
import org.eclipse.cdt.internal.ui.refactoring.extractconstant.ExtractConstantInfo;
import org.eclipse.cdt.internal.ui.refactoring.extractconstant.ExtractConstantRefactoring;
import org.eclipse.cdt.internal.ui.refactoring.utils.VisibilityEnum;
@@ -25,8 +32,8 @@ import org.eclipse.cdt.internal.ui.refactoring.utils.VisibilityEnum;
* Tests for Extract Constant refactoring.
*/
public class ExtractConstantRefactoringTest extends RefactoringTestBase {
- private String extractedConstantName = "EXTRACTED";
- private VisibilityEnum visibility = VisibilityEnum.v_private;
+ private String extractedConstantName;
+ private VisibilityEnum visibility;
private ExtractConstantRefactoring refactoring;
public ExtractConstantRefactoringTest() {
@@ -41,6 +48,13 @@ public class ExtractConstantRefactoringTest extends RefactoringTestBase {
return suite(ExtractConstantRefactoringTest.class);
}
+ @Before
+ public void setUp() throws Exception {
+ extractedConstantName = "EXTRACTED";
+ visibility = VisibilityEnum.v_private;
+ super.setUp();
+ }
+
@Override
protected CRefactoring createRefactoring() {
refactoring = new ExtractConstantRefactoring(getSelectedTranslationUnit(), getSelection(),
@@ -792,4 +806,211 @@ public class ExtractConstantRefactoringTest extends RefactoringTestBase {
public void testHistoryReplaceNumberPrivate() throws Exception {
assertRefactoringSuccess();
}
+
+ //A.cpp
+ //void foo() {
+ // int a = -(/*$*/+(~(10))/*$$*/);
+ // int b = -(+(~(10)));
+ // int c = -(+(~(11)));
+ // int d = +(~(10));
+ // int e = (~10);
+ // int f = -(+(-(10)));
+ //}
+ //=
+ //A.cpp
+ //namespace {
+ //
+ //const int EXTRACTED = +(~(10));
+ //
+ //}
+ //
+ //void foo() {
+ // int a = -(EXTRACTED);
+ // int b = -(EXTRACTED);
+ // int c = -(+(~(11)));
+ // int d = EXTRACTED;
+ // int e = (~10);
+ // int f = -(+(-(10)));
+ //}
+ public void testExtractionOfUnaryExpressions() throws Exception {
+ assertRefactoringSuccess();
+ }
+
+ //A.cpp
+ //void foo() {
+ // int i = /*$*/2 * 21/*$$*/;
+ //}
+ //=
+ //namespace {
+ //
+ //const int EXTRACTED = 2 * 21;
+ //
+ //}
+ //
+ //void foo() {
+ // int i = EXTRACTED;
+ //}
+ public void testSingleBinaryExpression() throws Exception {
+ assertRefactoringSuccess();
+ }
+
+ //A.cpp
+ //void foo(int) {
+ // foo(/*$*/2 * 21/*$$*/);
+ //}
+ //=
+ //namespace {
+ //
+ //const int EXTRACTED = 2 * 21;
+ //
+ //}
+ //
+ //void foo(int) {
+ // foo(EXTRACTED);
+ //}
+ public void testBinaryExpressionInFunctionCall() throws Exception {
+ assertRefactoringSuccess();
+ }
+
+ //A.cpp
+ //void foo(int, int) {
+ // foo(/*$*/2, 21/*$$*/);
+ //}
+ public void testExtractTwoIndependentLiterals() throws Exception {
+ assertRefactoringFailure();
+ }
+
+ //A.cpp
+ //void foo(int, int) {
+ // /*$*/foo(2, 21)/*$$*/;
+ //}
+ public void testExtractFunctionCall() throws Exception {
+ assertRefactoringFailure();
+ }
+
+ //A.cpp
+ //void foo() {
+ // int i = 42;
+ //}
+ public void testNoSelection() throws Exception {
+ assertRefactoringFailure();
+ }
+
+ //A.cpp
+ //void foo() {
+ // int i = 15;
+ // int j = /*$*/i/*$$*/;
+ //}
+ public void testExtractIdentifier() throws Exception {
+ assertRefactoringFailure();
+ }
+
+ //A.cpp
+ //void foo() {
+ // int i = 4/*$*//*$$*/2;
+ //}
+ //=
+ //namespace {
+ //
+ //const int EXTRACTED = 42;
+ //
+ //}
+ //
+ //void foo() {
+ // int i = EXTRACTED;
+ //}
+ public void testCarretInLiteral() throws Exception {
+ assertRefactoringSuccess();
+ }
+
+ //A.cpp
+ //void foo() {
+ // int i = 42/*$*//*$$*/;
+ //}
+ //=
+ //namespace {
+ //
+ //const int EXTRACTED = 42;
+ //
+ //}
+ //
+ //void foo() {
+ // int i = EXTRACTED;
+ //}
+ public void testCarretAtLiteral() throws Exception {
+ assertRefactoringSuccess();
+ }
+
+
+ //A.cpp
+ //int i = /*$*/42/*$$*/;
+ public void testDefaultNameForIntegerLiteral() throws Exception {
+ runUpToInitialConditions();
+ ExtractConstantInfo refactoringInfo = refactoring.getRefactoringInfo();
+ assertEquals("_42", refactoringInfo.getName());
+ }
+
+ // A.cpp
+ // namespace NS_1 {
+ // int i_in_NS1, j_in_NS1;
+ // struct S_in_NS1;
+ // }
+ // namespace NS_2 {
+ // int i_in_NS2;
+ // struct S_in_NS2;
+ // }
+ // using NS_1::j_in_NS1;
+ // using namespace NS_2;
+ // int global_variable;
+ // void free_function();
+ // struct S {
+ // void function(int parameter) {
+ // int local_before;
+ // int local_at = /*$*/42/*$$*/;
+ // {
+ // int nested;
+ // }
+ // int local_after;
+ // }
+ // int member_variable;
+ // void member_function();
+ // };
+ public void testOccupiedAndFreeNamesInContext() throws Exception {
+ runUpToInitialConditions();
+ ExtractConstantInfo refactoringInfo = refactoring.getRefactoringInfo();
+
+ String[] expectedOccupiedNames = { "free_function", "function",
+ "global_variable", "i_in_NS2", "j_in_NS1", "local_after",
+ "local_at", "local_before", "member_function",
+ "member_variable", "parameter", "NS_1", "NS_2", "S",
+ "S_in_NS2" };
+ String[] expectedFreeNames = { "_42", "i_in_NS1", "nested", "S_in_NS1" };
+ String[] allNames = combine(expectedOccupiedNames, expectedFreeNames);
+ String[] usedNames = Arrays.stream(allNames)
+ .filter(refactoringInfo::isNameUsed)
+ .toArray(String[]::new);
+ String[] freeNames = Arrays.stream(allNames)
+ .filter(not(refactoringInfo::isNameUsed))
+ .toArray(String[]::new);
+
+ assertEquals(Arrays.toString(expectedOccupiedNames), Arrays.toString(usedNames));
+ assertEquals(Arrays.toString(expectedFreeNames), Arrays.toString(freeNames));
+ }
+
+ private <T> Predicate<? super T> not(Predicate<? super T> predicate) {
+ return predicate.negate()::test;
+ }
+
+ private String[] combine(String[] array1, String[] array2) {
+ String[] result = new String[array1.length + array2.length];
+ System.arraycopy(array1, 0, result, 0, array1.length);
+ System.arraycopy(array2, 0, result, array1.length, array2.length);
+ return result;
+ }
+
+ private void runUpToInitialConditions() throws CoreException {
+ createRefactoring();
+ refactoring.setContext(new CRefactoringContext(refactoring));
+ refactoring.checkInitialConditions(npm());
+ }
}
diff --git a/core/org.eclipse.cdt.ui/.classpath b/core/org.eclipse.cdt.ui/.classpath
index 9c85daf9f2..9500e6b71b 100644
--- a/core/org.eclipse.cdt.ui/.classpath
+++ b/core/org.eclipse.cdt.ui/.classpath
@@ -4,7 +4,7 @@
<classpathentry kind="src" path="utils.ui"/>
<classpathentry kind="src" path="browser"/>
<classpathentry kind="src" path="templateengine"/>
- <classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.7"/>
+ <classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.8"/>
<classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/>
<classpathentry kind="output" path="bin"/>
</classpath>
diff --git a/core/org.eclipse.cdt.ui/.settings/org.eclipse.jdt.core.prefs b/core/org.eclipse.cdt.ui/.settings/org.eclipse.jdt.core.prefs
index 3933fad141..ca337be63c 100644
--- a/core/org.eclipse.cdt.ui/.settings/org.eclipse.jdt.core.prefs
+++ b/core/org.eclipse.cdt.ui/.settings/org.eclipse.jdt.core.prefs
@@ -1,8 +1,8 @@
eclipse.preferences.version=1
org.eclipse.jdt.core.compiler.codegen.inlineJsrBytecode=enabled
-org.eclipse.jdt.core.compiler.codegen.targetPlatform=1.7
+org.eclipse.jdt.core.compiler.codegen.targetPlatform=1.8
org.eclipse.jdt.core.compiler.codegen.unusedLocal=preserve
-org.eclipse.jdt.core.compiler.compliance=1.7
+org.eclipse.jdt.core.compiler.compliance=1.8
org.eclipse.jdt.core.compiler.debug.lineNumber=generate
org.eclipse.jdt.core.compiler.debug.localVariable=generate
org.eclipse.jdt.core.compiler.debug.sourceFile=generate
@@ -78,7 +78,7 @@ org.eclipse.jdt.core.compiler.problem.unusedParameterWhenOverridingConcrete=disa
org.eclipse.jdt.core.compiler.problem.unusedPrivateMember=warning
org.eclipse.jdt.core.compiler.problem.unusedWarningToken=warning
org.eclipse.jdt.core.compiler.problem.varargsArgumentNeedCast=warning
-org.eclipse.jdt.core.compiler.source=1.7
+org.eclipse.jdt.core.compiler.source=1.8
org.eclipse.jdt.core.formatter.align_type_members_on_columns=false
org.eclipse.jdt.core.formatter.alignment_for_arguments_in_allocation_expression=16
org.eclipse.jdt.core.formatter.alignment_for_arguments_in_annotation=0
diff --git a/core/org.eclipse.cdt.ui/META-INF/MANIFEST.MF b/core/org.eclipse.cdt.ui/META-INF/MANIFEST.MF
index bc93949aad..9e6eb75818 100644
--- a/core/org.eclipse.cdt.ui/META-INF/MANIFEST.MF
+++ b/core/org.eclipse.cdt.ui/META-INF/MANIFEST.MF
@@ -124,4 +124,4 @@ Require-Bundle: org.eclipse.cdt.core;bundle-version="[5.2.0,7.0.0)",
com.ibm.icu;bundle-version="4.4.2",
org.eclipse.e4.ui.css.swt.theme
Bundle-ActivationPolicy: lazy
-Bundle-RequiredExecutionEnvironment: JavaSE-1.7
+Bundle-RequiredExecutionEnvironment: JavaSE-1.8
diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/MethodContext.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/MethodContext.java
index 0d413a2c32..f6f0c702a6 100644
--- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/MethodContext.java
+++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/MethodContext.java
@@ -189,6 +189,10 @@ public class MethodContext {
return bind1.equals(bind2);
}
+ public static boolean haveSameClass(MethodContext context1, MethodContext context2) {
+ return isSameClass(context1.declarationName, context2.declarationName);
+ }
+
private static ICPPClassType getClassBinding(IASTName declName1) {
if (declName1.getParent().getParent().getParent() instanceof ICPPASTCompositeTypeSpecifier) {
ICPPASTCompositeTypeSpecifier compTypeSpec =
diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantInfo.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantInfo.java
index 256328d053..dcc0a16a0d 100644
--- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantInfo.java
+++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantInfo.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2008, 2012 Institute for Software, HSR Hochschule fuer Technik
+ * Copyright (c) 2008, 2016 Institute for Software, HSR Hochschule fuer Technik
* Rapperswil, University of applied sciences and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
@@ -9,9 +9,12 @@
* Contributors:
* Institute for Software (IFS) - initial API and implementation
* Sergey Prigogin (Google)
+ * Thomas Corbat (IFS)
******************************************************************************/
package org.eclipse.cdt.internal.ui.refactoring.extractconstant;
+import java.util.function.Predicate;
+
import org.eclipse.cdt.internal.ui.refactoring.MethodContext;
import org.eclipse.cdt.internal.ui.refactoring.VariableNameInformation;
import org.eclipse.cdt.internal.ui.refactoring.utils.VisibilityEnum;
@@ -22,6 +25,16 @@ import org.eclipse.cdt.internal.ui.refactoring.utils.VisibilityEnum;
public class ExtractConstantInfo extends VariableNameInformation {
private VisibilityEnum visibility = VisibilityEnum.v_private;
private MethodContext methodContext;
+ private Predicate<String> nameUsedChecker = (String) -> false;
+ private boolean replaceAllLiterals = true;
+
+ public boolean isReplaceAllOccurences() {
+ return replaceAllLiterals;
+ }
+
+ public void setReplaceAllLiterals(boolean replaceAllLiterals) {
+ this.replaceAllLiterals = replaceAllLiterals;
+ }
public VisibilityEnum getVisibility() {
return visibility;
@@ -37,4 +50,12 @@ public class ExtractConstantInfo extends VariableNameInformation {
public void setMethodContext(MethodContext context) {
methodContext = context;
}
+
+ public void setNameUsedChecker(Predicate<String> nameOccupiedChecker) {
+ this.nameUsedChecker = nameOccupiedChecker;
+ }
+
+ public boolean isNameUsed(String name) {
+ return nameUsedChecker.test(name);
+ }
}
diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoring.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoring.java
index fadf1de069..5b05ed2306 100644
--- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoring.java
+++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoring.java
@@ -9,13 +9,16 @@
* Contributors:
* Institute for Software - initial API and implementation
* Sergey Prigogin (Google)
+ * Thomas Corbat (IFS)
*******************************************************************************/
package org.eclipse.cdt.internal.ui.refactoring.extractconstant;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
+import java.util.stream.Collectors;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IProgressMonitor;
@@ -23,7 +26,6 @@ import org.eclipse.core.runtime.OperationCanceledException;
import org.eclipse.core.runtime.Platform;
import org.eclipse.core.runtime.SubMonitor;
import org.eclipse.core.runtime.preferences.IPreferencesService;
-import org.eclipse.jface.text.Region;
import org.eclipse.jface.viewers.ISelection;
import org.eclipse.ltk.core.refactoring.RefactoringDescriptor;
import org.eclipse.ltk.core.refactoring.RefactoringStatus;
@@ -31,13 +33,13 @@ import org.eclipse.text.edits.TextEditGroup;
import org.eclipse.cdt.core.dom.ast.ASTNodeFactoryFactory;
import org.eclipse.cdt.core.dom.ast.ASTVisitor;
+import org.eclipse.cdt.core.dom.ast.IASTBinaryExpression;
import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier;
import org.eclipse.cdt.core.dom.ast.IASTDeclaration;
import org.eclipse.cdt.core.dom.ast.IASTDeclarator;
import org.eclipse.cdt.core.dom.ast.IASTEqualsInitializer;
import org.eclipse.cdt.core.dom.ast.IASTExpression;
-import org.eclipse.cdt.core.dom.ast.IASTFileLocation;
-import org.eclipse.cdt.core.dom.ast.IASTFunctionDefinition;
+import org.eclipse.cdt.core.dom.ast.IASTIdExpression;
import org.eclipse.cdt.core.dom.ast.IASTLiteralExpression;
import org.eclipse.cdt.core.dom.ast.IASTMacroExpansionLocation;
import org.eclipse.cdt.core.dom.ast.IASTNode;
@@ -46,6 +48,7 @@ import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
import org.eclipse.cdt.core.dom.ast.IASTUnaryExpression;
import org.eclipse.cdt.core.dom.ast.IBinding;
import org.eclipse.cdt.core.dom.ast.INodeFactory;
+import org.eclipse.cdt.core.dom.ast.IScope;
import org.eclipse.cdt.core.dom.ast.IType;
import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTCompositeTypeSpecifier;
import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTNamespaceDefinition;
@@ -54,24 +57,19 @@ import org.eclipse.cdt.core.dom.rewrite.ASTRewrite;
import org.eclipse.cdt.core.dom.rewrite.DeclarationGenerator;
import org.eclipse.cdt.core.model.ICElement;
import org.eclipse.cdt.core.model.ICProject;
-import org.eclipse.cdt.core.model.ITranslationUnit;
import org.eclipse.cdt.ui.CUIPlugin;
import org.eclipse.cdt.ui.PreferenceConstants;
-import org.eclipse.cdt.internal.core.dom.parser.ASTQueries;
-import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTEqualsInitializer;
-import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTIdExpression;
-import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTLiteralExpression;
-import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTName;
-import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTSimpleDeclaration;
-import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPMethod;
+import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVisitor;
import org.eclipse.cdt.internal.ui.refactoring.CRefactoring;
import org.eclipse.cdt.internal.ui.refactoring.CRefactoringDescriptor;
import org.eclipse.cdt.internal.ui.refactoring.ClassMemberInserter;
import org.eclipse.cdt.internal.ui.refactoring.MethodContext;
import org.eclipse.cdt.internal.ui.refactoring.ModificationCollector;
+import org.eclipse.cdt.internal.ui.refactoring.utils.IdentifierHelper;
import org.eclipse.cdt.internal.ui.refactoring.utils.NodeHelper;
+import org.eclipse.cdt.internal.ui.refactoring.utils.SelectionHelper;
import org.eclipse.cdt.internal.ui.util.NameComposer;
/**
@@ -80,201 +78,146 @@ import org.eclipse.cdt.internal.ui.util.NameComposer;
* @author Mirko Stocker
*/
public class ExtractConstantRefactoring extends CRefactoring {
+ private final class SelectedExpressionFinderVisitor extends ASTVisitor {
+ {
+ shouldVisitExpressions = true;
+ }
+
+ private IASTExpression selectedExpression;
+
+ @Override
+ public int visit(IASTExpression expression) {
+ if (SelectionHelper.nodeMatchesSelection(expression, selectedRegion)) {
+ selectedExpression = expression;
+ return PROCESS_ABORT;
+ } else if (expression instanceof IASTLiteralExpression &&
+ SelectionHelper.isSelectionInsideNode(expression, selectedRegion)) {
+ selectedExpression = expression;
+ return PROCESS_ABORT;
+ }
+ return super.visit(expression);
+ }
+ }
+
+ private static final String PREFIX_FOR_NAME_WITH_LEADING_DIGIT = "_"; //$NON-NLS-1$
+
public static final String ID =
"org.eclipse.cdt.ui.refactoring.extractconstant.ExtractConstantRefactoring"; //$NON-NLS-1$
- private IASTLiteralExpression target;
+ private IASTExpression target;
private final ExtractConstantInfo info;
- private final ArrayList<IASTExpression> literalsToReplace = new ArrayList<IASTExpression>();
public ExtractConstantRefactoring(ICElement element, ISelection selection, ICProject project) {
super(element, selection, project);
this.info = new ExtractConstantInfo();
- name = Messages.ExtractConstantRefactoring_ExtractConst;
+ name = Messages.ExtractConstantRefactoring_ExtractConst;
}
@Override
- public RefactoringStatus checkInitialConditions(IProgressMonitor pm) throws CoreException, OperationCanceledException {
- SubMonitor sm = SubMonitor.convert(pm, 10);
+ public RefactoringStatus checkInitialConditions(IProgressMonitor monitor) throws CoreException, OperationCanceledException {
+ SubMonitor subMonitor = SubMonitor.convert(monitor, 12);
- try {
- RefactoringStatus status = super.checkInitialConditions(sm.newChild(8));
- if (status.hasError()) {
- return status;
- }
-
- Collection<IASTLiteralExpression> literalExpressionCollection = findAllLiterals(sm.newChild(1));
- if (literalExpressionCollection.isEmpty()) {
- initStatus.addFatalError(Messages.ExtractConstantRefactoring_LiteralMustBeSelected);
- return initStatus;
- }
-
- boolean oneMarked =
- selectedRegion != null && isOneMarked(literalExpressionCollection, selectedRegion);
- if (!oneMarked) {
- // None or more than one literal selected
- if (target == null) {
- // No l found;
- initStatus.addFatalError(Messages.ExtractConstantRefactoring_NoLiteralSelected);
- } else {
- // To many selection found
- initStatus.addFatalError(Messages.ExtractConstantRefactoring_TooManyLiteralSelected);
- }
- return initStatus;
- }
-
- findAllNodesForReplacement(literalExpressionCollection);
-
- info.addNamesToUsedNames(findAllDeclaredNames());
- if (info.getName().isEmpty()) {
- info.setName(getDefaultName(target));
- }
- info.setMethodContext(NodeHelper.findMethodContext(target, refactoringContext, sm.newChild(1)));
- return initStatus;
- } finally {
- sm.done();
+ RefactoringStatus status = super.checkInitialConditions(subMonitor.split(8));
+ if (status.hasError()) {
+ return status;
}
- }
- private String getDefaultName(IASTLiteralExpression literal) {
- String nameString = literal.toString();
- switch (literal.getKind()) {
- case IASTLiteralExpression.lk_char_constant:
- case IASTLiteralExpression.lk_string_literal:
- int beginIndex = 1;
- if (nameString.startsWith("L")) { //$NON-NLS-1$
- beginIndex = 2;
- }
- final int len= nameString.length();
- if (beginIndex < len && len > 0) {
- nameString = nameString.substring(beginIndex, len - 1);
- }
- break;
+ if (selectedRegion == null) {
+ status.addFatalError(Messages.ExtractConstantRefactoring_LiteralMustBeSelected);
+ return status;
+ }
- default:
- break;
+ IASTExpression selectedExpression = findSelectedExpression(subMonitor.split(1));
+ if (selectedExpression == null) {
+ status.addFatalError(Messages.ExtractConstantRefactoring_LiteralMustBeSelected);
+ return status;
}
- IPreferencesService preferences = Platform.getPreferencesService();
- int capitalization = preferences.getInt(CUIPlugin.PLUGIN_ID,
- PreferenceConstants.NAME_STYLE_CONSTANT_CAPITALIZATION,
- PreferenceConstants.NAME_STYLE_CAPITALIZATION_UPPER_CASE, null);
- String wordDelimiter = preferences.getString(CUIPlugin.PLUGIN_ID,
- PreferenceConstants.NAME_STYLE_CONSTANT_WORD_DELIMITER, "_", null); //$NON-NLS-1$
- String prefix = preferences.getString(CUIPlugin.PLUGIN_ID,
- PreferenceConstants.NAME_STYLE_CONSTANT_PREFIX, "", null); //$NON-NLS-1$
- String suffix = preferences.getString(CUIPlugin.PLUGIN_ID,
- PreferenceConstants.NAME_STYLE_CONSTANT_SUFFIX, "", null); //$NON-NLS-1$
- NameComposer composer = new NameComposer(capitalization, wordDelimiter, prefix, suffix);
- return composer.compose(nameString);
- }
+ if (!isExtractableExpression(selectedExpression)) {
+ status.addFatalError(Messages.ExtractConstantRefactoring_LiteralMustBeSelected);
+ }
- private ArrayList<String> findAllDeclaredNames() {
- ArrayList<String>names = new ArrayList<String>();
- IASTFunctionDefinition funcDef = ASTQueries.findAncestorWithType(target, IASTFunctionDefinition.class);
- ICPPASTCompositeTypeSpecifier comTypeSpec = getCompositeTypeSpecifier(funcDef);
- if (comTypeSpec != null) {
- for(IASTDeclaration dec : comTypeSpec.getMembers()) {
- if (dec instanceof IASTSimpleDeclaration) {
- IASTSimpleDeclaration simpDec = (IASTSimpleDeclaration) dec;
- for(IASTDeclarator decor : simpDec.getDeclarators()) {
- names.add(decor.getName().getRawSignature());
- }
- }
- }
+ Collection<IASTLiteralExpression> literalExpressionCollection = findAllLiterals(subMonitor.split(1));
+ if (literalExpressionCollection.isEmpty()) {
+ status.addFatalError(Messages.ExtractConstantRefactoring_LiteralMustBeSelected);
+ return status;
}
- return names;
- }
- private ICPPASTCompositeTypeSpecifier getCompositeTypeSpecifier(IASTFunctionDefinition funcDef) {
- if (funcDef != null) {
- IBinding binding = funcDef.getDeclarator().getName().resolveBinding();
- if (binding instanceof CPPMethod) {
- CPPMethod methode = (CPPMethod) binding;
- IASTNode[] declarations = methode.getDeclarations();
-
- IASTNode decl;
- if (declarations != null) {
- decl = declarations[0];
- } else {
- decl = methode.getDefinition();
- }
+ target = selectedExpression;
- IASTNode spec = decl.getParent().getParent();
- if (spec instanceof ICPPASTCompositeTypeSpecifier) {
- ICPPASTCompositeTypeSpecifier compTypeSpec = (ICPPASTCompositeTypeSpecifier) spec;
- return compTypeSpec;
- }
- }
+ if (info.getName().isEmpty()) {
+ info.setName(getDefaultName(target));
}
- return null;
+ info.setMethodContext(NodeHelper.findMethodContext(target, refactoringContext, subMonitor.split(1)));
+ subMonitor.split(1);
+ IScope containingScope = CPPVisitor.getContainingScope(target);
+ IASTTranslationUnit ast = target.getTranslationUnit();
+ info.setNameUsedChecker((String name) -> {
+ IBinding[] bindingsForName = containingScope.find(name, ast);
+ return bindingsForName.length != 0;
+ });
+ return status;
}
- private void findAllNodesForReplacement(Collection<IASTLiteralExpression> literalExpressionCollection) {
- if (target.getParent() instanceof IASTUnaryExpression) {
- IASTUnaryExpression unary = (IASTUnaryExpression) target.getParent();
- for (IASTLiteralExpression expression : literalExpressionCollection) {
- if (target.getKind() == expression.getKind()
- && target.toString().equals(expression.toString())
- && expression.getParent() instanceof IASTUnaryExpression
- && unary.getOperator() == ((IASTUnaryExpression)expression.getParent()).getOperator()) {
- literalsToReplace.add(((IASTUnaryExpression)expression.getParent()));
- }
- }
- } else {
- for (IASTLiteralExpression expression : literalExpressionCollection) {
- if (target.getKind() == expression.getKind()
- && target.toString().equals(expression.toString())) {
- literalsToReplace.add(expression);
- }
- }
- }
+ private IASTExpression findSelectedExpression(IProgressMonitor monitor)
+ throws OperationCanceledException, CoreException {
+ SubMonitor subMonitor = SubMonitor.convert(monitor, 5);
+
+ IASTTranslationUnit ast = getAST(tu, subMonitor.split(4));
+ subMonitor.split(1);
+ SelectedExpressionFinderVisitor expressionFinder = new SelectedExpressionFinderVisitor();
+ ast.accept(expressionFinder);
+ return expressionFinder.selectedExpression;
}
- private boolean isOneMarked(Collection<IASTLiteralExpression> selectedNodes,
- Region textSelection) {
- boolean oneMarked = false;
- for (IASTLiteralExpression expression : selectedNodes) {
- if (expression.isPartOfTranslationUnitFile() &&
- isExpressionInSelection(expression, textSelection)) {
- if (target == null) {
- target = expression;
- oneMarked = true;
- } else if (!isTargetChild(expression)) {
- oneMarked = false;
- }
- }
+ private boolean isExtractableExpression(IASTExpression expression) {
+ if (expression instanceof IASTLiteralExpression) {
+ return true;
+ }
+ if (expression instanceof IASTUnaryExpression) {
+ IASTUnaryExpression unaryExpression = (IASTUnaryExpression) expression;
+ return isExtractableExpression(unaryExpression.getOperand());
}
- return oneMarked;
+ if (expression instanceof IASTBinaryExpression) {
+ IASTBinaryExpression binaryExpression = (IASTBinaryExpression) expression;
+ return isExtractableExpression(binaryExpression.getOperand1()) &&
+ isExtractableExpression(binaryExpression.getOperand2());
+ }
+ return false;
}
- private static boolean isExpressionInSelection(IASTExpression expression, Region selection) {
- IASTFileLocation location = expression.getFileLocation();
- int expressionStart = location.getNodeOffset();
- int expressionEnd = expressionStart + location.getNodeLength();
- int selectionStart = selection.getOffset();
- int selectionEnd = selectionStart + selection.getLength();
- return expressionStart >= selectionStart && expressionEnd <= selectionEnd;
+ private String getDefaultName(IASTExpression expression) {
+ String nameString = expression.getRawSignature();
+ NameComposer composer = createNameComposer();
+ String composedName = composer.compose(nameString);
+ if (IdentifierHelper.isLeadingADigit(composedName)) {
+ composedName = PREFIX_FOR_NAME_WITH_LEADING_DIGIT + composedName;
+ }
+ return composedName;
}
- private boolean isTargetChild(IASTExpression child) {
- if (target == null) {
- return false;
- }
- IASTNode node = child;
- while (node != null) {
- if (node.getParent() == target)
- return true;
- node = node.getParent();
- }
- return false;
+ private static NameComposer createNameComposer() {
+ IPreferencesService preferences = Platform.getPreferencesService();
+ int capitalization = preferences.getInt(CUIPlugin.PLUGIN_ID,
+ PreferenceConstants.NAME_STYLE_CONSTANT_CAPITALIZATION,
+ PreferenceConstants.NAME_STYLE_CAPITALIZATION_UPPER_CASE, null);
+ String wordDelimiter = preferences.getString(CUIPlugin.PLUGIN_ID,
+ PreferenceConstants.NAME_STYLE_CONSTANT_WORD_DELIMITER, "_", null); //$NON-NLS-1$
+ String prefix = preferences.getString(CUIPlugin.PLUGIN_ID,
+ PreferenceConstants.NAME_STYLE_CONSTANT_PREFIX, "", null); //$NON-NLS-1$
+ String suffix = preferences.getString(CUIPlugin.PLUGIN_ID,
+ PreferenceConstants.NAME_STYLE_CONSTANT_SUFFIX, "", null); //$NON-NLS-1$
+ NameComposer composer = new NameComposer(capitalization, wordDelimiter, prefix, suffix);
+ return composer;
}
- private Collection<IASTLiteralExpression> findAllLiterals(IProgressMonitor pm)
+ private Collection<IASTLiteralExpression> findAllLiterals(IProgressMonitor monitor)
throws OperationCanceledException, CoreException {
- final Collection<IASTLiteralExpression> result = new ArrayList<IASTLiteralExpression>();
+ SubMonitor subMonitor = SubMonitor.convert(monitor, 5);
- IASTTranslationUnit ast = getAST(tu, pm);
+ final Collection<IASTLiteralExpression> result = new ArrayList<>();
+ IASTTranslationUnit ast = getAST(tu, subMonitor.split(4));
+ subMonitor.split(1);
ast.accept(new ASTVisitor() {
{
shouldVisitExpressions = true;
@@ -297,71 +240,134 @@ public class ExtractConstantRefactoring extends CRefactoring {
}
@Override
- protected void collectModifications(IProgressMonitor pm, ModificationCollector collector)
+ protected void collectModifications(IProgressMonitor monitor, ModificationCollector collector)
throws CoreException, OperationCanceledException{
- SubMonitor progress = SubMonitor.convert(pm, 10);
+ SubMonitor subMonitor = SubMonitor.convert(monitor, 10);
+ Collection<IASTExpression> expressionsToReplace = findExpressionsToExtract(subMonitor.split(4));
+ Collection<IASTExpression> expressionsToReplaceInSameContext =
+ filterLiteralsInSameContext(expressionsToReplace, subMonitor.split(3));
+ replaceLiteralsWithConstant(expressionsToReplaceInSameContext, collector, subMonitor.split(2));
+ insertConstantDeclaration(collector, subMonitor.split(1));
+ }
+
+ private void insertConstantDeclaration(ModificationCollector collector, IProgressMonitor monitor) throws CoreException {
+ SubMonitor subMonitor = SubMonitor.convert(monitor, 10);
+
MethodContext context = info.getMethodContext();
- Collection<IASTExpression> locLiteralsToReplace = new ArrayList<IASTExpression>();
+ IASTTranslationUnit ast = getAST(tu, subMonitor.split(9));
+ subMonitor.split(1);
+ String constName = info.getName();
+ if (context.getType() == MethodContext.ContextType.METHOD) {
+ IASTDeclaration methodDeclaration = context.getMethodDeclaration();
+ ICPPASTCompositeTypeSpecifier classDefinition = (ICPPASTCompositeTypeSpecifier) methodDeclaration.getParent();
+ IASTDeclaration memberDeclaration = createConstantDeclarationForClass(constName);
+ ClassMemberInserter.createChange(classDefinition, info.getVisibility(), memberDeclaration, true, collector);
+ } else {
+ IASTDeclaration nodes = createGlobalConstantDeclaration(constName, ast.getASTNodeFactory());
+ ASTRewrite rewriter = collector.rewriterForTranslationUnit(ast);
+ TextEditGroup editGroup = new TextEditGroup(Messages.ExtractConstantRefactoring_CreateConstant);
+ rewriter.insertBefore(ast, getFirstNode(ast), nodes, editGroup);
+ }
+ }
- IASTTranslationUnit ast = getAST(tu, progress.newChild(9));
+ private void replaceLiteralsWithConstant(Collection<IASTExpression> literals, ModificationCollector collector, IProgressMonitor monitor) {
+ SubMonitor subMonitor = SubMonitor.convert(monitor, literals.size());
+ String constName = info.getName();
+ for (IASTExpression each : literals) {
+ subMonitor.split(1);
+ IASTTranslationUnit translationUnit = each.getTranslationUnit();
+ ASTRewrite rewrite = collector.rewriterForTranslationUnit(translationUnit);
+ INodeFactory nodeFactory = translationUnit.getASTNodeFactory();
+ IASTIdExpression idExpression = nodeFactory.newIdExpression(nodeFactory.newName(constName.toCharArray()));
+ rewrite.replace(each, idExpression, new TextEditGroup(Messages.ExtractConstantRefactoring_ReplaceLiteral));
+ }
+ }
+
+ private Collection<IASTExpression> filterLiteralsInSameContext(Collection<IASTExpression> literalsToReplace, IProgressMonitor monitor) throws CoreException {
+ SubMonitor subMonitor = SubMonitor.convert(monitor, 1);
+
+ MethodContext context = info.getMethodContext();
+ Collection<IASTExpression> locLiteralsToReplace = new ArrayList<>();
if (context.getType() == MethodContext.ContextType.METHOD) {
- SubMonitor loopProgress = progress.newChild(1).setWorkRemaining(literalsToReplace.size());
+ SubMonitor loopMonitor = subMonitor.split(literalsToReplace.size());
for (IASTExpression expression : literalsToReplace) {
- MethodContext exprContext =
- NodeHelper.findMethodContext(expression, refactoringContext, loopProgress.newChild(1));
+ MethodContext exprContext = NodeHelper.findMethodContext(expression, refactoringContext, loopMonitor.split(1));
if (exprContext.getType() == MethodContext.ContextType.METHOD) {
if (context.getMethodQName() != null) {
if (MethodContext.isSameClass(exprContext.getMethodQName(), context.getMethodQName())) {
locLiteralsToReplace.add(expression);
}
- } else {
- if (MethodContext.isSameClass(exprContext.getMethodDeclarationName(),
- context.getMethodDeclarationName())) {
- locLiteralsToReplace.add(expression);
- }
+ } else if (MethodContext.haveSameClass(exprContext, context)) {
+ locLiteralsToReplace.add(expression);
}
}
}
} else {
- for (IASTExpression expression : literalsToReplace) {
- ITranslationUnit expressionTu = expression.getTranslationUnit().getOriginatingTranslationUnit();
- if (expressionTu.getResource() != null) {
- locLiteralsToReplace.add(expression);
+ subMonitor.split(1);
+ literalsToReplace.stream()
+ .filter(expr -> expr.getTranslationUnit().getOriginatingTranslationUnit() != null)
+ .collect(Collectors.toCollection(() -> locLiteralsToReplace));
+ }
+ return locLiteralsToReplace;
+ }
+
+ private Collection<IASTExpression> findExpressionsToExtract(IProgressMonitor monitor) throws OperationCanceledException, CoreException {
+ SubMonitor subMonitor = SubMonitor.convert(monitor, 5);
+
+ final Collection<IASTExpression> result = new ArrayList<>();
+ IASTTranslationUnit ast = getAST(tu, subMonitor.split(4));
+ subMonitor.split(1);
+ ast.accept(new ASTVisitor() {
+ {
+ shouldVisitExpressions = true;
+ }
+
+ @Override
+ public int visit(IASTExpression expression) {
+ if (isSameExpressionTree(expression, target)) {
+ if (!(expression.getNodeLocations().length == 1 &&
+ expression.getNodeLocations()[0] instanceof IASTMacroExpansionLocation)) {
+ result.add(expression);
+ }
}
+ return super.visit(expression);
}
- }
+ });
- // Create all changes for literals
- String constName = info.getName();
- createLiteralToConstantChanges(constName, locLiteralsToReplace, collector);
+ return result;
+ }
- if (context.getType() == MethodContext.ContextType.METHOD) {
- ICPPASTCompositeTypeSpecifier classDefinition =
- (ICPPASTCompositeTypeSpecifier) context.getMethodDeclaration().getParent();
- ClassMemberInserter.createChange(classDefinition, info.getVisibility(),
- getConstNodesClass(constName), true, collector);
- } else {
- IASTDeclaration nodes = getConstNodesGlobal(constName, ast.getASTNodeFactory());
- ASTRewrite rewriter = collector.rewriterForTranslationUnit(ast);
- rewriter.insertBefore(ast, getFirstNode(ast), nodes,
- new TextEditGroup(Messages.ExtractConstantRefactoring_CreateConstant));
+ private static boolean isSameExpressionTree(IASTExpression expression1, IASTExpression expression2) {
+ if (expression1 instanceof IASTLiteralExpression && expression2 instanceof IASTLiteralExpression) {
+ IASTLiteralExpression literalExpression1 = (IASTLiteralExpression) expression1;
+ IASTLiteralExpression literalExpression2 = (IASTLiteralExpression) expression2;
+ return literalExpression1.getKind() == literalExpression2.getKind() &&
+ String.valueOf(expression1).equals(String.valueOf(expression2));
}
+ if (expression1 instanceof IASTUnaryExpression && expression2 instanceof IASTUnaryExpression) {
+ IASTUnaryExpression unaryExpression1 = (IASTUnaryExpression) expression1;
+ IASTUnaryExpression unaryExpression2 = (IASTUnaryExpression) expression2;
+ if (unaryExpression1.getOperator() == unaryExpression2.getOperator()) {
+ return isSameExpressionTree(unaryExpression1.getOperand(), unaryExpression2.getOperand());
+ }
+ }
+ if (expression1 instanceof IASTBinaryExpression && expression2 instanceof IASTBinaryExpression) {
+ IASTBinaryExpression binaryExpression1 = (IASTBinaryExpression) expression1;
+ IASTBinaryExpression binaryExpression2 = (IASTBinaryExpression) expression2;
+ if (binaryExpression1.getOperator() == binaryExpression2.getOperator()) {
+ return isSameExpressionTree(binaryExpression1.getOperand1(), binaryExpression2.getOperand1())
+ && isSameExpressionTree(binaryExpression1.getOperand2(), binaryExpression2.getOperand2());
+ }
+ }
+ return false;
}
/**
* @return the first node in the translation unit or null
*/
private static IASTNode getFirstNode(IASTTranslationUnit ast) {
- IASTDeclaration firstNode = null;
- for (IASTDeclaration each : ast.getDeclarations()) {
- if (firstNode == null) {
- firstNode = each;
- } else if (each.getNodeLocations() != null &&
- each.getNodeLocations()[0].getNodeOffset() < firstNode.getNodeLocations()[0].getNodeOffset()) {
- firstNode = each;
- }
- }
- return firstNode;
+ IASTDeclaration[] declarations = ast.getDeclarations();
+ return Arrays.stream(declarations).filter(decl -> decl.isPartOfTranslationUnitFile()).findFirst().orElse(null);
}
@Override
@@ -374,7 +380,7 @@ public class ExtractConstantRefactoring extends CRefactoring {
}
private Map<String, String> getArgumentMap() {
- Map<String, String> arguments = new HashMap<String, String>();
+ Map<String, String> arguments = new HashMap<>();
arguments.put(CRefactoringDescriptor.FILE_NAME, tu.getLocationURI().toString());
arguments.put(CRefactoringDescriptor.SELECTION, selectedRegion.getOffset() + "," + selectedRegion.getLength()); //$NON-NLS-1$
arguments.put(ExtractConstantRefactoringDescriptor.NAME, info.getName());
@@ -382,18 +388,7 @@ public class ExtractConstantRefactoring extends CRefactoring {
return arguments;
}
- private void createLiteralToConstantChanges(String constName,
- Iterable<? extends IASTExpression> literals, ModificationCollector collector) {
- for (IASTExpression each : literals) {
- ASTRewrite rewrite = collector.rewriterForTranslationUnit(each.getTranslationUnit());
- CPPASTIdExpression idExpression =
- new CPPASTIdExpression(new CPPASTName(constName.toCharArray()));
- rewrite.replace(each, idExpression,
- new TextEditGroup(Messages.ExtractConstantRefactoring_ReplaceLiteral));
- }
- }
-
- private IASTSimpleDeclaration getConstNodes(String newName) {
+ private IASTSimpleDeclaration createConstantDeclaration(String newName) {
ICPPNodeFactory factory = ASTNodeFactoryFactory.getDefaultCPPNodeFactory();
DeclarationGenerator generator = DeclarationGenerator.create(factory);
@@ -404,30 +399,21 @@ public class ExtractConstantRefactoring extends CRefactoring {
IASTDeclarator declarator = generator.createDeclaratorFromType(type, newName.toCharArray());
- IASTSimpleDeclaration simple = new CPPASTSimpleDeclaration();
- simple.setDeclSpecifier(declSpec);
+ IASTSimpleDeclaration simple = factory.newSimpleDeclaration(declSpec);
- IASTEqualsInitializer init = new CPPASTEqualsInitializer();
- if (target.getParent() instanceof IASTUnaryExpression) {
- IASTUnaryExpression unary = (IASTUnaryExpression) target.getParent();
- init.setInitializerClause(unary);
- } else {
- CPPASTLiteralExpression expression =
- new CPPASTLiteralExpression(target.getKind(), target.getValue());
- init.setInitializerClause(expression);
- }
+ IASTEqualsInitializer init = factory.newEqualsInitializer(target.copy());
declarator.setInitializer(init);
simple.addDeclarator(declarator);
return simple;
}
- private IASTDeclaration getConstNodesGlobal(String newName, INodeFactory nodeFactory) {
- IASTSimpleDeclaration simple = getConstNodes(newName);
+ private IASTDeclaration createGlobalConstantDeclaration(String newName, INodeFactory nodeFactory) {
+ IASTSimpleDeclaration simple = createConstantDeclaration(newName);
if (nodeFactory instanceof ICPPNodeFactory) {
ICPPASTNamespaceDefinition namespace =
- ((ICPPNodeFactory) nodeFactory).newNamespaceDefinition(new CPPASTName());
+ ((ICPPNodeFactory) nodeFactory).newNamespaceDefinition(nodeFactory.newName());
namespace.addDeclaration(simple);
return namespace;
}
@@ -436,8 +422,8 @@ public class ExtractConstantRefactoring extends CRefactoring {
return simple;
}
- private IASTDeclaration getConstNodesClass(String newName) {
- IASTSimpleDeclaration simple = getConstNodes(newName);
+ private IASTDeclaration createConstantDeclarationForClass(String newName) {
+ IASTSimpleDeclaration simple = createConstantDeclaration(newName);
simple.getDeclSpecifier().setStorageClass(IASTDeclSpecifier.sc_static);
return simple;
}
diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/Messages.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/Messages.java
index fcfbb94c3c..e16241c69f 100644
--- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/Messages.java
+++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/Messages.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2008, 2012 Institute for Software, HSR Hochschule fuer Technik
+ * Copyright (c) 2008, 2016 Institute for Software, HSR Hochschule fuer Technik
* Rapperswil, University of applied sciences and others
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
@@ -20,7 +20,6 @@ final class Messages extends NLS {
public static String ExtractConstantRefactoring_ExtractConst;
public static String ExtractConstantRefactoring_LiteralMustBeSelected;
public static String ExtractConstantRefactoring_NoLiteralSelected;
- public static String ExtractConstantRefactoring_TooManyLiteralSelected;
public static String ExtractConstantRefactoring_CreateConstant;
public static String ExtractConstantRefactoring_ReplaceLiteral;
diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/Messages.properties b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/Messages.properties
index c1c67fa6b0..33b6323c96 100644
--- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/Messages.properties
+++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/Messages.properties
@@ -1,5 +1,5 @@
###############################################################################
-# Copyright (c) 2008, 2012 Institute for Software, HSR Hochschule fuer Technik
+# Copyright (c) 2008, 2016 Institute for Software, HSR Hochschule fuer Technik
# Rapperswil, University of applied sciences and others
# All rights reserved. This program and the accompanying materials
# are made available under the terms of the Eclipse Public License v1.0
@@ -16,7 +16,6 @@ InputPage_NameAlreadyDefined=An element named ''{0}'' is already defined in this
ExtractConstantRefactoring_ExtractConst=Extract Constant
ExtractConstantRefactoring_LiteralMustBeSelected=An literal expression must be selected to activate this refactoring.
ExtractConstantRefactoring_NoLiteralSelected=No selected literal.
-ExtractConstantRefactoring_TooManyLiteralSelected=Too many literals selected.
ExtractConstantRefactoring_CreateConstant=Create constant
ExtractConstantRefactoring_ReplaceLiteral=Replace a literal
diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/IdentifierHelper.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/IdentifierHelper.java
index 908a81c4f8..c226c14e23 100644
--- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/IdentifierHelper.java
+++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/IdentifierHelper.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2008, 2012 Institute for Software, HSR Hochschule fuer Technik
+ * Copyright (c) 2008, 2016 Institute for Software, HSR Hochschule fuer Technik
* Rapperswil, University of applied sciences and others
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
@@ -29,6 +29,9 @@ import org.eclipse.cdt.internal.core.parser.token.KeywordSets;
* @author Thomas Corbat
*/
public class IdentifierHelper {
+ private static Pattern IDENTIFIER_PATTERN = Pattern.compile("[a-zA-Z_]\\w*"); //$NON-NLS-1$
+ private static Pattern INVALID_CHARACTER_PATTERN = Pattern.compile("\\W"); //$NON-NLS-1$
+
/**
* @param identifier to check
* @return an instance of IdentifierResult that holds the outcome of the validation
@@ -37,7 +40,7 @@ public class IdentifierHelper {
if (identifier == null) {
return null;
}
- if (isCorrect(identifier)) {
+ if (isValidIdentifier(identifier)) {
if (isKeyword(identifier)) {
return new IdentifierResult(IdentifierResult.KEYWORD, NLS.bind(Messages.IdentifierHelper_isKeyword, identifier));
}
@@ -54,34 +57,21 @@ public class IdentifierHelper {
}
private static boolean isKeyword(String identifier) {
- for (String currentKeyword : getKeywords()) {
- if (identifier.equals(currentKeyword)) {
- return true;
- }
- }
- return false;
+ Set<String> keywords = KeywordSets.getKeywords(KeywordSetKey.KEYWORDS, ParserLanguage.CPP);
+ return keywords.contains(identifier);
}
private static boolean hasIllegalCharacters(String identifier) {
- Pattern p = Pattern.compile("\\W"); //$NON-NLS-1$
- Matcher m = p.matcher(identifier);
+ Matcher m = INVALID_CHARACTER_PATTERN.matcher(identifier);
return m.find();
}
- private static boolean isLeadingADigit(String identifier) {
- Pattern p = Pattern.compile("\\d.*"); //$NON-NLS-1$
- Matcher m = p.matcher(identifier);
- return m.matches();
+ public static boolean isLeadingADigit(String identifier) {
+ return identifier.length() > 0 && Character.isDigit(identifier.charAt(0));
}
- private static boolean isCorrect(String identifier) {
- Pattern p = Pattern.compile("[a-zA-Z_]\\w*"); //$NON-NLS-1$
- Matcher m = p.matcher(identifier);
+ private static boolean isValidIdentifier(String identifier) {
+ Matcher m = IDENTIFIER_PATTERN.matcher(identifier);
return m.matches();
}
-
- public static String[] getKeywords() {
- Set<String> keywords= KeywordSets.getKeywords(KeywordSetKey.KEYWORDS, ParserLanguage.CPP);
- return keywords.toArray(new String[keywords.size()]);
- }
}
diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/NodeHelper.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/NodeHelper.java
index a76eaadc3b..60eae1625b 100755
--- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/NodeHelper.java
+++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/NodeHelper.java
@@ -109,7 +109,9 @@ public class NodeHelper {
context.setType(MethodContext.ContextType.FUNCTION);
}
}
- getMethodContexWithIndex(refactoringContext, translationUnit, name, context, pm);
+ if (name != null) {
+ getMethodContexWithIndex(refactoringContext, translationUnit, name, context, pm);
+ }
return context;
}
diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/SelectionHelper.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/SelectionHelper.java
index 9fcad7bdd4..0d5cd13e8b 100644
--- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/SelectionHelper.java
+++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/SelectionHelper.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2008, 2012 Institute for Software, HSR Hochschule fuer Technik
+ * Copyright (c) 2008, 2016 Institute for Software, HSR Hochschule fuer Technik
* Rapperswil, University of applied sciences and others
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
@@ -9,6 +9,7 @@
* Contributors:
* Institute for Software - initial API and implementation
* Sergey Prigogin (Google)
+ * Thomas Corbat (IFS)
*******************************************************************************/
package org.eclipse.cdt.internal.ui.refactoring.utils;
@@ -95,6 +96,14 @@ public class SelectionHelper {
return node.isPartOfTranslationUnitFile() && isNodeInsideRegion(node, selection);
}
+ public static boolean isSelectionInsideNode(IASTNode node, Region selection) {
+ return node.isPartOfTranslationUnitFile() && isRegionInside(selection, getNodeSpan(node));
+ }
+
+ public static boolean nodeMatchesSelection(IASTNode node, Region region) {
+ return getNodeSpan(node).equals(region);
+ }
+
protected static Region getNodeSpan(IASTNode region) {
int start = Integer.MAX_VALUE;
int nodeLength = 0;

Back to the top