diff --git a/core/org.eclipse.cdt.ui.tests/.classpath b/core/org.eclipse.cdt.ui.tests/.classpath index 918a14ebbc8..b4784f4c94f 100644 --- a/core/org.eclipse.cdt.ui.tests/.classpath +++ b/core/org.eclipse.cdt.ui.tests/.classpath @@ -2,7 +2,7 @@ - + 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 4af03ef8a88..e5c000a63d1 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 894da93baf9..934830e40d1 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 3606b7524b6..d99cc6daf13 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 Predicate not(Predicate 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 9c85daf9f2e..9500e6b71bd 100644 --- a/core/org.eclipse.cdt.ui/.classpath +++ b/core/org.eclipse.cdt.ui/.classpath @@ -4,7 +4,7 @@ - + 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 3933fad1414..ca337be63c5 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 bc93949aadd..9e6eb758184 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 0d413a2c32b..f6f0c702a6f 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 256328d053d..dcc0a16a0d5 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 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 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 fadf1de0698..5b05ed2306c 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 literalsToReplace = new ArrayList(); 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 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(); - } - } - - 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; - - default: - break; + RefactoringStatus status = super.checkInitialConditions(subMonitor.split(8)); + if (status.hasError()) { + 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); - } - - private ArrayList findAllDeclaredNames() { - ArrayListnames = new ArrayList(); - 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()); - } - } - } + if (selectedRegion == null) { + 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(); - } - - IASTNode spec = decl.getParent().getParent(); - if (spec instanceof ICPPASTCompositeTypeSpecifier) { - ICPPASTCompositeTypeSpecifier compTypeSpec = (ICPPASTCompositeTypeSpecifier) spec; - return compTypeSpec; - } - } + IASTExpression selectedExpression = findSelectedExpression(subMonitor.split(1)); + if (selectedExpression == null) { + status.addFatalError(Messages.ExtractConstantRefactoring_LiteralMustBeSelected); + return status; } - return null; - } - private void findAllNodesForReplacement(Collection 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); - } - } + if (!isExtractableExpression(selectedExpression)) { + status.addFatalError(Messages.ExtractConstantRefactoring_LiteralMustBeSelected); } - } - private boolean isOneMarked(Collection 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; - } - } + Collection literalExpressionCollection = findAllLiterals(subMonitor.split(1)); + if (literalExpressionCollection.isEmpty()) { + status.addFatalError(Messages.ExtractConstantRefactoring_LiteralMustBeSelected); + return status; } - return oneMarked; - } - 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; - } + target = selectedExpression; - private boolean isTargetChild(IASTExpression child) { - if (target == null) { - return false; + if (info.getName().isEmpty()) { + info.setName(getDefaultName(target)); } - IASTNode node = child; - while (node != null) { - if (node.getParent() == target) - return true; - node = node.getParent(); + 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 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 isExtractableExpression(IASTExpression expression) { + if (expression instanceof IASTLiteralExpression) { + return true; + } + if (expression instanceof IASTUnaryExpression) { + IASTUnaryExpression unaryExpression = (IASTUnaryExpression) expression; + return isExtractableExpression(unaryExpression.getOperand()); + } + if (expression instanceof IASTBinaryExpression) { + IASTBinaryExpression binaryExpression = (IASTBinaryExpression) expression; + return isExtractableExpression(binaryExpression.getOperand1()) && + isExtractableExpression(binaryExpression.getOperand2()); } return false; } - private Collection findAllLiterals(IProgressMonitor pm) - throws OperationCanceledException, CoreException { - final Collection result = new ArrayList(); + 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; + } - IASTTranslationUnit ast = getAST(tu, pm); + 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 findAllLiterals(IProgressMonitor monitor) + throws OperationCanceledException, CoreException { + SubMonitor subMonitor = SubMonitor.convert(monitor, 5); + + final Collection 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); - MethodContext context = info.getMethodContext(); - Collection locLiteralsToReplace = new ArrayList(); + SubMonitor subMonitor = SubMonitor.convert(monitor, 10); + Collection expressionsToReplace = findExpressionsToExtract(subMonitor.split(4)); + Collection expressionsToReplaceInSameContext = + filterLiteralsInSameContext(expressionsToReplace, subMonitor.split(3)); + replaceLiteralsWithConstant(expressionsToReplaceInSameContext, collector, subMonitor.split(2)); + insertConstantDeclaration(collector, subMonitor.split(1)); + } - IASTTranslationUnit ast = getAST(tu, progress.newChild(9)); + private void insertConstantDeclaration(ModificationCollector collector, IProgressMonitor monitor) throws CoreException { + SubMonitor subMonitor = SubMonitor.convert(monitor, 10); + + MethodContext context = info.getMethodContext(); + IASTTranslationUnit ast = getAST(tu, subMonitor.split(9)); + subMonitor.split(1); + String constName = info.getName(); if (context.getType() == MethodContext.ContextType.METHOD) { - SubMonitor loopProgress = progress.newChild(1).setWorkRemaining(literalsToReplace.size()); + 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); + } + } + + private void replaceLiteralsWithConstant(Collection 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 filterLiteralsInSameContext(Collection literalsToReplace, IProgressMonitor monitor) throws CoreException { + SubMonitor subMonitor = SubMonitor.convert(monitor, 1); + + MethodContext context = info.getMethodContext(); + Collection locLiteralsToReplace = new ArrayList<>(); + if (context.getType() == MethodContext.ContextType.METHOD) { + 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 findExpressionsToExtract(IProgressMonitor monitor) throws OperationCanceledException, CoreException { + SubMonitor subMonitor = SubMonitor.convert(monitor, 5); + + final Collection 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); + } + }); + + return result; + } + + 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()); } } - - // Create all changes for literals - String constName = info.getName(); - createLiteralToConstantChanges(constName, locLiteralsToReplace, collector); - - 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)); + 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 getArgumentMap() { - Map arguments = new HashMap(); + Map 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 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 fcfbb94c3c7..e16241c69fe 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 c1c67fa6b04..33b6323c963 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 908a81c4f8a..c226c14e23f 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 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 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 a76eaadc3b0..60eae1625b7 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 9fcad7bdd40..0d5cd13e8b7 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;