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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
