Fix for bug#54325 Refactor: Check for name collision

core/org.eclipse.cdt.core/ChangeLog

@@ -1,3 +1,7 @@
+2004-03-19 Hoda Amer
+	The CModelBuilder sets TranslationUnit.isStructureKnown() based on
+	both parser errors and IProblems sent to the callback.
+	
 2004-03-19 David Inglis
 
 	Added logging on failures in CCorePlugin.

core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/CModelBuilder.java

@@ -165,7 +165,7 @@ public class CModelBuilder {
 		{ 
 			generateModelElements();
 			// important to know if the unit has parse errors or not
-			translationUnit.getElementInfo().setIsStructureKnown(hasNoErrors);
+			translationUnit.getElementInfo().setIsStructureKnown(hasNoErrors && quickParseCallback.hasNoProblems());
 		}
 		catch( NullPointerException npe )
 		{

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/parser/IQuickParseCallback.java

@@ -25,4 +25,5 @@ public interface IQuickParseCallback extends ISourceElementRequestor
      */
     public abstract IASTCompilationUnit getCompilationUnit();
     public abstract Iterator iterateOffsetableElements();
+    public boolean hasNoProblems();
 }

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/parser/QuickParseCallback.java

@@ -31,6 +31,7 @@ public class QuickParseCallback extends NullSourceElementRequestor implements IQ
 	protected IASTCompilationUnit compilationUnit = null;
 	protected List inclusions = new ArrayList(); 
 	protected List macros = new ArrayList(); 
+	protected boolean hasNoProblems = true;
 	
 	public Iterator getInclusions()
 	{
@@ -194,4 +195,23 @@ public class QuickParseCallback extends NullSourceElementRequestor implements IQ
 	{
 		return new OffsetableIterator();
 	}	
+	/* (non-Javadoc)
+	 * @see org.eclipse.cdt.core.parser.ISourceElementRequestor#acceptProblem(org.eclipse.cdt.core.parser.IProblem)
+	 */
+	public boolean acceptProblem(IProblem problem) {
+		setHasNoProblems(false);
+		return super.acceptProblem(problem);
+	}
+	/**
+	 * @return Returns the hasProblems.
+	 */
+	public boolean hasNoProblems() {
+		return hasNoProblems;
+	}
+	/**
+	 * @param hasProblems The hasProblems to set.
+	 */
+	public void setHasNoProblems(boolean hasProblems) {
+		this.hasNoProblems = hasProblems;
+	}
 }

core/org.eclipse.cdt.ui/ChangeLog

@@ -1,3 +1,6 @@
+2004-03-19 Hoda Amer
+	Fix for bug#54325 Refactor: Check for name collision
+	
 2004-03-18 Alain Magloire
 	New icons
 	* icons/full/obj16/cfolder_obj.gif

core/org.eclipse.cdt.ui/refactor/org/eclipse/cdt/internal/corext/refactoring/Checks.java

@@ -10,15 +10,22 @@
 ***********************************************************************/
 package org.eclipse.cdt.internal.corext.refactoring;
 
+import java.util.ArrayList;
+import java.util.List;
+
 import org.eclipse.cdt.core.CConventions;
 import org.eclipse.cdt.core.model.CModelException;
 import org.eclipse.cdt.core.model.CoreModel;
 import org.eclipse.cdt.core.model.ICElement;
+import org.eclipse.cdt.core.model.ICModelMarker;
 import org.eclipse.cdt.core.model.ITranslationUnit;
 import org.eclipse.cdt.internal.corext.refactoring.base.RefactoringStatus;
 import org.eclipse.cdt.internal.ui.util.Resources;
 import org.eclipse.core.resources.IFile;
+import org.eclipse.core.resources.IMarker;
+import org.eclipse.core.resources.IResource;
 import org.eclipse.core.resources.ResourcesPlugin;
+import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.IStatus;
 
@@ -147,7 +154,7 @@ public class Checks {
 		if (tu == null)
 			return RefactoringStatus.createFatalErrorStatus(RefactoringCoreMessages.getString("Checks.cu_not_created"));	 //$NON-NLS-1$
 		else if (! tu.isStructureKnown())
-			return RefactoringStatus.createFatalErrorStatus(RefactoringCoreMessages.getString("Checks.cu_not_parsed"));	 //$NON-NLS-1$
+			return RefactoringStatus.createErrorStatus(RefactoringCoreMessages.getString("Checks.cu_not_parsed"));	 //$NON-NLS-1$
 		return new RefactoringStatus();
 	}
 	//-------- validateEdit checks ----
@@ -172,4 +179,60 @@ public class Checks {
 			return false;
 		return true;
 	}
+	
+	/**
+	 * From SearchResultGroup[] passed as the parameter
+	 * this method removes all those that correspond to a non-parsable ITranslationUnit
+	 * and returns it as a result.
+	 * Status object collect the result of checking.
+	 */	
+	public static SearchResultGroup[] excludeTranslationUnits(SearchResultGroup[] grouped, RefactoringStatus status) throws CModelException{
+		List result= new ArrayList();
+		boolean wasEmpty= grouped.length == 0;
+		for (int i= 0; i < grouped.length; i++){	
+			IResource resource= grouped[i].getResource();
+			ICElement element= CoreModel.getDefault().create(resource);
+			if (! (element instanceof ITranslationUnit))
+				continue;
+			//XXX this is a workaround 	for a jcore feature that shows errors in cus only when you get the original element
+			ITranslationUnit cu= (ITranslationUnit)CoreModel.getDefault().create(resource);
+			if (! cu.isStructureKnown()){
+				String path= cu.getResource().getFullPath().toOSString();
+				status.addError(RefactoringCoreMessages.getFormattedString("Checks.cannot_be_parsed", path)); //$NON-NLS-1$
+				continue; //removed, go to the next one
+			}
+			result.add(grouped[i]);	
+		}
+		
+		if ((!wasEmpty) && result.isEmpty())
+			status.addFatalError(RefactoringCoreMessages.getString("Checks.all_excluded")); //$NON-NLS-1$
+		
+		return (SearchResultGroup[])result.toArray(new SearchResultGroup[result.size()]);
+	}
+	
+	public static RefactoringStatus checkCompileErrorsInAffectedFiles(SearchResultGroup[] grouped) throws CModelException {
+		RefactoringStatus result= new RefactoringStatus();
+		for (int i= 0; i < grouped.length; i++){
+			IResource resource= grouped[i].getResource();
+			if (hasCompileErrors(resource))
+				result.addFatalError(RefactoringCoreMessages.getFormattedString("Checks.cu_has_compile_errors", resource.getFullPath().makeRelative())); //$NON-NLS-1$
+		}
+		return result;
+	}
+	
+	private static boolean hasCompileErrors(IResource resource) throws CModelException {
+		try {
+			IMarker[] problemMarkers= resource.findMarkers(ICModelMarker.C_MODEL_PROBLEM_MARKER, true, IResource.DEPTH_INFINITE);
+			for (int i= 0; i < problemMarkers.length; i++) {
+				if (problemMarkers[i].getAttribute(IMarker.SEVERITY, -1) == IMarker.SEVERITY_ERROR)
+					return true;
+			}
+			return false;
+		} catch (CModelException e){
+			throw e;		
+		} catch (CoreException e){
+			throw new CModelException(e);
+		}
+	}
+	
 }

