[PATCH] D88875: [clangd] Add basic keyword-name-validation in rename.

[Haojian Wu via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88875

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp


Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -516,6 +516,7 @@
     const char* ErrorMessage; // null if no error
     bool IsHeaderFile;
     const SymbolIndex *Index;
+    llvm::StringRef NewName = "DummyName";
   };
   TestTU OtherFile = TestTU::withCode("Outside s; auto ss = &foo;");
   const char *CommonHeader = R"cpp(
@@ -542,6 +543,11 @@
       )cpp",
        nullptr, HeaderFile, Index},
 
+      {R"cpp(
+        void ^f();
+      )cpp",
+       "keyword", HeaderFile, Index, "return"},
+
       {R"cpp(// disallow -- symbol is indexable and has other refs in index.
         void f() {
           Out^side s;
@@ -639,7 +645,7 @@
       TU.ExtraArgs.push_back("-xobjective-c++-header");
     }
     auto AST = TU.build();
-    llvm::StringRef NewName = "dummyNewName";
+    llvm::StringRef NewName = Case.NewName;
     auto Results =
         rename({T.point(), NewName, AST, testPath(TU.Filename), Case.Index});
     bool WantRename = true;
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -120,6 +120,9 @@
   UsedOutsideFile, // for within-file rename only.
   UnsupportedSymbol,
   AmbiguousSymbol,
+
+  // name validation.
+  RenameToKeywords,
 };
 
 llvm::Optional<ReasonToReject> renameable(const NamedDecl &RenameDecl,
@@ -208,6 +211,8 @@
       return "symbol is not a supported kind (e.g. namespace, macro)";
     case ReasonToReject::AmbiguousSymbol:
       return "there are multiple symbols at the given location";
+    case ReasonToReject::RenameToKeywords:
+      return "symbol is renamed to a reserved keyword";
     }
     llvm_unreachable("unhandled reason kind");
   };
@@ -471,6 +476,10 @@
     return makeError(ReasonToReject::NoSymbolFound);
   if (DeclsUnderCursor.size() > 1)
     return makeError(ReasonToReject::AmbiguousSymbol);
+  // Empty name is legal -- this allows rename API to return all occurrences
+  // even the name is unknown.
+  if (!RInputs.NewName.empty() && isKeyword(RInputs.NewName, AST.getLangOpts()))
+    return makeError(ReasonToReject::RenameToKeywords);
 
   const auto &RenameDecl =
       llvm::cast<NamedDecl>(*(*DeclsUnderCursor.begin())->getCanonicalDecl());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D88875.296377.patch
Type: text/x-patch
Size: 2491 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201006/c9ceb22b/attachment-0001.bin>