[llvm-branch-commits] [clang-tools-extra] fb6f425 - [clangd] Add metrics for invalid name.

[Haojian Wu via llvm-branch-commits]

Author: Haojian Wu
Date: 2020-11-25T10:50:43+01:00
New Revision: fb6f425d1b06480f4e61109852b1761cc15c81c1

URL: https://github.com/llvm/llvm-project/commit/fb6f425d1b06480f4e61109852b1761cc15c81c1
DIFF: https://github.com/llvm/llvm-project/commit/fb6f425d1b06480f4e61109852b1761cc15c81c1.diff

LOG: [clangd] Add metrics for invalid name.

Differential Revision: https://reviews.llvm.org/D92082

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp
index e7924b4add09..78aaa9930cd4 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -341,6 +341,15 @@ struct InvalidName {
   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::Error makeError(InvalidName Reason) {
 llvm::Optional<InvalidName> 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<InvalidName> 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
index 68a6a666a895..d109b5139b2e 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1031,6 +1031,7 @@ TEST(RenameTest, PrepareRename) {
   EXPECT_THAT(FooCC.ranges(),
               testing::UnorderedElementsAreArray(Results->LocalChanges));
 
+  trace::TestTracer Tracer;
   // Name validation.
   Results =
       runPrepareRename(Server, FooCCPath, FooCC.point(),
@@ -1038,6 +1039,8 @@ TEST(RenameTest, PrepareRename) {
   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(),