[clangd] Don't rename the namespace.

[hokein]

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.

[sammccall]

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

167

can't rename symbols of this kind?

"supported" lacks context I think.

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

137

(why this change?)

[hokein]

Address review comments.

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

137

for the cases here, we want the main file treat as a header file, using -xc++ here would make clang treat it as a .cc file.

[sammccall]

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

137

It sounds like this is unrelated to the current change, and is designed to address tests that were passing by mistake (rename was failing because the file was not a header, not for the desired reason.

Can we split up the fix into another patch, and verify it by asserting on the error message?

[hokein]

Verify the error message in the test.

[hokein]

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

137

Done in this patch, adding the error message when doing the verification.

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

llvm::Optional<std::string> getOtherRefFile(const NamedDecl &Decl,
   });
   return OtherFile;
 }
 
 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;
   bool DeclaredInMainFile =
       SM.isWrittenInMainFile(SM.getExpansionLoc(Decl.getLocation()));
 
   // If the symbol is declared in the main file (which is not a header), we
   // rename it.
if (auto Reject = renamableWithinFile(*RenameDecl, File, Index)) {
     auto Message = [](ReasonToReject Reason) {
       switch (Reason) {
       case NoIndexProvided:
         return "symbol may be used in other files (no index available)";
       case UsedOutsideFile:
         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");
     };
     return llvm::make_error<llvm::StringError>(
         llvm::formatv("Cannot rename symbol: {0}", Message(*Reject)),
         llvm::inconvertibleErrorCode());
   }
 

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

const char *Headers[] = {
         }
       )cpp",
 
       R"cpp(// disallow -- symbol is not indexable.
         namespace {
         class Unin^dexable {};
         }
       )cpp",
+
+      R"cpp(// disallow -- namespace symbol isn't supported
+        namespace fo^o {}
+      )cpp",
   };
   const char *CommonHeader = "class Outside {};";
   TestTU OtherFile = TestTU::withCode("Outside s;");
   OtherFile.HeaderCode = CommonHeader;
   OtherFile.Filename = "other.cc";
   // The index has a "Outside" reference.
   auto OtherFileIndex = OtherFile.index();
 
   for (const char *Header : Headers) {
     Annotations T(Header);
     // 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(),
                                     "dummyNewName", OtherFileIndex.get());
     bool WantRename = true;
     if (T.ranges().empty())
       WantRename = false;
     if (!WantRename) {

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

ParsedAST TestTU::build() const {
   // guard without messing up offsets). In this case, use an intermediate file.
   std::string ThunkContents = "#import \"" + FullHeaderName + "\"\n";
 
   llvm::StringMap<std::string> Files(AdditionalFiles);
   Files[FullFilename] = Code;
   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()) {
     Cmd.push_back("-include");
     Cmd.push_back(ImplicitHeaderGuard ? ImportThunk.c_str()
                                       : 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()};
   Inputs.CompileCommand.Directory = testRoot();
   Inputs.Contents = Code;
   Inputs.FS = buildTestFS(Files);
   Inputs.Opts = ParseOptions();
   Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks;