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 @@ -20,11 +20,14 @@ #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 namespace clang { namespace clangd { @@ -627,7 +630,8 @@ 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 +639,25 @@ 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."); + WorkspaceEdit WE; + WE.changes.emplace(); + auto FS = FSProvider.getFileSystem(); + for (const auto &It : R->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)) + 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)); }; Server->applyTweak(Params.tweakArgs->file.file(), Params.tweakArgs->selection, Params.tweakArgs->tweakID, 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 @@ -13,6 +13,7 @@ #include "Format.h" #include "FormattedString.h" #include "Headers.h" +#include "Logger.h" #include "Protocol.h" #include "SemanticHighlighting.h" #include "SourceCode.h" @@ -392,7 +393,8 @@ void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID, Callback CB) { auto Action = [File = File.str(), Sel, TweakID = TweakID.str(), - CB = std::move(CB)](Expected InpAST) mutable { + CB = std::move(CB), FS = FSProvider.getFileSystem()]( + Expected InpAST) mutable { if (!InpAST) return CB(InpAST.takeError()); auto Selection = tweakSelection(Sel, *InpAST); @@ -404,16 +406,15 @@ 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()); + 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)) { + llvm::handleAllErrors(std::move(Err), [&](llvm::ErrorInfoBase &EIB) { + elog("Failed to format {0}: {1}", It.first(), EIB.message()); + }); + } } 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,25 @@ FileDigest digest(StringRef Content); Optional digestFile(const SourceManager &SM, FileID FID); +/// 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 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; +}; + // 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 +206,15 @@ 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 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 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 @@ -837,5 +845,49 @@ 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; +} + +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 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,14 +70,9 @@ 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::StringMap 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; @@ -127,6 +125,15 @@ llvm::Expected> prepareTweak(StringRef TweakID, const Tweak::Selection &S); +/// Returns an Effect containing Replacements as the ApplyEdits for the file +/// pointed by FID. +Tweak::Effect fileEdit(const SourceManager &SM, FileID FID, + tooling::Replacements Replacements); + +/// Convinience wrapper around above. +Tweak::Effect mainFileEdit(const SourceManager &SM, + tooling::Replacements Replacements); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/refactor/Tweak.cpp b/clang-tools-extra/clangd/refactor/Tweak.cpp --- a/clang-tools-extra/clangd/refactor/Tweak.cpp +++ b/clang-tools-extra/clangd/refactor/Tweak.cpp @@ -81,5 +81,23 @@ return std::move(T); } +Tweak::Effect fileEdit(const SourceManager &SM, FileID FID, + tooling::Replacements Replacements) { + Tweak::Effect E; + + auto FE = SM.getFileEntryForID(FID); + assert(FE && "Generating edits for unknown file!"); + llvm::StringRef FilePath = FE->getName(); + Edit Ed(SM.getBufferData(FID), std::move(Replacements)); + E.ApplyEdits.try_emplace(FilePath, std::move(Ed)); + + return E; +} + +Tweak::Effect mainFileEdit(const SourceManager &SM, + tooling::Replacements Replacements) { + return fileEdit(SM, SM.getMainFileID(), std::move(Replacements)); +} + } // namespace clangd } // namespace clang 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 mainFileEdit(SM, 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,7 @@ Expansion(SrcMgr, CharSourceRange(CachedLocation->getSourceRange(), true), PrettyTypeName); - return Tweak::Effect::applyEdit(tooling::Replacements(Expansion)); + return mainFileEdit(SrcMgr, 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,9 @@ 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 mainFileEdit(SM, 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,7 @@ // replace expression with variable name if (auto Err = Result.add(Target->replaceWithVar(Range, VarName))) return std::move(Err); - return Effect::applyEdit(Result); + return mainFileEdit(Inputs.AST.getSourceManager(), 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,12 @@ } 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 mainFileEdit(SM, 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,7 @@ ElseRng->getBegin(), ElseCode.size(), ThenCode))) return std::move(Err); - return Effect::applyEdit(Result); + return mainFileEdit(SrcMgr, 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,14 +98,16 @@ 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 TweakTest::isAvailable() const { 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.empty()) 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,