Author: hokein Date: Mon Feb 11 07:18:11 2019 New Revision: 353712 URL: http://llvm.org/viewvc/llvm-project?rev=353712&view=rev Log: Revamp the "[clangd] Format tweak's replacements"
Summary: This patch contains two parts: 1) reverts commit r353306. 2) move the format logic out from tweaks, keep tweaks API unchanged. Reviewers: sammccall, ilya-biryukov Reviewed By: ilya-biryukov Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D58051 Added: clang-tools-extra/trunk/test/clangd/tweaks-format.test Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/ClangdUnit.cpp clang-tools-extra/trunk/clangd/Compiler.h clang-tools-extra/trunk/clangd/refactor/Tweak.cpp clang-tools-extra/trunk/clangd/refactor/Tweak.h clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=353712&r1=353711&r2=353712&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Mon Feb 11 07:18:11 2019 @@ -152,9 +152,6 @@ void ClangdServer::addDocument(PathRef F Opts.ClangTidyOpts = tidy::ClangTidyOptions::getDefaults(); if (ClangTidyOptProvider) Opts.ClangTidyOpts = ClangTidyOptProvider->getOptions(File); - // FIXME: cache this. - Opts.Style = - getFormatStyleForFile(File, Contents, FSProvider.getFileSystem().get()); Opts.SuggestMissingIncludes = SuggestMissingIncludes; // FIXME: some build systems like Bazel will take time to preparing // environment to build the file, it would be nice if we could emit a @@ -365,8 +362,9 @@ void ClangdServer::enumerateTweaks(PathR void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID, Callback<tooling::Replacements> CB) { - auto Action = [Sel](decltype(CB) CB, std::string File, std::string TweakID, - Expected<InputsAndAST> InpAST) { + auto Action = [Sel, this](decltype(CB) CB, std::string File, + std::string TweakID, + Expected<InputsAndAST> InpAST) { if (!InpAST) return CB(InpAST.takeError()); auto Selection = tweakSelection(Sel, *InpAST); @@ -375,7 +373,15 @@ void ClangdServer::applyTweak(PathRef Fi auto A = prepareTweak(TweakID, *Selection); if (!A) return CB(A.takeError()); - return CB((*A)->apply(*Selection, InpAST->Inputs.Opts.Style)); + auto RawReplacements = (*A)->apply(*Selection); + if (!RawReplacements) + return CB(RawReplacements.takeError()); + // FIXME: this function has I/O operations (find .clang-format file), figure + // out a way to cache the format style. + auto Style = getFormatStyleForFile(File, InpAST->Inputs.Contents, + InpAST->Inputs.FS.get()); + return CB( + cleanupAndFormat(InpAST->Inputs.Contents, *RawReplacements, Style)); }; WorkScheduler.runWithAST( "ApplyTweak", File, Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=353712&r1=353711&r2=353712&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Mon Feb 11 07:18:11 2019 @@ -308,8 +308,9 @@ ParsedAST::build(std::unique_ptr<Compile llvm::Optional<IncludeFixer> FixIncludes; auto BuildDir = VFS->getCurrentWorkingDirectory(); if (Opts.SuggestMissingIncludes && Index && !BuildDir.getError()) { + auto Style = getFormatStyleForFile(MainInput.getFile(), Content, VFS.get()); auto Inserter = std::make_shared<IncludeInserter>( - MainInput.getFile(), Content, Opts.Style, BuildDir.get(), + MainInput.getFile(), Content, Style, BuildDir.get(), Clang->getPreprocessor().getHeaderSearchInfo()); if (Preamble) { for (const auto &Inc : Preamble->Includes.MainFileIncludes) Modified: clang-tools-extra/trunk/clangd/Compiler.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Compiler.h?rev=353712&r1=353711&r2=353712&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Compiler.h (original) +++ clang-tools-extra/trunk/clangd/Compiler.h Mon Feb 11 07:18:11 2019 @@ -17,7 +17,6 @@ #include "../clang-tidy/ClangTidyOptions.h" #include "index/Index.h" -#include "clang/Format/Format.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/PrecompiledPreamble.h" @@ -39,7 +38,6 @@ public: struct ParseOptions { tidy::ClangTidyOptions ClangTidyOpts; bool SuggestMissingIncludes = false; - format::FormatStyle Style; }; /// Information required to run clang, e.g. to parse AST or do code completion. Modified: clang-tools-extra/trunk/clangd/refactor/Tweak.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/Tweak.cpp?rev=353712&r1=353711&r2=353712&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/refactor/Tweak.cpp (original) +++ clang-tools-extra/trunk/clangd/refactor/Tweak.cpp Mon Feb 11 07:18:11 2019 @@ -46,14 +46,6 @@ Tweak::Selection::Selection(ParsedAST &A Cursor = SM.getComposedLoc(SM.getMainFileID(), RangeBegin); } -Expected<tooling::Replacements> Tweak::apply(const Selection &Sel, - const format::FormatStyle &Style) { - auto RawReplacements = execute(Sel); - if (!RawReplacements) - return RawReplacements; - return cleanupAndFormat(Sel.Code, *RawReplacements, Style); -} - std::vector<std::unique_ptr<Tweak>> prepareTweaks(const Tweak::Selection &S) { validateRegistry(); Modified: clang-tools-extra/trunk/clangd/refactor/Tweak.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/Tweak.h?rev=353712&r1=353711&r2=353712&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/refactor/Tweak.h (original) +++ clang-tools-extra/trunk/clangd/refactor/Tweak.h Mon Feb 11 07:18:11 2019 @@ -22,7 +22,6 @@ #include "ClangdUnit.h" #include "Protocol.h" #include "Selection.h" -#include "clang/Format/Format.h" #include "clang/Tooling/Core/Replacement.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/StringRef.h" @@ -48,7 +47,7 @@ public: ParsedAST &AST; /// A location of the cursor in the editor. SourceLocation Cursor; - /// The AST nodes that were selected. + // The AST nodes that were selected. SelectionTree ASTSelection; // FIXME: provide a way to get sources and ASTs for other files. }; @@ -64,20 +63,13 @@ public: /// should be moved into 'apply'. /// Returns true iff the action is available and apply() can be called on it. virtual bool prepare(const Selection &Sel) = 0; - /// Format and apply the actual changes generated from the second stage of the - /// action. + /// Run the second stage of the action that would produce the actual changes. /// EXPECTS: prepare() was called and returned true. - Expected<tooling::Replacements> apply(const Selection &Sel, - const format::FormatStyle &Style); + virtual Expected<tooling::Replacements> apply(const Selection &Sel) = 0; /// A one-line title of the action that should be shown to the users in the /// UI. /// EXPECTS: prepare() was called and returned true. virtual std::string title() const = 0; - -protected: - /// Run the second stage of the action that would produce the actual changes. - /// EXPECTS: prepare() was called and returned true. - virtual Expected<tooling::Replacements> execute(const Selection &Sel) = 0; }; // All tweaks must be registered in the .cpp file next to their definition. Modified: clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp?rev=353712&r1=353711&r2=353712&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp (original) +++ clang-tools-extra/trunk/clangd/refactor/tweaks/SwapIfBranches.cpp Mon Feb 11 07:18:11 2019 @@ -37,11 +37,9 @@ public: const char *id() const override final; bool prepare(const Selection &Inputs) override; + Expected<tooling::Replacements> apply(const Selection &Inputs) override; std::string title() const override; -protected: - Expected<tooling::Replacements> execute(const Selection &Inputs) override; - private: const IfStmt *If = nullptr; }; @@ -62,8 +60,7 @@ bool SwapIfBranches::prepare(const Selec dyn_cast_or_null<CompoundStmt>(If->getElse()); } -Expected<tooling::Replacements> -SwapIfBranches::execute(const Selection &Inputs) { +Expected<tooling::Replacements> SwapIfBranches::apply(const Selection &Inputs) { auto &Ctx = Inputs.AST.getASTContext(); auto &SrcMgr = Ctx.getSourceManager(); Added: clang-tools-extra/trunk/test/clangd/tweaks-format.test URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/tweaks-format.test?rev=353712&view=auto ============================================================================== --- clang-tools-extra/trunk/test/clangd/tweaks-format.test (added) +++ clang-tools-extra/trunk/test/clangd/tweaks-format.test Mon Feb 11 07:18:11 2019 @@ -0,0 +1,50 @@ +# RUN: clangd -lit-test < %s | FileCheck %s +{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}} +--- +{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cc","languageId":"cpp","version":1,"text":"int f() { if (true) { return 1; } else {} }"}}} +--- +{"jsonrpc":"2.0","id":5,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///main.cc"},"range":{"start":{"line":0,"character":11},"end":{"line":0,"character":11}},"context":{"diagnostics":[]}}} +--- +{"jsonrpc":"2.0","id":6,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"test:///main.cc","selection":{"end":{"character":11,"line":0},"start":{"character":11,"line":0}},"tweakID":"SwapIfBranches"}]}} +# CHECK: "newText": "\n ", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 10, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 9, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: { +# CHECK-NEXT: "newText": "{\n }", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 33, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 20, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: }, +# CHECK-NEXT: { +# CHECK-NEXT: "newText": "{\n return 1;\n }\n", +# CHECK-NEXT: "range": { +# CHECK-NEXT: "end": { +# CHECK-NEXT: "character": 42, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: }, +# CHECK-NEXT: "start": { +# CHECK-NEXT: "character": 39, +# CHECK-NEXT: "line": 0 +# CHECK-NEXT: } +# CHECK-NEXT: } +# CHECK-NEXT: } +--- +{"jsonrpc":"2.0","id":3,"method":"shutdown"} +--- +{"jsonrpc":"2.0","method":"exit"} Modified: clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp?rev=353712&r1=353711&r2=353712&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/TweakTests.cpp Mon Feb 11 07:18:11 2019 @@ -98,7 +98,7 @@ llvm::Expected<std::string> apply(String auto T = prepareTweak(ID, S); if (!T) return T.takeError(); - auto Replacements = (*T)->apply(S, clang::format::getLLVMStyle()); + auto Replacements = (*T)->apply(S); if (!Replacements) return Replacements.takeError(); return applyAllReplacements(Code.code(), *Replacements); @@ -127,40 +127,12 @@ TEST(TweakTest, SwapIfBranches) { llvm::StringLiteral Input = R"cpp( void test() { - ^if (true) { - return 100; - } else { - continue; - } + ^if (true) { return 100; } else { continue; } } )cpp"; llvm::StringLiteral Output = R"cpp( void test() { - if (true) { - continue; - } else { - return 100; - } - } - )cpp"; - checkTransform(ID, Input, Output); - - Input = R"cpp( - void test() { - ^if () { - return 100; - } else { - continue; - } - } - )cpp"; - Output = R"cpp( - void test() { - if () { - continue; - } else { - return 100; - } + if (true) { continue; } else { return 100; } } )cpp"; checkTransform(ID, Input, Output); @@ -172,11 +144,7 @@ TEST(TweakTest, SwapIfBranches) { )cpp"; Output = R"cpp( void test() { - if () { - continue; - } else { - return 100; - } + if () { continue; } else { return 100; } } )cpp"; checkTransform(ID, Input, Output); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
