Validate clang-tidy check names.

[alexfh]

Summary

ClangTidy and its frontends rely on check names being restricted to
certain rules, e.g. having a comma in a check name would break -checks=
parameter parsing. So we need to validate check names early.

[djasper]

clang-tidy/ClangTidy.cpp

294 ↗

(On Diff #12698)

Should we really validate the analyzer check names here? And if so, should we assert?

I'd be more comfortable moving at least the assert to somewhere in the static analyzer itself, so that people working on it know what valid names are and don't have to discover that by breaking an assert build of clang-tidy.

Or, alternatively, people can't add more analyzer checks by linking additional libraries to clang-tidy, right? Should we just have a test that calls "clang-tidy -list-checks" and verifies all the names?

323 ↗

(On Diff #12698)

It almost feels like it is not worth implementing and testing this function. I'd be fine with simply defining the string as a constant and then calling llvm::Regex().match() at the two call sites. Specifically, if it is located here, people might think that they have to do something with this function when writing a check.

clang-tidy/ClangTidy.h

89 ↗

(On Diff #12698)

Maybe: .., if \p Name can be used as a check name.

[alexfh]

Use a test instead of runtime checks.

[djasper]

I think this is fine for the analyzer checks. However, as people can link in their checks, it might make sense to do runtime checks for those. WDYT?

[alexfh]

In D4982#7, @djasper wrote:

I think this is fine for the analyzer checks. However, as people can link in their checks, it might make sense to do runtime checks for those. WDYT?

Well, if someone links their checks in, they should still run clang-tidy tests.

[alexfh]

PIng

[alexfh]

Daily pIng ;)

[djasper]

But they won't run clang-tidy checks, but I can see reasons for keeping the implementation simpler. Either way, this is a strict improvement, so looks good.

[alexfh]

This has already landed unintentionally in r217155.

test/clang-tidy/validate-check-names.cpp

+// Check names may only contain alphanumeric characters, '-', '_', and '.'.
+// RUN: clang-tidy -checks=* -list-checks - -- | grep '^    ' | cut -b5- | not grep -v '^[a-zA-Z0-9_.\-]\+$'