[clang] 8000d50 - [clangd] Strip /showIncludes in clangd compile commands

[Arthur Eubanks via cfe-commits]

Author: Arthur Eubanks
Date: 2020-04-26T18:57:39-07:00
New Revision: 8000d506afcdbb3aee7aa8f876688dd094c6eb85

URL: https://github.com/llvm/llvm-project/commit/8000d506afcdbb3aee7aa8f876688dd094c6eb85
DIFF: https://github.com/llvm/llvm-project/commit/8000d506afcdbb3aee7aa8f876688dd094c6eb85.diff

LOG: [clangd] Strip /showIncludes in clangd compile commands

In command lines with /showIncludes, clangd would output includes to stdout, breaking clients.

Summary: Fixes https://github.com/clangd/clangd/issues/322.

Reviewers: sammccall, kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D78836

Added: 
    

Modified: 
    clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
    clang/lib/Tooling/ArgumentsAdjusters.cpp
    clang/unittests/Tooling/ToolingTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
index fc98b56de2d6..cad987291623 100644
--- a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -79,6 +79,20 @@ TEST(CommandMangler, StripOutput) {
     EXPECT_THAT(Cmd, Not(Contains(Stripped)));
 }
 
+TEST(CommandMangler, StripShowIncludes) {
+  auto Mangler = CommandMangler::forTests();
+  std::vector<std::string> Cmd = {"clang-cl", "/showIncludes", "foo.cc"};
+  Mangler.adjust(Cmd);
+  EXPECT_THAT(Cmd, Not(Contains("/showIncludes")));
+}
+
+TEST(CommandMangler, StripShowIncludesUser) {
+  auto Mangler = CommandMangler::forTests();
+  std::vector<std::string> Cmd = {"clang-cl", "/showIncludes:user", "foo.cc"};
+  Mangler.adjust(Cmd);
+  EXPECT_THAT(Cmd, Not(Contains("/showIncludes:user")));
+}
+
 TEST(CommandMangler, ClangPath) {
   auto Mangler = CommandMangler::forTests();
   Mangler.ClangPath = testPath("fake/clang");

diff  --git a/clang/lib/Tooling/ArgumentsAdjusters.cpp b/clang/lib/Tooling/ArgumentsAdjusters.cpp
index 62ee954e3096..d8cd11efedd2 100644
--- a/clang/lib/Tooling/ArgumentsAdjusters.cpp
+++ b/clang/lib/Tooling/ArgumentsAdjusters.cpp
@@ -98,7 +98,7 @@ ArgumentsAdjuster getClangStripDependencyFileAdjuster() {
       StringRef Arg = Args[i];
       // All dependency-file options begin with -M. These include -MM,
       // -MF, -MG, -MP, -MT, -MQ, -MD, and -MMD.
-      if (!Arg.startswith("-M")) {
+      if (!Arg.startswith("-M") && !Arg.startswith("/showIncludes")) {
         AdjustedArgs.push_back(Args[i]);
         continue;
       }

diff  --git a/clang/unittests/Tooling/ToolingTest.cpp b/clang/unittests/Tooling/ToolingTest.cpp
index 0ff66206e06e..782b3f6a2b44 100644
--- a/clang/unittests/Tooling/ToolingTest.cpp
+++ b/clang/unittests/Tooling/ToolingTest.cpp
@@ -530,6 +530,62 @@ TEST(ClangToolTest, StripDependencyFileAdjuster) {
   EXPECT_TRUE(HasFlag("-w"));
 }
 
+// Check getClangStripDependencyFileAdjuster strips /showIncludes
+TEST(ClangToolTest, StripDependencyFileAdjusterShowIncludes) {
+  FixedCompilationDatabase Compilations("/", {"/showIncludes", "-c"});
+
+  ClangTool Tool(Compilations, std::vector<std::string>(1, "/a.cc"));
+  Tool.mapVirtualFile("/a.cc", "void a() {}");
+
+  std::unique_ptr<FrontendActionFactory> Action(
+      newFrontendActionFactory<SyntaxOnlyAction>());
+
+  CommandLineArguments FinalArgs;
+  ArgumentsAdjuster CheckFlagsAdjuster =
+      [&FinalArgs](const CommandLineArguments &Args, StringRef /*unused*/) {
+        FinalArgs = Args;
+        return Args;
+      };
+  Tool.clearArgumentsAdjusters();
+  Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
+  Tool.appendArgumentsAdjuster(CheckFlagsAdjuster);
+  Tool.run(Action.get());
+
+  auto HasFlag = [&FinalArgs](const std::string &Flag) {
+    return llvm::find(FinalArgs, Flag) != FinalArgs.end();
+  };
+  EXPECT_FALSE(HasFlag("/showIncludes"));
+  EXPECT_TRUE(HasFlag("-c"));
+}
+
+// Check getClangStripDependencyFileAdjuster strips /showIncludes:user
+TEST(ClangToolTest, StripDependencyFileAdjusterShowIncludesUser) {
+  FixedCompilationDatabase Compilations("/", {"/showIncludes:user", "-c"});
+
+  ClangTool Tool(Compilations, std::vector<std::string>(1, "/a.cc"));
+  Tool.mapVirtualFile("/a.cc", "void a() {}");
+
+  std::unique_ptr<FrontendActionFactory> Action(
+      newFrontendActionFactory<SyntaxOnlyAction>());
+
+  CommandLineArguments FinalArgs;
+  ArgumentsAdjuster CheckFlagsAdjuster =
+      [&FinalArgs](const CommandLineArguments &Args, StringRef /*unused*/) {
+        FinalArgs = Args;
+        return Args;
+      };
+  Tool.clearArgumentsAdjusters();
+  Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
+  Tool.appendArgumentsAdjuster(CheckFlagsAdjuster);
+  Tool.run(Action.get());
+
+  auto HasFlag = [&FinalArgs](const std::string &Flag) {
+    return llvm::find(FinalArgs, Flag) != FinalArgs.end();
+  };
+  EXPECT_FALSE(HasFlag("/showIncludes:user"));
+  EXPECT_TRUE(HasFlag("-c"));
+}
+
 // Check getClangStripPluginsAdjuster strips plugin related args.
 TEST(ClangToolTest, StripPluginsAdjuster) {
   FixedCompilationDatabase Compilations(