Bug 488605 - Organize Includes adds an include for a header included by

the partner header

Change-Id: I16ca2afc2ecbe1b47c9a71be5e0c170c9ac0d08d

core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/includes/IncludeOrganizerTest.java

@@ -24,7 +24,6 @@ import org.eclipse.cdt.internal.ui.refactoring.includes.HeaderSubstitutionMap;
 import org.eclipse.cdt.internal.ui.refactoring.includes.IHeaderChooser;
 import org.eclipse.cdt.internal.ui.refactoring.includes.IncludeMap;
 import org.eclipse.cdt.internal.ui.refactoring.includes.IncludeOrganizer;
-import org.eclipse.cdt.internal.ui.refactoring.includes.IncludePreferences.UnusedStatementDisposition;
 import org.eclipse.cdt.internal.ui.refactoring.includes.SymbolExportMap;
 
 import junit.framework.Test;
@@ -310,12 +309,27 @@ public class IncludeOrganizerTest extends IncludesTestBase {
 	//}
 	public void testExistingPartnerIncludeIsNotRemoved() throws Exception {
 		IPreferenceStore preferenceStore = getPreferenceStore();
-		preferenceStore.setValue(PreferenceConstants.INCLUDES_UNUSED_STATEMENTS_DISPOSITION,
-				UnusedStatementDisposition.REMOVE.toString());
 		preferenceStore.setValue(PreferenceConstants.FORWARD_DECLARE_FUNCTIONS, true);
 		assertExpectedResults();
 	}
 
+	//a.h
+	//class A {};
+
+	//f.h
+	//#include "a.h"
+
+	//f.cpp
+	//#include "f.h"
+	//A a;
+	//====================
+	//#include "f.h"
+	//
+	//A a;
+	public void testPartnerInclusion() throws Exception {
+		assertExpectedResults();
+	}
+
 	//h1.h
 	//class A {};
 
@@ -464,8 +478,6 @@ public class IncludeOrganizerTest extends IncludesTestBase {
 	//M1(a, 1);
 	public void testMacro() throws Exception {
 		IPreferenceStore preferenceStore = getPreferenceStore();
-		preferenceStore.setValue(PreferenceConstants.INCLUDES_UNUSED_STATEMENTS_DISPOSITION,
-				UnusedStatementDisposition.REMOVE.toString());
 		assertExpectedResults();
 	}
 
@@ -491,8 +503,6 @@ public class IncludeOrganizerTest extends IncludesTestBase {
 	//}
 	public void testExportedSymbol() throws Exception {
 		IPreferenceStore preferenceStore = getPreferenceStore();
-		preferenceStore.setValue(PreferenceConstants.INCLUDES_UNUSED_STATEMENTS_DISPOSITION,
-				UnusedStatementDisposition.REMOVE.toString());
 		SymbolExportMap symbolExportMap = new SymbolExportMap(new String[] { "NULL", "string.h" });
 		preferenceStore.setValue(PreferenceConstants.INCLUDES_SYMBOL_EXPORTING_HEADERS,
 				SymbolExportMap.serializeMaps(Collections.singletonList(symbolExportMap)));
@@ -670,8 +680,6 @@ public class IncludeOrganizerTest extends IncludesTestBase {
 	public void testForwardDeclarations() throws Exception {
 		// TODO(sprigogin): Move ns1 outside of other namespaces after IncludeOrganizer starts using ASTWriter.
 		IPreferenceStore preferenceStore = getPreferenceStore();
-		preferenceStore.setValue(PreferenceConstants.INCLUDES_UNUSED_STATEMENTS_DISPOSITION,
-				UnusedStatementDisposition.REMOVE.toString());
 		preferenceStore.setValue(PreferenceConstants.FORWARD_DECLARE_FUNCTIONS, true);
 		assertExpectedResults();
 	}

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludeCreationContext.java

@@ -152,4 +152,16 @@ public class IncludeCreationContext extends InclusionContext {
 	public final boolean wasIncludedPreviously(IPath header) {
 		return fHeadersIncludedPreviously.contains(header);
 	}
+
+	/**
+	 * Returns the path of the partner header included previously, or {@code null} if the partner was not
+	 * included.
+	 */
+	public final IPath getPartnerHeaderIncludedPreviously() {
+		for (IPath path : fHeadersIncludedPreviously) {
+			if (isPartnerFile(path))
+				return path;
+		}
+		return null;
+	}
 }

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludeOrganizer.java

@@ -1,5 +1,4 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2015 Google, Inc 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
@@ -193,7 +192,7 @@ public class IncludeOrganizer {
 		IIndexFileSet reachableHeaders = ast.getIndexFileSet();
 
 		List<InclusionRequest> requests = createInclusionRequests(ast, bindingsToInclude, false, reachableHeaders);
-		processInclusionRequests(requests, headerSubstitutor);
+		processInclusionRequests(requests, existingIncludes, headerSubstitutor);
 
 		NodeCommentMap commentedNodeMap = ASTCommenter.getCommentedNodeMap(ast);
 
@@ -599,10 +598,10 @@ public class IncludeOrganizer {
 	}
 
 	private void processInclusionRequests(List<InclusionRequest> requests,
-			HeaderSubstitutor headerSubstitutor) throws CoreException {
+			IASTPreprocessorIncludeStatement[] existingIncludes, HeaderSubstitutor headerSubstitutor)
+			throws CoreException {
 		// Add partner header if necessary.
-		HashSet<IIndexFile> includedByPartner = fContext.getPreferences().allowPartnerIndirectInclusion ?
+		IIndexFile partnerHeader = null;
-				new HashSet<IIndexFile>() : null;
 		for (InclusionRequest request : requests) {
 			List<IPath> candidatePaths = request.getCandidatePaths();
 			if (candidatePaths.size() == 1) {
@@ -610,24 +609,35 @@ public class IncludeOrganizer {
 				if (fContext.isPartnerFile(path)) {
 					request.resolve(path);
 					fContext.addHeaderToInclude(path);
-					if (includedByPartner != null) {
+					partnerHeader = request.getDeclaringFiles().keySet().iterator().next();
-						try {
+					break;
-							IIndexFile indexFile = request.getDeclaringFiles().keySet().iterator().next();
+				}
-							if (!includedByPartner.contains(indexFile)) {
+			}
-								for (IIndexInclude include : indexFile.getIncludes()) {
+		}
-									IIndexFileLocation headerLocation = include.getIncludesLocation();
+
-									if (headerLocation != null) {
+		if (fContext.getPreferences().allowPartnerIndirectInclusion) {
-										fContext.addHeaderAlreadyIncluded(getAbsolutePath(headerLocation));
+			// Mark all headers included by the partner header as already included.
-									}
+			if (partnerHeader == null) {
-								}
+				for (IASTPreprocessorIncludeStatement include : existingIncludes) {
-								includedByPartner.add(indexFile);
+					if (include.isPartOfTranslationUnitFile()) {
+						IIndexFile header = include.getImportedIndexFile();
+						if (header != null) {
+							if (fContext.isPartnerFile(new Path(include.getPath()))) {
+								partnerHeader = header;
+								break;
 							}
-						} catch (CoreException e) {
-							CUIPlugin.log(e);
 						}
 					}
 				}
 			}
+			if (partnerHeader != null) {
+				for (IIndexInclude include : partnerHeader.getIncludes()) {
+					IIndexFileLocation headerLocation = include.getIncludesLocation();
+					if (headerLocation != null) {
+						fContext.addHeaderAlreadyIncluded(getAbsolutePath(headerLocation));
+					}
+				}
+			}
 		}
 
 		// Process headers that are either indirectly included or have unique representatives.