core/org.eclipse.cdt.ui/refactor/org/eclipse/cdt/internal/corext/refactoring/refactoring.properties

@@ -326,19 +326,19 @@ RenameTypeRefactoring.choose_another_name=Please choose another name.
 RenameTypeRefactoring.creating_change=Preparing preview...
 RenameTypeRefactoring.rename_constructor=rename constructor
 RenameTypeRefactoring.searching=Searching for references...
-RenameTypeRefactoring.update_reference=update type reference
+RenameTypeRefactoring.update_reference=update element reference
 RenameTypeRefactoring.name=Rename ''{0}'' to ''{1}''
-RenameTypeRefactoring.enclosed=Type ''{0}'' is enclosed in a type named ''{1}''
-RenameTypeRefactoring.encloses=Type ''{0}'' encloses a type named ''{1}''
+RenameTypeRefactoring.enclosed=Element ''{0}'' is enclosed in a type named ''{1}''
+RenameTypeRefactoring.encloses=Element ''{0}'' encloses a type named ''{1}''
 RenameTypeRefactoring.exists=Type named ''{0}'' already exists in package ''{1}''
 RenameTypeRefactoring.imported=Type named ''{0}'' is imported (single-type-import) in ''{1}'' (a translation unit must not import and declare a type with the same name)
