From 95e3b558d51b6d16abec7f05f45713fb79e0f2f7 Mon Sep 17 00:00:00 2001 From: Emanuel Graf Date: Tue, 20 Oct 2009 10:19:04 +0000 Subject: [PATCH] FIXED - bug 291190: Refactoring tests use the index without read-locks https://bugs.eclipse.org/bugs/show_bug.cgi?id=291190 --- .../internal/ui/refactoring/CRefactoring.java | 6 + .../ui/refactoring/MethodContext.java | 4 + .../extractfunction/ExtractExpression.java | 86 +++--- .../ExtractFunctionRefactoring.java | 251 +++++++++--------- .../extractfunction/SimilarFinderVisitor.java | 27 +- .../extractfunction/TrailName.java | 49 ++-- .../TrailNodeEqualityChecker.java | 117 ++++---- .../GenerateGettersAndSettersRefactoring.java | 57 ++-- .../ui/refactoring/utils/NodeHelper.java | 49 ++-- 9 files changed, 328 insertions(+), 318 deletions(-) diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/CRefactoring.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/CRefactoring.java index cf3a30b0d3c..b41d67122e9 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/CRefactoring.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/CRefactoring.java @@ -301,6 +301,12 @@ public abstract class CRefactoring extends Refactoring { return fIndex; } + + + public IASTTranslationUnit getUnit() { + return unit; + } + protected ArrayList findAllMarkedNames() { final ArrayList namesVector = new ArrayList(); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/MethodContext.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/MethodContext.java index 824fd44b3ba..a2453666e7e 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/MethodContext.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/MethodContext.java @@ -194,6 +194,10 @@ public class MethodContext { return null; } + public boolean isInline() { + return qname == null; + } + private static ICPPClassType getClassBinding(ICPPASTQualifiedName qname){ IASTName classname = qname.getNames()[qname.getNames().length - 2]; ICPPClassType bind = (ICPPClassType)classname.resolveBinding(); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractExpression.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractExpression.java index 64b804155f1..0ffeb50483e 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractExpression.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractExpression.java @@ -15,7 +15,6 @@ import java.util.List; import org.eclipse.text.edits.TextEditGroup; -import org.eclipse.cdt.core.dom.ast.DOMException; import org.eclipse.cdt.core.dom.ast.IASTBinaryExpression; import org.eclipse.cdt.core.dom.ast.IASTCompoundStatement; import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier; @@ -29,10 +28,12 @@ import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclSpecifier; import org.eclipse.cdt.core.dom.ast.IBasicType; import org.eclipse.cdt.core.dom.ast.IBinding; import org.eclipse.cdt.core.dom.ast.IType; +import org.eclipse.cdt.core.dom.ast.ITypedef; import org.eclipse.cdt.core.dom.ast.IBasicType.Kind; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTBinaryExpression; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTNewExpression; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTQualifiedName; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPClassType; import org.eclipse.cdt.core.dom.rewrite.ASTRewrite; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTBinaryExpression; @@ -47,10 +48,8 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTReturnStatement; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTSimpleDeclSpecifier; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTSimpleDeclaration; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPBasicType; -import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPClassType; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPFunction; -import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPNamespace; -import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPTypedef; +import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVisitor; import org.eclipse.cdt.internal.ui.refactoring.NodeContainer.NameInformation; @@ -120,8 +119,11 @@ public class ExtractExpression extends ExtractedFunctionConstructionHelper { } return createSimpleDeclSpecifier(Kind.eVoid); } - - return declSpecifier.copy(); + if(declSpecifier.isFrozen()) { + return declSpecifier.copy(); + }else { + return declSpecifier; + } } private IASTDeclSpecifier handleLiteralExpression(IASTLiteralExpression extractedNode) { @@ -207,67 +209,41 @@ public class ExtractExpression extends ExtractedFunctionConstructionHelper { CPPBasicType basicType = (CPPBasicType) expressionType; return createSimpleDeclSpecifier(basicType.getKind()); - } else if (expressionType instanceof CPPTypedef) { + } else if (expressionType instanceof ITypedef) { - return getDeclSpecForType(((CPPTypedef)expressionType)); + return getDeclSpecForType(((ITypedef)expressionType)); - } else if (expressionType instanceof CPPClassType) { + } else if (expressionType instanceof ICPPClassType) { - return getDeclSpecForType((CPPClassType)expressionType); + return getDeclSpecForType((ICPPClassType)expressionType); } return null; } - private CPPASTNamedTypeSpecifier getDeclSpecForType(CPPClassType classType) { - + private IASTDeclSpecifier getDeclSpecForType(ICPPClassType expressionType) { IASTName name = null; - try { - IBinding bind = classType.getOwner(); - if (bind instanceof CPPNamespace) { - ICPPASTQualifiedName qname = getQname(classType, bind); - qname.addName((IASTName) classType.getDefinition().copy()); - name = qname; - }else { - name = (IASTName) classType.getDefinition(); - } - } catch (DOMException e) { - // TODO Auto-generated catch block - e.printStackTrace(); - name= new CPPASTName(classType.getNameCharArray()); + + char[][] qualifiedNameCharArray = CPPVisitor.getQualifiedNameCharArray(expressionType); + name = new CPPASTQualifiedName(); + for (char[] cs : qualifiedNameCharArray) { + ((ICPPASTQualifiedName) name).addName(new CPPASTName(cs)); } - - return new CPPASTNamedTypeSpecifier(name.copy()); + + return new CPPASTNamedTypeSpecifier(name); } - private ICPPASTQualifiedName getQname(IBinding classType, IBinding bind) { - CPPNamespace namespace = (CPPNamespace) bind; - char[][] names = namespace.getFullyQualifiedNameCharArray(); - CPPASTQualifiedName qname = new CPPASTQualifiedName(); - for (char[] string : names) { - qname.addName(new CPPASTName(string)); + private CPPASTNamedTypeSpecifier getDeclSpecForType(ITypedef classType) { + + IASTName name = null; + char[][] qualifiedNameCharArray = CPPVisitor.getQualifiedNameCharArray(classType); + name = new CPPASTQualifiedName(); + for (char[] cs : qualifiedNameCharArray) { + ((ICPPASTQualifiedName) name).addName(new CPPASTName(cs)); } - return qname; + + return new CPPASTNamedTypeSpecifier(name); } - private IASTDeclSpecifier getDeclSpecForType(CPPTypedef typedef) { - IASTName name = null; - try { - IBinding bind = typedef.getOwner(); - if (bind instanceof CPPNamespace) { - ICPPASTQualifiedName qname = getQname(typedef, bind); - qname.addName((IASTName) typedef.getDefinition().copy()); - name = qname; - }else { - name = (IASTName) typedef.getDefinition(); - } - } catch (DOMException e) { - // TODO Auto-generated catch block - e.printStackTrace(); - name= new CPPASTName(typedef.getNameCharArray()); - } - return new CPPASTNamedTypeSpecifier(name.copy()); - } - private static IASTDeclSpecifier createSimpleDeclSpecifier(IBasicType.Kind type) { IASTSimpleDeclSpecifier declSpec = new CPPASTSimpleDeclSpecifier(); declSpec.setType(type); @@ -307,8 +283,8 @@ public class ExtractExpression extends ExtractedFunctionConstructionHelper { return declaration.getDeclSpecifier(); } } - }else if(binding instanceof CPPTypedef) { - CPPTypedef typedef = (CPPTypedef) binding; + }else if(binding instanceof ITypedef) { + ITypedef typedef = (ITypedef) binding; return new CPPASTNamedTypeSpecifier(new CPPASTName(typedef.getNameCharArray())); } 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 8682806a125..defe251be1c 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 @@ -57,7 +57,6 @@ import org.eclipse.cdt.core.dom.ast.IASTNode; import org.eclipse.cdt.core.dom.ast.IASTParameterDeclaration; import org.eclipse.cdt.core.dom.ast.IASTPointerOperator; import org.eclipse.cdt.core.dom.ast.IASTReturnStatement; -import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclSpecifier; import org.eclipse.cdt.core.dom.ast.IASTSimpleDeclaration; import org.eclipse.cdt.core.dom.ast.IASTStandardFunctionDeclarator; import org.eclipse.cdt.core.dom.ast.IASTStatement; @@ -72,11 +71,9 @@ 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.ICPPASTOperatorName; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTQualifiedName; -import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTSimpleDeclSpecifier; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTemplateDeclaration; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTemplateId; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTemplateParameter; -import org.eclipse.cdt.core.dom.ast.cpp.ICPPBinding; import org.eclipse.cdt.core.dom.ast.cpp.ICPPClassType; import org.eclipse.cdt.core.dom.ast.cpp.ICPPMethod; import org.eclipse.cdt.core.dom.rewrite.ASTRewrite; @@ -98,7 +95,6 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTInitializerExpression; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTName; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTQualifiedName; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTReturnStatement; -import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTSimpleDeclSpecifier; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTSimpleDeclaration; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTTemplateDeclaration; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPNodeFactory; @@ -155,72 +151,83 @@ public class ExtractFunctionRefactoring extends CRefactoring { @Override public RefactoringStatus checkInitialConditions(IProgressMonitor pm) - throws CoreException, OperationCanceledException { + throws CoreException, OperationCanceledException { SubMonitor sm = SubMonitor.convert(pm, 10); - RefactoringStatus status = super.checkInitialConditions(sm.newChild(6)); + try { + lockIndex(); - container = findExtractableNodes(); - sm.worked(1); + try { + super.checkInitialConditions(sm.newChild(6)); - if (isProgressMonitorCanceld(sm, initStatus)) - return initStatus; + container = findExtractableNodes(); + sm.worked(1); - checkForNonExtractableStatements(container, status); - sm.worked(1); + if (isProgressMonitorCanceld(sm, initStatus)) + return initStatus; - if (isProgressMonitorCanceld(sm, initStatus)) - return initStatus; + checkForNonExtractableStatements(container, initStatus); + sm.worked(1); - container.findAllNames(); - markWriteAccess(); - sm.worked(1); + if (isProgressMonitorCanceld(sm, initStatus)) + return initStatus; - if (isProgressMonitorCanceld(sm, initStatus)) - return initStatus; + container.findAllNames(); + markWriteAccess(); + sm.worked(1); - container.getAllAfterUsedNames(); - info.setAllUsedNames(container.getUsedNamesUnique()); + if (isProgressMonitorCanceld(sm, initStatus)) + return initStatus; - if (container.size() < 1) { - status.addFatalError(Messages.ExtractFunctionRefactoring_NoStmtSelected); - sm.done(); - return status; - } + container.getAllAfterUsedNames(); + info.setAllUsedNames(container.getUsedNamesUnique()); - if (container.getAllDeclaredInScope().size() > 1) { - status.addFatalError(Messages.ExtractFunctionRefactoring_TooManySelected); - } else if (container.getAllDeclaredInScope().size() == 1) { - info.setInScopeDeclaredVariable(container.getAllDeclaredInScope().get(0)); - } - - extractedFunctionConstructionHelper = ExtractedFunctionConstructionHelper + if (container.size() < 1) { + initStatus.addFatalError(Messages.ExtractFunctionRefactoring_NoStmtSelected); + sm.done(); + return initStatus; + } + + if (container.getAllDeclaredInScope().size() > 1) { + initStatus.addFatalError(Messages.ExtractFunctionRefactoring_TooManySelected); + } else if (container.getAllDeclaredInScope().size() == 1) { + info.setInScopeDeclaredVariable(container.getAllDeclaredInScope().get(0)); + } + + extractedFunctionConstructionHelper = ExtractedFunctionConstructionHelper .createFor(container.getNodesToWrite()); - boolean isExtractExpression = container.getNodesToWrite().get(0) instanceof IASTExpression; - info.setExtractExpression(isExtractExpression); + 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); - - if (info.getInScopeDeclaredVariable() != null) { - info.getInScopeDeclaredVariable().setUserSetIsReturnValue(true); - } - for (int i = 0; i < info.getAllUsedNames().size(); i++) { - if (!info.getAllUsedNames().get(i).isDeclarationInScope()) { - NameInformation name = info.getAllUsedNames().get(i); - if(!name.isReturnValue()) { - name.setUserSetIsReference(name.isReference()); + info.setDeclarator(getDeclaration(container.getNodesToWrite().get(0))); + MethodContext context = NodeHelper.findMethodContext(container.getNodesToWrite().get(0), getIndex()); + info.setMethodContext(context); + + if (info.getInScopeDeclaredVariable() != null) { + info.getInScopeDeclaredVariable().setUserSetIsReturnValue(true); } + for (int i = 0; i < info.getAllUsedNames().size(); i++) { + if (!info.getAllUsedNames().get(i).isDeclarationInScope()) { + NameInformation name = info.getAllUsedNames().get(i); + if(!name.isReturnValue()) { + name.setUserSetIsReference(name.isReference()); + } + } + } + if(unit != null) { + IIndex index = CCorePlugin.getIndexManager().getIndex(project); + unit.setIndex(index); + } + + sm.done(); } + finally { + unlockIndex(); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); } - if(unit != null) { - IIndex index = CCorePlugin.getIndexManager().getIndex(project); - unit.setIndex(index); - } - - sm.done(); - return status; + return initStatus; } private void markWriteAccess() throws CoreException { @@ -279,56 +286,77 @@ public class ExtractFunctionRefactoring extends CRefactoring { @Override public RefactoringStatus checkFinalConditions(IProgressMonitor pm) - throws CoreException, OperationCanceledException { - RefactoringStatus status = super.checkFinalConditions(pm); + throws CoreException, OperationCanceledException { + RefactoringStatus finalConditions = null; + try { + lockIndex(); + try { + finalConditions = super.checkFinalConditions(pm); - final IASTName astMethodName = new CPPASTName(info.getMethodName() - .toCharArray()); - MethodContext context = NodeHelper.findMethodContext(container.getNodesToWrite().get(0), getIndex()); - if (context.getType() == ContextType.METHOD) { - ICPPASTCompositeTypeSpecifier classDeclaration = (ICPPASTCompositeTypeSpecifier) context + final IASTName astMethodName = new CPPASTName(info.getMethodName() + .toCharArray()); + MethodContext context = NodeHelper.findMethodContext(container.getNodesToWrite().get(0), getIndex()); + + if (context.getType() == ContextType.METHOD && !context.isInline()) { + ICPPASTCompositeTypeSpecifier classDeclaration = (ICPPASTCompositeTypeSpecifier) context .getMethodDeclaration().getParent(); - IASTSimpleDeclaration methodDeclaration = getDeclaration(astMethodName); + IASTSimpleDeclaration methodDeclaration = getDeclaration(astMethodName); - if (isMethodAllreadyDefined(methodDeclaration, classDeclaration)) { - status.addError(Messages.ExtractFunctionRefactoring_NameInUse); - return status; + if (isMethodAllreadyDefined(methodDeclaration, classDeclaration, getIndex())) { + finalConditions.addError(Messages.ExtractFunctionRefactoring_NameInUse); + return finalConditions; + } + } + for (NameInformation name : info.getAllUsedNames()) { + if (name.isUserSetIsReturnValue()) { + info.setReturnVariable(name); + } + + } } - } - for (NameInformation name : info.getAllUsedNames()) { - if (name.isUserSetIsReturnValue()) { - info.setReturnVariable(name); + finally { + unlockIndex(); } - + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); } - - return status; + return finalConditions; } @Override protected void collectModifications(IProgressMonitor pm, ModificationCollector collector) throws CoreException, OperationCanceledException { - final IASTName astMethodName = new CPPASTName(info.getMethodName() - .toCharArray()); + try { + lockIndex(); + try { + final IASTName astMethodName = new CPPASTName(info.getMethodName() + .toCharArray()); - MethodContext context = NodeHelper.findMethodContext(container.getNodesToWrite().get(0), getIndex()); + MethodContext context = NodeHelper.findMethodContext(container.getNodesToWrite().get(0), getIndex()); - // Create Declaration in Class - if (context.getType() == ContextType.METHOD) { - createMethodDeclaration(astMethodName, context, collector); - } - // Create Method Definition - IASTNode firstNode = container.getNodesToWrite().get(0); - IPath implPath = new Path(firstNode.getContainingFilename()); - final IFile implementationFile = ResourcesPlugin.getWorkspace() + // 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, - implementationFile, collector); + createMethodDefinition(astMethodName, context, firstNode, + implementationFile, collector); - createMethodCalls(astMethodName, implementationFile, context, collector); + createMethodCalls(astMethodName, implementationFile, context, collector); + } + finally { + unlockIndex(); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } } @@ -476,7 +504,7 @@ public class ExtractFunctionRefactoring extends CRefactoring { } else { // Save Name Sequenz Number IASTName name = (IASTName) node; - TrailName trailName = new TrailName(); + TrailName trailName = new TrailName(name); int actCount = trailCounter.getObject().intValue(); if (nameTrail.containsKey(name.getRawSignature())) { Integer value = nameTrail.get(name.getRawSignature()); @@ -486,7 +514,6 @@ public class ExtractFunctionRefactoring extends CRefactoring { nameTrail.put(name.getRawSignature(), trailCounter.getObject()); } trailName.setNameNumber(actCount); - trailName.setRealName(name); if (info.getReturnVariable() != null && info.getReturnVariable().getName().getRawSignature().equals( @@ -494,33 +521,6 @@ public class ExtractFunctionRefactoring extends CRefactoring { returnNumber.setObject(Integer.valueOf(actCount)); } - // Save type informations for the name - IBinding bind = name.resolveBinding(); - IASTName[] declNames = name.getTranslationUnit().getDeclarationsInAST(bind); - if (declNames.length > 0) { - IASTNode tmpNode = ASTHelper.getDeclarationForNode(declNames[0]); - - IBinding declbind = declNames[0].resolveBinding(); - if (declbind instanceof ICPPBinding) { - ICPPBinding cppBind = (ICPPBinding) declbind; - try { - trailName.setGloballyQualified(cppBind.isGloballyQualified()); - } catch (DOMException e) { - ILog logger = CUIPlugin.getDefault().getLog(); - IStatus status = new Status(IStatus.WARNING, CUIPlugin.PLUGIN_ID, - IStatus.OK, e.getMessage(), e); - - logger.log(status); - } - } - - if (tmpNode != null) { - trailName.setDeclaration(tmpNode); - } else { - hasNameResolvingForSimilarError = true; - } - } - trail.add(trailName); return PROCESS_SKIP; } @@ -536,9 +536,9 @@ public class ExtractFunctionRefactoring extends CRefactoring { return trail; } - protected boolean isStatementInTrail(IASTStatement stmt, final Vector trail) { + protected boolean isStatementInTrail(IASTStatement stmt, final Vector trail, IIndex index) { final Container same = new Container(Boolean.TRUE); - final TrailNodeEqualityChecker equalityChecker = new TrailNodeEqualityChecker(names, namesCounter); + final TrailNodeEqualityChecker equalityChecker = new TrailNodeEqualityChecker(names, namesCounter, index); stmt.accept(new CPPASTAllVisitor() { @Override @@ -578,9 +578,9 @@ public class ExtractFunctionRefactoring extends CRefactoring { private boolean isMethodAllreadyDefined( IASTSimpleDeclaration methodDeclaration, - ICPPASTCompositeTypeSpecifier classDeclaration) { + ICPPASTCompositeTypeSpecifier classDeclaration, IIndex index) { TrailNodeEqualityChecker equalityChecker = new TrailNodeEqualityChecker( - names, namesCounter); + names, namesCounter, index); IBinding bind = classDeclaration.getName().resolveBinding(); IASTStandardFunctionDeclarator declarator = (IASTStandardFunctionDeclarator) methodDeclaration @@ -647,8 +647,10 @@ public class ExtractFunctionRefactoring extends CRefactoring { ICPPASTQualifiedName qname = new CPPASTQualifiedName(); if (context.getType() == ContextType.METHOD) { - for (int i = 0; i < (context.getMethodQName().getNames().length - 1); i++) { - qname.addName(new CPPASTName(context.getMethodQName().getNames()[i].toCharArray())); + if(context.getMethodQName() != null) { + for (int i = 0; i < (context.getMethodQName().getNames().length - 1); i++) { + qname.addName(new CPPASTName(context.getMethodQName().getNames()[i].toCharArray())); + } } } qname.addName(astMethodName); @@ -656,9 +658,8 @@ public class ExtractFunctionRefactoring extends CRefactoring { IASTFunctionDefinition func = new CPPASTFunctionDefinition(); func.setParent(unit); - ICPPASTSimpleDeclSpecifier dummyDeclSpecifier = new CPPASTSimpleDeclSpecifier(); - func.setDeclSpecifier(dummyDeclSpecifier); - dummyDeclSpecifier.setType(IASTSimpleDeclSpecifier.t_void); + IASTDeclSpecifier returnType = getReturnType(); + func.setDeclSpecifier(returnType); IASTStandardFunctionDeclarator createdFunctionDeclarator = extractedFunctionConstructionHelper .createFunctionDeclarator(qname, info.getDeclarator(), info @@ -688,8 +689,6 @@ public class ExtractFunctionRefactoring extends CRefactoring { subRW = rewriter.insertBefore(insertpoint.getParent(), insertpoint, func, group); } - subRW.replace(dummyDeclSpecifier, getReturnType(), group); - extractedFunctionConstructionHelper.constructMethodBody(compound, container.getNodesToWrite(), subRW, group); 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 95f0db77a9a..1758b5df0a7 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 @@ -63,7 +63,7 @@ final class SimilarFinderVisitor extends CPPASTVisitor { boolean isAllreadyInMainRefactoring = isInSelection(stmt); if( (!isAllreadyInMainRefactoring) - && this.refactoring.isStatementInTrail(stmt, trail)){ + && this.refactoring.isStatementInTrail(stmt, trail, this.refactoring.getIndex())){ stmtToReplace.add(stmt); similarContainer.add(stmt); ++i; @@ -99,24 +99,23 @@ final class SimilarFinderVisitor extends CPPASTVisitor { } if(similarOnReturnWays){ - System.out.println(6); - IASTNode call = refactoring.getMethodCall(name, - this.refactoring.nameTrail, this.refactoring.names, - this.refactoring.container, similarContainer); - ASTRewrite rewrite = collector.rewriterForTranslationUnit(stmtToReplace.get(0) - .getTranslationUnit()); - TextEditGroup editGroup = new TextEditGroup(Messages.SimilarFinderVisitor_replaceDuplicateCode); - rewrite.replace(stmtToReplace.get(0), call, editGroup); - if (stmtToReplace.size() > 1) { - for (int i = 1; i < stmtToReplace.size(); ++i) { - rewrite.remove(stmtToReplace.get(i), editGroup); + IASTNode call = refactoring.getMethodCall(name, + this.refactoring.nameTrail, this.refactoring.names, + this.refactoring.container, similarContainer); + ASTRewrite rewrite = collector.rewriterForTranslationUnit(stmtToReplace.get(0) + .getTranslationUnit()); + TextEditGroup editGroup = new TextEditGroup(Messages.SimilarFinderVisitor_replaceDuplicateCode); + rewrite.replace(stmtToReplace.get(0), call, editGroup); + if (stmtToReplace.size() > 1) { + for (int i = 1; i < stmtToReplace.size(); ++i) { + rewrite.remove(stmtToReplace.get(i), editGroup); + } } } - } clear(); } - + return PROCESS_SKIP; } else { clear(); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/TrailName.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/TrailName.java index b5992849859..4dfef059ab1 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/TrailName.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/TrailName.java @@ -11,26 +11,28 @@ *******************************************************************************/ package org.eclipse.cdt.internal.ui.refactoring.extractfunction; +import org.eclipse.cdt.core.dom.ast.DOMException; import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier; import org.eclipse.cdt.core.dom.ast.IASTName; import org.eclipse.cdt.core.dom.ast.IASTNode; +import org.eclipse.cdt.core.dom.ast.IBinding; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPBinding; -import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTName; +import org.eclipse.cdt.internal.core.dom.parser.ASTNode; import org.eclipse.cdt.internal.ui.refactoring.utils.ASTHelper; -class TrailName extends CPPASTName { +class TrailName extends ASTNode{ private int nameNumber; private IASTNode declaration = null; private IASTName realName = null; - private boolean isGloballyQualified = false; - - @Override - public String getRawSignature() { - return realName.getRawSignature(); - } + public TrailName(IASTName realName) { + super(); + this.realName = realName; + } + public int getNameNumber() { return nameNumber; } @@ -39,14 +41,6 @@ class TrailName extends CPPASTName { this.nameNumber = nameNumber; } - public IASTNode getDeclaration() { - return declaration; - } - - public void setDeclaration(IASTNode declaration) { - this.declaration = declaration; - } - public IASTDeclSpecifier getDeclSpecifier() { return ASTHelper.getDeclarationSpecifier(declaration); } @@ -55,20 +49,19 @@ class TrailName extends CPPASTName { return realName; } - public void setRealName(IASTName realName) { - this.realName = realName; - } - public boolean isGloballyQualified() { - return isGloballyQualified; + IBinding bind = realName.resolveBinding(); + try { + if (bind instanceof ICPPBinding) { + ICPPBinding cppBind = (ICPPBinding) bind; + return cppBind.isGloballyQualified(); + } + } catch (DOMException e) { + } + return false; } - public void setGloballyQualified(boolean isGloballyQualified) { - this.isGloballyQualified = isGloballyQualified; - } - - @Override - public char[] toCharArray() { - return realName.toCharArray(); + public IASTNode copy() { + throw new UnsupportedOperationException(); } } 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 2012022851e..d1fb24434ae 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 @@ -18,7 +18,9 @@ import java.util.List; import java.util.Map; import org.eclipse.core.runtime.Assert; +import org.eclipse.core.runtime.CoreException; +import org.eclipse.cdt.core.dom.ast.DOMException; import org.eclipse.cdt.core.dom.ast.IASTASMDeclaration; import org.eclipse.cdt.core.dom.ast.IASTBinaryExpression; import org.eclipse.cdt.core.dom.ast.IASTCompositeTypeSpecifier; @@ -28,6 +30,7 @@ import org.eclipse.cdt.core.dom.ast.IASTDeclarator; import org.eclipse.cdt.core.dom.ast.IASTElaboratedTypeSpecifier; import org.eclipse.cdt.core.dom.ast.IASTExpression; import org.eclipse.cdt.core.dom.ast.IASTFieldReference; +import org.eclipse.cdt.core.dom.ast.IASTFileLocation; import org.eclipse.cdt.core.dom.ast.IASTInitializer; import org.eclipse.cdt.core.dom.ast.IASTLiteralExpression; import org.eclipse.cdt.core.dom.ast.IASTName; @@ -41,6 +44,7 @@ import org.eclipse.cdt.core.dom.ast.IASTStatement; import org.eclipse.cdt.core.dom.ast.IASTTypeIdExpression; import org.eclipse.cdt.core.dom.ast.IASTUnaryExpression; import org.eclipse.cdt.core.dom.ast.IBinding; +import org.eclipse.cdt.core.dom.ast.IType; import org.eclipse.cdt.core.dom.ast.c.ICASTDeclSpecifier; import org.eclipse.cdt.core.dom.ast.c.ICASTPointer; import org.eclipse.cdt.core.dom.ast.c.ICASTSimpleDeclSpecifier; @@ -59,24 +63,28 @@ import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTemplateDeclaration; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTypenameExpression; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTUsingDeclaration; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTVisibilityLabel; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPVariable; import org.eclipse.cdt.core.dom.ast.gnu.cpp.IGPPASTDeclSpecifier; import org.eclipse.cdt.core.dom.ast.gnu.cpp.IGPPASTExplicitTemplateInstantiation; import org.eclipse.cdt.core.dom.ast.gnu.cpp.IGPPASTPointer; import org.eclipse.cdt.core.dom.ast.gnu.cpp.IGPPASTSimpleDeclSpecifier; +import org.eclipse.cdt.core.index.IIndex; +import org.eclipse.cdt.core.index.IIndexName; import org.eclipse.cdt.internal.ui.refactoring.Container; import org.eclipse.cdt.internal.ui.refactoring.EqualityChecker; -import org.eclipse.cdt.internal.ui.refactoring.utils.ASTHelper; public class TrailNodeEqualityChecker implements EqualityChecker { private final Map names; private final Container namesCounter; + private IIndex index; - public TrailNodeEqualityChecker(Map names, Container namesCounter) { + public TrailNodeEqualityChecker(Map names, Container namesCounter, IIndex index) { super(); this.names = names; this.namesCounter = namesCounter; + this.index = index; } public boolean isEquals(IASTNode trailNode, IASTNode node) { @@ -165,13 +173,12 @@ public class TrailNodeEqualityChecker implements EqualityChecker { ICPPASTNamedTypeSpecifier decl = (ICPPASTNamedTypeSpecifier) node; - boolean isSame = isDeclSpecifierEquals(trailDecl, decl) + return isDeclSpecifierEquals(trailDecl, decl) && isSameNamedTypeSpecifierName(trailDecl, decl) && trailDecl.isTypename() == decl.isTypename() && trailDecl.isExplicit() == decl.isExplicit() && trailDecl.isFriend() == decl.isFriend() && trailDecl.isVirtual() == decl.isVirtual(); - return isSame; } else if (trailNode instanceof IASTNamedTypeSpecifier) { IASTNamedTypeSpecifier trailDecl = (IASTNamedTypeSpecifier) trailNode; IASTNamedTypeSpecifier decl = (IASTNamedTypeSpecifier) node; @@ -408,53 +415,69 @@ public class TrailNodeEqualityChecker implements EqualityChecker { if(actCount != trailName.getNameNumber()){ return false; } - - IBinding bind = name.resolveBinding(); - IASTName[] declNames = name.getTranslationUnit().getDeclarationsInAST(bind); - if(declNames.length > 0){ - IASTNode tmpNode = ASTHelper.getDeclarationForNode(declNames[0]); - - if(tmpNode != null){ - if(trailName.isGloballyQualified()){ - //global Node - if(tmpNode.equals(trailName.getDeclaration())){ - return true; - } - } else { - //localNode - IASTDeclSpecifier decl = ASTHelper.getDeclarationSpecifier(tmpNode); - IASTDeclSpecifier trailDecl = trailName.getDeclSpecifier(); - - IASTDeclarator declarator = ASTHelper.getDeclaratorForNode(declNames[0]); - IASTDeclarator trailDeclarator = ASTHelper.getDeclaratorForNode(trailName.getDeclaration()); - - IASTPointerOperator[] pointerOperators1 = declarator.getPointerOperators(); - IASTPointerOperator[] pointerOperators2 = trailDeclarator.getPointerOperators(); - - if(trailDecl != null && decl != null - && decl.getStorageClass() == trailDecl.getStorageClass() - && ASTHelper.samePointers(pointerOperators1, pointerOperators2, this)){ - if (decl instanceof IASTSimpleDeclSpecifier - && trailDecl instanceof IASTSimpleDeclSpecifier) { - IASTSimpleDeclSpecifier simpleDecl = (IASTSimpleDeclSpecifier) decl; - IASTSimpleDeclSpecifier simpleTrailDecl = (IASTSimpleDeclSpecifier) trailDecl; - if(simpleDecl.getType() == simpleTrailDecl.getType()){ - return true; - } - } else if (decl instanceof IASTNamedTypeSpecifier - && trailDecl instanceof IASTNamedTypeSpecifier) { - - IASTNamedTypeSpecifier namedDecl = (IASTNamedTypeSpecifier) decl; - IASTNamedTypeSpecifier trailNamedDecl = (IASTNamedTypeSpecifier) trailDecl; - if(namedDecl.getName().getRawSignature().equals(trailNamedDecl.getName().getRawSignature())){ - return true; - } - + + if(trailName.isGloballyQualified()) { + IBinding realBind = trailName.getRealName().resolveBinding(); + IBinding nameBind = name.resolveBinding(); + 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())) { + continue; + }else { + return false; } - } + } + return true; + }else { + return false; } + } catch (InterruptedException e) {} + catch (CoreException e) {} + finally { + index.releaseReadLock(); + } + }else { + IType oType = getType(trailName.getRealName().resolveBinding()); + IType nType = getType(name.resolveBinding()); + if(oType.isSameType(nType)) { + return true; } } return false; } + + private IType getType(IBinding binding) { + try { + if (binding instanceof ICPPVariable) { + ICPPVariable var = (ICPPVariable) binding; + return var.getType(); + } + } catch (DOMException e) { + return new NullType(); + } + return new NullType(); + } + + private class NullType implements IType{ + + @Override + public Object clone() { + try { + return super.clone(); + } catch (CloneNotSupportedException e) { + return null; + } + } + + public boolean isSameType(IType type) { + return false; + } + + } } 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 ff5074b4642..02b0acc19ab 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 @@ -116,13 +116,20 @@ public class GenerateGettersAndSettersRefactoring extends CRefactoring { @Override public RefactoringStatus checkFinalConditions(IProgressMonitor pm) throws CoreException, - OperationCanceledException { - RefactoringStatus finalStatus = super.checkFinalConditions(pm); - if(!context.isImplementationInHeader()) { - definitionInsertLocation = findInsertLocation(); - if(file.equals(definitionInsertLocation.getInsertFile())) { - finalStatus.addInfo(Messages.GenerateGettersAndSettersRefactoring_NoImplFile); + OperationCanceledException { + RefactoringStatus finalStatus = null; + try { + lockIndex(); + finalStatus = super.checkFinalConditions(pm); + if(!context.isImplementationInHeader()) { + definitionInsertLocation = findInsertLocation(); + if(file.equals(definitionInsertLocation.getInsertFile())) { + finalStatus.addInfo(Messages.GenerateGettersAndSettersRefactoring_NoImplFile); + } } + } catch (InterruptedException e) {} + finally { + unlockIndex(); } return finalStatus; } @@ -206,24 +213,30 @@ public class GenerateGettersAndSettersRefactoring extends CRefactoring { @Override protected void collectModifications(IProgressMonitor pm,ModificationCollector collector) throws CoreException, OperationCanceledException { - ArrayList getterAndSetters = new ArrayList(); - ArrayList definitions = new ArrayList(); - for(GetterSetterInsertEditProvider currentProvider : context.selectedFunctions){ - if(context.isImplementationInHeader()) { - getterAndSetters.add(currentProvider.getFunctionDefinition(false)); - }else { - getterAndSetters.add(currentProvider.getFunctionDeclaration()); - definitions.add(currentProvider.getFunctionDefinition(true)); + try { + lockIndex(); + ArrayList getterAndSetters = new ArrayList(); + ArrayList definitions = new ArrayList(); + for(GetterSetterInsertEditProvider currentProvider : context.selectedFunctions){ + if(context.isImplementationInHeader()) { + getterAndSetters.add(currentProvider.getFunctionDefinition(false)); + }else { + getterAndSetters.add(currentProvider.getFunctionDeclaration()); + definitions.add(currentProvider.getFunctionDefinition(true)); + } } - } - if(!context.isImplementationInHeader()) { - addDefinition(collector, definitions); - } - ICPPASTCompositeTypeSpecifier classDefinition = (ICPPASTCompositeTypeSpecifier) context.existingFields.get(context.existingFields.size()-1).getParent(); + if(!context.isImplementationInHeader()) { + addDefinition(collector, definitions); + } + ICPPASTCompositeTypeSpecifier classDefinition = (ICPPASTCompositeTypeSpecifier) context.existingFields.get(context.existingFields.size()-1).getParent(); + + AddDeclarationNodeToClassChange.createChange(classDefinition, VisibilityEnum.v_public, getterAndSetters, false, collector); + } catch (InterruptedException e) {} + finally { + unlockIndex(); + } + - AddDeclarationNodeToClassChange.createChange(classDefinition, VisibilityEnum.v_public, getterAndSetters, false, collector); - - } private void addDefinition(ModificationCollector collector, ArrayList definitions) diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/NodeHelper.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/NodeHelper.java index 9a3c4adacc1..8877ad8d7ed 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/NodeHelper.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/NodeHelper.java @@ -37,7 +37,6 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTTranslationUnit; import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVisitor; import org.eclipse.cdt.internal.ui.refactoring.MethodContext; -import org.eclipse.cdt.internal.ui.refactoring.MethodContext.ContextType; /** * General class for common Node operations. @@ -150,34 +149,32 @@ public class NodeHelper { private static void getMethodContexWithIndex(IIndex index, IASTTranslationUnit translationUnit, MethodContext context, IASTName name) throws CoreException { - if(name instanceof ICPPASTQualifiedName){ - ICPPASTQualifiedName qname =( ICPPASTQualifiedName )name; - context.setMethodQName(qname); - IBinding bind = qname.resolveBinding(); - if (bind instanceof ICPPMethod) { - context.setType(MethodContext.ContextType.METHOD); - IIndexName[] decl; - decl = index.findDeclarations(bind); - String tuFileLoc = translationUnit.getFileLocation().getFileName(); - for (IIndexName tmpname : decl) { - IASTTranslationUnit locTu = translationUnit; - if(!tuFileLoc.equals(tmpname.getFileLocation().getFileName())) { - locTu = TranslationUnitHelper.loadTranslationUnit(tmpname.getFileLocation().getFileName(), false); - } - IASTName declName = DeclarationFinder.findDeclarationInTranslationUnit(locTu, tmpname); - if(declName != null) { - IASTNode methoddefinition = declName.getParent().getParent(); - if (methoddefinition instanceof IASTSimpleDeclaration) { - context.setMethodDeclarationName(declName); - } + IBinding bind = name.resolveBinding(); + if (bind instanceof ICPPMethod) { + context.setType(MethodContext.ContextType.METHOD); + IIndexName[] decl; + decl = index.findDeclarations(bind); + String tuFileLoc = translationUnit.getFileLocation().getFileName(); + if(decl.length == 0) { + context.setMethodDeclarationName(name); + } + for (IIndexName tmpname : decl) { + IASTTranslationUnit locTu = translationUnit; + if(!tuFileLoc.equals(tmpname.getFileLocation().getFileName())) { + locTu = TranslationUnitHelper.loadTranslationUnit(tmpname.getFileLocation().getFileName(), false); + } + IASTName declName = DeclarationFinder.findDeclarationInTranslationUnit(locTu, tmpname); + if(declName != null) { + IASTNode methoddefinition = declName.getParent().getParent(); + if (methoddefinition instanceof IASTSimpleDeclaration || methoddefinition instanceof IASTFunctionDefinition) { + context.setMethodDeclarationName(declName); } } } - }else { - if (name.getParent().getParent().getParent() instanceof ICPPASTCompositeTypeSpecifier) { - context.setType(ContextType.METHOD); - context.setMethodDeclarationName(name); - } + } + if(name instanceof ICPPASTQualifiedName){ + ICPPASTQualifiedName qname =( ICPPASTQualifiedName )name; + context.setMethodQName(qname); } }