Bug 485422 - Organize includes: header substitution maps not always used

With this change the heuristic header substitution tries to find a
header that is better than the one computed using regular substitution
rules, but defaults to that header if it can't find a better one.

Change-Id: I811857b2ccf66e2db4161801f34db8464834ce12

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

@@ -33,6 +33,10 @@ import org.eclipse.cdt.ui.PreferenceConstants;
 import org.eclipse.cdt.internal.corext.codemanipulation.IncludeInfo;
 
 public class HeaderSubstitutor {
+	private static int HEURISTIC_SCORE_NAME_MATCH = 1;
+	private static int HEURISTIC_SCORE_NO_EXTENSION = 2;
+	private static int HEURISTIC_SCORE_MAX = HEURISTIC_SCORE_NAME_MATCH + HEURISTIC_SCORE_NO_EXTENSION;
+
 	private final IncludeCreationContext fContext;
 	private IncludeMap[] fIncludeMaps;
 	private SymbolExportMap fSymbolExportMap;
@@ -192,65 +196,65 @@ public class HeaderSubstitutor {
 
 	/**
 	 * Performs heuristic header substitution.
+	 *
+	 * @param request the inclusion request
+	 * @param preferredHeader the header selected using regular header substitution rules
 	 */
-	public IPath getPreferredRepresentativeHeaderByHeuristic(InclusionRequest request) {
+	public IPath getPreferredRepresentativeHeaderByHeuristic(InclusionRequest request, IPath preferredHeader) {
-		Set<IIndexFile> indexFiles = request.getDeclaringFiles().keySet();
+		if (!fContext.isCXXLanguage())
+			return preferredHeader;
+
 		String symbolName = request.getBinding().getName();
+		int bestScore = getScore(preferredHeader.toString(), symbolName);
+		if (bestScore == HEURISTIC_SCORE_MAX)
+			return preferredHeader;  // Nothing can be better than preferredHeader.
+
+		IIndexFile bestCandidate = null;
+		Set<IIndexFile> indexFiles = request.getDeclaringFiles().keySet();
 		ArrayDeque<IIndexFile> front = new ArrayDeque<>();
 		HashSet<IIndexFile> processed = new HashSet<>();
-		IIndexFile bestCandidate = null;
+		front.addAll(indexFiles);
-		IIndexFile candidateWithoutExtension = null;
+		processed.addAll(indexFiles);
-		IIndexFile candidateWithMatchingName = null;
 
 		try {
 			// Look for headers matching by name and headers without an extension.
-			if (fContext.isCXXLanguage()) {
+			while (!front.isEmpty()) {
-				front.addAll(indexFiles);
+				IIndexFile file = front.remove();
-				processed.addAll(indexFiles);
+				String path = IncludeUtil.getPath(file);
 
-				while (!front.isEmpty()) {
+				int score = getScore(path, symbolName);
-					IIndexFile file = front.remove();
+				if (score > bestScore) {
-					String path = IncludeUtil.getPath(file);
+					bestScore = score;
+					bestCandidate = file;
+				}
 
-					if (getFilename(path).equalsIgnoreCase(symbolName)) {
+				// Process the next level of the include hierarchy.
-						if (!hasExtension(path)) {
+				IIndexInclude[] includes = fContext.getIndex().findIncludedBy(file, 0);
-							// A C++ header without an extension and with a name which matches the name
+				for (IIndexInclude include : includes) {
-							// of the symbol that should be declared is a perfect candidate for inclusion.
+					IIndexFile includer = include.getIncludedBy();
-							bestCandidate = file;
+					if (!processed.contains(includer)) {
-							break;
+						front.add(includer);
-						}
+						processed.add(includer);
-						if (candidateWithMatchingName == null)
-							candidateWithMatchingName = file;
-					} else if (!hasExtension(path)) {
-						if (candidateWithoutExtension == null)
-							candidateWithoutExtension = file;
-					}
-
-					// Process the next level of the include hierarchy.
-					IIndexInclude[] includes = fContext.getIndex().findIncludedBy(file, 0);
-					for (IIndexInclude include : includes) {
-						IIndexFile includer = include.getIncludedBy();
-						if (!processed.contains(includer)) {
-							front.add(includer);
-							processed.add(includer);
-						}
 					}
 				}
 			}
 
-			if (bestCandidate == null)
-				bestCandidate = candidateWithoutExtension;
-
-			if (bestCandidate == null)
-				bestCandidate = candidateWithMatchingName;
-
 			if (bestCandidate != null)
 				return IndexLocationFactory.getAbsolutePath(bestCandidate.getLocation());
 		} catch (CoreException e) {
 			CUIPlugin.log(e);
 		}
 
-		return request.getCandidatePaths().iterator().next();
+		return preferredHeader;
+	}
+
+	private static int getScore(String path, String symbolName) {
+		int score = 0;
+		if (getFilename(path).equalsIgnoreCase(symbolName))
+			score += HEURISTIC_SCORE_NAME_MATCH;
+		if (!hasExtension(path))
+			score += HEURISTIC_SCORE_NO_EXTENSION;
+		return score;
 	}
 
 	/**

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

@@ -488,7 +488,7 @@ public class IncludeCreator {
 				boolean reachable = ast.getIndexFileSet().contains(file);
 				InclusionRequest request =
 						new InclusionRequest(binding, Collections.singletonMap(file, header), reachable);
-				header = headerSubstitutor.getPreferredRepresentativeHeaderByHeuristic(request);
+				header = headerSubstitutor.getPreferredRepresentativeHeaderByHeuristic(request, header);
 			}
 			IncludeGroupStyle style = fContext.getIncludeStyle(header);
 			include = fContext.createIncludeInfo(header, style);

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

@@ -696,7 +696,7 @@ public class IncludeOrganizer {
 					} else {
 						IPath header = headerSubstitutor.getPreferredRepresentativeHeader(path);
 						if (header.equals(path) && fContext.getPreferences().heuristicHeaderSubstitution) {
-							header = headerSubstitutor.getPreferredRepresentativeHeaderByHeuristic(request);
+							header = headerSubstitutor.getPreferredRepresentativeHeaderByHeuristic(request, header);
 						}
 						request.resolve(header);
 						if (DEBUG_HEADER_SUBSTITUTION) {