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(), _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
