kadircet updated this revision to Diff 218878.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Define more strict semantics around filename


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66637/new/

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.cpp
  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
@@ -7,16 +7,28 @@
 //===----------------------------------------------------------------------===//
 
 #include "Annotations.h"
+#include "ClangdUnit.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"
@@ -113,11 +125,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,
@@ -127,6 +143,27 @@
   EXPECT_EQ(Output, std::string(*Result)) << Input;
 }
 
+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, Succeeded());
+    EXPECT_EQ(Res->first, testPath(Path));
+  }
+}
+
 TWEAK_TEST(SwapIfBranches);
 TEST_F(SwapIfBranchesTest, Test) {
   Context = Function;
@@ -495,7 +532,7 @@
   checkTransform(ID, Input, Output);
 
   checkTransform(ID,
-  R"cpp(
+                 R"cpp(
 [[void f1();
 void f2();]]
 )cpp",
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,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<llvm::StringRef> TweakTest::isAvailable() const {
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,7 @@
                                                  ElseRng->getBegin(),
                                                  ElseCode.size(), ThenCode)))
     return std::move(Err);
-  return Effect::applyEdit(Result);
+  return Effect::mainFileEdit(SrcMgr, 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,12 @@
 }
 
 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
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,7 @@
   // 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
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,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 Effect::mainFileEdit(SM, 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,7 @@
       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,
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,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::mainFileEdit(SM, 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
@@ -20,11 +20,17 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_TWEAK_H
 
 #include "ClangdUnit.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 @@
   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 @@
 // 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
 
Index: clang-tools-extra/clangd/refactor/Tweak.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Tweak.cpp
+++ clang-tools-extra/clangd/refactor/Tweak.cpp
@@ -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,27 @@
   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: " +
+                                     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
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"
@@ -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;
@@ -184,11 +187,34 @@
                                           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);
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,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>
 
@@ -491,7 +499,7 @@
   }
 
   // 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:
@@ -837,5 +845,57 @@
   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
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ 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<Tweak::Effect> CB) {
   auto Action = [File = File.str(), Sel, TweakID = TweakID.str(),
-                 CB = std::move(CB)](Expected<InputsAndAST> InpAST) mutable {
+                 CB = std::move(CB), FS = FSProvider.getFileSystem()](
+                    Expected<InputsAndAST> 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));
   };
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -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 @@
   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 @@
       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 @@
         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,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to