[clang-tools-extra] [clangd] forward clang-tidy's readability-identifier-naming fix to textDocument/rename (PR #78454)

Tom Praschan via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 31 10:57:53 PST 2024


tom-anders wrote:

Thanks for the feedback!

> Thanks, the approach in this patch looks pretty good to me.
> 
> My only feedback is to encapsulate the "hard coding" into a function like:
> 
> ```
> Option<CodeActionResult::Rename> TryConvertToRename(const Diag *D, const Fix *F)
> ```
> 
> because I can imagine in the future coming across other diagnostics that are conceptually renames that we may want to handle this way.

:heavy_check_mark: 

> Regarding testing: I find lit-tests pretty hard to read and maintain; a nicer alternative might be a gtest in `ClangdLSPServerTests` (e.g. see [this one](https://searchfox.org/llvm/rev/23b233c8adad5b81e185e50d04356fab64c2f870/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp#198) for testing call hierarchy incoming calls).

Perfect, thanks for the hint! I had to adapt `LSPClient` a bit, it usually would flag every server->client call as an error, but in this case we actually expect a `workspace/applyEdit` after we trigger the renaming.


https://github.com/llvm/llvm-project/pull/78454


More information about the cfe-commits mailing list