[PATCH] D96125: [clangd] Diagfixer for layering violations

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

Introduce a diagfixer that can update build system files in the face of
layering violation warnings.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96125

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/Diagnostics.h


Index: clang-tools-extra/clangd/Diagnostics.h
===================================================================
--- clang-tools-extra/clangd/Diagnostics.h
+++ clang-tools-extra/clangd/Diagnostics.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_DIAGNOSTICS_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_DIAGNOSTICS_H
 
+#include "BuildSystem.h"
 #include "Protocol.h"
 #include "support/Path.h"
 #include "clang/Basic/Diagnostic.h"
@@ -76,6 +77,7 @@
   std::string Message;
   /// TextEdits from clang's fix-its. Must be non-empty.
   llvm::SmallVector<TextEdit, 1> Edits;
+  llvm::Optional<Command> Cmd;
 };
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Fix &F);
 
@@ -168,6 +170,8 @@
 /// (This strips err_ and -W prefix so we can match with or without them.)
 llvm::StringRef normalizeSuppressedCode(llvm::StringRef);
 
+StoreDiags::DiagFixer missingDependencyFixer(const BuildSystem *BS);
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/Diagnostics.cpp
===================================================================
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -29,6 +29,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Capacity.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/Signals.h"
@@ -368,12 +369,17 @@
 }
 
 CodeAction toCodeAction(const Fix &F, const URIForFile &File) {
+  assert(!F.Edits.empty() || F.Cmd.hasValue());
   CodeAction Action;
   Action.title = F.Message;
   Action.kind = std::string(CodeAction::QUICKFIX_KIND);
-  Action.edit.emplace();
-  Action.edit->changes.emplace();
-  (*Action.edit->changes)[File.uri()] = {F.Edits.begin(), F.Edits.end()};
+  if (!F.Edits.empty()) {
+    Action.edit.emplace();
+    Action.edit->changes.emplace();
+    (*Action.edit->changes)[File.uri()] = {F.Edits.begin(), F.Edits.end()};
+  }
+  if (F.Cmd)
+    Action.command = *F.Cmd;
   return Action;
 }
 
@@ -819,5 +825,37 @@
   return Code;
 }
 
+StoreDiags::DiagFixer missingDependencyFixer(const BuildSystem *BS) {
+  return [BS](DiagnosticsEngine::Level,
+              const clang::Diagnostic &D) -> std::vector<Fix> {
+    if (D.getID() != clang::diag::err_undeclared_use_of_module)
+      return {};
+    clang::clangd::trace::Span Span("DiagFixerDependencyInserter");
+    auto FromModule = D.getArgStdStr(0);
+    auto ToModule = BS->getOwningTarget(D.getArgStdStr(1));
+    if (!ToModule) {
+      clang::clangd::elog("Couldn't find owning module for {0}: {1}",
+                          D.getArgStdStr(1), ToModule.takeError());
+      return {};
+    }
+    // Rather than generating the edit here, we can defer that to diagnostic
+    // resolve time (at the cost of false positives, when BUILD file can't be
+    // edited).
+    auto Edit = BS->addDependency(FromModule, ToModule->front());
+    if (!Edit) {
+      clang::clangd::elog("Couldn't insert dep from {0} to {1}: {2}",
+                          FromModule, ToModule->front(), Edit.takeError());
+      return {};
+    }
+    Fix F;
+    F.Cmd.emplace();
+    F.Cmd->title =
+        llvm::formatv("Add deps {0} to {1}", ToModule->front(), FromModule);
+    F.Cmd->workspaceEdit = std::move(*Edit);
+    F.Cmd->command = Command::CLANGD_APPLY_FIX_COMMAND.str();
+    F.Message = F.Cmd->title;
+    return {std::move(F)};
+  };
+}
 } // namespace clangd
 } // namespace clang


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D96125.321718.patch
Type: text/x-patch
Size: 3506 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210205/27a8bb7a/attachment-0001.bin>