[clang-tools-extra] r326773 - [clangd] Sort includes when formatting code or inserting new includes.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Tue Mar  6 02:42:50 2018
New Revision: 326773

URL: http://llvm.org/viewvc/llvm-project?rev=326773&view=rev
Log:
[clangd] Sort includes when formatting code or inserting new includes.

Reviewers: hokein, ilya-biryukov

Subscribers: klimek, jkorous-apple, cfe-commits

Differential Revision: https://reviews.llvm.org/D44138

Modified:
    clang-tools-extra/trunk/clangd/ClangdServer.cpp
    clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=326773&r1=326772&r2=326773&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Mar  6 02:42:50 2018
@@ -353,8 +353,11 @@ ClangdServer::insertInclude(PathRef File
   // Replacement with offset UINT_MAX and length 0 will be treated as include
   // insertion.
   tooling::Replacement R(File, /*Offset=*/UINT_MAX, 0, "#include " + ToInclude);
-  return format::cleanupAroundReplacements(Code, tooling::Replacements(R),
-                                           *Style);
+  auto Replaces = format::cleanupAroundReplacements(
+      Code, tooling::Replacements(R), *Style);
+  if (!Replaces)
+    return Replaces;
+  return formatReplacements(Code, *Replaces, *Style);
 }
 
 llvm::Optional<std::string> ClangdServer::getDocument(PathRef File) {
@@ -465,13 +468,21 @@ ClangdServer::formatCode(llvm::StringRef
                          ArrayRef<tooling::Range> Ranges) {
   // Call clang-format.
   auto TaggedFS = FSProvider.getTaggedFileSystem(File);
-  auto StyleOrError =
+  auto Style =
       format::getStyle("file", File, "LLVM", Code, TaggedFS.Value.get());
-  if (!StyleOrError) {
-    return StyleOrError.takeError();
-  } else {
-    return format::reformat(StyleOrError.get(), Code, Ranges, File);
-  }
+  if (!Style)
+    return Style.takeError();
+
+  tooling::Replacements IncludeReplaces =
+      format::sortIncludes(*Style, Code, Ranges, File);
+  auto Changed = tooling::applyAllReplacements(Code, IncludeReplaces);
+  if (!Changed)
+    return Changed.takeError();
+
+  return IncludeReplaces.merge(format::reformat(
+      Style.get(), *Changed,
+      tooling::calculateRangesAfterReplacements(IncludeReplaces, Ranges),
+      File));
 }
 
 void ClangdServer::findDocumentHighlights(

Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp?rev=326773&r1=326772&r2=326773&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Tue Mar  6 02:42:50 2018
@@ -945,6 +945,7 @@ TEST_F(ClangdVFSTest, InsertIncludes) {
 
   auto FooCpp = testPath("foo.cpp");
   const auto Code = R"cpp(
+#include "z.h"
 #include "x.h"
 
 void f() {}
@@ -952,15 +953,18 @@ void f() {}
   FS.Files[FooCpp] = Code;
   runAddDocument(Server, FooCpp, Code);
 
-  auto Inserted = [&](llvm::StringRef Original, llvm::StringRef Preferred,
-                      llvm::StringRef Expected) {
+  auto ChangedCode = [&](llvm::StringRef Original, llvm::StringRef Preferred) {
     auto Replaces = Server.insertInclude(
         FooCpp, Code, Original, Preferred.empty() ? Original : Preferred);
     EXPECT_TRUE(static_cast<bool>(Replaces));
     auto Changed = tooling::applyAllReplacements(Code, *Replaces);
     EXPECT_TRUE(static_cast<bool>(Changed));
-    return llvm::StringRef(*Changed).contains(
-        (llvm::Twine("#include ") + Expected + "").str());
+    return *Changed;
+  };
+  auto Inserted = [&](llvm::StringRef Original, llvm::StringRef Preferred,
+                      llvm::StringRef Expected) {
+    return llvm::StringRef(ChangedCode(Original, Preferred))
+        .contains((llvm::Twine("#include ") + Expected + "").str());
   };
 
   EXPECT_TRUE(Inserted("\"y.h\"", /*Preferred=*/"","\"y.h\""));
@@ -976,6 +980,45 @@ void f() {}
                        /*Preferred=*/"<Y.h>", "<Y.h>"));
   EXPECT_TRUE(Inserted(OriginalHeader, PreferredHeader, "\"Y.h\""));
   EXPECT_TRUE(Inserted("<y.h>", PreferredHeader, "\"Y.h\""));
+
+  // Check that includes are sorted.
+  const auto Expected = R"cpp(
+#include "x.h"
+#include "y.h"
+#include "z.h"
+
+void f() {}
+)cpp";
+  EXPECT_EQ(Expected, ChangedCode("\"y.h\"", /*Preferred=*/""));
+}
+
+TEST_F(ClangdVFSTest, FormatCode) {
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  MockCompilationDatabase CDB;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  auto Path = testPath("foo.cpp");
+  std::string Code = R"cpp(
+#include "y.h"
+#include "x.h"
+
+void f(  )  {}
+)cpp";
+  std::string Expected = R"cpp(
+#include "x.h"
+#include "y.h"
+
+void f() {}
+)cpp";
+  FS.Files[Path] = Code;
+  runAddDocument(Server, Path, Code);
+
+  auto Replaces = Server.formatFile(Code, Path);
+  EXPECT_TRUE(static_cast<bool>(Replaces));
+  auto Changed = tooling::applyAllReplacements(Code, *Replaces);
+  EXPECT_TRUE(static_cast<bool>(Changed));
+  EXPECT_EQ(Expected, *Changed);
 }
 
 } // namespace