-RenameTypeRefactoring.member_type_exists=Another member type named ''{0}'' already exists in ''{1}''
-RenameTypeRefactoring.enclosed_type_native=A type enclosed in type ''{0}'' declares a native method. Renaming will cause an unsatisfied link error on runtime.
-RenameTypeRefactoring.name_conflict1=Name conflict with type ''{0}'' in ''{1}''
+RenameTypeRefactoring.member_type_exists=Another element named ''{0}'' already exists in ''{1}''
+RenameTypeRefactoring.enclosed_type_native=An element enclosed in type ''{0}'' declares a native method. Renaming will cause an unsatisfied link error on runtime.
+RenameTypeRefactoring.name_conflict1=Name conflict with element ''{0}'' in ''{1}''
 RenameTypeRefactoring.searching_text=searching for text matches
 RenameTypeRefactoring.update=Type declaration update
-RenameTypeRefactoring.does_not_exist=Type ''{0}'' does not exist in the saved version of ''{1}''
-RenameTypeRefactoring.will_not_rename=Compilation unit will not be renamed
+RenameTypeRefactoring.does_not_exist=Element ''{0}'' does not exist in the saved version of ''{1}''
+RenameTypeRefactoring.will_not_rename=Translation unit will not be renamed
 RenameTypeRefactoring.local_type=Local Type declared inside ''{0}'' is named {1}
 RenameTypeRefactoring.member_type=Member Type declared inside ''{0}'' is named {1}
 RenameTypeRefactoring.another_type=Another type named ''{0} is referenced in ''{1}''

core/org.eclipse.cdt.ui/refactor/org/eclipse/cdt/internal/corext/refactoring/rename/RenameElementProcessor.java

@@ -23,6 +23,7 @@ import org.eclipse.cdt.core.model.IFunctionDeclaration;
 import org.eclipse.cdt.core.model.IMethod;
 import org.eclipse.cdt.core.model.IMethodDeclaration;
 import org.eclipse.cdt.core.model.INamespace;
+import org.eclipse.cdt.core.model.IParent;
 import org.eclipse.cdt.core.model.ISourceReference;
 import org.eclipse.cdt.core.model.IStructure;
 import org.eclipse.cdt.core.model.ITranslationUnit;
@@ -166,6 +167,14 @@ public class RenameElementProcessor extends RenameProcessor implements IReferenc
 			result = Checks.checkIdentifier(newName);
 		}
 		
+		if (!(fCElement instanceof IFunctionDeclaration)){
+			if(checkSiblingsCollision().hasError()){
+				String msg= RefactoringCoreMessages.getFormattedString("RenameTypeRefactoring.member_type_exists", //$NON-NLS-1$
+						new String[]{fNewElementName, fCElement.getParent().getElementName()});
+				result.addFatalError(msg);		
+			}
+		}
+		
 		if (Checks.isAlreadyNamed(fCElement, newName))
 			result.addFatalError(RefactoringCoreMessages.getString("RenameTypeRefactoring.choose_another_name"));	 //$NON-NLS-1$
 		return result;
@@ -232,12 +241,32 @@ public class RenameElementProcessor extends RenameProcessor implements IReferenc
 				return result;
 			pm.worked(5);
 
