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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
