[PATCH] D100731: [clangd] Omit parameter hint for setter functions

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
Herald added a subscriber: cfe-commits.


================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:114
+    //   void setTimeout(int timeoutMillis);
+    return Name.substr(3).equals_lower(ParamNames[0]);
+  }
----------------
nridge wrote:
> njames93 wrote:
> > This heuristic is far from perfect. There's definitely diminishing returns for the more advanced you make it, but I'd argue it makes sense to strip any underscore from the Name, e.g,
> > `void set_timeout(int timeout)`
> Good call, snake_case is definitely worth supporting
This doesn't handle multi-word variable names if params use snake case and functions don't, e.g.:

e.g. `void setExceptionHandler(EHFunc exception_handler)`

This is fine for now but probably worth a comment.
Ultimately replacing `equals_lower` with some `sloppy_equals` that ignores case and skips underscores would solve this neatly.


================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:108
+
+    // In addition to checking that the function has one parameter and its
+    // name starts with "set", also check that the part after "set" matches
----------------
This is nice and conservative, but a bit style dependent.
e.g. `setSema(Sema &S)`

Seems fine for now, maybe we want to tweak at some point.


================
Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:344
+      void set_parent(S* parent);
+      void setTimeout(int timeoutMillis);
+    };
----------------
can you add setTimeoutMillis(int timeout_millis) just to show the current behavior?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100731/new/

https://reviews.llvm.org/D100731