diff --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp --- a/clang-tools-extra/clangd/refactor/Rename.cpp +++ b/clang-tools-extra/clangd/refactor/Rename.cpp @@ -341,6 +341,15 @@ Kind K; std::string Details; }; +std::string toString(InvalidName::Kind K) { + switch (K) { + case InvalidName::Keywords: + return "Keywords"; + case InvalidName::Conflict: + return "Conflict"; + } + llvm_unreachable("unhandled InvalidName kind"); +} llvm::Error makeError(InvalidName Reason) { auto Message = [](InvalidName Reason) { @@ -361,18 +370,25 @@ llvm::Optional checkName(const NamedDecl &RenameDecl, llvm::StringRef NewName) { trace::Span Tracer("CheckName"); + static constexpr trace::Metric InvalidNameMetric( + "rename_name_invalid", trace::Metric::Counter, "invalid_kind"); auto &ASTCtx = RenameDecl.getASTContext(); + llvm::Optional Result; if (isKeyword(NewName, ASTCtx.getLangOpts())) - return InvalidName{InvalidName::Keywords, NewName.str()}; - // Name conflict detection. - // Function conflicts are subtle (overloading), so ignore them. - if (RenameDecl.getKind() != Decl::Function) { - if (auto *Conflict = lookupSiblingWithName(ASTCtx, RenameDecl, NewName)) - return InvalidName{ - InvalidName::Conflict, - Conflict->getLocation().printToString(ASTCtx.getSourceManager())}; + Result = InvalidName{InvalidName::Keywords, NewName.str()}; + else { + // Name conflict detection. + // Function conflicts are subtle (overloading), so ignore them. + if (RenameDecl.getKind() != Decl::Function) { + if (auto *Conflict = lookupSiblingWithName(ASTCtx, RenameDecl, NewName)) + Result = InvalidName{ + InvalidName::Conflict, + Conflict->getLocation().printToString(ASTCtx.getSourceManager())}; + } } - return llvm::None; + if (Result) + InvalidNameMetric.record(1, toString(Result->K)); + return Result; } // AST-based rename, it renames all occurrences in the main file. diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp --- a/clang-tools-extra/clangd/unittests/RenameTests.cpp +++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp @@ -1031,6 +1031,7 @@ EXPECT_THAT(FooCC.ranges(), testing::UnorderedElementsAreArray(Results->LocalChanges)); + trace::TestTracer Tracer; // Name validation. Results = runPrepareRename(Server, FooCCPath, FooCC.point(), @@ -1038,6 +1039,8 @@ EXPECT_FALSE(Results); EXPECT_THAT(llvm::toString(Results.takeError()), testing::HasSubstr("keyword")); + EXPECT_THAT(Tracer.takeMetric("rename_name_invalid", "Keywords"), + ElementsAre(1)); // Single-file rename on global symbols, we should report an error. Results = runPrepareRename(Server, FooCCPath, FooCC.point(),