hokein created this revision.
hokein added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov,
mgorny.
Herald added a project: clang.
This patch adds a simple mechanism to disallow global rename
on std symbols. We might extend it to other symbols, e.g. protobuf.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D73450
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/refactor/RenameBlacklist.cpp
clang-tools-extra/clangd/refactor/RenameBlacklist.h
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
@@ -383,12 +383,12 @@
// Typedef.
R"cpp(
- namespace std {
+ namespace ns {
class basic_string {};
typedef basic_string [[s^tring]];
- } // namespace std
+ } // namespace ns
- std::[[s^tring]] foo();
+ ns::[[s^tring]] foo();
)cpp",
// Variable.
@@ -450,7 +450,7 @@
for (const auto &RenamePos : Code.points()) {
auto RenameResult =
rename({RenamePos, NewName, AST, testPath(TU.Filename)});
- ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
+ ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError() << T;
ASSERT_EQ(1u, RenameResult->size());
EXPECT_EQ(applyEdits(std::move(*RenameResult)).front().second,
expectedResult(Code, NewName));
@@ -539,6 +539,13 @@
void fo^o() {})cpp",
"used outside main file", !HeaderFile, nullptr /*no index*/},
+ {R"cpp(// disallow rename on blacklisted symbols (e.g. std symbols)
+ namespace std {
+ class str^ing {};
+ }
+ )cpp",
+ "not a supported kind", !HeaderFile, Index},
+
{R"cpp(
void foo(int);
void foo(char);
Index: clang-tools-extra/clangd/refactor/RenameBlacklist.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/refactor/RenameBlacklist.h
@@ -0,0 +1,24 @@
+//===--- RenameBlacklist.h - ------------------------------------*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_RENAME_BLACKLIST_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_RENAME_BLACKLIST_H
+
+#include "clang/AST/Decl.h"
+#include "llvm/ADT/DenseSet.h"
+
+namespace clang {
+namespace clangd {
+
+// Returns true if the given decl is disallowed for rename.
+bool isRenameBlacklisted(const NamedDecl &RenameDecl);
+
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_RENAME_BLACKLIST_H
Index: clang-tools-extra/clangd/refactor/RenameBlacklist.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/refactor/RenameBlacklist.cpp
@@ -0,0 +1,24 @@
+//===--- RenameBlacklist.cpp - ----------------------------------*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "refactor/RenameBlacklist.h"
+
+namespace clang {
+namespace clangd {
+
+bool isRenameBlacklisted(const NamedDecl &RenameDecl) {
+ static const auto *StdSymbols = new llvm::DenseSet<llvm::StringRef>({
+#define SYMBOL(Name, NameSpace, Header) {#NameSpace #Name},
+#include "StdSymbolMap.inc"
+#undef SYMBOL
+ });
+ return StdSymbols->count(RenameDecl.getQualifiedNameAsString());
+}
+
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -14,6 +14,7 @@
#include "Selection.h"
#include "SourceCode.h"
#include "index/SymbolCollector.h"
+#include "refactor/RenameBlacklist.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/Basic/SourceLocation.h"
@@ -105,7 +106,7 @@
AmbiguousSymbol,
};
-llvm::Optional<ReasonToReject> renameable(const Decl &RenameDecl,
+llvm::Optional<ReasonToReject> renameable(const NamedDecl &RenameDecl,
StringRef MainFilePath,
const SymbolIndex *Index,
bool CrossFile) {
@@ -120,6 +121,9 @@
if (RenameDecl.getParentFunctionOrMethod())
return None;
+ if (isRenameBlacklisted(RenameDecl))
+ return ReasonToReject::UnsupportedSymbol;
+
// Check whether the symbol being rename is indexable.
auto &ASTCtx = RenameDecl.getASTContext();
bool MainFileIsHeader = isHeaderFile(MainFilePath, ASTCtx.getLangOpts());
@@ -131,11 +135,9 @@
IsMainFileOnly = false;
else if (!DeclaredInMainFile)
IsMainFileOnly = false;
- bool IsIndexable =
- isa<NamedDecl>(RenameDecl) &&
- SymbolCollector::shouldCollectSymbol(
- cast<NamedDecl>(RenameDecl), RenameDecl.getASTContext(),
- SymbolCollector::Options(), IsMainFileOnly);
+ bool IsIndexable = SymbolCollector::shouldCollectSymbol(
+ RenameDecl, RenameDecl.getASTContext(), SymbolCollector::Options(),
+ IsMainFileOnly);
if (!IsIndexable) // If the symbol is not indexable, we disallow rename.
return ReasonToReject::NonIndexable;
@@ -467,13 +469,13 @@
if (DeclsUnderCursor.size() > 1)
return makeError(ReasonToReject::AmbiguousSymbol);
- const auto *RenameDecl = llvm::dyn_cast<NamedDecl>(*DeclsUnderCursor.begin());
+ const auto *RenameDecl = llvm::dyn_cast<NamedDecl>(
+ (*DeclsUnderCursor.begin())->getCanonicalDecl());
if (!RenameDecl)
return makeError(ReasonToReject::UnsupportedSymbol);
- auto Reject =
- renameable(*RenameDecl->getCanonicalDecl(), RInputs.MainFilePath,
- RInputs.Index, RInputs.AllowCrossFile);
+ auto Reject = renameable(*RenameDecl, RInputs.MainFilePath, RInputs.Index,
+ RInputs.AllowCrossFile);
if (Reject)
return makeError(*Reject);
Index: clang-tools-extra/clangd/CMakeLists.txt
===================================================================
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -107,6 +107,7 @@
index/dex/Trigram.cpp
refactor/Rename.cpp
+ refactor/RenameBlacklist.cpp
refactor/Tweak.cpp
LINK_LIBS
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
