[clangd] Support multifile edits as output of Tweaks

Summary:
First patch for propogating multifile changes from tweak outputs to LSP
WorkspaceEdits.

Uses SM to convert tooling::Replacements to TextEdits.
Errors out if there are any inconsistencies between the draft version and the
version generated the edits.

Reviewers: sammccall

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits

Tags: #clang

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

llvm-svn: 371392
This commit is contained in:
Kadir Cetinkaya 2019-09-09 12:28:44 +00:00
parent 7c5697c8b2
commit 5b270932cc
15 changed files with 271 additions and 68 deletions

View File

@ -8,6 +8,7 @@
#include "ClangdLSPServer.h"
#include "Diagnostics.h"
#include "DraftStore.h"
#include "FormattedString.h"
#include "GlobalCompilationDatabase.h"
#include "Protocol.h"
@ -20,11 +21,15 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/SHA1.h"
#include "llvm/Support/ScopedPrinter.h"
#include <cstddef>
#include <string>
namespace clang {
namespace clangd {
@ -93,6 +98,35 @@ std::vector<std::vector<std::string>> buildHighlightScopeLookupTable() {
return LookupTable;
}
// Makes sure edits in \p E are applicable to latest file contents reported by
// editor. If not generates an error message containing information about files
// that needs to be saved.
llvm::Error validateEdits(const DraftStore &DraftMgr, const Tweak::Effect &E) {
size_t InvalidFileCount = 0;
llvm::StringRef LastInvalidFile;
for (const auto &It : E.ApplyEdits) {
if (auto Draft = DraftMgr.getDraft(It.first())) {
// If the file is open in user's editor, make sure the version we
// saw and current version are compatible as this is the text that
// will be replaced by editors.
if (!It.second.canApplyTo(*Draft)) {
++InvalidFileCount;
LastInvalidFile = It.first();
}
}
}
if (!InvalidFileCount)
return llvm::Error::success();
if (InvalidFileCount == 1)
return llvm::createStringError(llvm::inconvertibleErrorCode(),
"File must be saved first: " +
LastInvalidFile);
return llvm::createStringError(
llvm::inconvertibleErrorCode(),
"Files must be saved first: " + LastInvalidFile + " (and " +
llvm::to_string(InvalidFileCount - 1) + " others)");
}
} // namespace
// MessageHandler dispatches incoming LSP messages.
@ -627,7 +661,8 @@ void ClangdLSPServer::onCommand(const ExecuteCommandParams &Params,
if (!R)
return Reply(R.takeError());
assert(R->ShowMessage || (R->ApplyEdit && "tweak has no effect"));
assert(R->ShowMessage ||
(!R->ApplyEdits.empty() && "tweak has no effect"));
if (R->ShowMessage) {
ShowMessageParams Msg;
@ -635,15 +670,21 @@ void ClangdLSPServer::onCommand(const ExecuteCommandParams &Params,
Msg.type = MessageType::Info;
notify("window/showMessage", Msg);
}
if (R->ApplyEdit) {
WorkspaceEdit WE;
WE.changes.emplace();
(*WE.changes)[File.uri()] = replacementsToEdits(Code, *R->ApplyEdit);
// ApplyEdit will take care of calling Reply().
return ApplyEdit(std::move(WE), "Tweak applied.", std::move(Reply));
}
// When no edit is specified, make sure we Reply().
return Reply("Tweak applied.");
if (R->ApplyEdits.empty())
return Reply("Tweak applied.");
if (auto Err = validateEdits(DraftMgr, *R))
return Reply(std::move(Err));
WorkspaceEdit WE;
WE.changes.emplace();
for (const auto &It : R->ApplyEdits) {
(*WE.changes)[URI::create(It.first()).toString()] =
It.second.asTextEdits();
}
// ApplyEdit will take care of calling Reply().
return ApplyEdit(std::move(WE), "Tweak applied.", std::move(Reply));
};
Server->applyTweak(Params.tweakArgs->file.file(),
Params.tweakArgs->selection, Params.tweakArgs->tweakID,

View File

@ -12,6 +12,7 @@
#include "Format.h"
#include "FormattedString.h"
#include "Headers.h"
#include "Logger.h"
#include "ParsedAST.h"
#include "Preamble.h"
#include "Protocol.h"
@ -388,32 +389,29 @@ void ClangdServer::enumerateTweaks(PathRef File, Range Sel,
void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
Callback<Tweak::Effect> CB) {
auto Action = [File = File.str(), Sel, TweakID = TweakID.str(),
CB = std::move(CB)](Expected<InputsAndAST> InpAST) mutable {
if (!InpAST)
return CB(InpAST.takeError());
auto Selection = tweakSelection(Sel, *InpAST);
if (!Selection)
return CB(Selection.takeError());
auto A = prepareTweak(TweakID, *Selection);
if (!A)
return CB(A.takeError());
auto Effect = (*A)->apply(*Selection);
if (!Effect)
return CB(Effect.takeError());
if (Effect->ApplyEdit) {
// 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());
if (auto Formatted = cleanupAndFormat(InpAST->Inputs.Contents,
*Effect->ApplyEdit, Style))
Effect->ApplyEdit = std::move(*Formatted);
else
elog("Failed to format replacements: {0}", Formatted.takeError());
}
return CB(std::move(*Effect));
};
auto Action =
[File = File.str(), Sel, TweakID = TweakID.str(), CB = std::move(CB),
FS = FSProvider.getFileSystem()](Expected<InputsAndAST> InpAST) mutable {
if (!InpAST)
return CB(InpAST.takeError());
auto Selection = tweakSelection(Sel, *InpAST);
if (!Selection)
return CB(Selection.takeError());
auto A = prepareTweak(TweakID, *Selection);
if (!A)
return CB(A.takeError());
auto Effect = (*A)->apply(*Selection);
if (!Effect)
return CB(Effect.takeError());
for (auto &It : Effect->ApplyEdits) {
Edit &E = It.second;
format::FormatStyle Style =
getFormatStyleForFile(File, E.InitialCode, FS.get());
if (llvm::Error Err = reformatEdit(E, Style))
elog("Failed to format {0}: {1}", It.first(), std::move(Err));
}
return CB(std::move(*Effect));
};
WorkScheduler.runWithAST("ApplyTweak", File, std::move(Action));
}

View File

@ -11,6 +11,7 @@
#include "FuzzyMatch.h"
#include "Logger.h"
#include "Protocol.h"
#include "refactor/Tweak.h"
#include "clang/AST/ASTContext.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
@ -19,14 +20,21 @@
#include "clang/Format/Format.h"
#include "clang/Lex/Lexer.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/Core/Replacement.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/None.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/LineIterator.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/SHA1.h"
#include "llvm/Support/VirtualFileSystem.h"
#include "llvm/Support/xxhash.h"
#include <algorithm>
@ -562,7 +570,7 @@ llvm::Optional<std::string> getCanonicalPath(const FileEntry *F,
}
// Handle the symbolic link path case where the current working directory
// (getCurrentWorkingDirectory) is a symlink./ We always want to the real
// (getCurrentWorkingDirectory) is a symlink. We always want to the real
// file path (instead of the symlink path) for the C++ symbols.
//
// Consider the following example:
@ -908,5 +916,57 @@ llvm::Optional<DefinedMacro> locateMacroAt(SourceLocation Loc,
return None;
}
llvm::Expected<std::string> Edit::apply() const {
return tooling::applyAllReplacements(InitialCode, Replacements);
}
std::vector<TextEdit> Edit::asTextEdits() const {
return replacementsToEdits(InitialCode, Replacements);
}
bool Edit::canApplyTo(llvm::StringRef Code) const {
// Create line iterators, since line numbers are important while applying our
// edit we cannot skip blank lines.
auto LHS = llvm::MemoryBuffer::getMemBuffer(Code);
llvm::line_iterator LHSIt(*LHS, /*SkipBlanks=*/false);
auto RHS = llvm::MemoryBuffer::getMemBuffer(InitialCode);
llvm::line_iterator RHSIt(*RHS, /*SkipBlanks=*/false);
// Compare the InitialCode we prepared the edit for with the Code we received
// line by line to make sure there are no differences.
// FIXME: This check is too conservative now, it should be enough to only
// check lines around the replacements contained inside the Edit.
while (!LHSIt.is_at_eof() && !RHSIt.is_at_eof()) {
if (*LHSIt != *RHSIt)
return false;
++LHSIt;
++RHSIt;
}
// After we reach EOF for any of the files we make sure the other one doesn't
// contain any additional content except empty lines, they should not
// interfere with the edit we produced.
while (!LHSIt.is_at_eof()) {
if (!LHSIt->empty())
return false;
++LHSIt;
}
while (!RHSIt.is_at_eof()) {
if (!RHSIt->empty())
return false;
++RHSIt;
}
return true;
}
llvm::Error reformatEdit(Edit &E, const format::FormatStyle &Style) {
if (auto NewEdits = cleanupAndFormat(E.InitialCode, E.Replacements, Style))
E.Replacements = std::move(*NewEdits);
else
return NewEdits.takeError();
return llvm::Error::success();
}
} // namespace clangd
} // namespace clang

View File

@ -12,6 +12,7 @@
//===----------------------------------------------------------------------===//
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H
#include "Context.h"
#include "Protocol.h"
#include "clang/Basic/Diagnostic.h"
@ -22,7 +23,9 @@
#include "clang/Tooling/Core/Replacement.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSet.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/SHA1.h"
#include <string>
namespace clang {
class SourceManager;
@ -197,11 +200,34 @@ format::FormatStyle getFormatStyleForFile(llvm::StringRef File,
llvm::StringRef Content,
llvm::vfs::FileSystem *FS);
// Cleanup and format the given replacements.
/// Cleanup and format the given replacements.
llvm::Expected<tooling::Replacements>
cleanupAndFormat(StringRef Code, const tooling::Replacements &Replaces,
const format::FormatStyle &Style);
/// A set of edits generated for a single file. Can verify whether it is safe to
/// apply these edits to a code block.
struct Edit {
tooling::Replacements Replacements;
std::string InitialCode;
Edit(llvm::StringRef Code, tooling::Replacements Reps)
: Replacements(std::move(Reps)), InitialCode(Code) {}
/// Returns the file contents after changes are applied.
llvm::Expected<std::string> apply() const;
/// Represents Replacements as TextEdits that are available for use in LSP.
std::vector<TextEdit> asTextEdits() const;
/// Checks whether the Replacements are applicable to given Code.
bool canApplyTo(llvm::StringRef Code) const;
};
/// Formats the edits and code around it according to Style. Changes
/// Replacements to formatted ones if succeeds.
llvm::Error reformatEdit(Edit &E, const format::FormatStyle &Style);
/// Collects identifiers with counts in the source code.
llvm::StringMap<unsigned> collectIdentifiers(llvm::StringRef Content,
const format::FormatStyle &Style);

View File

@ -7,12 +7,18 @@
//===----------------------------------------------------------------------===//
#include "Tweak.h"
#include "Logger.h"
#include "Path.h"
#include "SourceCode.h"
#include "llvm/ADT/None.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/Registry.h"
#include <functional>
#include <memory>
#include <utility>
LLVM_INSTANTIATE_REGISTRY(llvm::Registry<clang::clangd::Tweak>)
@ -81,5 +87,28 @@ llvm::Expected<std::unique_ptr<Tweak>> prepareTweak(StringRef ID,
return std::move(T);
}
llvm::Expected<std::pair<Path, Edit>>
Tweak::Effect::fileEdit(const SourceManager &SM, FileID FID,
tooling::Replacements Replacements) {
Edit Ed(SM.getBufferData(FID), std::move(Replacements));
if (auto FilePath = getCanonicalPath(SM.getFileEntryForID(FID), SM))
return std::make_pair(*FilePath, std::move(Ed));
return llvm::createStringError(
llvm::inconvertibleErrorCode(),
"Failed to get absolute path for edited file: " +
SM.getFileEntryForID(FID)->getName());
}
llvm::Expected<Tweak::Effect>
Tweak::Effect::mainFileEdit(const SourceManager &SM,
tooling::Replacements Replacements) {
auto PathAndEdit = fileEdit(SM, SM.getMainFileID(), std::move(Replacements));
if (!PathAndEdit)
return PathAndEdit.takeError();
Tweak::Effect E;
E.ApplyEdits.try_emplace(PathAndEdit->first, PathAndEdit->second);
return E;
}
} // namespace clangd
} // namespace clang

