Hi Sam, this looks good!
I found just one small detail.
It seems to me that in case ID is undefined (hits the default case in getDiagnosticCode) we are calling std::string non-explicit constructor with nullptr which is UB.
Rebase to head and expand scope a bit:
- now also setting code for clang-tidy checks
- to enable this to be used from the C++ API, the string code is now on Diag
- and also expose Source over LSP (just an oversight?)
It's a good question, it depends how this is surfaced, and we may want to tweak the behavior or suppress entirely in some cases.
I think at least some are useful:
- clang-tidy check names are things users need to know about (used for configuration)
- for warnings, we quite likely should replace with the most specific warning category (e.g. "unreachable-code-loop-increment"), again these are used for configuration (-W)
- for others, maybe we should at least trim the err_ prefix, or maybe drop them entirely.
Yeah, this isn't really part of clang, and seems to be part of static analyzer.
This is an if statement - if the pointer is null we never assign to std::string.
I see, I believe we can decide on what tweaks to perform after landing this patch and seeing behaviors in different editors, but up to you.
As for the lit-tests, it would be great to have a diag with source clang-tidy if it is not too much of a hustle.