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,19 @@ 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) { + // If the file is open in user's editor, make sure the version we saw + // and current version are compatible. + if (auto Draft = DraftMgr.getDraft(It.first())) { + if (!It.second.canApplyTo(*Draft)) + return Reply((It.first() + " needs to be saved").str()); + } + (*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)); } 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" @@ -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 namespace clang { class SourceManager; @@ -36,6 +39,26 @@ FileDigest digest(StringRef Content); Optional digestFile(const SourceManager &SM, FileID FID); +/// Represents a set of edits generated for a single file. +struct Edit { + tooling::Replacements Replacements; + + Edit(llvm::StringRef Code, tooling::Replacements Reps) + : Replacements(std::move(Reps)), InitialCode(Code) {} + + /// Returns the file contents after changes are applied. + llvm::Expected apply() const; + + /// Represents Replacements as TextEdits that are available for use in LSP. + std::vector asTextEdits() const; + + /// Checks whether the Replacements are applicable to given Code. + bool canApplyTo(llvm::StringRef Code) const; + +private: + std::string InitialCode; +}; + // 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 +207,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,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 @@ -567,6 +575,43 @@ 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.canApplyTo(Contents)) { + 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.Replacements, Style)) + ApplyEdit.Replacements = std::move(*NewEdits); + else + elog("Failed to format reps: {0}", NewEdits.takeError()); + } + return llvm::Error::success(); +} + template static void lex(llvm::StringRef Code, const format::FormatStyle &Style, Action A) { @@ -837,5 +882,41 @@ return None; } +llvm::Expected Edit::apply() const { + return tooling::applyAllReplacements(InitialCode, Replacements); +} + +std::vector Edit::asTextEdits() const { + return replacementsToEdits(InitialCode, Replacements); +} + +bool Edit::canApplyTo(llvm::StringRef Code) const { + 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); + + while (!LHSIt.is_at_eof() && !RHSIt.is_at_eof()) { + if (*LHSIt != *RHSIt) + return false; + ++LHSIt; + ++RHSIt; + } + + // We only allow trailing empty lines. + while (!LHSIt.is_at_eof()) { + if (!LHSIt->empty()) + return false; + ++LHSIt; + } + while (!RHSIt.is_at_eof()) { + if (!RHSIt->empty()) + return false; + ++RHSIt; + } + return true; +} + } // namespace clangd } // namespace clang 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,12 @@ 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(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(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(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(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(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(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,11 @@ 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)) + if (Result->ApplyEdits) { + 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()); 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.Replacements); } void checkTransform(llvm::StringRef ID, llvm::StringRef Input,