alexfh added inline comments. ================ Comment at: clang-tidy-vs/ClangTidy/ClangTidyProperties.cs:82 @@ +81,3 @@ + public bool CERTDCL50 + { + get { return GetInheritableProperty<bool>("CERTDCL50").Value; } ---------------- zturner wrote: > alexfh wrote: > > zturner wrote: > > > Are the .rst files in the repo somewhere already? I don't see them. > > > > > > As for the display name, I agree this one is bad (I forgot to change it). > > > But you can look at some of the ones below for better examples. For > > > example, I find `I.22 - Complex Global Initializers` to be a better short > > > descriptor than `cppcoreguidelines-interfaces-global-init`. > > > > > > What about a hand-maintained Yaml file that adheres to a format similar > > > to the following? > > > > > > ``` > > > --- > > > Checks: > > > - Name: cert-dcl54-cpp > > > Label: Overloaded allocation function pairs > > > Description: Checks for violations of CERT DCL54-CPP - Overload > > > allocation and deallocation functions as a pair in the same scope > > > Category: CERT Secure Coding Standards > > > - Name: cppcoreguidelines-interfaces-global-init > > > Label: I.22 - Complex Global Initializers > > > Description: Checks for violations of Core Guideline I.22 - Avoid > > > complex initializers of global objects > > > Category: C++ Core Guidelines > > > ... > > > > > > ``` > > > > > > Some file somewhere is going to have to be maintained by hand, and since > > > a file such as this doesn't appear to exist in clang-tidy already, we > > > might as well use a format that we already have good tools to parse at > > > runtime, such as Yaml. > > The .rst files are there: > > https://reviews.llvm.org/diffusion/L/browse/clang-tools-extra/trunk/docs/clang-tidy/checks/ > > > > Docs + source code are already enough hassle to maintain, so adding yet > > another file with a similar information seems too much. We can start with a > > hand-written YAML file and later add a script to generate it from the docs. > > I'm not sure though, what `Label` could be mapped on. Do you need it for > > the property editor or can you get away with just the name? > Well you can look at the screenshot to see how I'm using it. Basically the > left hand side of the grid are all the "label" values. It's what people are > going to see when deciding whether they want to enable a check or not. So it > should be mostly self-explanatory without having to click on it to get more > detail. The name kinda is, but the point of the extension is to be > friendlier than editing files by hand, so I think we should try to provide a > friendlier name too. When you're using the command line tool you have no > choice but to specify the check name, but if you're using a UI I think we can > give the user a little bit more info about each check. > > Generating YAML from docs seems like a decent idea. Perhaps to map label we > could extend the rst format a little to add the label to it. Sort of like > "summary" vs "description". > > I'll do a hand-written Yaml file for now, and think about the best way to > generate the Yaml from RST in a later patch. SG.
Re: label vs check name. I think, even if we show a friendlier label (which we'll need to require for all new checks and back-fill for the existing ones), the extension should also show the check name somewhere for transparency. It's supposed to be human-readable to some degree (not a proper explanation though, rather a hint to what the check is about). It's also shown in the diagnostic messages, so there's no point in hiding it elsewhere. https://reviews.llvm.org/D23848 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits