[PATCH] D92082: [clangd] Add metrics for invalid name.

[Haojian Wu via Phabricator via cfe-commits]

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.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92082

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
@@ -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(),
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ 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<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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D92082.307538.patch
Type: text/x-patch
Size: 2998 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201125/b80da7cf/attachment.bin>