[PATCH] D88875: [clangd] Add basic keyword-name-validation in rename.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:124
+
+  // name validation.
+  RenameToKeywords,
----------------
nit: not sure we need a separate section for these, I can imagine at most keyword/conflict/shadow


================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:215
+    case ReasonToReject::RenameToKeywords:
+      return "symbol is renamed to a reserved keyword";
     }
----------------
nit: either "reserved [word|identifier|name]" or "keyword", but not both

This could be friendlier, what about "the chosen name is a keyword"?


================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:479
     return makeError(ReasonToReject::AmbiguousSymbol);
+  // Empty name is legal -- this allows rename API to return all occurrences
+  // even the name is unknown.
----------------
Now that prepareRename only exposes ranges rather than edits, do we still need this empty special case, or can we go back to using "clangd_rename_dummy" or so?

(Sorry for the back and forth here)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88875