[clang-tools-extra] 753b247 - [clangd] Omit parameter hint if parameter name comment is present

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2021-04-25T19:20:10-04:00
New Revision: 753b247d71d7e74dd6998735848a2d9e0b6317de

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

LOG: [clangd] Omit parameter hint if parameter name comment is present

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/InlayHints.cpp
    clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index 54b109443b30..948a9e511e7f 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -19,7 +19,13 @@ namespace clangd {
 class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
 public:
   InlayHintVisitor(std::vector<InlayHint> &Results, ParsedAST &AST)
-      : Results(Results), AST(AST.getASTContext()) {}
+      : Results(Results), AST(AST.getASTContext()),
+        MainFileID(AST.getSourceManager().getMainFileID()) {
+    bool Invalid = false;
+    llvm::StringRef Buf =
+        AST.getSourceManager().getBufferData(MainFileID, &Invalid);
+    MainFileBuf = Invalid ? StringRef{} : Buf;
+  }
 
   bool VisitCXXConstructExpr(CXXConstructExpr *E) {
     // Weed out constructor calls that don't look like a function call with
@@ -102,11 +108,37 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
     if (ParamName == getSpelledIdentifier(Arg))
       return false;
 
-    // FIXME: Exclude argument expressions preceded by a /*paramName=*/ comment.
+    // Exclude argument expressions preceded by a /*paramName*/.
+    if (isPrecededByParamNameComment(Arg, ParamName))
+      return false;
 
     return true;
   }
 
+  // Checks if "E" is spelled in the main file and preceded by a C-style comment
+  // whose contents match ParamName (allowing for whitespace and an optional "="
+  // at the end.
+  bool isPrecededByParamNameComment(const Expr *E, StringRef ParamName) {
+    auto &SM = AST.getSourceManager();
+    auto ExprStartLoc = SM.getTopMacroCallerLoc(E->getBeginLoc());
+    auto Decomposed = SM.getDecomposedLoc(ExprStartLoc);
+    if (Decomposed.first != MainFileID)
+      return false;
+
+    StringRef SourcePrefix = MainFileBuf.substr(0, Decomposed.second);
+    // Allow whitespace between comment and expression.
+    SourcePrefix = SourcePrefix.rtrim();
+    // Check for comment ending.
+    if (!SourcePrefix.consume_back("*/"))
+      return false;
+    // Allow whitespace and "=" at end of comment.
+    SourcePrefix = SourcePrefix.rtrim().rtrim('=').rtrim();
+    // Other than that, the comment must contain exactly ParamName.
+    if (!SourcePrefix.consume_back(ParamName))
+      return false;
+    return SourcePrefix.rtrim().endswith("/*");
+  }
+
   // If "E" spells a single unqualified identifier, return that name.
   // Otherwise, return an empty string.
   static StringRef getSpelledIdentifier(const Expr *E) {
@@ -208,6 +240,8 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
 
   std::vector<InlayHint> &Results;
   ASTContext &AST;
+  FileID MainFileID;
+  StringRef MainFileBuf;
 };
 
 std::vector<InlayHint> inlayHints(ParsedAST &AST) {

diff  --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 6a39dc9d923a..086c502181ef 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -322,6 +322,20 @@ TEST(ParameterHints, UserDefinedLiteral) {
   )cpp");
 }
 
+TEST(ParameterHints, ParamNameComment) {
+  // Do not hint an argument which already has a comment
+  // with the parameter name preceding it.
+  assertParameterHints(R"cpp(
+    void foo(int param);
+    void bar() {
+      foo(/*param*/42);
+      foo( /* param = */ 42);
+      foo(/* the answer */$param[[42]]);
+    }
+  )cpp",
+                       ExpectedHint{"param: ", "param"});
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang