[clang-tools-extra] r353712 - Revamp the "[clangd] Format tweak's replacements"
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 11 07:18:12 PST 2019
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);
More information about the cfe-commits
mailing list