From 69562ae63e0fb6e8f055aeb20eddfd34ad91e8d7 Mon Sep 17 00:00:00 2001 From: Sergey Prigogin Date: Fri, 26 Feb 2016 19:47:40 -0800 Subject: [PATCH] Bug 488605 - Organize Includes adds an include for a header included by the partner header Change-Id: I16ca2afc2ecbe1b47c9a71be5e0c170c9ac0d08d --- .../includes/IncludeOrganizerTest.java | 26 +++++++---- .../includes/IncludeCreationContext.java | 12 +++++ .../includes/IncludeOrganizer.java | 46 +++++++++++-------- 3 files changed, 57 insertions(+), 27 deletions(-) diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/includes/IncludeOrganizerTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/includes/IncludeOrganizerTest.java index 4f4242a6cf4..1de8fa872ba 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/includes/IncludeOrganizerTest.java +++ b/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(); } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludeCreationContext.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludeCreationContext.java index 7a4f447b0c1..e3eef3b1948 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludeCreationContext.java +++ b/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; + } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludeOrganizer.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludeOrganizer.java index f35f48c8859..b8e7c287183 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludeOrganizer.java +++ b/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 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 requests, - HeaderSubstitutor headerSubstitutor) throws CoreException { + IASTPreprocessorIncludeStatement[] existingIncludes, HeaderSubstitutor headerSubstitutor) + throws CoreException { // Add partner header if necessary. - HashSet includedByPartner = fContext.getPreferences().allowPartnerIndirectInclusion ? - new HashSet() : null; + IIndexFile partnerHeader = null; for (InclusionRequest request : requests) { List 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) { - try { - IIndexFile indexFile = request.getDeclaringFiles().keySet().iterator().next(); - if (!includedByPartner.contains(indexFile)) { - for (IIndexInclude include : indexFile.getIncludes()) { - IIndexFileLocation headerLocation = include.getIncludesLocation(); - if (headerLocation != null) { - fContext.addHeaderAlreadyIncluded(getAbsolutePath(headerLocation)); - } - } - includedByPartner.add(indexFile); + partnerHeader = request.getDeclaringFiles().keySet().iterator().next(); + break; + } + } + } + + if (fContext.getPreferences().allowPartnerIndirectInclusion) { + // Mark all headers included by the partner header as already included. + if (partnerHeader == null) { + for (IASTPreprocessorIncludeStatement include : existingIncludes) { + 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.