View File

@ -20,11 +20,17 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_TWEAK_H
#include "ParsedAST.h"
#include "Path.h"
#include "Protocol.h"
#include "Selection.h"
#include "SourceCode.h"
#include "clang/Tooling/Core/Replacement.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include <string>
namespace clang {
namespace clangd {
@ -67,19 +73,27 @@ public:
struct Effect {
/// A message to be displayed to the user.
llvm::Optional<std::string> ShowMessage;
/// An edit to apply to the input file.
llvm::Optional<tooling::Replacements> ApplyEdit;
/// A mapping from file path(the one used for accessing the underlying VFS)
/// to edits.
llvm::StringMap<Edit> ApplyEdits;
static Effect applyEdit(tooling::Replacements R) {
Effect E;
E.ApplyEdit = std::move(R);
return E;
}
static Effect showMessage(StringRef S) {
Effect E;
E.ShowMessage = S;
return E;
}
/// Path is the absolute, symlink-resolved path for the file pointed by FID
/// in SM. Edit is generated from Replacements.
/// Fails if cannot figure out absolute path for FID.
static llvm::Expected<std::pair<Path, Edit>>
fileEdit(const SourceManager &SM, FileID FID,
tooling::Replacements Replacements);
/// Creates an effect with an Edit for the main file.
/// Fails if cannot figure out absolute path for main file.
static llvm::Expected<Tweak::Effect>
mainFileEdit(const SourceManager &SM, tooling::Replacements Replacements);
};
virtual ~Tweak() = default;
@ -126,7 +140,6 @@ prepareTweaks(const Tweak::Selection &S,
// If prepare() returns true, returns the corresponding tweak.
llvm::Expected<std::unique_ptr<Tweak>> prepareTweak(StringRef TweakID,
const Tweak::Selection &S);
} // namespace clangd
} // namespace clang

View File

@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "SemanticHighlighting.h"
#include "refactor/Tweak.h"
#include "llvm/ADT/StringRef.h"
namespace clang {
namespace clangd {
@ -56,6 +57,7 @@ Expected<Tweak::Effect> AnnotateHighlightings::apply(const Selection &Inputs) {
}
auto &SM = Inputs.AST.getSourceManager();
tooling::Replacements Result;
llvm::StringRef FilePath = SM.getFilename(Inputs.Cursor);
for (const auto &Token : HighlightingTokens) {
assert(Token.R.start.line == Token.R.end.line &&
"Token must be at the same line");
@ -64,12 +66,12 @@ Expected<Tweak::Effect> AnnotateHighlightings::apply(const Selection &Inputs) {
return InsertOffset.takeError();
auto InsertReplacement = tooling::Replacement(
SM.getFileEntryForID(SM.getMainFileID())->getName(), *InsertOffset, 0,
FilePath, *InsertOffset, 0,
("/* " + toTextMateScope(Token.Kind) + " */").str());
if (auto Err = Result.add(InsertReplacement))
return std::move(Err);
}
return Effect::applyEdit(Result);
return Effect::mainFileEdit(SM, std::move(Result));
}
} // namespace

View File

@ -101,7 +101,7 @@ Expected<Tweak::Effect> ExpandAutoType::apply(const Selection& Inputs) {
Expansion(SrcMgr, CharSourceRange(CachedLocation->getSourceRange(), true),
PrettyTypeName);
return Tweak::Effect::applyEdit(tooling::Replacements(Expansion));
return Effect::mainFileEdit(SrcMgr, tooling::Replacements(Expansion));
}
llvm::Error ExpandAutoType::createErrorMessage(const std::string& Message,

View File

@ -104,7 +104,7 @@ bool ExpandMacro::prepare(const Selection &Inputs) {
}
Expected<Tweak::Effect> ExpandMacro::apply(const Selection &Inputs) {
auto &SM = Inputs.AST.getASTContext().getSourceManager();
auto &SM = Inputs.AST.getSourceManager();
std::string Replacement;
for (const syntax::Token &T : Expansion.Expanded) {
@ -120,11 +120,9 @@ Expected<Tweak::Effect> ExpandMacro::apply(const Selection &Inputs) {
CharSourceRange::getCharRange(Expansion.Spelled.front().location(),
Expansion.Spelled.back().endLocation());
Tweak::Effect E;
E.ApplyEdit.emplace();
llvm::cantFail(
E.ApplyEdit->add(tooling::Replacement(SM, MacroRange, Replacement)));
return E;
tooling::Replacements Reps;
llvm::cantFail(Reps.add(tooling::Replacement(SM, MacroRange, Replacement)));
return Effect::mainFileEdit(SM, std::move(Reps));
}
std::string ExpandMacro::title() const {

View File

@ -645,7 +645,7 @@ Expected<Tweak::Effect> ExtractFunction::apply(const Selection &Inputs) {
return std::move(Err);
if (auto Err = Result.add(replaceWithFuncCall(*ExtractedFunc, SM, LangOpts)))
return std::move(Err);
return Effect::applyEdit(Result);
return Effect::mainFileEdit(SM, std::move(Result));
}
} // namespace

View File

@ -468,7 +468,7 @@ Expected<Tweak::Effect> ExtractVariable::apply(const Selection &Inputs) {
// replace expression with variable name
if (auto Err = Result.add(Target->replaceWithVar(Range, VarName)))
return std::move(Err);
return Effect::applyEdit(Result);
return Effect::mainFileEdit(Inputs.AST.getSourceManager(), std::move(Result));
}
} // namespace

View File

@ -88,10 +88,12 @@ bool RawStringLiteral::prepare(const Selection &Inputs) {
}
Expected<Tweak::Effect> RawStringLiteral::apply(const Selection &Inputs) {
return Effect::applyEdit(tooling::Replacements(
auto &SM = Inputs.AST.getSourceManager();
auto Reps = tooling::Replacements(
tooling::Replacement(Inputs.AST.getSourceManager(), Str,
("R\"(" + Str->getBytes() + ")\"").str(),
Inputs.AST.getASTContext().getLangOpts())));
Inputs.AST.getASTContext().getLangOpts()));
return Effect::mainFileEdit(SM, std::move(Reps));
}
} // namespace

View File

@ -90,7 +90,7 @@ Expected<Tweak::Effect> SwapIfBranches::apply(const Selection &Inputs) {
ElseRng->getBegin(),
ElseCode.size(), ThenCode)))
return std::move(Err);
return Effect::applyEdit(Result);
return Effect::mainFileEdit(SrcMgr, std::move(Result));
}
} // namespace

