V2: Bug 410074 - Refactoring throws NullPointerException

Change-Id: Ic2b69a8954a3b9a437a1e8dafaff2f92b10caa6f
Reviewed-on: https://git.eclipse.org/r/13740
Reviewed-by: Sergey Prigogin <eclipse.sprigogin@gmail.com>
IP-Clean: Sergey Prigogin <eclipse.sprigogin@gmail.com>
Tested-by: Sergey Prigogin <eclipse.sprigogin@gmail.com>

core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/implementmethod/ImplementMethodRefactoringTest.java

@@ -271,9 +271,12 @@ public class ImplementMethodRefactoringTest extends RefactoringTestBase {
 	//void function_with_impl();
 
 	//A.cpp
+	//#include "A.h"
 	//void function_with_impl() {
 	//}
 	//====================
+	//#include "A.h"
+	//
 	//void function() {
 	//}
 	//

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/MethodContext.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2008, 2010 Institute for Software, HSR Hochschule fuer Technik
+ * Copyright (c) 2008, 2013 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,6 +8,7 @@
  *
  * Contributors:
  *     Institute for Software - initial API and implementation
+ *     Sergey Prigogin (Google)
  *******************************************************************************/
 package org.eclipse.cdt.internal.ui.refactoring;
 
@@ -57,17 +58,15 @@ public class MethodContext {
 	}
 
 	public IASTDeclaration getMethodDeclaration() {
-		IASTNode parent = declarationName.getParent().getParent();
+		if (declarationName != null) {
-		if (parent instanceof IASTDeclaration) {
+			IASTNode parent = declarationName.getParent().getParent();
-			return (IASTDeclaration) parent;
+			if (parent instanceof IASTDeclaration) {
+				return (IASTDeclaration) parent;
+			}
 		}
 		return null;
 	}
 
-	public Visibility getMethodDeclarationVisibility() {
-		return Visibility.getVisibility(declarationName);
-	}
-
 	public void setMethodQName(ICPPASTQualifiedName qname) {
 		this.qname = qname;
 	}

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/ExtractFunctionRefactoring.java

@@ -215,6 +215,10 @@ public class ExtractFunctionRefactoring extends CRefactoring {
 			info.setDeclarator(getDeclaration(container.getNodesToWrite().get(0)));
 			MethodContext context =	NodeHelper.findMethodContext(container.getNodesToWrite().get(0),
 					refactoringContext, sm.newChild(1));
+			if (context.getType() == ContextType.METHOD && context.getMethodDeclarationName() == null) {
+				initStatus.addFatalError(Messages.ExtractFunctionRefactoring_no_declaration_of_surrounding_method);
+				return initStatus;
+			}
 			info.setMethodContext(context);
 			return initStatus;
 		} finally {
@@ -290,8 +294,9 @@ public class ExtractFunctionRefactoring extends CRefactoring {
 		MethodContext context = info.getMethodContext();
 
 		if (context.getType() == ContextType.METHOD && !context.isInline()) {
+			IASTDeclaration contextDeclaration = context.getMethodDeclaration();
 			ICPPASTCompositeTypeSpecifier classDeclaration =
-					(ICPPASTCompositeTypeSpecifier) context.getMethodDeclaration().getParent();
+					(ICPPASTCompositeTypeSpecifier) contextDeclaration.getParent();
 			IASTSimpleDeclaration methodDeclaration = getDeclaration(methodName);
 
 			if (isMethodAllreadyDefined(methodDeclaration, classDeclaration, getIndex())) {

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/Messages.java

@@ -18,6 +18,7 @@ public final class Messages extends NLS {
 	public static String ExtractFunctionRefactoring_ExtractFunction;
 	public static String ExtractFunctionRefactoring_NoStmtSelected;
 	public static String ExtractFunctionRefactoring_TooManySelected;
+	public static String ExtractFunctionRefactoring_no_declaration_of_surrounding_method;
 	public static String ExtractFunctionRefactoring_name_in_use;
 	public static String ExtractFunctionRefactoring_parameter_name_in_use;
 	public static String ExtractFunctionRefactoring_duplicate_parameter;

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/extractfunction/Messages.properties

@@ -13,6 +13,7 @@
 ExtractFunctionRefactoring_ExtractFunction=Extract Function
 ExtractFunctionRefactoring_NoStmtSelected=No statement selected
 ExtractFunctionRefactoring_TooManySelected=Too many declarations in selection.
+ExtractFunctionRefactoring_no_declaration_of_surrounding_method=Unable to find declaration of the surrounding method.
 ExtractFunctionRefactoring_name_in_use=Name already in use.
 ExtractFunctionRefactoring_parameter_name_in_use=''{0}'' is already used as a name in the selected code
 ExtractFunctionRefactoring_duplicate_parameter=A parameter ''{0}'' already exists

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/utils/DefinitionFinder.java

@@ -31,6 +31,7 @@ import org.eclipse.cdt.core.dom.ast.IBinding;
 import org.eclipse.cdt.core.dom.ast.cpp.ICPPMember;
 import org.eclipse.cdt.core.index.IIndex;
 import org.eclipse.cdt.core.index.IIndexBinding;
+import org.eclipse.cdt.core.index.IIndexFile;
 import org.eclipse.cdt.core.index.IIndexName;
 import org.eclipse.cdt.core.model.CoreModelUtil;
 import org.eclipse.cdt.core.model.ITranslationUnit;
@@ -63,7 +64,7 @@ public class DefinitionFinder {
 		if (binding == null) {
 			return null;
 		}
-		return getDefinition(binding, context, pm);
+		return getDefinition(binding, name.getTranslationUnit(), context, pm);
 	}
 
 	/**
@@ -71,14 +72,17 @@ public class DefinitionFinder {
 	 * of dirty editors.
 	 * 
 	 * @param binding the binding to find the definition for
+	 * @param contextTu the translation unit that determines the set of files to search for
+	 *     the definition. Only the files directly or indirectly included by the translation unit
+	 *     are considered. 
 	 * @param context the refactoring context
 	 * @param pm the progress monitor
 	 * @return the definition name, or {@code null} if there is no definition or if it is
 	 *     not unique.
 	 * @throws CoreException thrown in case of errors
 	 */
-	public static IASTName getDefinition(IBinding binding, CRefactoringContext context,
+	public static IASTName getDefinition(IBinding binding, IASTTranslationUnit contextTu,
-			IProgressMonitor pm) throws CoreException {
+			CRefactoringContext context, IProgressMonitor pm) throws CoreException {
 		SubMonitor sm = SubMonitor.convert(pm, 10);
 		IIndex index = context.getIndex();
 		if (index == null) {
@@ -97,12 +101,16 @@ public class DefinitionFinder {
 			if (sm.isCanceled()) {
 				throw new OperationCanceledException();
 			}
-			ITranslationUnit tu = CoreModelUtil.findTranslationUnitForLocation(
+			IIndexFile indexFile = name.getFile();
-					name.getFile().getLocation(), null);
+			if (contextTu.getASTFileSet().contains(indexFile) ||
-			if (searchedFiles.add(tu.getLocation().toOSString())) {
+					contextTu.getIndexFileSet().contains(indexFile)) {
-				findDefinitionsInTranslationUnit(indexBinding, tu, context, definitions, pm);
+				ITranslationUnit tu = CoreModelUtil.findTranslationUnitForLocation(
-				if (definitions.size() > 1)
+						indexFile.getLocation(), null);
-					return null;
+				if (searchedFiles.add(tu.getLocation().toOSString())) {
+					findDefinitionsInTranslationUnit(indexBinding, tu, context, definitions, pm);
+					if (definitions.size() > 1)
+						return null;
+				}
 			}
 			loopProgress.setWorkRemaining(--remainingCount);
 		}
@@ -230,7 +238,7 @@ public class DefinitionFinder {
 		IBinding binding = memberName.resolveBinding();
 		if (!(binding instanceof ICPPMember))
 			return null;
-		return getMemberDeclaration((ICPPMember) binding, context, pm);
+		return getMemberDeclaration((ICPPMember) binding, memberName.getTranslationUnit(), context, pm);
 	}
 
 	/**
@@ -239,15 +247,18 @@ public class DefinitionFinder {
 	 * editors.
 	 * 
 	 * @param member the class member binding to find the declaration for
+	 * @param contextTu the translation unit that determines the set of files to search for
+	 *     the declaration. Only the files directly or indirectly included by the translation unit
+	 *     are considered. 
 	 * @param context the refactoring context
 	 * @param pm the progress monitor
 	 * @return the declaration name, or {@code null} if there is no declaration or if it is
 	 *     not unique.
 	 * @throws CoreException thrown in case of errors
 	 */
-	public static IASTName getMemberDeclaration(ICPPMember member, CRefactoringContext context,
+	public static IASTName getMemberDeclaration(ICPPMember member, IASTTranslationUnit contextTu,
-			IProgressMonitor pm) throws CoreException, OperationCanceledException {
+			CRefactoringContext context, IProgressMonitor pm) throws CoreException, OperationCanceledException {
-		IASTName classDefintionName = getDefinition(member.getClassOwner(), context, pm);
+		IASTName classDefintionName = getDefinition(member.getClassOwner(), contextTu, context, pm);
 		if (classDefintionName == null)
 			return null;
 		IASTCompositeTypeSpecifier compositeTypeSpecifier =