Author: Haojian Wu Date: 2020-10-06T15:47:57+02:00 New Revision: 8a3cbb1535a92dcc0ac3bd8fc64216a465b8506a
URL: https://github.com/llvm/llvm-project/commit/8a3cbb1535a92dcc0ac3bd8fc64216a465b8506a DIFF: https://github.com/llvm/llvm-project/commit/8a3cbb1535a92dcc0ac3bd8fc64216a465b8506a.diff LOG: [clangd] Add basic keyword-name-validation in rename. Differential Revision: https://reviews.llvm.org/D88875 Added: Modified: clang-tools-extra/clangd/ClangdServer.cpp clang-tools-extra/clangd/ClangdServer.h clang-tools-extra/clangd/refactor/Rename.cpp clang-tools-extra/clangd/unittests/RenameTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 0840155fc8f9..d38e115a6796 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -412,9 +412,9 @@ void ClangdServer::prepareRename(PathRef File, Position Pos, // - for cross-file rename, we deliberately pass a nullptr index to save // the cost, thus the result may be incomplete as it only contains // main-file occurrences; - auto Results = clangd::rename({Pos, /*NewName*/ "", InpAST->AST, File, - RenameOpts.AllowCrossFile ? nullptr : Index, - RenameOpts}); + auto Results = clangd::rename( + {Pos, /*NewName=*/"__clangd_rename_dummy", InpAST->AST, File, + RenameOpts.AllowCrossFile ? nullptr : Index, RenameOpts}); if (!Results) { // LSP says to return null on failure, but that will result in a generic // failure message. If we send an LSP error response, clients can surface diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index a9d46fa5278f..d03f50069746 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -273,9 +273,6 @@ class ClangdServer { StringRef TriggerText, Callback<std::vector<TextEdit>> CB); /// Test the validity of a rename operation. - /// - /// The returned result describes edits in the main-file only (all - /// occurrences of the renamed symbol are simply deleted. void prepareRename(PathRef File, Position Pos, const RenameOptions &RenameOpts, Callback<RenameResult> CB); diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp index 9de3302564fd..e072853dac9f 100644 --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/clang-tools-extra/clangd/refactor/Rename.cpp @@ -120,6 +120,9 @@ enum class ReasonToReject { UsedOutsideFile, // for within-file rename only. UnsupportedSymbol, AmbiguousSymbol, + + // name validation. + RenameToKeywords, }; llvm::Optional<ReasonToReject> renameable(const NamedDecl &RenameDecl, @@ -208,6 +211,8 @@ llvm::Error makeError(ReasonToReject Reason) { 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 "the chosen name is a keyword"; } llvm_unreachable("unhandled reason kind"); }; @@ -471,6 +476,8 @@ llvm::Expected<RenameResult> rename(const RenameInputs &RInputs) { return makeError(ReasonToReject::NoSymbolFound); if (DeclsUnderCursor.size() > 1) return makeError(ReasonToReject::AmbiguousSymbol); + if (isKeyword(RInputs.NewName, AST.getLangOpts())) + return makeError(ReasonToReject::RenameToKeywords); const auto &RenameDecl = llvm::cast<NamedDecl>(*(*DeclsUnderCursor.begin())->getCanonicalDecl()); diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp index cc2454e9d04e..d925dfa36f50 100644 --- a/clang-tools-extra/clangd/unittests/RenameTests.cpp +++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -516,6 +516,7 @@ TEST(RenameTest, Renameable) { 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 @@ TEST(RenameTest, Renameable) { )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 @@ TEST(RenameTest, Renameable) { 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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits