[clang-tools-extra] 043e965 - [clangd] Add inlay hints for mutable reference parameters

[Nathan Ridge via cfe-commits]

Author: Tobias Ribizel
Date: 2022-05-02T15:57:23-04:00
New Revision: 043e9650240866701bee89922d54bf7af059ed30

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

LOG: [clangd] Add inlay hints for mutable reference parameters

Add a & prefix to all parameter inlay hints that refer to a non-const l-value reference. That makes it easier to identify them even if semantic highlighting is not used (where this is already available)

Reviewed By: nridge

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

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 bd124aa2a23d5..c5876ac5554b6 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -10,6 +10,7 @@
 #include "Config.h"
 #include "HeuristicResolver.h"
 #include "ParsedAST.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
@@ -392,6 +393,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
     // Don't show hints for variadic parameters.
     size_t FixedParamCount = getFixedParamCount(Callee);
     size_t ArgCount = std::min(FixedParamCount, Args.size());
+    auto Params = Callee->parameters();
 
     NameVec ParameterNames = chooseParameterNames(Callee, ArgCount);
 
@@ -402,12 +404,14 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
 
     for (size_t I = 0; I < ArgCount; ++I) {
       StringRef Name = ParameterNames[I];
-      if (!shouldHint(Args[I], Name))
-        continue;
+      bool NameHint = shouldHintName(Args[I], Name);
+      bool ReferenceHint = shouldHintReference(Params[I]);
 
-      addInlayHint(Args[I]->getSourceRange(), HintSide::Left,
-                   InlayHintKind::ParameterHint, /*Prefix=*/"", Name,
-                   /*Suffix=*/": ");
+      if (NameHint || ReferenceHint) {
+        addInlayHint(Args[I]->getSourceRange(), HintSide::Left,
+                     InlayHintKind::ParameterHint, ReferenceHint ? "&" : "",
+                     NameHint ? Name : "", ": ");
+      }
     }
   }
 
@@ -434,12 +438,12 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
     return WhatItIsSetting.equals_insensitive(ParamNames[0]);
   }
 
-  bool shouldHint(const Expr *Arg, StringRef ParamName) {
+  bool shouldHintName(const Expr *Arg, StringRef ParamName) {
     if (ParamName.empty())
       return false;
 
     // If the argument expression is a single name and it matches the
-    // parameter name exactly, omit the hint.
+    // parameter name exactly, omit the name hint.
     if (ParamName == getSpelledIdentifier(Arg))
       return false;
 
@@ -450,6 +454,13 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
     return true;
   }
 
+  bool shouldHintReference(const ParmVarDecl *Param) {
+    // If the parameter is a non-const reference type, print an inlay hint
+    auto Type = Param->getType();
+    return Type->isLValueReferenceType() &&
+           !Type.getNonReferenceType().isConstQualified();
+  }
+
   // 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.
@@ -563,7 +574,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
     return Result;
   }
 
-  // We pass HintSide rather than SourceLocation because we want to ensure 
+  // We pass HintSide rather than SourceLocation because we want to ensure
   // it is in the same file as the common file range.
   void addInlayHint(SourceRange R, HintSide Side, InlayHintKind Kind,
                     llvm::StringRef Prefix, llvm::StringRef Label,

diff  --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index b0f8b2556d2cf..1b39ebb8de5ae 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -138,6 +138,38 @@ TEST(ParameterHints, NoName) {
   )cpp");
 }
 
+TEST(ParameterHints, NoNameConstReference) {
+  // No hint for anonymous const l-value ref parameter.
+  assertParameterHints(R"cpp(
+    void foo(const int&);
+    void bar() {
+      foo(42);
+    }
+  )cpp");
+}
+
+TEST(ParameterHints, NoNameReference) {
+  // Reference hint for anonymous l-value ref parameter.
+  assertParameterHints(R"cpp(
+    void foo(int&);
+    void bar() {
+      int i;
+      foo($param[[i]]);
+    }
+  )cpp",
+                       ExpectedHint{"&: ", "param"});
+}
+
+TEST(ParameterHints, NoNameRValueReference) {
+  // No reference hint for anonymous r-value ref parameter.
+  assertParameterHints(R"cpp(
+    void foo(int&&);
+    void bar() {
+      foo(42);
+    }
+  )cpp");
+}
+
 TEST(ParameterHints, NameInDefinition) {
   // Parameter name picked up from definition if necessary.
   assertParameterHints(R"cpp(
@@ -162,6 +194,66 @@ TEST(ParameterHints, NameMismatch) {
                        ExpectedHint{"good: ", "good"});
 }
 
+TEST(ParameterHints, NameConstReference) {
+  // Only name hint for const l-value ref parameter.
+  assertParameterHints(R"cpp(
+    void foo(const int& param);
+    void bar() {
+      foo($param[[42]]);
+    }
+  )cpp",
+                       ExpectedHint{"param: ", "param"});
+}
+
+TEST(ParameterHints, NameTypeAliasConstReference) {
+  // Only name hint for const l-value ref parameter via type alias.
+  assertParameterHints(R"cpp(
+    using alias = const int&;
+    void foo(alias param);
+    void bar() {
+      int i;
+      foo($param[[i]]);
+    }
+  )cpp",
+                       ExpectedHint{"param: ", "param"});
+}
+
+TEST(ParameterHints, NameReference) {
+  // Reference and name hint for l-value ref parameter.
+  assertParameterHints(R"cpp(
+    void foo(int& param);
+    void bar() {
+      int i;
+      foo($param[[i]]);
+    }
+  )cpp",
+                       ExpectedHint{"&param: ", "param"});
+}
+
+TEST(ParameterHints, NameTypeAliasReference) {
+  // Reference and name hint for l-value ref parameter via type alias.
+  assertParameterHints(R"cpp(
+    using alias = int&;
+    void foo(alias param);
+    void bar() {
+      int i;
+      foo($param[[i]]);
+    }
+  )cpp",
+                       ExpectedHint{"&param: ", "param"});
+}
+
+TEST(ParameterHints, NameRValueReference) {
+  // Only name hint for r-value ref parameter.
+  assertParameterHints(R"cpp(
+    void foo(int&& param);
+    void bar() {
+      foo($param[[42]]);
+    }
+  )cpp",
+                       ExpectedHint{"param: ", "param"});
+}
+
 TEST(ParameterHints, Operator) {
   // No hint for operator call with operator syntax.
   assertParameterHints(R"cpp(
@@ -301,6 +393,21 @@ TEST(ParameterHints, ArgMatchesParam) {
                        ExpectedHint{"param: ", "param"});
 }
 
+TEST(ParameterHints, ArgMatchesParamReference) {
+  assertParameterHints(R"cpp(
+    void foo(int& param);
+    void foo2(const int& param);
+    void bar() {
+      int param;
+      // show reference hint on mutable reference
+      foo($param[[param]]);
+      // but not on const reference
+      foo2(param);
+    }
+  )cpp",
+                       ExpectedHint{"&: ", "param"});
+}
+
 TEST(ParameterHints, LeadingUnderscore) {
   assertParameterHints(R"cpp(
     void foo(int p1, int _p2, int __p3);