mirrors/cdt
1
0
Fork 0
mirror of https://github.com/eclipse-cdt/cdt synced 2025-04-21 21:52:10 +02:00
Code Issues Wiki Activity

Bug 45203. Added transitive closing of include maps.

Browse source
This commit is contained in:
Sergey Prigogin 2013-03-30 10:37:13 -07:00
parent ae409bedd6
commit 54919b0124
10 changed files with 320 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/includes/BindingClassifierTest.java
View file

@ -33,6 +33,9 @@ import org.eclipse.cdt.ui.testplugin.CTestPlugin;
import org.eclipse.cdt.internal.ui.refactoring.includes.BindingClassifier;
import org.eclipse.cdt.internal.ui.refactoring.includes.InclusionContext;
/**
* Tests for {@link BindingClassifier}.
*/
public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase {
private IIndex fIndex;
private InclusionContext fContext;

103
core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/includes/IncludeMapTest.java Normal file
View file

@ -0,0 +1,103 @@
/*******************************************************************************
* Copyright (c) 2013 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
* http://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
* Sergey Prigogin (Google) - initial API and implementation
*******************************************************************************/
package org.eclipse.cdt.ui.tests.refactoring.includes;
import junit.framework.TestCase;
import org.eclipse.cdt.internal.ui.refactoring.includes.IncludeMap;
/**
* Tests for {@link IncludeMap}.
*/
public class IncludeMapTest extends TestCase {
private void assertEqualMaps(IncludeMap expected, IncludeMap actual) {
assertEquals(expected.toString(), actual.toString());
}
public void testWeakCyclicMap() {
IncludeMap map = new IncludeMap(false, false, new String[] {
"a", "b",
"b", "c",
"c", "d",
"d", "b",
});
map.transitivelyClose();
IncludeMap expected = new IncludeMap(false, false, new String[] {
"a", "c",
"a", "b",
"a", "d",
"b", "d",
"b", "c",
"c", "d",
"c", "b",
"d", "b",
"d", "c",
});
assertEqualMaps(expected, map);
}
public void testStrongCyclicMap() {
IncludeMap map = new IncludeMap(true, false, new String[] {
"a", "b",
"b", "c",
"c", "d",
"d", "b",
});
map.transitivelyClose();
IncludeMap expected = new IncludeMap(true, false, new String[] {
"a", "c",
"c", "b",
"d", "b",
});
assertEqualMaps(expected, map);
}
public void testWeakMap() {
IncludeMap map = new IncludeMap(false, false, new String[] {
"a", "b",
"a", "c",
"c", "d",
"c", "e",
"d", "f",
});
map.transitivelyClose();
IncludeMap expected = new IncludeMap(false, false, new String[] {
"a", "b",
"a", "f",
"a", "d",
"a", "e",
"a", "c",
"c", "f",
"c", "d",
"c", "e",
"d", "f",
});
assertEqualMaps(expected, map);
}
public void testStrongMap() {
IncludeMap map = new IncludeMap(true, false, new String[] {
"a", "b",
"a", "c",
"c", "d",
"c", "e",
"d", "f",
});
map.transitivelyClose();
IncludeMap expected = new IncludeMap(true, false, new String[] {
"a", "b",
"c", "f",
"d", "f",
});
assertEqualMaps(expected, map);
}
}

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

@ -26,7 +26,7 @@ import org.eclipse.cdt.internal.ui.refactoring.includes.IHeaderChooser;
import org.eclipse.cdt.internal.ui.refactoring.includes.IncludeOrganizer;
/**
* Tests for IncludeOrganizer.
* Tests for {@link IncludeOrganizer}.
*/
public class IncludeOrganizerTest extends IncludesTestBase {

3
core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/includes/IncludesTestSuite.java
View file

@ -16,7 +16,8 @@ import junit.framework.TestSuite;
public class IncludesTestSuite extends TestSuite {
public static Test suite() throws Exception {
IncludesTestSuite suite = new IncludesTestSuite();
IncludesTestSuite suite = new IncludesTestSuite();
suite.addTestSuite(IncludeMapTest.class);
suite.addTest(BindingClassifierTest.suite());
suite.addTest(IncludeOrganizerTest.suite());
return suite;

3
core/org.eclipse.cdt.ui/.options
View file

@ -8,3 +8,6 @@ org.eclipse.cdt.ui/debug/SemanticHighlighting=false
# Enables debug information related to folding
org.eclipse.cdt.ui/debug/folding=false
# Enables debug information for header substitution in Organize Includes command.
org.eclipse.cdt.ui/debug/includeOrganizer/headerSubstitution=false

42
core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/HeaderSubstitutor.java
View file

@ -474,7 +474,6 @@ public class HeaderSubstitutor {
"<ios>", "<iostream>",
"<ios>", "<istream>",
"<ios>", "<ostream>",
"<iosfwd>", "<iostream>", // TODO(sprigogin): This should already be covered by <ios> -> <iostream> mapping
"<iosfwd>", "<ios>",
"<iosfwd>", "<streambuf>",
"<istream>", "<iostream>",
@ -489,10 +488,27 @@ public class HeaderSubstitutor {
private final InclusionContext fContext;
private List<IncludeMap> fIncludeMaps;
private final IncludeMap fIncludeMap;
private final IncludeMap fIncludeMapWeak;
private IncludeMap[] fIncludeMaps;
public HeaderSubstitutor(InclusionContext context) {
fContext = context;
fIncludeMap = new IncludeMap(cIncludeMap);
if (fContext.isCXXLanguage())
fIncludeMap.addAllMappings(cppIncludeMap);
fIncludeMapWeak = new IncludeMap(cIncludeMapWeak);
if (fContext.isCXXLanguage())
fIncludeMapWeak.addAllMappings(cppIncludeMapWeak);
}
public void addIncludeMap(IncludeMap map) {
if (fIncludeMaps != null)
throw new IllegalStateException("Modifications are not allowed after maps have been finalized"); //$NON-NLS-1$
if (map.isCppOnly() && !fContext.isCXXLanguage())
return;
IncludeMap receiver = map.isForcedReplacement() ? fIncludeMap : fIncludeMapWeak;
receiver.addAllMappings(map);
}
/**
@ -507,7 +523,7 @@ public class HeaderSubstitutor {
IncludeInfo includeInfo = fContext.getIncludeForHeaderFile(path);
if (includeInfo == null)
return null;
List<IncludeMap> maps = getAllIncludeMaps();
IncludeMap[] maps = getAllIncludeMaps();
for (IncludeMap map : maps) {
if (map.isForcedReplacement()) {
List<IncludeInfo> replacements = map.getMapping(includeInfo);
@ -535,7 +551,7 @@ public class HeaderSubstitutor {
// TODO(sprigogin): Take symbolIncludeMap into account.
List<IncludeInfo> candidates = new ArrayList<IncludeInfo>();
candidates.add(includeInfo);
List<IncludeMap> maps = getAllIncludeMaps();
IncludeMap[] maps = getAllIncludeMaps();
for (IncludeMap map : maps) {
for (int i = 0; i < candidates.size();) {
IncludeInfo candidate = candidates.get(i);
@ -641,21 +657,17 @@ public class HeaderSubstitutor {
return request.getCandidatePaths().iterator().next();
}
private List<IncludeMap> getAllIncludeMaps() {
private IncludeMap[] getAllIncludeMaps() {
if (fIncludeMaps == null) {
fIncludeMaps = new ArrayList<IncludeMap>();
for (IncludeMap map : INCLUDE_MAPS) {
if (fContext.isCXXLanguage() || !map.isCppOnly())
fIncludeMaps.add(map);
}
fIncludeMap.transitivelyClose();
fIncludeMapWeak.transitivelyClose();
fIncludeMaps = new IncludeMap[] { fIncludeMap, fIncludeMapWeak };
}
return fIncludeMaps;
}
/**
* Returns whether the given URI is an URI within the workspace
* @param uri
* @return
* Returns whether the given URI points within the workspace.
*/
private static boolean isWorkspaceFile(URI uri) {
for (IFile file : ResourceLookup.findFilesForLocationURI(uri)) {
@ -668,8 +680,6 @@ public class HeaderSubstitutor {
/**
* Returns whether the given path has a file suffix, or not.
* @param path
* @return
*/
private static boolean hasExtension(String path) {
return path.indexOf('.', path.lastIndexOf('/') + 1) >= 0;
@ -677,8 +687,6 @@ public class HeaderSubstitutor {
/**
* Returns the filename of the given path, without extension.
* @param path
* @return
*/
private static String getFilename(String path) {
int startPos = path.lastIndexOf('/') + 1;

26
core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludeInfo.java
View file

@ -10,7 +10,14 @@
*******************************************************************************/
package org.eclipse.cdt.internal.ui.refactoring.includes;
public class IncludeInfo {
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.Path;
import com.ibm.icu.text.Collator;
public class IncludeInfo implements Comparable<IncludeInfo> {
private static final Collator COLLATOR = Collator.getInstance();
private final String name;
private final boolean isSystem;
@ -84,4 +91,21 @@ public class IncludeInfo {
public String toString() {
return (isSystem ? '<' : '"') + name + (isSystem ? '>' : '"');
}
@Override
public int compareTo(IncludeInfo other) {
if (isSystem != other.isSystem) {
return isSystem ? -1 : 1;
}
IPath path1 = Path.fromOSString(name);
IPath path2 = Path.fromOSString(other.name);
int length1 = path1.segmentCount();
int length2 = path2.segmentCount();
for (int i = 0; i < length1 && i < length2; i++) {
int c = COLLATOR.compare(path1.segment(i), path2.segment(i));
if (c != 0)
return c;
}
return length1 - length2;
}
}

113
core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludeMap.java
View file

@ -11,9 +11,12 @@
package org.eclipse.cdt.internal.ui.refactoring.includes;
import java.io.StringReader;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
@ -22,7 +25,7 @@ import org.eclipse.ui.IMemento;
import org.eclipse.ui.WorkbenchException;
import org.eclipse.ui.XMLMemento;
class IncludeMap {
public class IncludeMap {
private static final String TAG_CPP_ONLY = "cpp_only"; //$NON-NLS-1$
private static final String TAG_FORCED_REPLACEMENT = "forced_replacement"; //$NON-NLS-1$
private static final String TAG_MAPPING = "mapping"; //$NON-NLS-1$
@ -56,6 +59,13 @@ class IncludeMap {
}
}
public IncludeMap(IncludeMap other) {
this.forcedReplacement = other.forcedReplacement;
this.cppOnly = other.cppOnly;
this.map = new HashMap<IncludeInfo, List<IncludeInfo>>(other.map.size());
addAllMappings(other);
}
/**
* Indicates that header file {@code to} should be used instead of {@code from}.
*
@ -63,6 +73,8 @@ class IncludeMap {
* @param to The header file to be used.
*/
protected void addMapping(IncludeInfo from, IncludeInfo to) {
if (from.equals(to))
return; // Don't allow mapping to itself.
List<IncludeInfo> list = map.get(from);
if (list == null) {
list = new ArrayList<IncludeInfo>(2);
@ -150,7 +162,7 @@ class IncludeMap {
return includeMap;
}
public static IncludeMap fromString(String str) {
public static IncludeMap fromSerializedMemento(String str) {
StringReader reader = new StringReader(str);
XMLMemento memento;
try {
@ -160,4 +172,101 @@ class IncludeMap {
}
return fromMemento(memento);
}
public void addAllMappings(IncludeMap other) {
if (other.forcedReplacement != forcedReplacement)
throw new IllegalArgumentException();
for (Entry<IncludeInfo, List<IncludeInfo>> entry : other.map.entrySet()) {
IncludeInfo source = entry.getKey();
List<IncludeInfo> otherTargets = entry.getValue();
List<IncludeInfo> targets = map.get(source);
if (targets == null) {
targets = new ArrayList<IncludeInfo>(otherTargets);
map.put(source, targets);
} else {
targets.addAll(otherTargets);
}
}
}
public void transitivelyClose() {
for (Entry<IncludeInfo, List<IncludeInfo>> entry : map.entrySet()) {
IncludeInfo source = entry.getKey();
List<IncludeInfo> targets = entry.getValue();
ArrayDeque<IncludeInfo> queue = new ArrayDeque<IncludeInfo>(targets);
targets.clear();
HashSet<IncludeInfo> seen = new HashSet<IncludeInfo>();
if (!forcedReplacement)
seen.add(source); // Don't allow mapping to itself.
int iterationsWithoutProgress = 0;
IncludeInfo target;
queueLoop: while ((target = queue.pollFirst()) != null) {
if (seen.contains(target))
continue;
List<IncludeInfo> newTargets = map.get(target);
if (newTargets != null) {
queue.addFirst(target);
boolean added = false;
for (int i = newTargets.size(); --i >=0;) {
IncludeInfo newTarget = newTargets.get(i);
if (!seen.contains(newTarget)) {
if (forcedReplacement && newTarget.equals(source)) {
break queueLoop; // Leave the mapping empty.
}
queue.addFirst(newTarget);
added = true;
}
}
// The second condition protects against an infinite loop.
if (!added || ++iterationsWithoutProgress >= map.size()) {
target = queue.pollFirst();
targets.add(target);
if (forcedReplacement)
break;
seen.add(target);
iterationsWithoutProgress = 0;
}
} else {
targets.add(target);
if (forcedReplacement)
break;
seen.add(target);
iterationsWithoutProgress = 0;
}
}
}
if (forcedReplacement) {
// Remove trivial mappings.
for (Iterator<Entry<IncludeInfo, List<IncludeInfo>>> iter = map.entrySet().iterator(); iter.hasNext();) {
Entry<IncludeInfo, List<IncludeInfo>> entry = iter.next();
IncludeInfo source = entry.getKey();
List<IncludeInfo> targets = entry.getValue();
if (targets.isEmpty() || (targets.size() == 1 && source.equals(targets.get(0)))) {
iter.remove();
}
}
}
}
/** For debugging only. */
@Override
public String toString() {
StringBuilder buf = new StringBuilder();
buf.append("forcedReplacement = ").append(forcedReplacement); //$NON-NLS-1$
buf.append(", cppOnly = ").append(cppOnly); //$NON-NLS-1$
ArrayList<IncludeInfo> sources = new ArrayList<IncludeInfo>(map.keySet());
Collections.sort(sources);
for (IncludeInfo source : sources) {
buf.append('\n');
buf.append(source);
buf.append(" -> "); //$NON-NLS-1$
List<IncludeInfo> targets = map.get(source);
for (int i = 0; i < targets.size(); i++) {
if (i > 0)
buf.append(", "); //$NON-NLS-1$
buf.append(targets.get(i));
}
}
return buf.toString();
}
}

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

@ -19,7 +19,6 @@ import static org.eclipse.cdt.internal.core.dom.parser.ASTTranslationUnit.getSta
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@ -32,6 +31,7 @@ import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.OperationCanceledException;
import org.eclipse.core.runtime.Path;
import org.eclipse.core.runtime.Platform;
import org.eclipse.jface.text.IRegion;
import org.eclipse.jface.text.Region;
import org.eclipse.text.edits.DeleteEdit;
@ -93,8 +93,9 @@ import org.eclipse.cdt.internal.ui.refactoring.includes.IncludeGroupStyle.Includ
* Organizes the include directives and forward declarations of a source or header file.
*/
public class IncludeOrganizer {
private static boolean DEBUG_HEADER_SUBSTITUTION = "true".equalsIgnoreCase(Platform.getDebugOption(CUIPlugin.PLUGIN_ID + "/debug/includeOrganizer/headerSubstitution")); //$NON-NLS-1$ //$NON-NLS-2$
private static class IncludePrototype {
private static class IncludePrototype implements Comparable<IncludePrototype> {
final IPath header; // null for existing unresolved includes
final IncludeInfo includeInfo; // never null
IASTPreprocessorIncludeStatement existingInclude; // null for newly added includes
@ -158,31 +159,15 @@ public class IncludeOrganizer {
public String toString() {
return header != null ? header.toPortableString() : includeInfo.toString();
}
@Override
public int compareTo(IncludePrototype other) {
return includeInfo.compareTo(other.includeInfo);
}
}
private static final Collator COLLATOR = Collator.getInstance();
private static final Comparator<IncludePrototype> INCLUDE_COMPARATOR = new Comparator<IncludePrototype>() {
@Override
public int compare(IncludePrototype include1, IncludePrototype include2) {
IncludeInfo includeInfo1 = include1.includeInfo;
IncludeInfo includeInfo2 = include2.includeInfo;
if (includeInfo1.isSystem() != includeInfo2.isSystem()) {
return includeInfo1.isSystem() ? -1 : 1;
}
IPath path1 = Path.fromOSString(includeInfo1.getName());
IPath path2 = Path.fromOSString(includeInfo2.getName());
int length1 = path1.segmentCount();
int length2 = path2.segmentCount();
for (int i = 0; i < length1 && i < length2; i++) {
int c = COLLATOR.compare(path1.segment(i), path2.segment(i));
if (c != 0)
return c;
}
return length1 - length2;
}
};
private final IHeaderChooser fHeaderChooser;
private final InclusionContext fContext;
private final String fLineDelimiter;
@ -291,7 +276,7 @@ public class IncludeOrganizer {
IncludeGroupStyle previousParentStyle = null;
for (List<IncludePrototype> prototypes : groupedPrototypes) {
if (prototypes != null && !prototypes.isEmpty()) {
Collections.sort(prototypes, INCLUDE_COMPARATOR);
Collections.sort(prototypes);
IncludeGroupStyle style = prototypes.get(0).style;
IncludeGroupStyle groupingStyle = getGroupingStyle(style);
IncludeGroupStyle parentStyle = getParentStyle(groupingStyle);
@ -912,6 +897,10 @@ public class IncludeOrganizer {
for (IPath path : candidatePaths) {
if (fContext.isIncluded(path)) {
request.resolve(path);
if (DEBUG_HEADER_SUBSTITUTION) {
System.out.println(request.toString() +
(fContext.isToBeIncluded(path) ? " (decided earlier)" : " (was previously included)")); //$NON-NLS-1$ //$NON-NLS-2$
}
break;
} else {
IPath header = headerSubstitutor.getUniqueRepresentativeHeader(path);
@ -926,6 +915,8 @@ public class IncludeOrganizer {
if (!request.isResolved() && allRepresented && representativeHeaders.size() == 1) {
IPath path = representativeHeaders.iterator().next();
request.resolve(path);
if (DEBUG_HEADER_SUBSTITUTION)
System.out.println(request.toString() + " (unique representative)"); //$NON-NLS-1$
if (!fContext.isAlreadyIncluded(path))
fContext.addHeaderToInclude(path);
}
@ -940,12 +931,19 @@ public class IncludeOrganizer {
IPath path = candidatePaths.iterator().next();
if (fContext.isIncluded(path)) {
request.resolve(path);
if (DEBUG_HEADER_SUBSTITUTION) {
System.out.println(request.toString() +
(fContext.isToBeIncluded(path) ? " (decided earlier)" : " (was previously included)")); //$NON-NLS-1$ //$NON-NLS-2$
}
} else {
IPath header = headerSubstitutor.getPreferredRepresentativeHeader(path);
if (header.equals(path) && fContext.getPreferences().heuristicHeaderSubstitution) {
header = headerSubstitutor.getPreferredRepresentativeHeaderByHeuristic(request);
}
request.resolve(header);
if (DEBUG_HEADER_SUBSTITUTION) {
System.out.println(request.toString() + " (preferred representative)"); //$NON-NLS-1$
}
if (!fContext.isAlreadyIncluded(header))
fContext.addHeaderToInclude(header);
}
@ -960,6 +958,10 @@ public class IncludeOrganizer {
for (IPath path : candidatePaths) {
if (fContext.isIncluded(path)) {
request.resolve(path);
if (DEBUG_HEADER_SUBSTITUTION) {
System.out.println(request.toString() +
(fContext.isToBeIncluded(path) ? " (decided earlier)" : " (was previously included)")); //$NON-NLS-1$ //$NON-NLS-2$
}
break;
}
}
@ -969,6 +971,9 @@ public class IncludeOrganizer {
throw new OperationCanceledException();
request.resolve(header);
if (DEBUG_HEADER_SUBSTITUTION) {
System.out.println(request.toString() + " (user's choice)"); //$NON-NLS-1$
}
if (!fContext.isAlreadyIncluded(header))
fContext.addHeaderToInclude(header);
}

18
core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/InclusionRequest.java
View file

@ -75,4 +75,22 @@ class InclusionRequest {
public boolean isResolved() {
return fResolvedPath != null;
}
/** For debugging only */
@Override
public String toString() {
StringBuilder buf = new StringBuilder();
buf.append(fBinding.getName());
buf.append(" defined in "); //$NON-NLS-1$
for (int i = 0; i < fCandidatePaths.size(); i++) {
if (i != 0)
buf.append(", "); //$NON-NLS-1$
buf.append(fCandidatePaths.get(i).toOSString());
}
if (fResolvedPath != null) {
buf.append(" represented by "); //$NON-NLS-1$
buf.append(fResolvedPath.toOSString());
}
return buf.toString();
}
}