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.
With this patch, we reject the rename if the new name would conflict with
any other decls in the decl context of the renamed decl.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D89790
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
@@ -630,6 +630,44 @@
class Foo {};
)cpp",
"no symbol", !HeaderFile, nullptr},
+
+ {R"cpp(
+ namespace {
+ int Conflict;
+ int Va^r;
+ }
+ )cpp",
+ "conflict", !HeaderFile, nullptr, "Conflict"},
+
+ {R"cpp(
+ int Conflict;
+ int Va^r;
+ )cpp",
+ "conflict", !HeaderFile, nullptr, "Conflict"},
+
+ {R"cpp(
+ class Foo {
+ int Conflict;
+ int Va^r;
+ };
+ )cpp",
+ "conflict", !HeaderFile, nullptr, "Conflict"},
+
+ {R"cpp(
+ enum E {
+ Conflict,
+ Fo^o,
+ };
+ )cpp",
+ "conflict", !HeaderFile, nullptr, "Conflict"},
+
+ {R"cpp(// FIXME: detecting local variables is not supported yet.
+ void func() {
+ int Conflict;
+ int [[V^ar]];
+ }
+ )cpp",
+ nullptr, !HeaderFile, nullptr, "Conflict"},
};
for (const auto& Case : Cases) {
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -254,6 +254,74 @@
return Results;
}
+// Lookup the declarations (if any) with the given Name in the given DC.
+NamedDecl *lookup(const ASTContext &Ctx, const DeclContext *DC,
+ llvm::StringRef Name) {
+ const auto &II = Ctx.Idents.get(Name);
+ DeclarationName LookupName(&II);
+ DeclContextLookupResult LookupResult;
+ switch (DC->getDeclKind()) {
+ case Decl::TranslationUnit:
+ case Decl::Namespace:
+ case Decl::Record:
+ case Decl::Enum:
+ case Decl::CXXRecord:
+ LookupResult = DC->lookup(LookupName);
+ break;
+ default:
+ break;
+ }
+ if (!LookupResult.empty())
+ return LookupResult.front();
+ return nullptr;
+}
+
+struct InvalidName {
+ enum Kind {
+ Keywords,
+ Conflict,
+ };
+ Kind K;
+ std::string Details;
+};
+
+llvm::Error makeError(InvalidName Reason) {
+ auto Message = [](InvalidName Reason) {
+ switch (Reason.K) {
+ case InvalidName::Keywords:
+ return llvm::formatv("the chosen name \"{0}\" is a keyword",
+ Reason.Details);
+ case InvalidName::Conflict:
+ return llvm::formatv("conflict with the symbol in {0}", Reason.Details);
+ }
+ llvm_unreachable("unhandled InvalidName kind");
+ };
+ return error("invalid name: {0}", Message(Reason));
+}
+
+// Check if we can rename the given RenameDecl into NewName.
+// Return details if the rename would produce a conflict.
+llvm::Optional<InvalidName> checkName(const NamedDecl &RenameDecl,
+ llvm::StringRef NewName) {
+ auto &ASTCtx = RenameDecl.getASTContext();
+ if (isKeyword(NewName, ASTCtx.getLangOpts()))
+ return InvalidName{InvalidName::Keywords, NewName.str()};
+ // FIXME: detecting function conflicts is tricky, e.g. we can have a same name
+ // for two overload functions.
+ if (RenameDecl.getKind() == Decl::Function)
+ return llvm::None;
+ // Perform a lookup in the decl context of the RenameDecl, to find out any
+ // conflicts if we perform the rename.
+ // (!) DeclContext::lookup doesn't perform lookup local decls in a
+ // function-kind DeclContext.
+ auto *Conflict = lookup(ASTCtx, RenameDecl.getDeclContext(), NewName);
+ if (!Conflict)
+ return llvm::None;
+ return InvalidName{
+ InvalidName::Conflict,
+ Conflict->getLocation().printToString(ASTCtx.getSourceManager())};
+}
+
// AST-based rename, it renames all occurrences in the main file.
llvm::Expected<tooling::Replacements>
renameWithinFile(ParsedAST &AST, const NamedDecl &RenameDecl,
@@ -476,11 +544,12 @@
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());
+ auto Invalid = checkName(RenameDecl, RInputs.NewName);
+ if (Invalid)
+ return makeError(*Invalid);
+
auto Reject = renameable(RenameDecl, RInputs.MainFilePath, RInputs.Index,
Opts.AllowCrossFile);
if (Reject)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
