[clangd] Don't rename the namespace.

Summary:
Also fix a small bug -- the extra argument "-xc++" doesn't overwrite the
language if the argument is present after the file name in the compiler
command.

Reviewers: sammccall

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

Tags: #clang

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

llvm-svn: 364392

clang-tools-extra/clangd/refactor/Rename.cpp

@@ -93,12 +93,15 @@ enum ReasonToReject {
   NoIndexProvided,
   NonIndexable,
   UsedOutsideFile,
+  UnsupportedSymbol,
 };
 
 // Check the symbol Decl is renameable (per the index) within the file.
 llvm::Optional<ReasonToReject> renamableWithinFile(const NamedDecl &Decl,
                                                    StringRef MainFile,
                                                    const SymbolIndex *Index) {
+  if (llvm::isa<NamespaceDecl>(&Decl))
+    return ReasonToReject::UnsupportedSymbol;
   auto &ASTCtx = Decl.getASTContext();
   const auto &SM = ASTCtx.getSourceManager();
   bool MainFileIsHeader = ASTCtx.getLangOpts().IsHeaderFile;
@@ -160,6 +163,8 @@ renameWithinFile(ParsedAST &AST, llvm::StringRef File, Position Pos,
         return "the symbol is used outside main file";
       case NonIndexable:
         return "symbol may be used in other files (not eligible for indexing)";
+      case UnsupportedSymbol:
+        return "symbol is not a supported kind (e.g. namespace)";
       }
       llvm_unreachable("unhandled reason kind");
     };

clang-tools-extra/clangd/unittests/RenameTests.cpp

@@ -93,28 +93,41 @@ TEST(RenameTest, SingleFile) {
 
 TEST(RenameTest, Renameable) {
   // Test cases where the symbol is declared in header.
-  const char *Headers[] = {
-      R"cpp(// allow -- function-local
+  struct Case {
+    const char* HeaderCode;
+    const char* ErrorMessage; // null if no error
+  };
+  Case Cases[] = {
+      {R"cpp(// allow -- function-local
         void f(int [[Lo^cal]]) {
           [[Local]] = 2;
         }
       )cpp",
+       nullptr},
 
-      R"cpp(// allow -- symbol is indexable and has no refs in index.
+      {R"cpp(// allow -- symbol is indexable and has no refs in index.
         void [[On^lyInThisFile]]();
       )cpp",
+       nullptr},
 
-      R"cpp(// disallow -- symbol is indexable and has other refs in index.
+      {R"cpp(// disallow -- symbol is indexable and has other refs in index.
         void f() {
           Out^side s;
         }
       )cpp",
+       "used outside main file"},
 
-      R"cpp(// disallow -- symbol is not indexable.
+      {R"cpp(// disallow -- symbol is not indexable.
         namespace {
         class Unin^dexable {};
         }
       )cpp",
+       "not eligible for indexing"},
+
+      {R"cpp(// disallow -- namespace symbol isn't supported
+        namespace fo^o {}
+      )cpp",
+       "not a supported kind"},
   };
   const char *CommonHeader = "class Outside {};";
   TestTU OtherFile = TestTU::withCode("Outside s;");
@@ -123,13 +136,14 @@ TEST(RenameTest, Renameable) {
   // The index has a "Outside" reference.
   auto OtherFileIndex = OtherFile.index();
 
-  for (const char *Header : Headers) {
-    Annotations T(Header);
+  for (const auto& Case : Cases) {
+    Annotations T(Case.HeaderCode);
     // We open the .h file as the main file.
     TestTU TU = TestTU::withCode(T.code());
     TU.Filename = "test.h";
     TU.HeaderCode = CommonHeader;
-    TU.ExtraArgs.push_back("-xc++");
+    // Parsing the .h file as C++ include.
+    TU.ExtraArgs.push_back("-xobjective-c++-header");
     auto AST = TU.build();
 
     auto Results = renameWithinFile(AST, testPath(TU.Filename), T.point(),
@@ -138,9 +152,11 @@ TEST(RenameTest, Renameable) {
     if (T.ranges().empty())
       WantRename = false;
     if (!WantRename) {
+      assert(Case.ErrorMessage && "Error message must be set!");
       EXPECT_FALSE(Results) << "expected renameWithinFile returned an error: "
                             << T.code();
-      llvm::consumeError(Results.takeError());
+      auto ActualMessage = llvm::toString(Results.takeError());
+      EXPECT_THAT(ActualMessage, testing::HasSubstr(Case.ErrorMessage));
     } else {
       EXPECT_TRUE(bool(Results)) << "renameWithinFile returned an error: "
                                  << llvm::toString(Results.takeError());

clang-tools-extra/clangd/unittests/TestTU.cpp

@@ -31,7 +31,7 @@ ParsedAST TestTU::build() const {
   Files[FullHeaderName] = HeaderCode;
   Files[ImportThunk] = ThunkContents;
 
-  std::vector<const char *> Cmd = {"clang", FullFilename.c_str()};
+  std::vector<const char *> Cmd = {"clang"};
   // FIXME: this shouldn't need to be conditional, but it breaks a
   // GoToDefinition test for some reason (getMacroArgExpandedLocation fails).
   if (!HeaderCode.empty()) {
@@ -40,6 +40,9 @@ ParsedAST TestTU::build() const {
                                       : FullHeaderName.c_str());
   }
   Cmd.insert(Cmd.end(), ExtraArgs.begin(), ExtraArgs.end());
+  // Put the file name at the end -- this allows the extra arg (-xc++) to
+  // override the language setting.
+  Cmd.push_back(FullFilename.c_str());
   ParseInputs Inputs;
   Inputs.CompileCommand.Filename = FullFilename;
   Inputs.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};