[clang-tools-extra] r269028 - [include-fixer] For now, only add the first suggested include.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Tue May 10 03:25:28 2016
New Revision: 269028

URL: http://llvm.org/viewvc/llvm-project?rev=269028&view=rev
Log:
[include-fixer] For now, only add the first suggested include.

We used a std::set which made the picked include somewhat random (well,
lexicographically sorted). Make the behavior more consistent by always
picking the first one we found.

Modified:
    clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
    clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp

Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp?rev=269028&r1=269027&r2=269028&view=diff
==============================================================================
--- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp Tue May 10 03:25:28 2016
@@ -178,26 +178,27 @@ public:
   bool Rewrite(clang::SourceManager &SourceManager,
                clang::HeaderSearch &HeaderSearch,
                std::vector<clang::tooling::Replacement> &replacements) {
-    for (const auto &ToTry : Untried) {
-      std::string ToAdd = "#include " +
-                          minimizeInclude(ToTry, SourceManager, HeaderSearch) +
-                          "\n";
-      DEBUG(llvm::dbgs() << "Adding " << ToAdd << "\n");
-
-      if (FirstIncludeOffset == -1U)
-        FirstIncludeOffset = 0;
-
-      replacements.push_back(clang::tooling::Replacement(
-          SourceManager, FileBegin.getLocWithOffset(FirstIncludeOffset), 0,
-          ToAdd));
-
-      // We currently abort after the first inserted include. The more
-      // includes we have the less safe this becomes due to error recovery
-      // changing the results.
-      // FIXME: Handle multiple includes at once.
-      return true;
-    }
-    return false;
+    if (Untried.empty())
+      return false;
+
+    const auto &ToTry = UntriedList.front();
+    std::string ToAdd = "#include " +
+                        minimizeInclude(ToTry, SourceManager, HeaderSearch) +
+                        "\n";
+    DEBUG(llvm::dbgs() << "Adding " << ToAdd << "\n");
+
+    if (FirstIncludeOffset == -1U)
+      FirstIncludeOffset = 0;
+
+    replacements.push_back(clang::tooling::Replacement(
+        SourceManager, FileBegin.getLocWithOffset(FirstIncludeOffset), 0,
+        ToAdd));
+
+    // We currently abort after the first inserted include. The more
+    // includes we have the less safe this becomes due to error recovery
+    // changing the results.
+    // FIXME: Handle multiple includes at once.
+    return true;
   }
 
   /// Gets the location at the very top of the file.
@@ -209,7 +210,8 @@ public:
   /// Add an include to the set of includes to try.
   /// \param include_path The include path to try.
   void TryInclude(const std::string &query, const std::string &include_path) {
-    Untried.insert(include_path);
+    if (Untried.insert(include_path).second)
+      UntriedList.push_back(include_path);
   }
 
 private:
@@ -255,8 +257,11 @@ private:
   /// be used as the insertion point for new include directives.
   unsigned FirstIncludeOffset = -1U;
 
-  /// Includes we have left to try.
+  /// Includes we have left to try. A set to unique them and a list to keep
+  /// track of the order. We prefer includes that were discovered early to avoid
+  /// getting caught in results from error recovery.
   std::set<std::string> Untried;
+  std::vector<std::string> UntriedList;
 
   /// Whether we should use the smallest possible include path.
   bool MinimizeIncludePaths = true;

Modified: clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp?rev=269028&r1=269027&r2=269028&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp (original)
+++ clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp Tue May 10 03:25:28 2016
@@ -49,6 +49,7 @@ static std::string runIncludeFixer(
     const std::vector<std::string> &ExtraArgs = std::vector<std::string>()) {
   std::map<std::string, std::vector<std::string>> XrefsMap = {
       {"std::string", {"<string>"}},
+      {"std::sting", {"\"sting\""}},
       {"std::string::size_type", {"<string>"}},
       {"a::b::foo", {"dir/otherdir/qux.h"}},
   };
@@ -114,6 +115,11 @@ TEST(IncludeFixer, MinimizeInclude) {
             runIncludeFixer("a::b::foo bar;\n", IncludePath));
 }
 
+TEST(IncludeFixer, MultipleMissingSymbols) {
+  EXPECT_EQ("#include <string>\nstd::string bar;\nstd::sting foo;\n",
+            runIncludeFixer("std::string bar;\nstd::sting foo;\n"));
+}
+
 } // namespace
 } // namespace include_fixer
 } // namespace clang