[PATCH] D77085: [clang-tidy] Added support for validating configuration options

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:25
+  llvm::raw_svector_ostream Output(Buffer);
+  Output << "warning: Option not found '" << OptionName << "'\n";
+  return std::string(Buffer);
----------------
aaron.ballman wrote:
> I think the diagnostic text should probably not start with a capital letter. Also, the name of the classes are confusing in that they say error but the diagnostic is a warning. When I hear "error", the class name makes me think this would stop compilation and give a nonzero result code from the program.
Sorry, I was unclear with what I was looking for.

It looks to me like the behavior of `OptionError` is to act more like a warning than an error in that a message is printed and execution of the tool continues, correct? If so, then I'd prefer to rename it to `OptionWarning` (unless you want to generalize it to be either a warning or an error with a return code, then `OptionDiagnostic` would be good), and all the subclasses to be `FooWarning`. Or did I misunderstand the design?

Also, I like having the `warning:` in the message (though we could bikeshed if we want to make it look less like a check warning by doing something like `warning [clang-tidy command line]:`), but maybe that's being added by `OptionsView::logErrToStdErr()`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77085/new/

https://reviews.llvm.org/D77085