+			result.merge(Checks.checkIfTuBroken(fCElement));
+			if (result.hasFatalError())
+				return result;
+			pm.worked(1);
+		
+			result.merge(checkEnclosingElements());
+			pm.worked(1);
+
+			result.merge(checkEnclosedElements());
+			pm.worked(1);
+
+			result.merge(checkSiblingsCollision());
+			pm.worked(1);
+			
+			if (result.hasFatalError())
+				return result;
+			
 			fReferences= null;
 			if (fUpdateReferences){
 				pm.setTaskName(RefactoringCoreMessages.getString("RenameTypeRefactoring.searching"));	 //$NON-NLS-1$
 				fReferences= getReferences(fCElement.getElementName(), new SubProgressMonitor(pm, 35));
 			}
-			pm.worked(10);
+			pm.worked(6);
+			
+			if (fUpdateReferences)
+				result.merge(analyzeAffectedTranslationUnits());
 
 			pm.setTaskName(RefactoringCoreMessages.getString("RenameTypeRefactoring.checking")); //$NON-NLS-1$
 			if (pm.isCanceled())
@@ -406,4 +435,70 @@ public class RenameElementProcessor extends RenameProcessor implements IReferenc
 		return orPattern;
 	}
 
+	private RefactoringStatus checkEnclosedElements() throws CoreException {
+		ICElement enclosedElement= findEnclosedElements(fCElement, fNewElementName);
+		if (enclosedElement == null)
+			return null;
+		String msg= RefactoringCoreMessages.getFormattedString("RenameTypeRefactoring.encloses",  //$NON-NLS-1$
+																		new String[]{fCElement.getElementName(), fNewElementName});
+		return RefactoringStatus.createErrorStatus(msg);
+	}
+
+	private RefactoringStatus checkEnclosingElements() throws CoreException {
+		ICElement enclosingElement= findEnclosingElements(fCElement, fNewElementName);
+		if (enclosingElement == null)
+			return null;
+			
+		String msg= RefactoringCoreMessages.getFormattedString("RenameTypeRefactoring.enclosed",//$NON-NLS-1$
+								new String[]{fCElement.getElementName(), fNewElementName});
+		return RefactoringStatus.createErrorStatus(msg);
+	}
+	
+	private static ICElement findEnclosedElements(ICElement element, String newName) throws CoreException {
+		if(element instanceof IParent){
+			ICElement[] enclosedTypes= ((IParent)element).getChildren();
+			for (int i= 0; i < enclosedTypes.length; i++){
+				if (newName.equals(enclosedTypes[i].getElementName()) || findEnclosedElements(enclosedTypes[i], newName) != null)
+					return enclosedTypes[i];
+			}
+		}
+		return null;
+	}
+		
+	private static ICElement findEnclosingElements(ICElement element, String newName) {
+		ICElement enclosing= element.getParent();
+		while (enclosing != null){
+			if (newName.equals(enclosing.getElementName()))
+				return enclosing;
+			else 
+				enclosing= enclosing.getParent();	
+		}
+		return null;
+	}
+	private RefactoringStatus checkSiblingsCollision() {		
+		RefactoringStatus result= new RefactoringStatus();
+		// get the siblings of the CElement and check if it has the same name
+		ICElement[] siblings= ((IParent)fCElement.getParent()).getChildren();
+		for (int i = 0; i <siblings.length; ++i ){
+			ICElement sibling = (ICElement)siblings[i];
+			if ((sibling.getElementName().equals(fNewElementName)) 
+			&& (sibling.getElementType() == fCElement.getElementType())  ) {
+				String msg= RefactoringCoreMessages.getFormattedString("RenameTypeRefactoring.member_type_exists", //$NON-NLS-1$
+						new String[]{fNewElementName, fCElement.getParent().getElementName()});
+				result.addError(msg);
+			}
+		}
+		return result;
+	}
+
+	private RefactoringStatus analyzeAffectedTranslationUnits() throws CoreException{
+		RefactoringStatus result= new RefactoringStatus();
+		fReferences= Checks.excludeTranslationUnits(fReferences, result);
+		if (result.hasFatalError())
+			return result;
+		
+		result.merge(Checks.checkCompileErrorsInAffectedFiles(fReferences));	
+		return result;
+	}
+	
 }