diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/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)); } diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/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)); }; diff --git a/clang-tools-extra/clangd/SourceCode.h b/clang-tools-extra/clangd/SourceCode.h --- a/clang-tools-extra/clangd/SourceCode.h +++ b/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 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 ContentDigests; + tooling::Replacements Reps; + llvm::Optional> 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 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 &ApplyEdits, + llvm::StringRef MainFilePath, + llvm::StringRef MainFileCode); + /// Collects identifiers with counts in the source code. llvm::StringMap collectIdentifiers(llvm::StringRef Content, const format::FormatStyle &Style); diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp --- a/clang-tools-extra/clangd/SourceCode.cpp +++ b/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 @@ -567,6 +572,45 @@ return formatReplacements(Code, std::move(*CleanReplaces), Style); } +llvm::Error formatAndGenerateEdits(llvm::vfs::FileSystem *FS, + llvm::StringMap &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 static void lex(llvm::StringRef Code, const format::FormatStyle &Style, Action A) { diff --git a/clang-tools-extra/clangd/refactor/Tweak.h b/clang-tools-extra/clangd/refactor/Tweak.h --- a/clang-tools-extra/clangd/refactor/Tweak.h +++ b/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 namespace clang { namespace clangd { @@ -67,12 +70,18 @@ struct Effect { /// A message to be displayed to the user. llvm::Optional ShowMessage; - /// An edit to apply to the input file. - llvm::Optional ApplyEdit; + /// A mapping from absolute file path to edits. + llvm::Optional> 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) { diff --git a/clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp b/clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp +++ b/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 diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp +++ b/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, diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp @@ -104,7 +104,7 @@ } Expected 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 { diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp +++ b/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 diff --git a/clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp b/clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/RawStringLiteral.cpp @@ -88,10 +88,13 @@ } Expected 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 diff --git a/clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp b/clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp +++ b/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 diff --git a/clang-tools-extra/clangd/unittests/TweakTesting.cpp b/clang-tools-extra/clangd/unittests/TweakTesting.cpp --- a/clang-tools-extra/clangd/unittests/TweakTesting.cpp +++ b/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()); diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp --- a/clang-tools-extra/clangd/unittests/TweakTests.cpp +++ b/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,