diff --git a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ImplementMethod.rts b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ImplementMethod.rts index ad0fb7421e4..ba804a149a3 100644 --- a/core/org.eclipse.cdt.ui.tests/resources/refactoring/ImplementMethod.rts +++ b/core/org.eclipse.cdt.ui.tests/resources/refactoring/ImplementMethod.rts @@ -2,6 +2,7 @@ //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config filename=A.h +infos=1 //@A.h class X { public: @@ -24,6 +25,7 @@ inline bool X::a(int int1) const //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config filename=A.h +infos=1 //@A.h class X { public: @@ -46,6 +48,7 @@ inline bool X::xy(int int1, int i) const //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config filename=A.h +infos=1 //@A.h template class A { @@ -76,6 +79,7 @@ template inline void A::test() //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config filename=A.h +infos=1 //@A.h template class A { @@ -137,6 +141,7 @@ void Demo::SubClass::test() //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config filename=A.h +infos=1 //@A.h class A { @@ -431,6 +436,7 @@ int A::foo(int param1, int param2) //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config filename=A.h +infos=1 //@A.h class A { @@ -581,6 +587,7 @@ namespace OuterSpace { //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config filename=A.h +infos=1 //@A.h template class A { @@ -714,6 +721,7 @@ void Test::doNothing(void) //#org.eclipse.cdt.ui.tests.refactoring.implementmethod.ImplementMethodRefactoringTest //@.config filename=TestClass.h +infos=1 //@TestClass.h #ifndef TESTCLASS_H_ #define TESTCLASS_H_ diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/implementmethod/ImplementMethodRefactoringTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/implementmethod/ImplementMethodRefactoringTest.java index 49fc46dab96..9d5d147179c 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/implementmethod/ImplementMethodRefactoringTest.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/implementmethod/ImplementMethodRefactoringTest.java @@ -8,6 +8,7 @@ * * Contributors: * Institute for Software - initial API and implementation + * Marc-Andre Laperle *******************************************************************************/ package org.eclipse.cdt.ui.tests.refactoring.implementmethod; @@ -18,10 +19,12 @@ import org.eclipse.core.resources.IFile; import org.eclipse.ltk.core.refactoring.Change; import org.eclipse.ltk.core.refactoring.RefactoringStatus; +import org.eclipse.cdt.core.model.CoreModel; +import org.eclipse.cdt.core.model.ICElement; import org.eclipse.cdt.ui.tests.refactoring.RefactoringTest; import org.eclipse.cdt.ui.tests.refactoring.TestSourceFile; -import org.eclipse.cdt.internal.ui.refactoring.CRefactoring; +import org.eclipse.cdt.internal.ui.refactoring.CRefactoring2; import org.eclipse.cdt.internal.ui.refactoring.implementmethod.ImplementMethodRefactoring; /** @@ -30,6 +33,7 @@ import org.eclipse.cdt.internal.ui.refactoring.implementmethod.ImplementMethodRe public class ImplementMethodRefactoringTest extends RefactoringTest { protected int finalWarnings; private int initialWarnings; + private int infos; public ImplementMethodRefactoringTest(String name, Collection files) { super(name, files); @@ -38,37 +42,36 @@ public class ImplementMethodRefactoringTest extends RefactoringTest { @Override protected void runTest() throws Throwable { IFile refFile = project.getFile(fileName); - CRefactoring refactoring = new ImplementMethodRefactoring(refFile, selection, null, cproject); - - try { - refactoring.lockIndex(); - RefactoringStatus checkInitialConditions = refactoring.checkInitialConditions(NULL_PROGRESS_MONITOR); + ICElement element = CoreModel.getDefault().create(refFile); + CRefactoring2 refactoring = new ImplementMethodRefactoring(element, selection, cproject, astCache); + RefactoringStatus checkInitialConditions = refactoring.checkInitialConditions(NULL_PROGRESS_MONITOR); - if(initialWarnings == 0) { - assertConditionsOk(checkInitialConditions); - } else { - assertConditionsFatalError(checkInitialConditions, initialWarnings); - return; - } - - refactoring.checkFinalConditions(NULL_PROGRESS_MONITOR); - RefactoringStatus finalConditions = refactoring.checkFinalConditions(NULL_PROGRESS_MONITOR); - if (finalWarnings == 0) { - Change createChange = refactoring.createChange(NULL_PROGRESS_MONITOR); - assertConditionsOk(finalConditions); - createChange.perform(NULL_PROGRESS_MONITOR); - } else { - assertConditionsWarning(finalConditions, finalWarnings); - } - compareFiles(fileMap); - } finally { - refactoring.unlockIndex(); + if (initialWarnings == 0) { + assertConditionsOk(checkInitialConditions); + } else { + assertConditionsFatalError(checkInitialConditions, initialWarnings); + return; } + + RefactoringStatus finalConditions = refactoring.checkFinalConditions(NULL_PROGRESS_MONITOR); + Change createChange = refactoring.createChange(NULL_PROGRESS_MONITOR); + + if (finalWarnings > 0) { + assertConditionsWarning(finalConditions, finalWarnings); + } else if (infos > 0) { + assertConditionsInfo(finalConditions, infos); + } else { + assertConditionsOk(finalConditions); + } + + createChange.perform(NULL_PROGRESS_MONITOR); + compareFiles(fileMap); } @Override protected void configureRefactoring(Properties refactoringProperties) { finalWarnings = new Integer(refactoringProperties.getProperty("finalWarnings", "0")).intValue(); //$NON-NLS-1$//$NON-NLS-2$ initialWarnings = Integer.parseInt(refactoringProperties.getProperty("initialWarnings", "0")); //$NON-NLS-1$//$NON-NLS-2$ + infos = Integer.parseInt(refactoringProperties.getProperty("infos", "0")); //$NON-NLS-1$//$NON-NLS-2$ } } diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/utils/DefinitionFinderTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/utils/DefinitionFinderTest.java index d5ef5cf5158..ec4d06b093e 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/utils/DefinitionFinderTest.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/utils/DefinitionFinderTest.java @@ -44,7 +44,7 @@ public class DefinitionFinderTest extends RefactoringTest { IASTTranslationUnit ast = astCache.getAST((ITranslationUnit) element, null); for (IASTDeclaration declaration : ast.getDeclarations()) { if (declaration instanceof IASTSimpleDeclaration) { - assertNotNull(DefinitionFinder2.getDefinition((IASTSimpleDeclaration) declaration, astCache)); + assertNotNull(DefinitionFinder2.getDefinition((IASTSimpleDeclaration) declaration, astCache, NULL_PROGRESS_MONITOR)); } } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GenerateGettersAndSettersRefactoring.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GenerateGettersAndSettersRefactoring.java index 85d096b457a..128871034d7 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GenerateGettersAndSettersRefactoring.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/GenerateGettersAndSettersRefactoring.java @@ -122,7 +122,7 @@ public class GenerateGettersAndSettersRefactoring extends CRefactoring2 { CheckConditionsContext checkContext) throws CoreException, OperationCanceledException { RefactoringStatus result = new RefactoringStatus(); if (!context.isImplementationInHeader()) { - findDefinitionInsertLocation(); + findDefinitionInsertLocation(pm); if (definitionInsertLocation == null || tu.equals(definitionInsertLocation.getTranslationUnit())) { result.addInfo(Messages.GenerateGettersAndSettersRefactoring_NoImplFile); } @@ -238,7 +238,7 @@ public class GenerateGettersAndSettersRefactoring extends CRefactoring2 { } } if (!context.isImplementationInHeader()) { - addDefinition(collector, definitions); + addDefinition(collector, definitions, pm); } ICPPASTCompositeTypeSpecifier classDefinition = (ICPPASTCompositeTypeSpecifier) context.existingFields.get(context.existingFields.size() - 1).getParent(); @@ -247,9 +247,9 @@ public class GenerateGettersAndSettersRefactoring extends CRefactoring2 { getterAndSetters, false, collector); } - private void addDefinition(ModificationCollector collector, List definitions) + private void addDefinition(ModificationCollector collector, List definitions, IProgressMonitor pm) throws CoreException { - findDefinitionInsertLocation(); + findDefinitionInsertLocation(pm); IASTNode parent = definitionInsertLocation.getParentOfNodeToInsertBefore(); IASTTranslationUnit ast = parent.getTranslationUnit(); ASTRewrite rewrite = collector.rewriterForTranslationUnit(ast); @@ -265,14 +265,15 @@ public class GenerateGettersAndSettersRefactoring extends CRefactoring2 { return context; } - private void findDefinitionInsertLocation() throws CoreException { + private void findDefinitionInsertLocation(IProgressMonitor pm) throws CoreException { if (definitionInsertLocation != null) { return; } IASTSimpleDeclaration decl = context.existingFields.get(0); - InsertLocation2 location = MethodDefinitionInsertLocationFinder2.find( - tu, decl.getFileLocation(), decl.getParent(), astCache); + MethodDefinitionInsertLocationFinder2 methodDefinitionInsertLocationFinder = new MethodDefinitionInsertLocationFinder2(); + InsertLocation2 location = methodDefinitionInsertLocationFinder.find( + tu, decl.getFileLocation(), decl.getParent(), astCache, pm); if (location.getFile() == null || NodeHelper.isContainedInTemplateDeclaration(decl)) { location.setNodeToInsertAfter(NodeHelper.findTopLevelParent(decl), tu); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/messages.properties b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/messages.properties index 937e6ce7f59..bb018b58168 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/messages.properties +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/gettersandsetters/messages.properties @@ -1,5 +1,5 @@ ############################################################################### -# Copyright (c) 2008, 2009 Institute for Software, HSR Hochschule fuer Technik +# Copyright (c) 2008, 2011 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 @@ -18,4 +18,4 @@ GenerateGettersAndSettersInputPage_SelectGetters=Select Getters GenerateGettersAndSettersInputPage_SelectSetters=Select Setters GenerateGettersAndSettersRefactoring_NoCassDefFound=No class definition found GenerateGettersAndSettersRefactoring_NoFields=The class does not contain any fields. -GenerateGettersAndSettersRefactoring_NoImplFile=No implementation file found. Insert definition into the header file. +GenerateGettersAndSettersRefactoring_NoImplFile=No implementation file found. Inserting definition into the header file. diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoring.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoring.java index 59752260814..630a15d7fb6 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoring.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoring.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2010 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2011 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 @@ -8,11 +8,14 @@ * * Contributors: * Institute for Software - initial API and implementation + * Marc-Andre Laperle *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring.implementmethod; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.eclipse.core.resources.IFile; import org.eclipse.core.runtime.CoreException; @@ -23,10 +26,13 @@ import org.eclipse.core.runtime.SubMonitor; 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.cdt.core.dom.ast.ASTNodeFactoryFactory; import org.eclipse.cdt.core.dom.ast.ASTVisitor; 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.IASTFunctionDeclarator; import org.eclipse.cdt.core.dom.ast.IASTFunctionDefinition; import org.eclipse.cdt.core.dom.ast.IASTName; @@ -35,23 +41,26 @@ import org.eclipse.cdt.core.dom.ast.IASTNode.CopyStyle; import org.eclipse.cdt.core.dom.ast.IASTPointerOperator; import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration; import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit; +import org.eclipse.cdt.core.dom.ast.IBinding; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTDeclSpecifier; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTFunctionDeclarator; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTQualifiedName; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTemplateDeclaration; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTemplateParameter; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPFunction; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPMethod; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPNodeFactory; import org.eclipse.cdt.core.dom.rewrite.ASTRewrite; +import org.eclipse.cdt.core.index.IIndex; +import org.eclipse.cdt.core.index.IIndexName; import org.eclipse.cdt.core.model.ICElement; import org.eclipse.cdt.core.model.ICProject; +import org.eclipse.cdt.ui.CUIPlugin; -import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTCompoundStatement; -import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTFunctionDeclarator; -import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTFunctionDefinition; -import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTTemplateDeclaration; - -import org.eclipse.cdt.internal.ui.refactoring.CRefactoring; +import org.eclipse.cdt.internal.ui.refactoring.CRefactoring2; import org.eclipse.cdt.internal.ui.refactoring.ModificationCollector; -import org.eclipse.cdt.internal.ui.refactoring.utils.DefinitionFinder; +import org.eclipse.cdt.internal.ui.refactoring.RefactoringASTCache; +import org.eclipse.cdt.internal.ui.refactoring.utils.Checks; import org.eclipse.cdt.internal.ui.refactoring.utils.NameHelper; import org.eclipse.cdt.internal.ui.refactoring.utils.NodeHelper; import org.eclipse.cdt.internal.ui.refactoring.utils.SelectionHelper; @@ -62,14 +71,18 @@ import org.eclipse.cdt.internal.ui.refactoring.utils.SelectionHelper; * * @author Mirko Stocker, Lukas Felber, Emanuel Graf */ -public class ImplementMethodRefactoring extends CRefactoring { - private InsertLocation insertLocation; - private CPPASTFunctionDeclarator createdMethodDeclarator; +public class ImplementMethodRefactoring extends CRefactoring2 { + private ICPPASTFunctionDeclarator createdMethodDeclarator; private ImplementMethodData data; + private MethodDefinitionInsertLocationFinder2 methodDefinitionInsertLocationFinder; + private Map insertLocations; + private static ICPPNodeFactory nodeFactory = ASTNodeFactoryFactory.getDefaultCPPNodeFactory(); - public ImplementMethodRefactoring(IFile file, ISelection selection, ICElement element, ICProject project) { - super(file, selection, element, project); + public ImplementMethodRefactoring(ICElement element, ISelection selection, ICProject project, RefactoringASTCache astCache) { + super(element, selection, project, astCache); data = new ImplementMethodData(); + methodDefinitionInsertLocationFinder = new MethodDefinitionInsertLocationFinder2(); + insertLocations = new HashMap(); } @Override @@ -78,10 +91,10 @@ public class ImplementMethodRefactoring extends CRefactoring { super.checkInitialConditions(sm.newChild(6)); if (!initStatus.hasFatalError()) { - data.setMethodDeclarations(findUnimplementedMethodDeclarations(ast)); + data.setMethodDeclarations(findUnimplementedMethodDeclarations(pm)); - if (region.getLength() > 0) { - IASTSimpleDeclaration methodDeclaration = SelectionHelper.findFirstSelectedDeclaration(region, ast); + if (selectedRegion.getLength() > 0) { + IASTSimpleDeclaration methodDeclaration = SelectionHelper.findFirstSelectedDeclaration(selectedRegion, astCache.getAST(tu, pm)); if (NodeHelper.isMethodDeclaration(methodDeclaration)) { for (MethodToImplementConfig config : data.getMethodDeclarations()) { if (config.getDeclaration() == methodDeclaration) { @@ -95,10 +108,10 @@ public class ImplementMethodRefactoring extends CRefactoring { return initStatus; } - private List findUnimplementedMethodDeclarations( - IASTTranslationUnit unit) { + private List findUnimplementedMethodDeclarations(IProgressMonitor pm) throws OperationCanceledException, CoreException { + IASTTranslationUnit ast = astCache.getAST(tu, pm); final List list = new ArrayList(); - unit.accept(new ASTVisitor() { + ast.accept(new ASTVisitor() { { shouldVisitDeclarations = true; } @@ -107,40 +120,76 @@ public class ImplementMethodRefactoring extends CRefactoring { public int visit(IASTDeclaration declaration) { if (declaration instanceof IASTSimpleDeclaration) { IASTSimpleDeclaration simpleDeclaration = (IASTSimpleDeclaration) declaration; - try { - if (NodeHelper.isMethodDeclaration(simpleDeclaration) && DefinitionFinder.getDefinition(simpleDeclaration, file) == null) { + if (NodeHelper.isMethodDeclaration(simpleDeclaration)) { + IASTDeclarator[] declarators = simpleDeclaration.getDeclarators(); + IBinding binding = declarators[0].getName().resolveBinding(); + if (isUnimplementedMethodBinding(binding)) { list.add(simpleDeclaration); + return ASTVisitor.PROCESS_SKIP; } - } catch (CoreException e) {} + } } return ASTVisitor.PROCESS_CONTINUE; } }); return list; } + + private boolean isUnimplementedMethodBinding(IBinding binding) { + if (binding instanceof ICPPFunction) { + if (binding instanceof ICPPMethod) { + ICPPMethod methodBinding = (ICPPMethod) binding; + if (methodBinding.isPureVirtual()) { + return false; //Ępure virtual not handled for now, see bug 303870 + } + } + + try { + IIndexName[] indexNames = astCache.getIndex().findNames(binding, IIndex.FIND_DEFINITIONS + | IIndex.SEARCH_ACROSS_LANGUAGE_BOUNDARIES); + if (indexNames.length == 0) { + return true; + } + } catch (CoreException e) { + CUIPlugin.log(e); + } + } + + return false; + } @Override protected void collectModifications(IProgressMonitor pm, ModificationCollector collector) throws CoreException, OperationCanceledException { List methodsToImplement = data.getMethodsToImplement(); - SubMonitor sm = SubMonitor.convert(pm, 4*methodsToImplement.size()); + SubMonitor sm = SubMonitor.convert(pm, 4 * methodsToImplement.size()); for (MethodToImplementConfig config : methodsToImplement) { createDefinition(collector, config, sm.newChild(4)); - } + } } protected void createDefinition(ModificationCollector collector, - MethodToImplementConfig config, IProgressMonitor subMonitor) throws CoreException { + MethodToImplementConfig config, IProgressMonitor subMonitor) throws CoreException, OperationCanceledException { + if (subMonitor.isCanceled()) { + throw new OperationCanceledException(); + } IASTSimpleDeclaration decl = config.getDeclaration(); - insertLocation = findInsertLocation(decl); + InsertLocation2 insertLocation = findInsertLocation(decl, subMonitor); + if (subMonitor.isCanceled()) { + throw new OperationCanceledException(); + } + subMonitor.worked(1); - IASTTranslationUnit targetUnit = insertLocation.getTargetTranslationUnit(); - IASTNode parent = insertLocation.getPartenOfNodeToInsertBefore(); - ASTRewrite translationUnitRewrite = collector.rewriterForTranslationUnit(targetUnit); + IASTNode parent = insertLocation.getParentOfNodeToInsertBefore(); + IASTTranslationUnit ast = parent.getTranslationUnit(); + ASTRewrite translationUnitRewrite = collector.rewriterForTranslationUnit(ast); subMonitor.worked(1); + if (subMonitor.isCanceled()) { + throw new OperationCanceledException(); + } IASTNode nodeToInsertBefore = insertLocation.getNodeToInsertBefore(); - IASTNode createdMethodDefinition = createFunctionDefinition(targetUnit, decl); + IASTNode createdMethodDefinition = createFunctionDefinition(ast, decl, insertLocation); subMonitor.worked(1); ASTRewrite methodRewrite = translationUnitRewrite.insertBefore(parent, nodeToInsertBefore, createdMethodDefinition , null); createParameterModifications(methodRewrite, config.getParaHandler()); @@ -169,35 +218,30 @@ public class ImplementMethodRefactoring extends CRefactoring { } } - private InsertLocation findInsertLocation(IASTSimpleDeclaration methodDeclaration) throws CoreException { - InsertLocation insertLocation = MethodDefinitionInsertLocationFinder.find(methodDeclaration.getFileLocation(), methodDeclaration.getParent(), file); - - if (!insertLocation.hasFile() || NodeHelper.isContainedInTemplateDeclaration(methodDeclaration)) { - insertLocation.setInsertFile(file); - insertLocation.setNodeToInsertAfter(NodeHelper.findTopLevelParent(methodDeclaration)); + private InsertLocation2 findInsertLocation(IASTSimpleDeclaration methodDeclaration, IProgressMonitor subMonitor) throws CoreException { + if (insertLocations.containsKey(methodDeclaration)) { + return insertLocations.get(methodDeclaration); } + InsertLocation2 insertLocation = methodDefinitionInsertLocationFinder.find(tu, methodDeclaration.getFileLocation(), methodDeclaration.getParent(), astCache, subMonitor); + if (insertLocation.getTranslationUnit() == null || NodeHelper.isContainedInTemplateDeclaration(methodDeclaration)) { + insertLocation.setNodeToInsertAfter(NodeHelper.findTopLevelParent(methodDeclaration), tu); + } + insertLocations.put(methodDeclaration, insertLocation); return insertLocation; } - private IASTDeclaration createFunctionDefinition(IASTTranslationUnit unit, IASTSimpleDeclaration methodDeclaration) throws CoreException { - return createFunctionDefinition( -methodDeclaration.getDeclSpecifier().copy(CopyStyle.withLocations), - (ICPPASTFunctionDeclarator) methodDeclaration.getDeclarators()[0], - methodDeclaration.getParent(), unit); - } - - private IASTDeclaration createFunctionDefinition(IASTDeclSpecifier declSpecifier, ICPPASTFunctionDeclarator functionDeclarator, - IASTNode declarationParent, IASTTranslationUnit unit) throws CoreException { - IASTFunctionDefinition func = new CPPASTFunctionDefinition(); - func.setParent(unit); + private IASTDeclaration createFunctionDefinition(IASTTranslationUnit unit, IASTSimpleDeclaration methodDeclaration, InsertLocation2 insertLocation) throws CoreException { + IASTDeclSpecifier declSpecifier = methodDeclaration.getDeclSpecifier().copy(CopyStyle.withLocations); + ICPPASTFunctionDeclarator functionDeclarator = (ICPPASTFunctionDeclarator) methodDeclaration.getDeclarators()[0]; + IASTNode declarationParent = methodDeclaration.getParent(); if (declSpecifier instanceof ICPPASTDeclSpecifier) { ((ICPPASTDeclSpecifier) declSpecifier).setVirtual(false); } String currentFileName = declarationParent.getNodeLocations()[0].asFileLocation().getFileName(); - if (Path.fromOSString(currentFileName).equals(insertLocation.getInsertFile().getLocation())) { + if (Path.fromOSString(currentFileName).equals(insertLocation.getFile().getLocation())) { declSpecifier.setInline(true); } @@ -205,38 +249,36 @@ methodDeclaration.getDeclSpecifier().copy(CopyStyle.withLocations), declSpecifier.setStorageClass(IASTDeclSpecifier.sc_unspecified); } - func.setDeclSpecifier(declSpecifier); + ICPPASTQualifiedName qName = createQualifiedNameFor(functionDeclarator, declarationParent, insertLocation); - ICPPASTQualifiedName qname = createQualifiedNameFor(functionDeclarator, declarationParent); - - createdMethodDeclarator = new CPPASTFunctionDeclarator(); - createdMethodDeclarator.setName(qname); + createdMethodDeclarator = nodeFactory.newFunctionDeclarator(qName); createdMethodDeclarator.setConst(functionDeclarator.isConst()); for (IASTPointerOperator pop : functionDeclarator.getPointerOperators()) { createdMethodDeclarator.addPointerOperator(pop.copy(CopyStyle.withLocations)); } - - func.setDeclarator(createdMethodDeclarator); - func.setBody(new CPPASTCompoundStatement()); + + IASTFunctionDefinition functionDefinition = nodeFactory.newFunctionDefinition(declSpecifier, createdMethodDeclarator, nodeFactory.newCompoundStatement()); + functionDefinition.setParent(unit); if (NodeHelper.isContainedInTemplateDeclaration(declarationParent)) { - CPPASTTemplateDeclaration templateDeclaration = new CPPASTTemplateDeclaration(); + ICPPASTTemplateDeclaration templateDeclaration = nodeFactory.newTemplateDeclaration(functionDefinition); templateDeclaration.setParent(unit); for (ICPPASTTemplateParameter templateParameter : ((ICPPASTTemplateDeclaration) declarationParent.getParent().getParent() ).getTemplateParameters()) { templateDeclaration.addTemplateParameter(templateParameter.copy(CopyStyle.withLocations)); } - templateDeclaration.setDeclaration(func); return templateDeclaration; } - return func; + return functionDefinition; } private ICPPASTQualifiedName createQualifiedNameFor(IASTFunctionDeclarator functionDeclarator, - IASTNode declarationParent) throws CoreException { + IASTNode declarationParent, InsertLocation2 insertLocation) throws CoreException { int insertOffset = insertLocation.getInsertPosition(); - return NameHelper.createQualifiedNameFor(functionDeclarator.getName(), file, functionDeclarator.getFileLocation().getNodeOffset(), insertLocation.getInsertFile(), insertOffset); + return NameHelper.createQualifiedNameFor( + functionDeclarator.getName(), tu, functionDeclarator.getFileLocation().getNodeOffset(), + insertLocation.getTranslationUnit(), insertOffset, astCache); } public ImplementMethodData getRefactoringData() { @@ -248,4 +290,52 @@ methodDeclaration.getDeclSpecifier().copy(CopyStyle.withLocations), // TODO egraf add Descriptor return null; } + + private IFile[] getAllFilesToModify() { + List files = new ArrayList(2); + IFile file = (IFile) tu.getResource(); + if (file != null) { + files.add(file); + } + + for (InsertLocation2 insertLocation : insertLocations.values()) { + if (insertLocation != null) { + file = insertLocation.getFile(); + if (file != null) { + files.add(file); + } + } + } + + return files.toArray(new IFile[files.size()]); + } + + @Override + protected RefactoringStatus checkFinalConditions(IProgressMonitor subProgressMonitor, + CheckConditionsContext checkContext) throws CoreException, OperationCanceledException { + RefactoringStatus result = new RefactoringStatus(); + if (isOneOrMoreImplementationInHeader(subProgressMonitor)) { + result.addInfo(Messages.ImplementMethodRefactoring_NoImplFile); + } + Checks.addModifiedFilesToChecker(getAllFilesToModify(), checkContext); + return result; + } + + private boolean isOneOrMoreImplementationInHeader(IProgressMonitor subProgressMonitor) throws CoreException { + for (MethodToImplementConfig config : data.getMethodsToImplement()) { + IASTSimpleDeclaration decl = config.getDeclaration(); + findInsertLocation(decl, subProgressMonitor); + } + + if (insertLocations.isEmpty()) { + return true; + } + + for (InsertLocation2 insertLocation : insertLocations.values()) { + if (insertLocation != null && tu.equals(insertLocation.getTranslationUnit())) { + return true; + } + } + return false; + } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoringRunner.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoringRunner.java index 4376a4ed727..58cf32b1fca 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoringRunner.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoringRunner.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2009 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2011 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,49 +7,40 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * Institute for Software - initial API and implementation + * Institute for Software - initial API and implementation + * Marc-Andre Laperle *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring.implementmethod; -import org.eclipse.core.resources.IFile; -import org.eclipse.core.runtime.CoreException; 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.ui.CUIPlugin; - -import org.eclipse.cdt.internal.ui.refactoring.RefactoringRunner; +import org.eclipse.cdt.internal.ui.refactoring.RefactoringASTCache; +import org.eclipse.cdt.internal.ui.refactoring.RefactoringRunner2; /** * @author Lukas Felber */ -public class ImplementMethodRefactoringRunner extends RefactoringRunner { +public class ImplementMethodRefactoringRunner extends RefactoringRunner2 { - public ImplementMethodRefactoringRunner(IFile file, ISelection selection, ICElement element, + public ImplementMethodRefactoringRunner(ICElement element, ISelection selection, IShellProvider shellProvider, ICProject cProject) { - super(file, selection, element, shellProvider, cProject); + super(element, selection, shellProvider, cProject); } @Override - public void run() { - ImplementMethodRefactoring refactoring = new ImplementMethodRefactoring(file, selection, celement, project); + public void run(RefactoringASTCache astCache) { + ImplementMethodRefactoring refactoring = new ImplementMethodRefactoring(element, selection, project, astCache); ImplementMethodRefactoringWizard wizard = new ImplementMethodRefactoringWizard(refactoring); RefactoringWizardOpenOperation operator = new RefactoringWizardOpenOperation(wizard); try { - refactoring.lockIndex(); - try { - operator.run(shellProvider.getShell(), refactoring.getName()); - } finally { - refactoring.unlockIndex(); - } + operator.run(shellProvider.getShell(), refactoring.getName()); } catch (InterruptedException e) { Thread.currentThread().interrupt(); - } catch (CoreException e) { - CUIPlugin.log(e); } } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoringWizard.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoringWizard.java index c052390e0fa..10e24aa267b 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoringWizard.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ImplementMethodRefactoringWizard.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2011 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,15 +7,21 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * Institute for Software - initial API and implementation + * Institute for Software - initial API and implementation + * Marc-Andre Laperle *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring.implementmethod; +import java.lang.reflect.InvocationTargetException; import java.util.HashMap; import java.util.Map; +import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.jface.operation.IRunnableWithProgress; import org.eclipse.ltk.ui.refactoring.RefactoringWizard; +import org.eclipse.cdt.ui.CUIPlugin; + /** * @author Mirko Stocker * @@ -46,4 +52,62 @@ public class ImplementMethodRefactoringWizard extends RefactoringWizard { public ParameterNamesInputPage getPageForConfig(MethodToImplementConfig config) { return pagesMap.get(config); } + + /** + * - When cancelling the wizard, RefactoringASTCache gets disposed and releases the lock on the index but + * the preview jobs might still be running and access the index or an index based AST so we need to make sure they + * are done before disposing the cache + * - When proceeding to the last page and finishing the wizard, the + * refactoring will run and possibly use concurrently the same ASTs that the jobs use, so we need to make + * sure the jobs are joined. + */ + protected void cancelAndJoinPreviewJobs() { + boolean isOnePreviewJobRunning = false; + for (ParameterNamesInputPage parameterNamesInputPage : pagesMap.values()) { + isOnePreviewJobRunning |= parameterNamesInputPage.cancelPreviewJob(); + } + + // There are good chances that one job is still running, show a progress bar to the user, join everything + if(isOnePreviewJobRunning) { + try { + getContainer().run(false, false, new IRunnableWithProgress() { + + public void run(IProgressMonitor monitor) throws InvocationTargetException, InterruptedException { + monitor.beginTask(Messages.ImplementMethodRefactoringWizard_CancelingPreviewGeneration, pagesMap.size() + 1); + monitor.worked(1); + + for (ParameterNamesInputPage parameterNamesInputPage : pagesMap.values()) { + parameterNamesInputPage.joinPreviewJob(); + monitor.worked(1); + } + + monitor.done(); + } + }); + } catch (InvocationTargetException e) { + CUIPlugin.log(e); + } catch (InterruptedException e) { + // ignore since not cancelable + } + } + // We don't take any chances, we still join everything. But there are good chances that the jobs are stopped + // so we don't show a progress bar. + else { + for (ParameterNamesInputPage parameterNamesInputPage : pagesMap.values()) { + parameterNamesInputPage.joinPreviewJob(); + } + } + } + + @Override + public boolean performCancel() { + cancelAndJoinPreviewJobs(); + return super.performCancel(); + } + + @Override + public boolean performFinish() { + cancelAndJoinPreviewJobs(); + return super.performFinish(); + } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/Messages.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/Messages.java index 5f84d78d888..bb2914fb5f0 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/Messages.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/Messages.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2005, 2009 IBM Corporation and others. + * Copyright (c) 2005, 2011 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -23,13 +23,16 @@ public final class Messages extends NLS { public static String ParameterNamesInputPage_Title; public static String ParameterNamesInputPage_CompleteMissingMails; - public static String PreviewGenerationNotPossible; public static String ImplementMethodInputPage_PageTitle; public static String ImplementMethodInputPage_SelectAll; public static String ImplementMethodInputPage_DeselectAll; public static String ImplementMethodRefactoringPage_GeneratingPreview; + public static String ImplementMethodRefactoringPage_PreviewCanceled; + public static String ImplementMethodRefactoringPage_PreviewGenerationNotPossible; public static String ImplementMethodRefactoring_NoMethodSelected; public static String ImplementMethodRefactoring_MethodHasImpl; + public static String ImplementMethodRefactoring_NoImplFile; + public static String ImplementMethodRefactoringWizard_CancelingPreviewGeneration; public static String ImplementMethodInputPage_Header; static { diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/MethodDefinitionInsertLocationFinder2.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/MethodDefinitionInsertLocationFinder2.java index 2b8ffc59944..2b3b8f7206d 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/MethodDefinitionInsertLocationFinder2.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/MethodDefinitionInsertLocationFinder2.java @@ -9,14 +9,19 @@ * Contributors: * Institute for Software - initial API and implementation * Sergey Prigogin (Google) + * Marc-Andre Laperle *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring.implementmethod; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.core.runtime.OperationCanceledException; import org.eclipse.cdt.core.dom.ast.IASTDeclaration; import org.eclipse.cdt.core.dom.ast.IASTFileLocation; @@ -42,23 +47,55 @@ import org.eclipse.cdt.internal.ui.refactoring.utils.NodeHelper; */ public class MethodDefinitionInsertLocationFinder2 { - public static InsertLocation2 find(ITranslationUnit declarationTu, IASTFileLocation methodDeclarationLocation, - IASTNode parent, RefactoringASTCache astCache) throws CoreException { + // We cache DefinitionFinder2.getDefinition results because refactorings like Implement Method might want to find multiple + // insert locations in the same translation unit. This prevents many redundant calls to DefinitionFinder2.getDefinition + // and speeds up the process quite a bit. Unfortunately, this has the minor side-effect or having to instantiate this class. + Map cachedDeclarationToDefinition = new HashMap(); + + public InsertLocation2 find(ITranslationUnit declarationTu, IASTFileLocation methodDeclarationLocation, + IASTNode parent, RefactoringASTCache astCache, IProgressMonitor pm) throws CoreException { IASTDeclaration[] declarations = NodeHelper.getDeclarations(parent); InsertLocation2 insertLocation = new InsertLocation2(); + + Collection allPreviousSimpleDeclarationsFromClassInReverseOrder = getAllPreviousSimpleDeclarationsFromClassInReverseOrder(declarations, methodDeclarationLocation, pm); + Collection allFollowingSimpleDeclarationsFromClass = getAllFollowingSimpleDeclarationsFromClass(declarations, methodDeclarationLocation, pm); - for (IASTSimpleDeclaration simpleDeclaration : getAllPreviousSimpleDeclarationsFromClassInReverseOrder( - declarations, methodDeclarationLocation)) { - IASTName definition = DefinitionFinder2.getDefinition(simpleDeclaration, astCache); - if (definition != null) { - insertLocation.setNodeToInsertAfter(findFirstSurroundingParentFunctionNode(definition), - definition.getTranslationUnit().getOriginatingTranslationUnit()); + for (IASTSimpleDeclaration simpleDeclaration : allPreviousSimpleDeclarationsFromClassInReverseOrder) { + if (pm != null && pm.isCanceled()) { + throw new OperationCanceledException(); } + + IASTName definition = null; + if (cachedDeclarationToDefinition.containsKey(simpleDeclaration)) { + definition = cachedDeclarationToDefinition.get(simpleDeclaration); + } else { + definition = DefinitionFinder2.getDefinition(simpleDeclaration, astCache, pm); + if (definition != null) { + cachedDeclarationToDefinition.put(simpleDeclaration, definition); + } + } + + if (definition != null) { + insertLocation.setNodeToInsertAfter(findFirstSurroundingParentFunctionNode( + definition), definition.getTranslationUnit().getOriginatingTranslationUnit()); + } } - for (IASTSimpleDeclaration simpleDeclaration : getAllFollowingSimpleDeclarationsFromClass( - declarations, methodDeclarationLocation)) { - IASTName definition = DefinitionFinder2.getDefinition(simpleDeclaration, astCache); + for (IASTSimpleDeclaration simpleDeclaration : allFollowingSimpleDeclarationsFromClass) { + if (pm != null && pm.isCanceled()) { + throw new OperationCanceledException(); + } + + IASTName definition = null; + if (cachedDeclarationToDefinition.containsKey(simpleDeclaration)) { + definition = cachedDeclarationToDefinition.get(simpleDeclaration); + } else { + definition = DefinitionFinder2.getDefinition(simpleDeclaration, astCache, pm); + if (definition != null) { + cachedDeclarationToDefinition.put(simpleDeclaration, definition); + } + } + if (definition != null) { insertLocation.setNodeToInsertBefore(findFirstSurroundingParentFunctionNode(definition), definition.getTranslationUnit().getOriginatingTranslationUnit()); @@ -105,13 +142,17 @@ public class MethodDefinitionInsertLocationFinder2 { * * @param declarations to be searched * @param methodPosition on which the search aborts + * @param pm * @return all declarations, sorted in reverse order */ private static Collection getAllPreviousSimpleDeclarationsFromClassInReverseOrder( - IASTDeclaration[] declarations, IASTFileLocation methodPosition) { + IASTDeclaration[] declarations, IASTFileLocation methodPosition, IProgressMonitor pm) { ArrayList outputDeclarations = new ArrayList(); if (declarations.length >= 0) { for (IASTDeclaration decl : declarations) { + if (pm != null && pm.isCanceled()) { + return outputDeclarations; + } if (decl.getFileLocation().getStartingLineNumber() >= methodPosition.getStartingLineNumber()) { break; } @@ -125,11 +166,14 @@ public class MethodDefinitionInsertLocationFinder2 { } private static Collection getAllFollowingSimpleDeclarationsFromClass( - IASTDeclaration[] declarations, IASTFileLocation methodPosition) { + IASTDeclaration[] declarations, IASTFileLocation methodPosition, IProgressMonitor pm) { ArrayList outputDeclarations = new ArrayList(); if (declarations.length >= 0) { for (IASTDeclaration decl : declarations) { + if (pm != null && pm.isCanceled()) { + return outputDeclarations; + } if (isMemberFunctionDeclaration(decl) && decl.getFileLocation().getStartingLineNumber() > methodPosition.getStartingLineNumber() ) { outputDeclarations.add((IASTSimpleDeclaration) decl); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ParameterNamesInputPage.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ParameterNamesInputPage.java index 277fdfcc2f0..8247988ebd6 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ParameterNamesInputPage.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/ParameterNamesInputPage.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2011 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,35 +7,38 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * Institute for Software - initial API and implementation + * Institute for Software - initial API and implementation + * Marc-Andre Laperle *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring.implementmethod; import java.util.HashMap; import org.eclipse.core.runtime.CoreException; -import org.eclipse.core.runtime.NullProgressMonitor; +import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.OperationCanceledException; +import org.eclipse.core.runtime.Status; +import org.eclipse.core.runtime.jobs.Job; import org.eclipse.jface.wizard.IWizardPage; import org.eclipse.ltk.core.refactoring.Change; import org.eclipse.ltk.core.refactoring.CompositeChange; import org.eclipse.ltk.ui.refactoring.UserInputWizardPage; import org.eclipse.swt.SWT; -import org.eclipse.swt.events.DisposeEvent; -import org.eclipse.swt.events.DisposeListener; import org.eclipse.swt.layout.GridData; import org.eclipse.swt.layout.GridLayout; import org.eclipse.swt.widgets.Composite; -import org.eclipse.swt.widgets.Label; import org.eclipse.text.edits.InsertEdit; import org.eclipse.text.edits.MultiTextEdit; +import org.eclipse.text.edits.TextEdit; +import org.eclipse.cdt.ui.CUIPlugin; import org.eclipse.cdt.ui.refactoring.CTextFileChange; import org.eclipse.cdt.internal.ui.preferences.formatter.TranslationUnitPreview; +import org.eclipse.cdt.internal.ui.refactoring.CCompositeChange; import org.eclipse.cdt.internal.ui.refactoring.ModificationCollector; import org.eclipse.cdt.internal.ui.refactoring.dialogs.ValidatingLabeledTextField; -import org.eclipse.cdt.internal.ui.refactoring.utils.DelayedJobRunner; /** * InputPage used by the ImplementMethod refactoring if its necessary to enter additional parameter names. @@ -45,9 +48,10 @@ import org.eclipse.cdt.internal.ui.refactoring.utils.DelayedJobRunner; */ public class ParameterNamesInputPage extends UserInputWizardPage { + private static final int PREVIEW_UPDATE_DELAY = 500; private MethodToImplementConfig config; private TranslationUnitPreview translationUnitPreview; - private DelayedJobRunner delayedPreviewUpdater; + private Job delayedPreviewUpdater; private ImplementMethodRefactoringWizard wizard; public ParameterNamesInputPage(MethodToImplementConfig config, ImplementMethodRefactoringWizard wizard) { @@ -62,9 +66,8 @@ public class ParameterNamesInputPage extends UserInputWizardPage { superComposite.setLayout(new GridLayout()); - Label label = new Label(superComposite, SWT.NONE); - label.setText(Messages.ParameterNamesInputPage_CompleteMissingMails); - label.setLayoutData(new GridData(GridData.GRAB_HORIZONTAL | GridData.HORIZONTAL_ALIGN_FILL)); + setTitle(Messages.ImplementMethodInputPage_PageTitle); + setMessage(Messages.ParameterNamesInputPage_CompleteMissingMails); ValidatingLabeledTextField validatingLabeledTextField = new ValidatingLabeledTextField(superComposite); validatingLabeledTextField.setLayoutData(new GridData(GridData.GRAB_HORIZONTAL | GridData.HORIZONTAL_ALIGN_FILL)); @@ -101,56 +104,79 @@ public class ParameterNamesInputPage extends UserInputWizardPage { } private InsertEdit getInsertEdit(CompositeChange compositeChange) { - for(Change actChange : compositeChange.getChildren()) { - if(actChange instanceof CompositeChange) { + for (Change actChange : compositeChange.getChildren()) { + if (actChange instanceof CompositeChange) { return getInsertEdit((CompositeChange) actChange); } else if (actChange instanceof CTextFileChange) { CTextFileChange textFileChange = (CTextFileChange) actChange; - MultiTextEdit multiEdit = (MultiTextEdit) textFileChange.getEdit(); - if(multiEdit.getChildrenSize() == 0) { - continue; + TextEdit edit = textFileChange.getEdit(); + if (edit instanceof MultiTextEdit) { + MultiTextEdit multiEdit = (MultiTextEdit) edit; + if (multiEdit.getChildrenSize() == 0) { + continue; + } + TextEdit textEdit = multiEdit.getChildren()[0]; + if (textEdit instanceof InsertEdit) { + return (InsertEdit) textEdit; + } } - return (InsertEdit) multiEdit.getChildren()[0]; } } return null; } - public String createFunctionDefinitionSignature() { + public String createFunctionDefinitionSignature(IProgressMonitor monitor) { try { ModificationCollector collector = new ModificationCollector(); - ((ImplementMethodRefactoring)wizard.getRefactoring()).createDefinition(collector, config, new NullProgressMonitor()); - InsertEdit insertEdit = getInsertEdit(collector.createFinalChange()); + ImplementMethodRefactoring implementMethodRefactoring = (ImplementMethodRefactoring)wizard.getRefactoring(); + CCompositeChange finalChange = null; + // We can have multiple preview jobs. We don't + // want multiple jobs concurrently using the same ASTs + synchronized (implementMethodRefactoring) { + implementMethodRefactoring.createDefinition(collector, config, monitor); + finalChange = collector.createFinalChange(); + } + InsertEdit insertEdit = getInsertEdit(finalChange); + if (insertEdit == null) { + return Messages.ImplementMethodRefactoringPage_PreviewGenerationNotPossible; + } + return insertEdit.getText().trim(); } catch (OperationCanceledException e) { - return Messages.PreviewGenerationNotPossible; + return Messages.ImplementMethodRefactoringPage_PreviewCanceled; } catch (CoreException e) { - return Messages.PreviewGenerationNotPossible; + return Messages.ImplementMethodRefactoringPage_PreviewGenerationNotPossible; } } private void createPreview(Composite superComposite) { translationUnitPreview = new TranslationUnitPreview(new HashMap(), superComposite); translationUnitPreview.getControl().setLayoutData(new GridData(GridData.GRAB_HORIZONTAL | GridData.HORIZONTAL_ALIGN_FILL)); - Runnable runnable = new Runnable() { - public void run() { + + delayedPreviewUpdater = new Job(Messages.ImplementMethodRefactoringPage_GeneratingPreview) { + @Override + protected IStatus run(IProgressMonitor monitor) { setPreviewText(Messages.ImplementMethodRefactoringPage_GeneratingPreview); - setPreviewText(createFunctionDefinitionSignature()); + String functionDefinitionSignature = createFunctionDefinitionSignature(monitor); + if (monitor.isCanceled()) { + return Status.CANCEL_STATUS; + } + setPreviewText(functionDefinitionSignature); + return Status.OK_STATUS; } private void setPreviewText(final String text) { - getShell().getDisplay().asyncExec(new Runnable() { - public void run() { - translationUnitPreview.setPreviewText(text); - }}); + if (getShell() != null && getShell().getDisplay() != null) { + getShell().getDisplay().asyncExec(new Runnable() { + public void run() { + if (translationUnitPreview.getControl() != null && !translationUnitPreview.getControl().isDisposed()) { + translationUnitPreview.setPreviewText(text); + } + }}); + } } }; - delayedPreviewUpdater = new DelayedJobRunner(runnable, 500); - delayedPreviewUpdater.start(); - superComposite.addDisposeListener(new DisposeListener() { - - public void widgetDisposed(DisposeEvent e) { - delayedPreviewUpdater.stop(); - }}); + + delayedPreviewUpdater.schedule(PREVIEW_UPDATE_DELAY); } @Override @@ -161,18 +187,35 @@ public class ParameterNamesInputPage extends UserInputWizardPage { @Override public IWizardPage getNextPage() { MethodToImplementConfig nextConfig = ((ImplementMethodRefactoring)wizard.getRefactoring()).getRefactoringData().getNextConfigNeedingParameterNames(config); - if(nextConfig != null) { + if (nextConfig != null) { return wizard.getPageForConfig(nextConfig); - }else { + } else { + wizard.cancelAndJoinPreviewJobs(); return computeSuccessorPage(); } } + protected boolean cancelPreviewJob() { + // We cannot rely on getState being accurate in all cases so we only use this + // as a hint to change the preview text + if (delayedPreviewUpdater.getState() != Job.NONE) { + translationUnitPreview.setPreviewText(Messages.ImplementMethodRefactoringPage_PreviewCanceled); + } + return !delayedPreviewUpdater.cancel(); + } + + protected void joinPreviewJob() { + try { + delayedPreviewUpdater.join(); + } catch (InterruptedException e1) { + CUIPlugin.log(e1); + } + } + private void updatePreview() { if (translationUnitPreview == null) { return; } - delayedPreviewUpdater.runJob(); - } - + delayedPreviewUpdater.schedule(PREVIEW_UPDATE_DELAY); + } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/messages.properties b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/messages.properties index e36707f20c1..1b60d0f979e 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/messages.properties +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/implementmethod/messages.properties @@ -1,5 +1,5 @@ ############################################################################### -# Copyright (c) 2008, 2009 Institute for Software, HSR Hochschule fuer Technik +# Copyright (c) 2008, 2011 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 # IBM Corporation +# Marc-Andre Laperle ############################################################################### ParameterNamesInputPage_Title=Implement Method ImplementMethodInputPage_Header=Select Methods to implement: @@ -16,8 +17,10 @@ ParameterNamesInputPage_CompleteMissingMails=Please complete the missing variabl ImplementMethodInputPage_PageTitle=Implement Method ImplementMethodInputPage_SelectAll=Select All ImplementMethodInputPage_DeselectAll=Deselect All -ImplementMethodRefactoring_MethodDefinition=Method Definition -PreviewGenerationNotPossible=Preview generation not possible +ImplementMethodRefactoringPage_PreviewGenerationNotPossible=Preview generation not possible ImplementMethodRefactoringPage_GeneratingPreview=Generating preview... +ImplementMethodRefactoringPage_PreviewCanceled=Preview canceled ImplementMethodRefactoring_NoMethodSelected=No method declaration selected -ImplementMethodRefactoring_MethodHasImpl=This method already has an implementation. \ No newline at end of file +ImplementMethodRefactoring_MethodHasImpl=This method already has an implementation. +ImplementMethodRefactoring_NoImplFile=No implementation file found for one or more method. Inserting definition(s) into the header file. +ImplementMethodRefactoringWizard_CancelingPreviewGeneration=Canceling preview generation \ No newline at end of file diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/DefinitionFinder2.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/DefinitionFinder2.java index 86c7baa48b5..ec9fe02dfb5 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/DefinitionFinder2.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/DefinitionFinder2.java @@ -44,7 +44,7 @@ import org.eclipse.cdt.internal.ui.util.EditorUtility; public class DefinitionFinder2 { public static IASTName getDefinition(IASTSimpleDeclaration simpleDeclaration, - RefactoringASTCache astCache) throws CoreException { + RefactoringASTCache astCache, IProgressMonitor pm) throws CoreException { IIndex index = astCache.getIndex(); IASTDeclarator declarator = simpleDeclaration.getDeclarators()[0]; if (index == null) { @@ -54,15 +54,18 @@ public class DefinitionFinder2 { if (binding == null) { return null; } - return getDefinition(binding, astCache, index); + return getDefinition(binding, astCache, index, pm); } private static IASTName getDefinition(IIndexBinding binding, - RefactoringASTCache astCache, IIndex index) throws CoreException { + RefactoringASTCache astCache, IIndex index, IProgressMonitor pm) throws CoreException { Set searchedFiles = new HashSet(); List definitions = new ArrayList(); IEditorPart[] dirtyEditors = EditorUtility.getDirtyEditors(true); for (IEditorPart editor : dirtyEditors) { + if (pm != null && pm.isCanceled()) { + throw new OperationCanceledException(); + } IEditorInput editorInput = editor.getEditorInput(); if (editorInput instanceof ITranslationUnitEditorInput) { ITranslationUnit tu = @@ -74,10 +77,13 @@ public class DefinitionFinder2 { IIndexName[] definitionsFromIndex = index.findDefinitions(binding); for (IIndexName name : definitionsFromIndex) { + if (pm != null && pm.isCanceled()) { + throw new OperationCanceledException(); + } ITranslationUnit tu = CoreModelUtil.findTranslationUnitForLocation( name.getFile().getLocation(), null); if (searchedFiles.add(tu.getLocation().toOSString())) { - findDefinitionsInTranslationUnit(binding, tu, astCache, definitions, null); + findDefinitionsInTranslationUnit(binding, tu, astCache, definitions, pm); } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/DelayedJobRunner.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/DelayedJobRunner.java deleted file mode 100644 index 6841da6e971..00000000000 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/DelayedJobRunner.java +++ /dev/null @@ -1,78 +0,0 @@ -/******************************************************************************* - * Copyright (c) 2008 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 - * which accompanies this distribution, and is available at - * http://www.eclipse.org/legal/epl-v10.html - * - * Contributors: - * Institute for Software - initial API and implementation - *******************************************************************************/ -package org.eclipse.cdt.internal.ui.refactoring.utils; - -/** - * Runs a job delayed by a given time when runJob gets called.
- * Only the time of the last runJob call is considered. So if runJob - * gets called before the time expires the first call will be ignored and the job will be run - * after the time of the second call + the delay time.
- * DelayedJobRunner isn't thread save (not neede till now).
- * Oh and one more thing. Do call the stop method when the runner isn't needed any more. ;-) - * - * @author Lukas Felber - * - */ -public class DelayedJobRunner { - private Runnable job; - private long lastUpdateEventTime; - private long delayTimeInMillis; - private boolean shouldStop; - private boolean shouldUpdate; - private static final long sleepTimeInMillis = 50; - - public DelayedJobRunner(Runnable job, long delayTimeInMillis) { - this.job = job; - this.delayTimeInMillis = delayTimeInMillis; - shouldUpdate = true; - } - - public void start() { - shouldStop = false; - new Thread( - new Runnable() { - public void run() { - startUpdateLoop(); - } - } - ).start(); - } - - public void runJob() { - shouldUpdate = true; - lastUpdateEventTime = System.currentTimeMillis(); - } - - private void startUpdateLoop() { - try { - while (!shouldStop) { - if (shouldUpdate && isDelayTimeOver()) { - lastUpdateEventTime = System.currentTimeMillis(); - shouldUpdate = false; - job.run(); - } - Thread.sleep(sleepTimeInMillis); - } - } catch (Exception e) { - //do nothing expect die. - } - } - - private boolean isDelayTimeOver() { - long currentTime = System.currentTimeMillis(); - return lastUpdateEventTime + delayTimeInMillis < currentTime; - } - - public void stop() { - this.shouldStop = true; - } -} diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/NameHelper.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/NameHelper.java index 4334ce4aa94..6b2b3889fb4 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/NameHelper.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/NameHelper.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2010 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2011 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 @@ -13,7 +13,6 @@ package org.eclipse.cdt.internal.ui.refactoring.utils; import java.util.regex.Pattern; -import org.eclipse.core.resources.IFile; import org.eclipse.core.runtime.CoreException; import org.eclipse.cdt.core.dom.ast.ASTTypeUtil; @@ -24,11 +23,14 @@ import org.eclipse.cdt.core.dom.ast.IBinding; import org.eclipse.cdt.core.dom.ast.IType; import org.eclipse.cdt.core.dom.ast.IVariable; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTQualifiedName; +import org.eclipse.cdt.core.model.ITranslationUnit; import org.eclipse.cdt.core.parser.Keywords; import org.eclipse.cdt.core.parser.util.CharArrayIntMap; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTQualifiedName; +import org.eclipse.cdt.internal.ui.refactoring.RefactoringASTCache; + /** * Helps with IASTNames. * @@ -56,19 +58,19 @@ public class NameHelper { * the namespace at the declaration position and the target namespace at the target position. * * @param declaratorName of the method or function - * @param declarationFile + * @param declarationTu translation unit of the method or function declaration + * @param insertFileTu translation unit of the file where the implementation is being inserted * @param selectionOffset the offset in the declarationFile, usually the position or selection of the declaration - * @param insertFile the target file in which the definition is inserted * @param insertLocation * @return the correct name for the target * @throws CoreException */ - public static ICPPASTQualifiedName createQualifiedNameFor(IASTName declaratorName, IFile declarationFile, int selectionOffset, IFile insertFile, int insertLocation) + public static ICPPASTQualifiedName createQualifiedNameFor(IASTName declaratorName, ITranslationUnit declarationTu, int selectionOffset, ITranslationUnit insertFileTu, int insertLocation, RefactoringASTCache astCache) throws CoreException { ICPPASTQualifiedName qname = new CPPASTQualifiedName(); - IASTName[] declarationNames = NamespaceHelper.getSurroundingNamespace(declarationFile, selectionOffset).getNames(); - IASTName[] implementationNames = NamespaceHelper.getSurroundingNamespace(insertFile, insertLocation).getNames(); + IASTName[] declarationNames = NamespaceHelper.getSurroundingNamespace(declarationTu, selectionOffset, astCache).getNames(); + IASTName[] implementationNames = NamespaceHelper.getSurroundingNamespace(insertFileTu, insertLocation, astCache).getNames(); for (int i = 0; i < declarationNames.length; i++) { if (i >= implementationNames.length) { diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/NamespaceHelper.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/NamespaceHelper.java index db5b4f8af74..f8ce66ae422 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/NamespaceHelper.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/NamespaceHelper.java @@ -11,8 +11,8 @@ *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring.utils; -import org.eclipse.core.resources.IFile; import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.IPath; import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier; import org.eclipse.cdt.core.dom.ast.IASTName; @@ -24,6 +24,7 @@ import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTNamespaceDefinition; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTQualifiedName; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTemplateDeclaration; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTemplateParameter; +import org.eclipse.cdt.core.model.ITranslationUnit; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTNamedTypeSpecifier; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTQualifiedName; @@ -31,27 +32,30 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTSimpleTypeTemplatePara import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTTemplateId; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTTypeId; +import org.eclipse.cdt.internal.ui.refactoring.RefactoringASTCache; + /** * Helper class to find Namespace informations. * @author Mirko Stocker */ public class NamespaceHelper { /** - * Returns the qualified name of all namespaces that are defined at the specified file and offset. + * Returns the qualified name of all namespaces that are defined at the specified translation unit and offset. * - * @param insertFile + * @param translationUnit * @param offset + * @param astCache * @return ICPPASTQualifiedName with the names of all namespaces * @throws CoreException */ - public static ICPPASTQualifiedName getSurroundingNamespace(final IFile insertFile, final int offset) + public static ICPPASTQualifiedName getSurroundingNamespace(final ITranslationUnit translationUnit, final int offset, RefactoringASTCache astCache) throws CoreException { final CPPASTQualifiedName qualifiedName = new CPPASTQualifiedName(); - TranslationUnitHelper.loadTranslationUnit(insertFile, false).accept(new CPPASTAllVisitor() { + astCache.getAST(translationUnit, null).accept(new CPPASTAllVisitor() { @Override public int visit(IASTDeclSpecifier declSpec) { - if (declSpec instanceof ICPPASTCompositeTypeSpecifier && checkFileNameAndLocation(insertFile, offset, declSpec)) { + if (declSpec instanceof ICPPASTCompositeTypeSpecifier && checkFileNameAndLocation(translationUnit.getLocation(), offset, declSpec)) { qualifiedName.addName(createNameWithTemplates(declSpec)); } return super.visit(declSpec); @@ -59,7 +63,7 @@ public class NamespaceHelper { @Override public int visit(ICPPASTNamespaceDefinition namespace) { - if (checkFileNameAndLocation(insertFile, offset, namespace)) { + if (checkFileNameAndLocation(translationUnit.getLocation(), offset, namespace)) { qualifiedName.addName((namespace).getName().copy()); } @@ -70,8 +74,8 @@ public class NamespaceHelper { return qualifiedName; } - private static boolean checkFileNameAndLocation(final IFile insertFile, final int offset, IASTNode namespace) { - boolean fileNameOk = namespace.getFileLocation().getFileName().endsWith(insertFile.getLocation().toOSString()); + private static boolean checkFileNameAndLocation(final IPath path, final int offset, IASTNode namespace) { + boolean fileNameOk = namespace.getFileLocation().getFileName().endsWith(path.toOSString()); if (!fileNameOk) { return false; } 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 f20c30f6c9f..db66437114a 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 @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2008, 2009 Institute for Software, HSR Hochschule fuer Technik + * Copyright (c) 2008, 2011 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 @@ -48,14 +48,14 @@ public class ImplementMethodAction extends RefactoringAction { @Override public void run(IShellProvider shellProvider, ICElement elem) { - new ImplementMethodRefactoringRunner(null, null, elem, shellProvider, elem.getCProject()).run(); + new ImplementMethodRefactoringRunner(elem, null, shellProvider, elem.getCProject()).run(); } @Override public void run(IShellProvider shellProvider, IWorkingCopy wc, ITextSelection selection) { IResource res = wc.getResource(); if (res instanceof IFile) { - new ImplementMethodRefactoringRunner((IFile) res, selection, null, shellProvider, wc.getCProject()).run(); + new ImplementMethodRefactoringRunner(wc, selection, shellProvider, wc.getCProject()).run(); } }