kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
First patch for propogating multifile changes from tweak outputs to LSP
WorkspaceEdits.
Uses FS to convert tooling::Replacements to TextEdits except the MainFile.
Errors out if there are any inconsistencies between the draft version and the
version generated the edits.
We exempt MainFile from this check to not regress existing use cases, since
clangd can refactor the mainfile even if there are unsaved changes in the
editor. But for other files it will make use of on-disk state rather than
on-editor state.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D66637
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/refactor/Tweak.h
clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp
clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
clang-tools-extra/clangd/unittests/TweakTesting.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp
Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -113,11 +113,15 @@
auto Effect = apply(ID, Input);
if (!Effect)
return Effect.takeError();
- if (!Effect->ApplyEdit)
+ if (!Effect->ApplyEdits)
return llvm::createStringError(llvm::inconvertibleErrorCode(),
"No replacements");
+ auto Edits = *Effect->ApplyEdits;
+ if (Edits.size() > 1)
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Multi-file edits");
Annotations Code(Input);
- return applyAllReplacements(Code.code(), *Effect->ApplyEdit);
+ return applyAllReplacements(Code.code(), Edits.begin()->second.Reps);
}
void checkTransform(llvm::StringRef ID, llvm::StringRef Input,
Index: clang-tools-extra/clangd/unittests/TweakTesting.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/TweakTesting.cpp
@@ -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,9 +98,12 @@
return "fail: " + llvm::toString(Result.takeError());
if (Result->ShowMessage)
return "message:\n" + *Result->ShowMessage;
- if (Result->ApplyEdit) {
+ if (Result->ApplyEdits) {
+ if (Result->ApplyEdits->size() > 1)
+ return "received multi-file edits";
+ auto ApplyEdit = Result->ApplyEdits->begin()->second;
if (auto NewText =
- tooling::applyAllReplacements(Input.code(), *Result->ApplyEdit))
+ tooling::applyAllReplacements(Input.code(), ApplyEdit.Reps))
return unwrap(Context, *NewText);
else
return "bad edits: " + llvm::toString(NewText.takeError());
Index: clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
@@ -90,7 +90,8 @@
ElseRng->getBegin(),
ElseCode.size(), ThenCode)))
return std::move(Err);
- return Effect::applyEdit(Result);
+ return Effect::applyEdit(SrcMgr.getFilename(Inputs.Cursor),
+ Edit::generateEdit(Inputs.Code, std::move(Result)));
}
} // namespace
Index: clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp
@@ -88,10 +88,13 @@
}
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::applyEdit(SM.getFilename(Inputs.Cursor),
+ Edit::generateEdit(Inputs.Code, std::move(Reps)));
}
} // namespace
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp
@@ -468,7 +468,9 @@
// replace expression with variable name
if (auto Err = Result.add(Target->replaceWithVar(Range, VarName)))
return std::move(Err);
- return Effect::applyEdit(Result);
+ return Effect::applyEdit(
+ Inputs.AST.getSourceManager().getFilename(Inputs.Cursor),
+ Edit::generateEdit(Inputs.Code, std::move(Result)));
}
} // namespace
Index: clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
@@ -104,7 +104,7 @@
}
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,10 @@
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::applyEdit(SM.getFilename(Inputs.Cursor),
+ Edit::generateEdit(Inputs.Code, std::move(Reps)));
}
std::string ExpandMacro::title() const {
Index: clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
@@ -101,7 +101,9 @@
Expansion(SrcMgr, CharSourceRange(CachedLocation->getSourceRange(), true),
PrettyTypeName);
- return Tweak::Effect::applyEdit(tooling::Replacements(Expansion));
+ return Tweak::Effect::applyEdit(
+ SrcMgr.getFilename(Inputs.Cursor),
+ Edit::generateEdit(Inputs.Code, tooling::Replacements(Expansion)));
}
llvm::Error ExpandAutoType::createErrorMessage(const std::string& Message,
Index: clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "SemanticHighlighting.h"
#include "refactor/Tweak.h"
+#include "llvm/ADT/StringRef.h"
namespace clang {
namespace clangd {
@@ -56,6 +57,7 @@
}
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,13 @@
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::applyEdit(FilePath,
+ Edit::generateEdit(Inputs.Code, std::move(Result)));
}
} // namespace
Index: clang-tools-extra/clangd/refactor/Tweak.h
===================================================================
--- clang-tools-extra/clangd/refactor/Tweak.h
+++ clang-tools-extra/clangd/refactor/Tweak.h
@@ -24,7 +24,10 @@
#include "Selection.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/SHA1.h"
+#include <array>
namespace clang {
namespace clangd {
@@ -67,12 +70,18 @@
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 absolute file path to edits.
+ llvm::Optional<llvm::StringMap<Edit>> ApplyEdits;
- static Effect applyEdit(tooling::Replacements R) {
+ void addEdit(StringRef FilePath, Edit Ed) {
+ assert(ApplyEdits);
+ ApplyEdits->try_emplace(FilePath, std::move(Ed));
+ }
+
+ static Effect applyEdit(StringRef FilePath, Edit Ed) {
Effect E;
- E.ApplyEdit = std::move(R);
+ E.ApplyEdits.emplace();
+ E.addEdit(FilePath, std::move(Ed));
return E;
}
static Effect showMessage(StringRef S) {
Index: clang-tools-extra/clangd/SourceCode.h
===================================================================
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -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"
@@ -36,6 +37,24 @@
FileDigest digest(StringRef Content);
Optional<FileDigest> digestFile(const SourceManager &SM, FileID FID);
+/// Represents a set of edits generated for a single file.
+struct Edit {
+ /// SHA1 hash of the file contents for the edits generated below. Clients
+ /// should verify contents they see are the same as this one before applying
+ /// edits.
+ std::array<uint8_t, 20> ContentDigests;
+ tooling::Replacements Reps;
+ llvm::Optional<std::vector<TextEdit>> Edits;
+
+ static Edit generateEdit(llvm::StringRef Code, tooling::Replacements Reps) {
+ Edit E;
+ E.Reps = std::move(Reps);
+ E.ContentDigests = llvm::SHA1::hash(
+ llvm::makeArrayRef(Code.bytes_begin(), Code.bytes_end()));
+ return E;
+ }
+};
+
// This context variable controls the behavior of functions in this file
// that convert between LSP offsets and native clang byte offsets.
// If not set, defaults to UTF-16 for backwards-compatibility.
@@ -184,11 +203,20 @@
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);
+/// Formats the edits in \p ApplyEdits and generates TextEdits from those.
+/// Ensures the files in FS are consistent with the files used while generating
+/// edits except the main file. For the mainfile it chechs the dirty versions
+/// are the same.
+llvm::Error formatAndGenerateEdits(llvm::vfs::FileSystem *FS,
+ llvm::StringMap<Edit> &ApplyEdits,
+ llvm::StringRef MainFilePath,
+ llvm::StringRef MainFileCode);
+
/// Collects identifiers with counts in the source code.
llvm::StringMap<unsigned> collectIdentifiers(llvm::StringRef Content,
const format::FormatStyle &Style);
Index: clang-tools-extra/clangd/SourceCode.cpp
===================================================================
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -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,18 @@
#include "clang/Format/Format.h"
#include "clang/Lex/Lexer.h"
#include "clang/Lex/Preprocessor.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/Path.h"
+#include "llvm/Support/SHA1.h"
+#include "llvm/Support/VirtualFileSystem.h"
#include "llvm/Support/xxhash.h"
#include <algorithm>
@@ -567,6 +572,45 @@
return formatReplacements(Code, std::move(*CleanReplaces), Style);
}
+llvm::Error formatAndGenerateEdits(llvm::vfs::FileSystem *FS,
+ llvm::StringMap<Edit> &ApplyEdits,
+ llvm::StringRef MainFilePath,
+ llvm::StringRef MainFileCode) {
+ for (auto &It : ApplyEdits) {
+ llvm::StringRef FilePath = It.first();
+ Edit &ApplyEdit = It.second;
+
+ llvm::StringRef Contents;
+ if (FilePath == MainFilePath) {
+ Contents = MainFileCode;
+ } else {
+ auto Buffer = FS->getBufferForFile(FilePath);
+ if (!Buffer)
+ return llvm::createStringError(
+ Buffer.getError(), "Couldn't open file %s for generating edits",
+ FilePath.data());
+ Contents = Buffer.get()->getBuffer();
+ }
+
+ if (ApplyEdit.ContentDigests !=
+ llvm::SHA1::hash(
+ llvm::makeArrayRef(Contents.bytes_begin(), Contents.size()))) {
+ return llvm::createStringError(
+ llvm::inconvertibleErrorCode(),
+ "File contents differ on disk for %s, please save", FilePath.data());
+ }
+ // FIXME: this function has I/O operations (find .clang-format file),
+ // figure out a way to cache the format style.
+ auto Style = getFormatStyleForFile(FilePath, Contents, FS);
+ if (auto NewEdits = cleanupAndFormat(Contents, ApplyEdit.Reps, Style))
+ ApplyEdit.Reps = std::move(*NewEdits);
+ else
+ elog("Failed to format reps: {0}", NewEdits.takeError());
+ ApplyEdit.Edits = replacementsToEdits(Contents, ApplyEdit.Reps);
+ }
+ return llvm::Error::success();
+}
+
template <typename Action>
static void lex(llvm::StringRef Code, const format::FormatStyle &Style,
Action A) {
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -404,16 +404,11 @@
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());
+ if (Effect->ApplyEdits) {
+ if (llvm::Error Err = formatAndGenerateEdits(InpAST->Inputs.FS.get(),
+ *Effect->ApplyEdits, File,
+ InpAST->Inputs.Contents))
+ return CB(std::move(Err));
}
return CB(std::move(*Effect));
};
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -24,6 +24,7 @@
#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"
namespace clang {
@@ -627,7 +628,7 @@
if (!R)
return Reply(R.takeError());
- assert(R->ShowMessage || (R->ApplyEdit && "tweak has no effect"));
+ assert(R->ShowMessage || (R->ApplyEdits && "tweak has no effect"));
if (R->ShowMessage) {
ShowMessageParams Msg;
@@ -635,10 +636,21 @@
Msg.type = MessageType::Info;
notify("window/showMessage", Msg);
}
- if (R->ApplyEdit) {
+ if (R->ApplyEdits) {
WorkspaceEdit WE;
WE.changes.emplace();
- (*WE.changes)[File.uri()] = replacementsToEdits(Code, *R->ApplyEdit);
+ for (const auto &It : *R->ApplyEdits) {
+ assert(It.second.Edits && "TextEdits hasn't been generated?");
+ if (auto Draft = DraftMgr.getDraft(It.first())) {
+ llvm::StringRef Contents = *Draft;
+ if (llvm::SHA1::hash(llvm::makeArrayRef(Contents.bytes_begin(),
+ Contents.size())) !=
+ It.second.ContentDigests)
+ return Reply((It.first() + " needs to be saved").str());
+ }
+ (*WE.changes)[URI::create(It.first()).toString()] =
+ std::move(*It.second.Edits);
+ }
// ApplyEdit will take care of calling Reply().
return ApplyEdit(std::move(WE), "Tweak applied.", std::move(Reply));
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