View File

@ -9,8 +9,8 @@
#include "TweakTesting.h"
#include "Annotations.h"
#include "refactor/Tweak.h"
#include "SourceCode.h"
#include "refactor/Tweak.h"
#include "clang/Tooling/Core/Replacement.h"
#include "llvm/Support/Error.h"
@ -98,14 +98,16 @@ std::string TweakTest::apply(llvm::StringRef MarkedCode) const {
return "fail: " + llvm::toString(Result.takeError());
if (Result->ShowMessage)
return "message:\n" + *Result->ShowMessage;
if (Result->ApplyEdit) {
if (auto NewText =
tooling::applyAllReplacements(Input.code(), *Result->ApplyEdit))
return unwrap(Context, *NewText);
else
return "bad edits: " + llvm::toString(NewText.takeError());
}
return "no effect";
if (Result->ApplyEdits.empty())
return "no effect";
if (Result->ApplyEdits.size() > 1)
return "received multi-file edits";
auto ApplyEdit = Result->ApplyEdits.begin()->second;
if (auto NewText = ApplyEdit.apply())
return unwrap(Context, *NewText);
else
return "bad edits: " + llvm::toString(NewText.takeError());
}
::testing::Matcher<llvm::StringRef> TweakTest::isAvailable() const {

View File

@ -8,15 +8,26 @@
#include "Annotations.h"
#include "SourceCode.h"
#include "TestFS.h"
#include "TestTU.h"
#include "TweakTesting.h"
#include "refactor/Tweak.h"
#include "clang/AST/Expr.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticIDs.h"
#include "clang/Basic/DiagnosticOptions.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/FileSystemOptions.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Rewrite/Core/Rewriter.h"
#include "clang/Tooling/Core/Replacement.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/VirtualFileSystem.h"
#include "llvm/Testing/Support/Error.h"
#include "gmock/gmock-matchers.h"
#include "gmock/gmock.h"
@ -31,6 +42,27 @@ namespace clang {
namespace clangd {
namespace {
TEST(FileEdits, AbsolutePath) {
auto RelPaths = {"a.h", "foo.cpp", "test/test.cpp"};
llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> MemFS(
new llvm::vfs::InMemoryFileSystem);
MemFS->setCurrentWorkingDirectory(testRoot());
for (auto Path : RelPaths)
MemFS->addFile(Path, 0, llvm::MemoryBuffer::getMemBuffer("", Path));
FileManager FM(FileSystemOptions(), MemFS);
DiagnosticsEngine DE(new DiagnosticIDs, new DiagnosticOptions);
SourceManager SM(DE, FM);
for (auto Path : RelPaths) {
auto FID = SM.createFileID(*FM.getFile(Path), SourceLocation(),
clang::SrcMgr::C_User);
auto Res = Tweak::Effect::fileEdit(SM, FID, tooling::Replacements());
ASSERT_THAT_EXPECTED(Res, llvm::Succeeded());
EXPECT_EQ(Res->first, testPath(Path));
}
}
TWEAK_TEST(SwapIfBranches);
TEST_F(SwapIfBranchesTest, Test) {
Context = Function;