From 7046aff5b19d3a2b565755920bfd93e2d0649d77 Mon Sep 17 00:00:00 2001 From: Markus Schorn Date: Thu, 4 Sep 2008 09:28:36 +0000 Subject: [PATCH] Using index for rename refactoring started off the editor, bug 245636. --- .../refactoring/rename/RefactoringTests.java | 20 ++-- .../refactoring/rename/RenameMacroTests.java | 14 +-- .../rename/RenameRegressionTests.java | 17 ++- .../tests/refactoring/rename/RenameTests.java | 101 ++++++++++++------ .../ui/refactoring/rename/ASTManager.java | 64 +++++++---- .../ui/refactoring/rename/CRefactory.java | 26 +++-- 6 files changed, 161 insertions(+), 81 deletions(-) diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/rename/RefactoringTests.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/rename/RefactoringTests.java index 644c53f65d4..fc772659935 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/rename/RefactoringTests.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/rename/RefactoringTests.java @@ -1,20 +1,17 @@ /******************************************************************************* - * Copyright (c) 2005, 2007 Wind River Systems, Inc. + * Copyright (c) 2005, 2008 Wind River Systems, Inc. * 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: - * Markus Schorn - initial API and implementation + * Markus Schorn - initial API and implementation ******************************************************************************/ - package org.eclipse.cdt.ui.tests.refactoring.rename; import java.io.StringWriter; -import org.eclipse.cdt.core.tests.BaseTestFramework; -import org.eclipse.cdt.internal.core.dom.SavedCodeReaderFactory; import org.eclipse.core.resources.IFile; import org.eclipse.ltk.core.refactoring.Change; import org.eclipse.ltk.core.refactoring.CompositeChange; @@ -28,6 +25,12 @@ import org.eclipse.text.edits.ReplaceEdit; import org.eclipse.text.edits.TextEdit; import org.eclipse.text.edits.TextEditGroup; +import org.eclipse.cdt.core.CCorePlugin; +import org.eclipse.cdt.core.dom.IPDOMManager; +import org.eclipse.cdt.core.tests.BaseTestFramework; + +import org.eclipse.cdt.internal.core.dom.SavedCodeReaderFactory; + /** * @author markus.schorn@windriver.com @@ -42,13 +45,16 @@ public class RefactoringTests extends BaseTestFramework { super(name); } - protected void setUp() throws Exception { + @Override + protected void setUp() throws Exception { super.setUp(); + CCorePlugin.getIndexManager().setIndexerId(cproject, IPDOMManager.ID_FAST_INDEXER); fBufferSize= FileCharSequenceProvider.BUFFER_SIZE; FileCharSequenceProvider.BUFFER_SIZE= 1024*4; } - protected void tearDown() throws Exception { + @Override + protected void tearDown() throws Exception { super.tearDown(); SavedCodeReaderFactory.getInstance().getCodeReaderCache().flush(); FileCharSequenceProvider.BUFFER_SIZE= fBufferSize; diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/rename/RenameMacroTests.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/rename/RenameMacroTests.java index a0098de9af8..dd5c7874575 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/rename/RenameMacroTests.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/rename/RenameMacroTests.java @@ -1,14 +1,13 @@ /******************************************************************************* - * Copyright (c) 2005, 2007 Wind River Systems, Inc. + * Copyright (c) 2005, 2008 Wind River Systems, Inc. * 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: - * Markus Schorn - initial API and implementation + * Markus Schorn - initial API and implementation *******************************************************************************/ - package org.eclipse.cdt.ui.tests.refactoring.rename; import java.io.StringWriter; @@ -20,9 +19,6 @@ import org.eclipse.core.resources.IFile; import org.eclipse.ltk.core.refactoring.Change; import org.eclipse.ltk.core.refactoring.RefactoringStatus; -/** - * @author markus.schorn@windriver.com - */ public class RenameMacroTests extends RenameTests { public RenameMacroTests(String name) { @@ -112,6 +108,9 @@ public class RenameMacroTests extends RenameTests { writer.write("static int s2(); \n"); //$NON-NLS-1$ writer.write("void f(int par1){ \n"); //$NON-NLS-1$ writer.write(" int w1; v1(); \n"); //$NON-NLS-1$ + writer.write(" extern_var; \n"); //$NON-NLS-1$ + writer.write(" var_def; \n"); //$NON-NLS-1$ + writer.write(" enum_item; \n"); //$NON-NLS-1$ writer.write("} \n"); //$NON-NLS-1$ String contents = writer.toString(); IFile cpp= importFile("test.cpp", contents ); //$NON-NLS-1$ @@ -119,11 +118,12 @@ public class RenameMacroTests extends RenameTests { writer = new StringWriter(); writer.write( "static int static_other_file(); \n" ); //$NON-NLS-1$ importFile( "other.cpp", writer.toString() ); //$NON-NLS-1$ + waitForIndexer(); int offset1= contents.indexOf("MACRO"); //$NON-NLS-1$ - // conflicting renamings + // conflicts after renaming RefactoringStatus status= checkConditions(cpp, offset1, "w1"); //$NON-NLS-1$ assertRefactoringError(status, "A conflict was encountered during refactoring. \n" + "Type of problem: Name conflict \n" + diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/rename/RenameRegressionTests.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/rename/RenameRegressionTests.java index f3d7eedfdf8..8ba92733d5e 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/rename/RenameRegressionTests.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/rename/RenameRegressionTests.java @@ -9,10 +9,6 @@ * IBM Corporation - initial API and implementation * Wind River Systems Inc. - ported for new rename implementation *******************************************************************************/ - -/* - * Created on Nov 10, 2004 - */ package org.eclipse.cdt.ui.tests.refactoring.rename; import java.io.StringWriter; @@ -396,6 +392,7 @@ public class RenameRegressionTests extends RenameTests { writer.write( "void Foo::aMethod(int x){} \n" ); //$NON-NLS-1$ String source = writer.toString(); IFile cppfile = importFile( "t.cpp", source ); //$NON-NLS-1$ + waitForIndexer(); int hoffset = header.indexOf( "aMethod" ) ; //$NON-NLS-1$ int cppoffset = source.indexOf( "aMethod" ) ; //$NON-NLS-1$ @@ -477,6 +474,8 @@ public class RenameRegressionTests extends RenameTests { writer.write( "} \n" ); //$NON-NLS-1$ String source = writer.toString(); IFile cpp = importFile( "t.cpp", source ); //$NON-NLS-1$ + waitForIndexer(); + //vp1 const int offset = header.indexOf( "method1" ); //$NON-NLS-1$ Change changes = getRefactorChanges(h, offset, "m1" ); //$NON-NLS-1$ @@ -512,6 +511,8 @@ public class RenameRegressionTests extends RenameTests { writer.write( "} \n" ); //$NON-NLS-1$ String source = writer.toString(); IFile cpp = importFile( "t.cpp", source ); //$NON-NLS-1$ + waitForIndexer(); + //vp1 static method declaration int offset = header.indexOf( "method1/*vp1*/" ); //$NON-NLS-1$ Change changes = getRefactorChanges(h, offset, "m1" ); //$NON-NLS-1$ @@ -549,6 +550,8 @@ public class RenameRegressionTests extends RenameTests { writer.write( "} \n" ); //$NON-NLS-1$ String source = writer.toString(); IFile cpp = importFile( "t.cpp", source ); //$NON-NLS-1$ + waitForIndexer(); + //vp1 volatile int offset = header.indexOf( "method1/*vp1*/" ) ; //$NON-NLS-1$ Change changes = getRefactorChanges(h, offset, "m1" ); //$NON-NLS-1$ @@ -577,6 +580,8 @@ public class RenameRegressionTests extends RenameTests { writer.write( "} \n" ); //$NON-NLS-1$ String source = writer.toString(); IFile cpp = importFile( "t.cpp", source ); //$NON-NLS-1$ + waitForIndexer(); + //vp1 inline int offset = header.indexOf( "method1/*vp1*/" ) ; //$NON-NLS-1$ Change changes = getRefactorChanges(h, offset, "m1" ); //$NON-NLS-1$ @@ -957,6 +962,8 @@ public class RenameRegressionTests extends RenameTests { String source = writer.toString(); IFile cpp=importFile( "Foo.cpp", source ); //$NON-NLS-1$ + waitForIndexer(); + int offset = header.indexOf( "E1" ) ; //$NON-NLS-1$ getRefactorChanges(h, offset, "Ooga" ); //$NON-NLS-1$ Change changes = getRefactorChanges(h, offset, "ooga" ); //$NON-NLS-1$ @@ -980,6 +987,8 @@ public class RenameRegressionTests extends RenameTests { String source = writer.toString(); IFile cpp = importFile( "Foo.cpp", source ); //$NON-NLS-1$ + waitForIndexer(); + int offset = header.indexOf( "att" ) ; //$NON-NLS-1$ Change changes = getRefactorChanges(h, offset, "ooga" ); //$NON-NLS-1$ diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/rename/RenameTests.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/rename/RenameTests.java index d4d4adfe91a..383ab3a9ea8 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/rename/RenameTests.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/rename/RenameTests.java @@ -1,23 +1,26 @@ /******************************************************************************* - * Copyright (c) 2005 Wind River Systems, Inc. + * Copyright (c) 2005, 2008 Wind River Systems, Inc. * 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: - * Markus Schorn - initial API and implementation + * Markus Schorn - initial API and implementation ******************************************************************************/ - package org.eclipse.cdt.ui.tests.refactoring.rename; import org.eclipse.core.resources.IFile; import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.IProgressMonitor; import org.eclipse.core.runtime.NullProgressMonitor; import org.eclipse.ltk.core.refactoring.Change; import org.eclipse.ltk.core.refactoring.RefactoringStatus; import org.eclipse.ltk.core.refactoring.RefactoringStatusEntry; +import org.eclipse.cdt.core.CCorePlugin; +import org.eclipse.cdt.core.index.IIndexManager; + import org.eclipse.cdt.internal.ui.refactoring.rename.CRefactoringArgument; import org.eclipse.cdt.internal.ui.refactoring.rename.CRefactory; import org.eclipse.cdt.internal.ui.refactoring.rename.CRenameProcessor; @@ -29,7 +32,9 @@ import org.eclipse.cdt.internal.ui.refactoring.rename.TextSearchWrapper; */ public class RenameTests extends RefactoringTests { - public RenameTests(String name) { + private static final IProgressMonitor NPM = new NullProgressMonitor(); + + public RenameTests(String name) { super(name); } @@ -45,18 +50,23 @@ public class RenameTests extends RefactoringTests { public Change getRefactorChanges(IFile file, int offset, String newName) throws Exception { CRenameRefactoring proc = createRefactoring(file, offset, newName); - RefactoringStatus rs = checkConditions(proc); - if (!rs.hasError()) { - Change change = proc.createChange( new NullProgressMonitor() ); - return change; - } - - fail ("Input check on "+ newName + " failed. "+rs.getEntryMatchingSeverity(RefactoringStatus.ERROR) ); //$NON-NLS-1$ //$NON-NLS-2$ - //rs.getFirstMessage(RefactoringStatus.ERROR) is not the message displayed in - //the UI for renaming a method to a constructor, the first message which is only - //a warning is shown in the UI. If you click preview, then the error and the warning - //is shown. - return null; + ((CRenameProcessor) proc.getProcessor()).lockIndex(); + try { + RefactoringStatus rs = checkConditions(proc); + if (!rs.hasError()) { + Change change = proc.createChange( new NullProgressMonitor() ); + return change; + } + + fail ("Input check on "+ newName + " failed. "+rs.getEntryMatchingSeverity(RefactoringStatus.ERROR) ); //$NON-NLS-1$ //$NON-NLS-2$ + //rs.getFirstMessage(RefactoringStatus.ERROR) is not the message displayed in + //the UI for renaming a method to a constructor, the first message which is only + //a warning is shown in the UI. If you click preview, then the error and the warning + //is shown. + return null; + } finally { + ((CRenameProcessor) proc.getProcessor()).unlockIndex(); + } } private CRenameRefactoring createRefactoring(IFile file, int offset, String newName) { @@ -71,24 +81,34 @@ public class RenameTests extends RefactoringTests { public String[] getRefactorMessages(IFile file, int offset, String newName) throws Exception { String[] result; CRenameRefactoring proc = createRefactoring(file, offset, newName); - RefactoringStatus rs = checkConditions(proc); - if (!rs.hasWarning()){ - fail ("Input check on "+ newName + " passed. There should have been warnings or errors. ") ; //$NON-NLS-1$ //$NON-NLS-2$ - return null; + ((CRenameProcessor) proc.getProcessor()).lockIndex(); + try { + RefactoringStatus rs = checkConditions(proc); + if (!rs.hasWarning()){ + fail ("Input check on "+ newName + " passed. There should have been warnings or errors. ") ; //$NON-NLS-1$ //$NON-NLS-2$ + return null; + } + RefactoringStatusEntry[] rse = rs.getEntries(); + result = new String[rse.length]; + for (int i=0; i< rse.length; i++){ + RefactoringStatusEntry entry = rse[i]; + result[i]=entry.getMessage(); + + } + return result; + } finally { + ((CRenameProcessor) proc.getProcessor()).unlockIndex(); } - RefactoringStatusEntry[] rse = rs.getEntries(); - result = new String[rse.length]; - for (int i=0; i< rse.length; i++){ - RefactoringStatusEntry entry = rse[i]; - result[i]=entry.getMessage(); - - } - return result; } public RefactoringStatus checkConditions(IFile file, int offset, String newName) throws Exception { CRenameRefactoring proc = createRefactoring(file, offset, newName); - return checkConditions(proc); + ((CRenameProcessor) proc.getProcessor()).lockIndex(); + try { + return checkConditions(proc); + } finally { + ((CRenameProcessor) proc.getProcessor()).unlockIndex(); + } } private RefactoringStatus checkConditions(CRenameRefactoring proc) throws CoreException { @@ -101,9 +121,14 @@ public class RenameTests extends RefactoringTests { public int getRefactorSeverity(IFile file, int offset, String newName) throws Exception { CRenameRefactoring proc = createRefactoring(file, offset, newName); - RefactoringStatus rs = checkConditions(proc); - - return (rs.getSeverity()); + ((CRenameProcessor) proc.getProcessor()).lockIndex(); + try { + RefactoringStatus rs = checkConditions(proc); + + return (rs.getSeverity()); + } finally { + ((CRenameProcessor) proc.getProcessor()).unlockIndex(); + } } protected int countOccurrences(String contents, String lookup) { @@ -115,5 +140,15 @@ public class RenameTests extends RefactoringTests { } return count; } - + + protected void waitForIndexer() throws InterruptedException { + final IIndexManager im = CCorePlugin.getIndexManager(); + int sleep= 1; + while (im.isIndexerSetupPostponed(cproject)) { + Thread.sleep(sleep); + sleep *= 2; + assertTrue(sleep < 2000); + } + assertTrue(im.joinIndexer(10000, NPM)); + } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/rename/ASTManager.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/rename/ASTManager.java index 801d7cfeb78..0d51f31f359 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/rename/ASTManager.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/rename/ASTManager.java @@ -98,6 +98,7 @@ import org.eclipse.cdt.internal.core.dom.parser.c.CVisitor; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPImplicitMethod; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPMethod; import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPVisitor; +import org.eclipse.cdt.internal.core.index.IIndexScope; /** @@ -384,7 +385,12 @@ public class ASTManager { public static String getName(IScope s1) { String name= null; try { - name= getNameOrNull(ASTInternal.getPhysicalNodeOfScope(s1)); + if (s1 instanceof IIndexScope) { + IIndexScope indexScope= (IIndexScope) s1; + name= indexScope.getScopeName().toString(); + } else { + name= getNameOrNull(ASTInternal.getPhysicalNodeOfScope(s1)); + } } catch (DOMException e) { } @@ -1115,27 +1121,45 @@ public class ASTManager { Integer cmpObj= fKnownBindings.get(binding); if (cmpObj != null) { cmp= cmpObj.intValue(); - } - else if (binding instanceof IProblemBinding) { + } else if (binding instanceof IProblemBinding) { cmp= UNKNOWN; handleProblemBinding(name.getTranslationUnit(), (IProblemBinding) binding, status); - } - else { - for (IBinding renameBinding : fValidBindings) { - try { - int cmp0= isSameBinding(binding, renameBinding); - if (cmp0 != FALSE) { - cmp= cmp0; - } - if (cmp0 == TRUE) { - break; - } - } - catch (DOMException e) { - handleDOMException(name.getTranslationUnit(), e, status); - cmp= UNKNOWN; - } - } + } else { + // check whether a qualifier has a problem binding + boolean problemInQualifier= false; + IASTNode parent= name.getParent(); + if (parent instanceof ICPPASTQualifiedName) { + IASTName[] names= ((ICPPASTQualifiedName) parent).getNames(); + for (IASTName n : names) { + if (n == name) + break; + final IBinding b = n.resolveBinding(); + if (b instanceof IProblemBinding) { + handleProblemBinding(name.getTranslationUnit(), (IProblemBinding) b, status); + problemInQualifier= true; + break; + } + } + } + if (problemInQualifier) { + cmp= UNKNOWN; + } else { + for (IBinding renameBinding : fValidBindings) { + try { + int cmp0= isSameBinding(binding, renameBinding); + if (cmp0 != FALSE) { + cmp= cmp0; + } + if (cmp0 == TRUE) { + break; + } + } + catch (DOMException e) { + handleDOMException(name.getTranslationUnit(), e, status); + cmp= UNKNOWN; + } + } + } fKnownBindings.put(binding, new Integer(cmp)); } switch(cmp) { diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/rename/CRefactory.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/rename/CRefactory.java index efb1354ebad..ba10724caff 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/rename/CRefactory.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/rename/CRefactory.java @@ -6,9 +6,8 @@ * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * Markus Schorn - initial API and implementation + * Markus Schorn - initial API and implementation ******************************************************************************/ - package org.eclipse.cdt.internal.ui.refactoring.rename; import java.util.Arrays; @@ -109,17 +108,24 @@ public class CRefactory { return; } CRefactoringArgument iarg= new CRefactoringArgument((IFile) res, s.getOffset(), s.getLength()); - CRenameRefactoring r= new CRenameRefactoring(new CRenameProcessor(this, iarg)); - RefactoringWizardOpenOperation op= - new RefactoringWizardOpenOperation(new CRenameRefactoringWizard(r)); - try { - op.run(shell, Messages.getString("CRefactory.title.rename")); //$NON-NLS-1$ + final CRenameProcessor processor = new CRenameProcessor(this, iarg); + try { + processor.lockIndex(); + try { + CRenameRefactoring r= new CRenameRefactoring(processor); + RefactoringWizardOpenOperation op= + new RefactoringWizardOpenOperation(new CRenameRefactoringWizard(r)); + op.run(shell, Messages.getString("CRefactory.title.rename")); //$NON-NLS-1$ + } finally { + processor.unlockIndex(); + } } catch (InterruptedException e) { - // operation was canceled - } + Thread.currentThread().interrupt(); + } catch (CoreException e) { + CCorePlugin.log(e); + } } - public TextSearchWrapper getTextSearch() { if (fTextSearch == null) { return new TextSearchWrapper();