From 58eed0ce4203ba2f6581a9c6f6820989db8945dc Mon Sep 17 00:00:00 2001 From: Sergey Prigogin Date: Fri, 17 Feb 2012 19:27:49 -0800 Subject: [PATCH] Use shared AST in Extract Function refactoring. --- .../ExtractFunctionRefactoringTest.java | 7 +- .../ui/refactoring/CRefactoring2.java | 2 +- .../ExtractConstantRefactoring.java | 72 +++-- ...xtractConstantRefactoringContribution.java | 6 +- .../ExtractFunctionRefactoring.java | 262 ++++++++---------- ...xtractFunctionRefactoringContribution.java | 2 +- ...ExtractFunctionRefactoringDescriptor.java} | 32 +-- .../ExtractFunctionRefactoringRunner.java | 27 +- .../NonExtractableStmtFinder.java | 3 +- .../extractfunction/SimilarFinderVisitor.java | 3 +- .../TrailNodeEqualityChecker.java | 44 +-- .../ExtractLocalVariableRefactoring.java | 5 +- ...tLocalVariableRefactoringContribution.java | 1 - .../ui/refactoring/utils/SelectionHelper.java | 64 +++-- .../ui/refactoring/actions/CRenameAction.java | 1 - .../actions/ExtractFunctionAction.java | 24 +- .../actions/ExtractLocalVariableAction.java | 3 +- .../actions/GettersAndSettersAction.java | 5 +- .../refactoring/actions/HideMethodAction.java | 1 + .../actions/ImplementMethodAction.java | 1 + .../actions/RefactoringAction.java | 2 +- .../actions/ToggleFunctionAction.java | 1 + 22 files changed, 259 insertions(+), 309 deletions(-) rename core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/{ExtractFunctionRefactoringDescription.java => ExtractFunctionRefactoringDescriptor.java} (71%) diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionRefactoringTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionRefactoringTest.java index cf8065c3ddd..74c9d4bfd13 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionRefactoringTest.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/extractfunction/ExtractFunctionRefactoringTest.java @@ -33,7 +33,6 @@ import org.eclipse.cdt.internal.ui.refactoring.utils.VisibilityEnum; */ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { private static final String NO_RETURN_VALUE = ""; - private ExtractFunctionInformation refactoringInfo; private String extractedFunctionName = "extracted"; private String returnValue; // Map from old names to new ones. @@ -43,6 +42,7 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { private VisibilityEnum visibility = VisibilityEnum.v_private; private boolean virtual; private boolean replaceDuplicates = true; + private ExtractFunctionRefactoring refactoring; public ExtractFunctionRefactoringTest() { super(); @@ -65,13 +65,14 @@ public class ExtractFunctionRefactoringTest extends RefactoringTestBase { @Override protected Refactoring createRefactoring() { - refactoringInfo = new ExtractFunctionInformation(); - return new ExtractFunctionRefactoring(getSelectedFile(), getSelection(), refactoringInfo, + refactoring = new ExtractFunctionRefactoring(getSelectedTranslationUnit(), getSelection(), getCProject()); + return refactoring; } @Override protected void simulateUserInput() { + ExtractFunctionInformation refactoringInfo = refactoring.getRefactoringInfo(); refactoringInfo.setMethodName(extractedFunctionName); refactoringInfo.setReplaceDuplicates(replaceDuplicates); if (refactoringInfo.getMandatoryReturnVariable() == null) { diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/CRefactoring2.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/CRefactoring2.java index 27266c4293a..392a6214030 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/CRefactoring2.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/CRefactoring2.java @@ -212,7 +212,7 @@ public abstract class CRefactoring2 extends Refactoring { @Override public int visit(IASTName name) { if (name.isPartOfTranslationUnitFile() && - SelectionHelper.isSelectionOnExpression(selectedRegion, name) && + SelectionHelper.doesNodeOverlapWithRegion(name, selectedRegion) && !(name instanceof ICPPASTQualifiedName)) { names.add(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 a9a4665bedf..0c3e9f3174a 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 @@ -98,47 +98,43 @@ public class ExtractConstantRefactoring extends CRefactoring2 { public RefactoringStatus checkInitialConditions(IProgressMonitor pm) throws CoreException, OperationCanceledException { SubMonitor sm = SubMonitor.convert(pm, 10); - RefactoringStatus status = super.checkInitialConditions(sm.newChild(7)); - if (status.hasError()) { - return status; - } - - Collection literalExpressionCollection = findAllLiterals(sm.newChild(1)); - if (literalExpressionCollection.isEmpty()) { - initStatus.addFatalError(Messages.ExtractConstantRefactoring_LiteralMustBeSelected); - return initStatus; - } - - if (isProgressMonitorCanceld(sm, initStatus)) - 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); + 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(); } - sm.worked(1); - - if (isProgressMonitorCanceld(sm, initStatus)) - return initStatus; - - findAllNodesForReplacement(literalExpressionCollection); - - info.addNamesToUsedNames(findAllDeclaredNames()); - if (info.getName().isEmpty()) { - info.setName(getDefaultName(target)); - } - info.setMethodContext(NodeHelper.findMethodContext(target, refactoringContext, sm.newChild(1))); - sm.done(); - return initStatus; } private String getDefaultName(IASTLiteralExpression literal) { diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoringContribution.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoringContribution.java index 7ecbea4523d..45ef7c16940 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoringContribution.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractconstant/ExtractConstantRefactoringContribution.java @@ -21,12 +21,10 @@ import org.eclipse.cdt.internal.ui.refactoring.CRefactoringContribution; * @author Emanuel Graf IFS */ public class ExtractConstantRefactoringContribution extends CRefactoringContribution { - @SuppressWarnings({ "unchecked", "rawtypes" }) @Override - public RefactoringDescriptor createDescriptor(String id, String project, - String description, String comment, Map arguments, int flags) - throws IllegalArgumentException { + public RefactoringDescriptor createDescriptor(String id, String project, String description, + String comment, Map arguments, int flags) throws IllegalArgumentException { if (id.equals(ExtractConstantRefactoring.ID)) { return new ExtractConstantRefactoringDescriptor(project, description, comment, arguments); } else { diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoring.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoring.java index 50b6dca145e..84cebbc093a 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoring.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoring.java @@ -20,19 +20,16 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; -import org.eclipse.core.resources.IFile; -import org.eclipse.core.resources.ResourcesPlugin; import org.eclipse.core.runtime.CoreException; -import org.eclipse.core.runtime.IPath; import org.eclipse.core.runtime.IProgressMonitor; import org.eclipse.core.runtime.OperationCanceledException; -import org.eclipse.core.runtime.Path; import org.eclipse.core.runtime.Platform; import org.eclipse.core.runtime.SubMonitor; import org.eclipse.core.runtime.preferences.IPreferencesService; import org.eclipse.jface.viewers.ISelection; import org.eclipse.ltk.core.refactoring.RefactoringDescriptor; import org.eclipse.ltk.core.refactoring.RefactoringStatus; +import org.eclipse.ltk.core.refactoring.participants.CheckConditionsContext; import org.eclipse.osgi.util.NLS; import org.eclipse.text.edits.TextEditGroup; @@ -61,6 +58,7 @@ import org.eclipse.cdt.core.dom.ast.IASTReturnStatement; import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration; import org.eclipse.cdt.core.dom.ast.IASTStandardFunctionDeclarator; import org.eclipse.cdt.core.dom.ast.IASTStatement; +import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit; import org.eclipse.cdt.core.dom.ast.IASTUnaryExpression; import org.eclipse.cdt.core.dom.ast.IBasicType; import org.eclipse.cdt.core.dom.ast.IBinding; @@ -83,6 +81,7 @@ import org.eclipse.cdt.core.dom.rewrite.ASTRewrite; import org.eclipse.cdt.core.dom.rewrite.TypeHelper; import org.eclipse.cdt.core.formatter.DefaultCodeFormatterOptions; import org.eclipse.cdt.core.index.IIndex; +import org.eclipse.cdt.core.model.ICElement; import org.eclipse.cdt.core.model.ICProject; import org.eclipse.cdt.ui.CUIPlugin; import org.eclipse.cdt.ui.PreferenceConstants; @@ -105,8 +104,8 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTTemplateDeclaration; import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil; import org.eclipse.cdt.internal.core.dom.rewrite.astwriter.ASTWriterVisitor; -import org.eclipse.cdt.internal.ui.refactoring.CRefactoring; -import org.eclipse.cdt.internal.ui.refactoring.CRefactoringDescription; +import org.eclipse.cdt.internal.ui.refactoring.CRefactoring2; +import org.eclipse.cdt.internal.ui.refactoring.CRefactoringDescriptor; import org.eclipse.cdt.internal.ui.refactoring.ClassMemberInserter; import org.eclipse.cdt.internal.ui.refactoring.Container; import org.eclipse.cdt.internal.ui.refactoring.MethodContext; @@ -122,7 +121,7 @@ import org.eclipse.cdt.internal.ui.refactoring.utils.NodeHelper; import org.eclipse.cdt.internal.ui.refactoring.utils.SelectionHelper; import org.eclipse.cdt.internal.ui.viewsupport.BasicElementLabels; -public class ExtractFunctionRefactoring extends CRefactoring { +public class ExtractFunctionRefactoring extends CRefactoring2 { public static final String ID = "org.eclipse.cdt.internal.ui.refactoring.extractfunction.ExtractFunctionRefactoring"; //$NON-NLS-1$ @@ -143,10 +142,12 @@ public class ExtractFunctionRefactoring extends CRefactoring { private INodeFactory nodeFactory; DefaultCodeFormatterOptions formattingOptions; - public ExtractFunctionRefactoring(IFile file, ISelection selection, - ExtractFunctionInformation info, ICProject project) { - super(file, selection, null, project); - this.info = info; + private IIndex index; + private IASTTranslationUnit ast; + + public ExtractFunctionRefactoring(ICElement element, ISelection selection, ICProject project) { + super(element, selection, project); + info = new ExtractFunctionInformation(); name = Messages.ExtractFunctionRefactoring_ExtractFunction; names = new HashMap(); namesCounter = new Container(NULL_INTEGER); @@ -157,81 +158,66 @@ public class ExtractFunctionRefactoring extends CRefactoring { @Override public RefactoringStatus checkInitialConditions(IProgressMonitor pm) - throws CoreException, OperationCanceledException { + throws CoreException, OperationCanceledException { SubMonitor sm = SubMonitor.convert(pm, 10); + try { - lockIndex(); - - try { - RefactoringStatus status = super.checkInitialConditions(sm.newChild(6)); - if (status.hasError()) { - return status; - } - - nodeFactory = ast.getASTNodeFactory(); - container = findExtractableNodes(); - sm.worked(1); - - if (isProgressMonitorCanceld(sm, initStatus)) - return initStatus; - - if (container.isEmpty()) { - initStatus.addFatalError(Messages.ExtractFunctionRefactoring_NoStmtSelected); - sm.done(); - return initStatus; - } - - checkForNonExtractableStatements(container, initStatus); - sm.worked(1); - - if (isProgressMonitorCanceld(sm, initStatus)) - return initStatus; - - List returnValueCandidates = container.getReturnValueCandidates(); - if (returnValueCandidates.size() > 1) { - initStatus.addFatalError(Messages.ExtractFunctionRefactoring_TooManySelected); - return initStatus; - } else if (returnValueCandidates.size() == 1) { - info.setMandatoryReturnVariable(returnValueCandidates.get(0)); - } - sm.worked(1); - - if (isProgressMonitorCanceld(sm, initStatus)) - return initStatus; - - info.setParameters(container.getParameterCandidates()); - initStatus.merge(checkParameterAndReturnTypes()); - if (initStatus.hasFatalError()) - return initStatus; - - extractor = FunctionExtractor.createFor(container.getNodesToWrite()); - - if (extractor.canChooseReturnValue() && info.getMandatoryReturnVariable() == null) { - chooseReturnVariable(); - } - - IPreferencesService preferences = Platform.getPreferencesService(); - final boolean outFirst = preferences.getBoolean(CUIPlugin.PLUGIN_ID, - PreferenceConstants.FUNCTION_OUTPUT_PARAMETERS_BEFORE_INPUT, false, - PreferenceConstants.getPreferenceScopes(project.getProject())); - info.sortParameters(outFirst); - - boolean isExtractExpression = container.getNodesToWrite().get(0) instanceof IASTExpression; - info.setExtractExpression(isExtractExpression); - - info.setDeclarator(getDeclaration(container.getNodesToWrite().get(0))); - MethodContext context = - NodeHelper.findMethodContext(container.getNodesToWrite().get(0), getIndex()); - info.setMethodContext(context); - - sm.done(); - } finally { - unlockIndex(); + RefactoringStatus status = super.checkInitialConditions(sm.newChild(8)); + if (status.hasError()) { + return status; } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); + + index = getIndex(); + ast = getAST(tu, sm.newChild(1)); + nodeFactory = ast.getASTNodeFactory(); + container = findExtractableNodes(); + + if (isProgressMonitorCanceld(sm, initStatus)) + return initStatus; + + if (container.isEmpty()) { + initStatus.addFatalError(Messages.ExtractFunctionRefactoring_NoStmtSelected); + return initStatus; + } + + checkForNonExtractableStatements(container, initStatus); + + List returnValueCandidates = container.getReturnValueCandidates(); + if (returnValueCandidates.size() > 1) { + initStatus.addFatalError(Messages.ExtractFunctionRefactoring_TooManySelected); + return initStatus; + } else if (returnValueCandidates.size() == 1) { + info.setMandatoryReturnVariable(returnValueCandidates.get(0)); + } + + info.setParameters(container.getParameterCandidates()); + initStatus.merge(checkParameterAndReturnTypes()); + if (initStatus.hasFatalError()) + return initStatus; + + extractor = FunctionExtractor.createFor(container.getNodesToWrite()); + + if (extractor.canChooseReturnValue() && info.getMandatoryReturnVariable() == null) { + chooseReturnVariable(); + } + + IPreferencesService preferences = Platform.getPreferencesService(); + final boolean outFirst = preferences.getBoolean(CUIPlugin.PLUGIN_ID, + PreferenceConstants.FUNCTION_OUTPUT_PARAMETERS_BEFORE_INPUT, false, + PreferenceConstants.getPreferenceScopes(project.getProject())); + info.sortParameters(outFirst); + + boolean isExtractExpression = container.getNodesToWrite().get(0) instanceof IASTExpression; + info.setExtractExpression(isExtractExpression); + + info.setDeclarator(getDeclaration(container.getNodesToWrite().get(0))); + MethodContext context = NodeHelper.findMethodContext(container.getNodesToWrite().get(0), + refactoringContext, sm.newChild(1)); + info.setMethodContext(context); + return initStatus; + } finally { + sm.done(); } - return initStatus; } private void chooseReturnVariable() { @@ -247,7 +233,8 @@ public class ExtractFunctionRefactoring extends CRefactoring { return; } } - if (candidate == null && !TypeHelper.shouldBePassedByReference(type, ast)) { + if (candidate == null && + !TypeHelper.shouldBePassedByReference(type, declarator.getTranslationUnit())) { candidate = param; } } @@ -256,9 +243,9 @@ public class ExtractFunctionRefactoring extends CRefactoring { candidate.setReturnValue(true); } - private void checkForNonExtractableStatements(NodeContainer cont, RefactoringStatus status) { + private void checkForNonExtractableStatements(NodeContainer container, RefactoringStatus status) { NonExtractableStmtFinder finder = new NonExtractableStmtFinder(); - for (IASTNode node : cont.getNodesToWrite()) { + for (IASTNode node : container.getNodesToWrite()) { node.accept(finder); if (finder.containsContinue()) { initStatus.addFatalError(Messages.ExtractFunctionRefactoring_Error_Continue); @@ -270,7 +257,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { } ReturnStatementFinder returnFinder = new ReturnStatementFinder(); - for (IASTNode node : cont.getNodesToWrite()) { + for (IASTNode node : container.getNodesToWrite()) { node.accept(returnFinder); if (returnFinder.containsReturn()) { initStatus.addFatalError(Messages.ExtractFunctionRefactoring_Error_Return); @@ -293,33 +280,21 @@ public class ExtractFunctionRefactoring extends CRefactoring { } @Override - public RefactoringStatus checkFinalConditions(IProgressMonitor pm) - throws CoreException, OperationCanceledException { - RefactoringStatus finalConditions = null; - try { - lockIndex(); - try { - finalConditions = super.checkFinalConditions(pm); + public RefactoringStatus checkFinalConditions(IProgressMonitor pm, CheckConditionsContext checkContext) + throws CoreException, OperationCanceledException { + RefactoringStatus finalConditions = new RefactoringStatus(); - final IASTName astMethodName = new CPPASTName(info.getMethodName().toCharArray()); - MethodContext context = - NodeHelper.findMethodContext(container.getNodesToWrite().get(0), getIndex()); + final IASTName methodName = new CPPASTName(info.getMethodName().toCharArray()); + MethodContext context = info.getMethodContext(); - if (context.getType() == ContextType.METHOD && !context.isInline()) { - ICPPASTCompositeTypeSpecifier classDeclaration = - (ICPPASTCompositeTypeSpecifier) context.getMethodDeclaration().getParent(); - IASTSimpleDeclaration methodDeclaration = getDeclaration(astMethodName); + if (context.getType() == ContextType.METHOD && !context.isInline()) { + ICPPASTCompositeTypeSpecifier classDeclaration = + (ICPPASTCompositeTypeSpecifier) context.getMethodDeclaration().getParent(); + IASTSimpleDeclaration methodDeclaration = getDeclaration(methodName); - if (isMethodAllreadyDefined(methodDeclaration, classDeclaration, getIndex())) { - finalConditions.addError(Messages.ExtractFunctionRefactoring_name_in_use); - return finalConditions; - } - } - } finally { - unlockIndex(); + if (isMethodAllreadyDefined(methodDeclaration, classDeclaration, getIndex())) { + finalConditions.addError(Messages.ExtractFunctionRefactoring_name_in_use); } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); } return finalConditions; } @@ -327,32 +302,18 @@ public class ExtractFunctionRefactoring extends CRefactoring { @Override protected void collectModifications(IProgressMonitor pm, ModificationCollector collector) throws CoreException, OperationCanceledException { - try { - lockIndex(); - try { - final IASTName astMethodName = new CPPASTName(info.getMethodName().toCharArray()); - MethodContext context = - NodeHelper.findMethodContext(container.getNodesToWrite().get(0), getIndex()); + final IASTName methodName = new CPPASTName(info.getMethodName().toCharArray()); + MethodContext context = info.getMethodContext(); - // Create Declaration in Class - if (context.getType() == ContextType.METHOD && !context.isInline()) { - createMethodDeclaration(astMethodName, context, collector); - } - // Create Method Definition - IASTNode firstNode = container.getNodesToWrite().get(0); - IPath implPath = new Path(firstNode.getContainingFilename()); - final IFile implementationFile = - ResourcesPlugin.getWorkspace().getRoot().getFileForLocation(implPath); - - createMethodDefinition(astMethodName, context, firstNode, collector); - - createMethodCalls(astMethodName, context, collector); - } finally { - unlockIndex(); - } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); + // Create declaration in class + if (context.getType() == ContextType.METHOD && !context.isInline()) { + createMethodDeclaration(methodName, context, collector); } + // Create method definition + IASTNode firstNode = container.getNodesToWrite().get(0); + createMethodDefinition(methodName, context, firstNode, collector); + + createMethodCalls(methodName, context, collector); } private void createMethodCalls(IASTName methodName, MethodContext context, @@ -443,11 +404,12 @@ public class ExtractFunctionRefactoring extends CRefactoring { methodDeclaration, false, collector); } - private void replaceSimilar(ModificationCollector collector, IASTName astMethodName) { + private void replaceSimilar(ModificationCollector collector, IASTName methodName) { // Find similar code final List nodesToRewriteWithoutComments = getNodesWithoutComments(container.getNodesToWrite()); final List initTrail = getTrail(nodesToRewriteWithoutComments); - ast.accept(new SimilarReplacerVisitor(this, container, collector, initTrail, astMethodName, + IASTTranslationUnit ast = nodesToRewriteWithoutComments.get(0).getTranslationUnit(); + ast.accept(new SimilarReplacerVisitor(this, container, collector, initTrail, methodName, nodesToRewriteWithoutComments)); } @@ -455,6 +417,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { final List nodesToRewriteWithoutComments = getNodesWithoutComments(container.getNodesToWrite()); final List initTrail = getTrail(nodesToRewriteWithoutComments); final int[] count = new int[1]; + IASTTranslationUnit ast = nodesToRewriteWithoutComments.get(0).getTranslationUnit(); ast.accept(new SimilarFinderVisitor(this, container, initTrail, nodesToRewriteWithoutComments) { @Override protected void foundSimilar() { @@ -485,10 +448,10 @@ public class ExtractFunctionRefactoring extends CRefactoring { @Override public int visitAll(IASTNode node) { if (node instanceof IASTComment) { - // Visit Comment, but don't add them to the trail + // Visit comments, but don't add them to the trail return super.visitAll(node); } else if (node instanceof IASTNamedTypeSpecifier) { - // Skip if somewhere is a named Type Specifier + // Skip if somewhere is a named type specifier trail.add(node); return PROCESS_SKIP; } else if (node instanceof IASTName) { @@ -498,7 +461,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { trail.add(node); return super.visitAll(node); } else { - // Save Name Sequence Number + // Save name sequence number IASTName name = (IASTName) node; TrailName trailName = new TrailName(name); int actCount = trailCounter.getObject().intValue(); @@ -531,7 +494,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { return trail; } - protected boolean isStatementInTrail(IASTStatement stmt, final List trail, IIndex index) { + boolean isStatementInTrail(IASTStatement stmt, final List trail) { final Container same = new Container(Boolean.TRUE); final TrailNodeEqualityChecker equalityChecker = new TrailNodeEqualityChecker(names, namesCounter, index); @@ -547,7 +510,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { } if (node instanceof IASTComment) { - // Visit Comment, but they are not in the trail + // Visit comments, but they are not in the trail return super.visitAll(node); } @@ -598,7 +561,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { (IASTParameterDeclaration) origParameterName[0].getParent().getParent(); IASTParameterDeclaration newParameter = declarator.getParameters()[i]; - // if not the same break; + // If not the same break; if (!(equalityChecker.isEquals(origParameter.getDeclSpecifier(), newParameter.getDeclSpecifier()) && ASTHelper.samePointers(origParameter.getDeclarator().getPointerOperators(), @@ -850,7 +813,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { @Override public int visit(IASTStatement stmt) { - if (SelectionHelper.isSelectedFile(region, stmt, file)) { + if (SelectionHelper.isNodeInsideSelection(stmt, selectedRegion)) { container.add(stmt); return PROCESS_SKIP; } @@ -859,7 +822,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { @Override public int visit(IASTExpression expression) { - if (SelectionHelper.isSelectedFile(region, expression, file)) { + if (SelectionHelper.isNodeInsideSelection(expression, selectedRegion)) { container.add(expression); return PROCESS_SKIP; } @@ -896,19 +859,20 @@ public class ExtractFunctionRefactoring extends CRefactoring { protected RefactoringDescriptor getRefactoringDescriptor() { Map arguments = getArgumentMap(); RefactoringDescriptor desc = - new ExtractFunctionRefactoringDescription(project.getProject().getName(), + new ExtractFunctionRefactoringDescriptor(project.getProject().getName(), "Extract Method Refactoring", "Create method " + info.getMethodName(), arguments); //$NON-NLS-1$//$NON-NLS-2$ return desc; } private Map getArgumentMap() { Map arguments = new HashMap(); - arguments.put(CRefactoringDescription.FILE_NAME, file.getLocationURI().toString()); - arguments.put(CRefactoringDescription.SELECTION, region.getOffset() + "," + region.getLength()); //$NON-NLS-1$ - arguments.put(ExtractFunctionRefactoringDescription.NAME, info.getMethodName()); - arguments.put(ExtractFunctionRefactoringDescription.VISIBILITY, + arguments.put(CRefactoringDescriptor.FILE_NAME, tu.getLocationURI().toString()); + arguments.put(CRefactoringDescriptor.SELECTION, + selectedRegion.getOffset() + "," + selectedRegion.getLength()); //$NON-NLS-1$ + arguments.put(ExtractFunctionRefactoringDescriptor.NAME, info.getMethodName()); + arguments.put(ExtractFunctionRefactoringDescriptor.VISIBILITY, info.getVisibility().toString()); - arguments.put(ExtractFunctionRefactoringDescription.REPLACE_DUPLICATES, + arguments.put(ExtractFunctionRefactoringDescriptor.REPLACE_DUPLICATES, Boolean.toString(info.isReplaceDuplicates())); return arguments; } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoringContribution.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoringContribution.java index 046e61f8c4a..afe139d5132 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoringContribution.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoringContribution.java @@ -26,7 +26,7 @@ public class ExtractFunctionRefactoringContribution extends CRefactoringContribu public RefactoringDescriptor createDescriptor(String id, String project, String description, String comment, Map arguments, int flags) throws IllegalArgumentException { if (id.equals(ExtractFunctionRefactoring.ID)) { - return new ExtractFunctionRefactoringDescription(project, description, comment, arguments); + return new ExtractFunctionRefactoringDescriptor(project, description, comment, arguments); } else { return null; } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoringDescription.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoringDescriptor.java similarity index 71% rename from core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoringDescription.java rename to core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoringDescriptor.java index 568ed006469..2c04b1e8e62 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoringDescription.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoringDescriptor.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2009 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2009, 2012 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 @@ -7,52 +7,48 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * Institute for Software (IFS)- initial API and implementation + * Institute for Software (IFS)- initial API and implementation + * Sergey Prigogin (Google) ******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring.extractfunction; import java.util.Map; -import org.eclipse.core.resources.IFile; import org.eclipse.core.runtime.CoreException; import org.eclipse.jface.viewers.ISelection; -import org.eclipse.ltk.core.refactoring.Refactoring; import org.eclipse.ltk.core.refactoring.RefactoringDescriptor; import org.eclipse.ltk.core.refactoring.RefactoringStatus; import org.eclipse.cdt.core.model.ICProject; -import org.eclipse.cdt.internal.ui.refactoring.CRefactoringDescription; +import org.eclipse.cdt.internal.ui.refactoring.CRefactoring2; +import org.eclipse.cdt.internal.ui.refactoring.CRefactoringDescriptor; import org.eclipse.cdt.internal.ui.refactoring.utils.VisibilityEnum; /** * @author Emanuel Graf IFS */ -public class ExtractFunctionRefactoringDescription extends CRefactoringDescription { +public class ExtractFunctionRefactoringDescriptor extends CRefactoringDescriptor { protected static final String NAME = "name"; //$NON-NLS-1$ protected static final String VISIBILITY = "visibility"; //$NON-NLS-1$ protected static final String REPLACE_DUPLICATES = "replaceDuplicates"; //$NON-NLS-1$ - public ExtractFunctionRefactoringDescription(String project, String description, String comment, + public ExtractFunctionRefactoringDescriptor(String project, String description, String comment, Map arguments) { super(ExtractFunctionRefactoring.ID, project, description, comment, RefactoringDescriptor.MULTI_CHANGE, arguments); } @Override - public Refactoring createRefactoring(RefactoringStatus status) throws CoreException { - IFile file; - ExtractFunctionInformation info = new ExtractFunctionInformation(); - ICProject proj; - + public CRefactoring2 createRefactoring(RefactoringStatus status) throws CoreException { + ISelection selection = getSelection(); + ICProject project = getCProject(); + ExtractFunctionRefactoring refactoring = + new ExtractFunctionRefactoring(getTranslationUnit(), selection, project); + ExtractFunctionInformation info = refactoring.getRefactoringInfo(); info.setMethodName(arguments.get(NAME)); info.setVisibility(VisibilityEnum.getEnumForStringRepresentation(arguments.get(VISIBILITY))); info.setReplaceDuplicates(Boolean.parseBoolean(arguments.get(REPLACE_DUPLICATES))); - - proj = getCProject(); - file = getFile(); - - ISelection selection = getSelection(); - return new ExtractFunctionRefactoring(file, selection, info, proj); + return refactoring; } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoringRunner.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoringRunner.java index 91efd5ab562..2186c7fe1b9 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoringRunner.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoringRunner.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2009 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2012 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 @@ -12,37 +12,30 @@ *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring.extractfunction; -import org.eclipse.core.resources.IFile; import org.eclipse.jface.viewers.ISelection; import org.eclipse.jface.window.IShellProvider; -import org.eclipse.ltk.ui.refactoring.RefactoringWizardOpenOperation; +import org.eclipse.cdt.core.model.ICElement; import org.eclipse.cdt.core.model.ICProject; -import org.eclipse.cdt.internal.ui.refactoring.RefactoringRunner; +import org.eclipse.cdt.internal.ui.refactoring.RefactoringRunner2; +import org.eclipse.cdt.internal.ui.refactoring.RefactoringSaveHelper; /** * @author Emanuel Graf */ -public class ExtractFunctionRefactoringRunner extends RefactoringRunner { +public class ExtractFunctionRefactoringRunner extends RefactoringRunner2 { - public ExtractFunctionRefactoringRunner(IFile file, ISelection selection, + public ExtractFunctionRefactoringRunner(ICElement element, ISelection selection, IShellProvider shellProvider, ICProject cProject) { - super(file, selection, null, shellProvider, cProject); + super(element, selection, shellProvider, cProject); } @Override public void run() { - ExtractFunctionInformation info = new ExtractFunctionInformation(); - - ExtractFunctionRefactoring refactoring = new ExtractFunctionRefactoring(file, selection, info, project); + ExtractFunctionRefactoring refactoring = + new ExtractFunctionRefactoring(element, selection, project); ExtractFunctionWizard wizard = new ExtractFunctionWizard(refactoring); - RefactoringWizardOpenOperation operator = new RefactoringWizardOpenOperation(wizard); - - try { - operator.run(shellProvider.getShell(), refactoring.getName()); - } catch (InterruptedException e) { - //initial condition checking got canceled by the user. - } + run(wizard, refactoring, RefactoringSaveHelper.SAVE_REFACTORING); } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/NonExtractableStmtFinder.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/NonExtractableStmtFinder.java index 901f4202238..bc4a39c87fa 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/NonExtractableStmtFinder.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/NonExtractableStmtFinder.java @@ -39,10 +39,11 @@ class NonExtractableStmtFinder extends ASTVisitor{ } else if (statement instanceof IASTBreakStatement) { containsBreakStmt = true; return ASTVisitor.PROCESS_SKIP; - } else if (statement instanceof IASTForStatement || //Extracting hole loop statements is ok + } else if (statement instanceof IASTForStatement || statement instanceof IASTWhileStatement || statement instanceof IASTSwitchStatement || statement instanceof IASTDoStatement) { + // Extracting a whole loop statement is allowed return ASTVisitor.PROCESS_SKIP; } return ASTVisitor.PROCESS_CONTINUE; diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/SimilarFinderVisitor.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/SimilarFinderVisitor.java index 52ebaa030be..06ec8cac935 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/SimilarFinderVisitor.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/SimilarFinderVisitor.java @@ -44,8 +44,7 @@ abstract class SimilarFinderVisitor extends ASTVisitor { @Override public int visit(IASTStatement statement) { - if (!isInSelection(statement) && - refactoring.isStatementInTrail(statement, trail, refactoring.getIndex())) { + if (!isInSelection(statement) && refactoring.isStatementInTrail(statement, trail)) { stmtToReplace.add(statement); similarContainer.add(statement); ++statementCount; diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/TrailNodeEqualityChecker.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/TrailNodeEqualityChecker.java index f6f2fbd84de..6ba975f6303 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/TrailNodeEqualityChecker.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/TrailNodeEqualityChecker.java @@ -61,6 +61,7 @@ import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTVisibilityLabel; import org.eclipse.cdt.core.dom.ast.cpp.ICPPVariable; import org.eclipse.cdt.core.index.IIndex; import org.eclipse.cdt.core.index.IIndexName; +import org.eclipse.cdt.ui.CUIPlugin; import org.eclipse.cdt.internal.ui.refactoring.Container; import org.eclipse.cdt.internal.ui.refactoring.EqualityChecker; @@ -70,7 +71,8 @@ public class TrailNodeEqualityChecker implements EqualityChecker { private final Container namesCounter; private final IIndex index; - public TrailNodeEqualityChecker(Map names, Container namesCounter, IIndex index) { + public TrailNodeEqualityChecker(Map names, Container namesCounter, + IIndex index) { super(); this.names = names; this.namesCounter = namesCounter; @@ -81,7 +83,7 @@ public class TrailNodeEqualityChecker implements EqualityChecker { public boolean isEquals(IASTNode trailNode, IASTNode node) { if ((trailNode instanceof TrailName && node instanceof IASTName) || Arrays.equals(getInterfaces(node), getInterfaces(trailNode))) { - //Is same type + // Is same type if (node instanceof IASTExpression) { return isExpressionEquals(trailNode, node); } else if (node instanceof IASTStatement) { @@ -93,7 +95,7 @@ public class TrailNodeEqualityChecker implements EqualityChecker { } else if (node instanceof IASTDeclarator) { return isDeclaratorEquals(trailNode, node); } else if (node instanceof IASTInitializer) { - //no speciality, is the same type return true + // No speciality, is the same type return true return true; } else if (node instanceof IASTDeclSpecifier) { return isDeclSpecifierEquals(trailNode, node); @@ -310,7 +312,7 @@ public class TrailNodeEqualityChecker implements EqualityChecker { return false; } } - return trailDeclSpeci.isConst() == declSpeci.isConst() + return trailDeclSpeci.isConst() == declSpeci.isConst() && trailDeclSpeci.isInline() == declSpeci.isInline() && trailDeclSpeci.isVolatile() == declSpeci.isVolatile() && trailDeclSpeci.isRestrict() == declSpeci.isRestrict() @@ -347,25 +349,25 @@ public class TrailNodeEqualityChecker implements EqualityChecker { if (trailName.isGloballyQualified()) { IBinding realBind = trailName.getRealName().resolveBinding(); IBinding nameBind = name.resolveBinding(); + IIndexName[] realDecs; + IIndexName[] nameDecs; try { - index.acquireReadLock(); - IIndexName[] realDecs = index.findDeclarations(realBind); - IIndexName[] nameDecs = index.findDeclarations(nameBind); - if (realDecs.length == nameDecs.length) { - for (int i = 0; i < realDecs.length; ++i) { - IASTFileLocation rfl = realDecs[i].getFileLocation(); - IASTFileLocation nfl = nameDecs[i].getFileLocation(); - if (rfl.getNodeOffset() != nfl.getNodeOffset() || !rfl.getFileName().equals(nfl.getFileName())) - return false; - } - return true; - } else { - return false; - } - } catch (InterruptedException e) { + realDecs = index.findDeclarations(realBind); + nameDecs = index.findDeclarations(nameBind); } catch (CoreException e) { - } finally { - index.releaseReadLock(); + CUIPlugin.log(e); + return false; + } + if (realDecs.length == nameDecs.length) { + for (int i = 0; i < realDecs.length; ++i) { + IASTFileLocation rfl = realDecs[i].getFileLocation(); + IASTFileLocation nfl = nameDecs[i].getFileLocation(); + if (rfl.getNodeOffset() != nfl.getNodeOffset() || !rfl.getFileName().equals(nfl.getFileName())) + return false; + } + return true; + } else { + return false; } } else { IType oType = getType(trailName.getRealName().resolveBinding()); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractlocalvariable/ExtractLocalVariableRefactoring.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractlocalvariable/ExtractLocalVariableRefactoring.java index 6316524c1bc..b28effeea1a 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractlocalvariable/ExtractLocalVariableRefactoring.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractlocalvariable/ExtractLocalVariableRefactoring.java @@ -70,8 +70,8 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPFunction; import org.eclipse.cdt.internal.ui.refactoring.CRefactoring2; import org.eclipse.cdt.internal.ui.refactoring.CRefactoringDescriptor; import org.eclipse.cdt.internal.ui.refactoring.ModificationCollector; -import org.eclipse.cdt.internal.ui.refactoring.VariableNameInformation; import org.eclipse.cdt.internal.ui.refactoring.NodeContainer; +import org.eclipse.cdt.internal.ui.refactoring.VariableNameInformation; 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; @@ -239,8 +239,7 @@ public class ExtractLocalVariableRefactoring extends CRefactoring2 { @Override public int visit(IASTExpression expression) { - if (expression.isPartOfTranslationUnitFile() && - SelectionHelper.isExpressionWhollyInSelection(selectedRegion, expression)) { + if (SelectionHelper.isNodeInsideSelection(expression, selectedRegion)) { container.add(expression); return PROCESS_SKIP; } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractlocalvariable/ExtractLocalVariableRefactoringContribution.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractlocalvariable/ExtractLocalVariableRefactoringContribution.java index 230da75a6c4..347a5933171 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractlocalvariable/ExtractLocalVariableRefactoringContribution.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractlocalvariable/ExtractLocalVariableRefactoringContribution.java @@ -21,7 +21,6 @@ import org.eclipse.cdt.internal.ui.refactoring.CRefactoringContribution; * @author Emanuel Graf IFS */ public class ExtractLocalVariableRefactoringContribution extends CRefactoringContribution { - @SuppressWarnings({ "unchecked", "rawtypes" }) @Override public RefactoringDescriptor createDescriptor(String id, String project, String description, 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 8ee3ea7f49e..7c506abfa54 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, 2009 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2012 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 @@ -7,7 +7,8 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * Institute for Software - initial API and implementation + * Institute for Software - initial API and implementation + * Sergey Prigogin (Google) *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring.utils; @@ -57,7 +58,7 @@ public class SelectionHelper { @Override public int visit(IASTDeclaration declaration) { if (declaration instanceof IASTSimpleDeclaration && - isSelectionOnExpression(textSelection, declaration)) { + doesNodeOverlapWithRegion(declaration, textSelection)) { container.setObject((IASTSimpleDeclaration) declaration); } return super.visit(declaration); @@ -67,22 +68,34 @@ public class SelectionHelper { return container.getObject(); } - public static boolean isSelectionOnExpression(Region textSelection, IASTNode expression) { - Region exprPos = createExpressionPosition(expression); - int exprStart = exprPos.getOffset(); - int selStart = textSelection.getOffset(); - int selEnd = selStart + textSelection.getLength(); - return exprStart + exprPos.getLength() >= selStart && exprStart <= selEnd; + public static boolean doesNodeOverlapWithRegion(IASTNode node, Region region) { + return doRegionsOverlap(getNodeSpan(node), region); } - public static boolean isExpressionWhollyInSelection(Region textSelection, IASTNode expression) { - Region exprPos = createExpressionPosition(expression); - int exprStart = exprPos.getOffset(); - int selStart = textSelection.getOffset(); - int selEnd = selStart + textSelection.getLength(); - return exprStart >= selStart && exprStart + exprPos.getLength() <= selEnd; + public static boolean isNodeInsideRegion(IASTNode node, Region region) { + return isRegionInside(getNodeSpan(node), region); } - + + /** + * Returns true if the first region is inside the second. + */ + private static boolean isRegionInside(Region region1, Region region2) { + int offset1 = region1.getOffset(); + int offset2 = region2.getOffset(); + return offset1 >= offset2 && + offset1 + region1.getLength() <= offset2 + region2.getLength(); + } + + /** + * Returns true if the two regions have at least one common point. + */ + private static boolean doRegionsOverlap(Region region1, Region region2) { + int offset1 = region1.getOffset(); + int offset2 = region2.getOffset(); + return offset1 + region1.getLength() >= offset2 && + offset1 <= offset2 + region2.getLength(); + } + public static boolean isInSameFile(IASTNode node, IFile file) { IPath path = new Path(node.getContainingFilename()); IWorkspaceRoot workspaceRoot = ResourcesPlugin.getWorkspace().getRoot(); @@ -93,22 +106,19 @@ public class SelectionHelper { public static boolean isInSameFileSelection(Region textSelection, IASTNode node, IFile file) { if (isInSameFile(node, file)) { - return SelectionHelper.isSelectionOnExpression(textSelection, node); + return doesNodeOverlapWithRegion(node, textSelection); } return false; } - public static boolean isSelectedFile(Region textSelection, IASTNode node, IFile file) { - if (isInSameFile(node, file)) { - return isExpressionWhollyInSelection(textSelection, node); - } - return false; + public static boolean isNodeInsideSelection(IASTNode node, Region selection) { + return node.isPartOfTranslationUnitFile() && isNodeInsideRegion(node, selection); } - - protected static Region createExpressionPosition(IASTNode expression) { + + protected static Region getNodeSpan(IASTNode region) { int start = Integer.MAX_VALUE; int nodeLength = 0; - IASTNodeLocation[] nodeLocations = expression.getNodeLocations(); + IASTNodeLocation[] nodeLocations = region.getNodeLocations(); if (nodeLocations.length != 1) { for (IASTNodeLocation location : nodeLocations) { if (location instanceof IASTMacroExpansionLocation) { @@ -119,7 +129,7 @@ public class SelectionHelper { } nodeLength += macroLoc.asFileLocation().getNodeLength(); } else { - IASTFileLocation loc = expression.getFileLocation(); + IASTFileLocation loc = region.getFileLocation(); int nodeOffset = loc.getNodeOffset(); if (nodeOffset < start) { start = nodeOffset; @@ -133,7 +143,7 @@ public class SelectionHelper { start = macroLoc.asFileLocation().getNodeOffset(); nodeLength = macroLoc.asFileLocation().getNodeLength(); } else { - IASTFileLocation loc = expression.getFileLocation(); + IASTFileLocation loc = region.getFileLocation(); start = loc.getNodeOffset(); nodeLength = loc.getNodeLength(); } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/CRenameAction.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/CRenameAction.java index a32edc6cb3e..49efacf9e00 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/CRenameAction.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/CRenameAction.java @@ -36,7 +36,6 @@ public class CRenameAction extends RefactoringAction { public CRenameAction() { super(Messages.CRenameAction_label); - setSaveRequired(false); } @Override diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ExtractFunctionAction.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ExtractFunctionAction.java index 2d7d38e0d29..ca68c34dddd 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ExtractFunctionAction.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ExtractFunctionAction.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2009 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2012 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 @@ -7,14 +7,12 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * Institute for Software (IFS)- initial API and implementation + * Institute for Software (IFS)- initial API and implementation + * Sergey Prigogin (Google) ******************************************************************************/ package org.eclipse.cdt.ui.refactoring.actions; -import org.eclipse.core.resources.IFile; -import org.eclipse.core.resources.IResource; import org.eclipse.jface.text.ITextSelection; -import org.eclipse.jface.viewers.ISelection; import org.eclipse.jface.window.IShellProvider; import org.eclipse.cdt.core.model.ICElement; @@ -23,11 +21,11 @@ import org.eclipse.cdt.core.model.IWorkingCopy; import org.eclipse.cdt.internal.ui.refactoring.extractfunction.ExtractFunctionRefactoringRunner; /** - * + * Launches an Extract Function refactoring. + * * @since 5.0 - * @author Emanuel Graf IFS - * * @noextend This class is not intended to be subclassed by clients. + * @author Emanuel Graf IFS */ public class ExtractFunctionAction extends RefactoringAction { @@ -40,12 +38,9 @@ public class ExtractFunctionAction extends RefactoringAction { } @Override - public void run(IShellProvider shellProvider, IWorkingCopy wc, - ITextSelection s) { - IResource res = wc.getResource(); - if (res instanceof IFile) { - final ISelection selection = fEditor.getSelectionProvider().getSelection(); - new ExtractFunctionRefactoringRunner((IFile) res, selection, fEditor.getSite(), wc.getCProject()).run(); + public void run(IShellProvider shellProvider, IWorkingCopy wc, ITextSelection selection) { + if (wc.getResource() != null) { + new ExtractFunctionRefactoringRunner(wc, selection, shellProvider, wc.getCProject()).run(); } } @@ -54,5 +49,4 @@ public class ExtractFunctionAction extends RefactoringAction { super.updateSelection(elem); setEnabled(false); } - } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ExtractLocalVariableAction.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ExtractLocalVariableAction.java index 6fd7993a400..15f03a47cb2 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ExtractLocalVariableAction.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ExtractLocalVariableAction.java @@ -9,7 +9,6 @@ * Markus Schorn - initial API and implementation * Sergey Prigogin (Google) *******************************************************************************/ - package org.eclipse.cdt.ui.refactoring.actions; import org.eclipse.jface.text.ITextSelection; @@ -21,7 +20,7 @@ import org.eclipse.cdt.core.model.IWorkingCopy; import org.eclipse.cdt.internal.ui.refactoring.extractlocalvariable.ExtractLocalVariableRefactoringRunner; /** - * Launches a extract local variable refactoring. + * Launches an Extract Local Variable refactoring. * @since 5.1 * @noextend This class is not intended to be subclassed by clients. */ diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/GettersAndSettersAction.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/GettersAndSettersAction.java index 2e07fdd7fee..04750c25edf 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/GettersAndSettersAction.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/GettersAndSettersAction.java @@ -13,7 +13,6 @@ package org.eclipse.cdt.ui.refactoring.actions; import org.eclipse.core.resources.IFile; -import org.eclipse.core.resources.IResource; import org.eclipse.jface.text.ITextSelection; import org.eclipse.jface.window.IShellProvider; import org.eclipse.ui.IEditorPart; @@ -34,7 +33,6 @@ public class GettersAndSettersAction extends RefactoringAction { public GettersAndSettersAction() { super(Messages.GettersAndSetters_label); - setSaveRequired(false); } /** @@ -52,8 +50,7 @@ public class GettersAndSettersAction extends RefactoringAction { @Override public void run(IShellProvider shellProvider, IWorkingCopy wc, ITextSelection s) { - IResource res= wc.getResource(); - if (res instanceof IFile) { + if (wc.getResource() != null) { new GenerateGettersAndSettersRefactoringRunner(wc, s, shellProvider, wc.getCProject()).run(); } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/HideMethodAction.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/HideMethodAction.java index 537c3f51dcb..0f86652d76f 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/HideMethodAction.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/HideMethodAction.java @@ -33,6 +33,7 @@ public class HideMethodAction extends RefactoringAction { public HideMethodAction() { super(Messages.HideMethodAction_label); + setSaveRequired(true); } @Override diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ImplementMethodAction.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ImplementMethodAction.java index db66437114a..e2512f25ff8 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ImplementMethodAction.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ImplementMethodAction.java @@ -36,6 +36,7 @@ public class ImplementMethodAction extends RefactoringAction { public ImplementMethodAction() { super(Messages.ImplementMethodAction_label); + setSaveRequired(true); } /** diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/RefactoringAction.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/RefactoringAction.java index be59b665ea1..93f0ebd0b9b 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/RefactoringAction.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/RefactoringAction.java @@ -38,7 +38,7 @@ public abstract class RefactoringAction extends Action { public RefactoringAction(String label) { super(label); - saveRequired = true; + saveRequired = false; } /** diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ToggleFunctionAction.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ToggleFunctionAction.java index 5d98d2799c6..fd092a2d753 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ToggleFunctionAction.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/refactoring/actions/ToggleFunctionAction.java @@ -38,6 +38,7 @@ public class ToggleFunctionAction extends RefactoringAction { public ToggleFunctionAction() { super(Messages.ToggleFunctionAction_label); + setSaveRequired(true); } @Override