diff --git a/clang-tools-extra/clang-tidy/google/TodoCommentCheck.h b/clang-tools-extra/clang-tidy/google/TodoCommentCheck.h --- a/clang-tools-extra/clang-tidy/google/TodoCommentCheck.h +++ b/clang-tools-extra/clang-tidy/google/TodoCommentCheck.h @@ -27,8 +27,13 @@ void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override { + Options.store(Opts, "UseV2Style", UseV2Style); + } + private: class TodoCommentHandler; + bool UseV2Style; std::unique_ptr Handler; }; diff --git a/clang-tools-extra/clang-tidy/google/TodoCommentCheck.cpp b/clang-tools-extra/clang-tidy/google/TodoCommentCheck.cpp --- a/clang-tools-extra/clang-tidy/google/TodoCommentCheck.cpp +++ b/clang-tools-extra/clang-tidy/google/TodoCommentCheck.cpp @@ -7,51 +7,97 @@ //===----------------------------------------------------------------------===// #include "TodoCommentCheck.h" +#include "clang/Basic/Diagnostic.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Lex/Preprocessor.h" +#include "llvm/Support/ErrorHandling.h" #include namespace clang::tidy::google::readability { +enum class TodoStyleVersion { + V1, + V2, +}; + +std::string_view getVersionRegex(TodoStyleVersion V) { + switch (V) { + case TodoStyleVersion::V1: + return "^// *TODO *(\\(.*\\))?:?( )?(.*)$"; + case TodoStyleVersion::V2: + return "^// *TODO *((: *[^ ]+( - .)?)|([(][^)]*[)])?).*$"; + } + llvm_unreachable("bad enum value"); +} + class TodoCommentCheck::TodoCommentHandler : public CommentHandler { public: - TodoCommentHandler(TodoCommentCheck &Check, std::optional User) - : Check(Check), User(User ? *User : "unknown"), - TodoMatch("^// *TODO *(\\(.*\\))?:?( )?(.*)$") {} + TodoCommentHandler(TodoCommentCheck &Check, std::optional User, + TodoStyleVersion V) + : Check(Check), User(User ? *User : "unknown"), Version(V), + TodoMatch(getVersionRegex(V)) {} bool HandleComment(Preprocessor &PP, SourceRange Range) override { StringRef Text = Lexer::getSourceText(CharSourceRange::getCharRange(Range), PP.getSourceManager(), PP.getLangOpts()); + switch (Version) { + case TodoStyleVersion::V1: + handleV1Comment(Text, Range); + return false; + case TodoStyleVersion::V2: + handleV2Comment(Text, Range); + return false; + } + } + +private: + void handleV1Comment(StringRef Text, SourceRange Range) { SmallVector Matches; if (!TodoMatch.match(Text, &Matches)) - return false; + return; StringRef Username = Matches[1]; StringRef Comment = Matches[3]; if (!Username.empty()) - return false; + return; std::string NewText = ("// TODO(" + Twine(User) + "): " + Comment).str(); Check.diag(Range.getBegin(), "missing username/bug in TODO") << FixItHint::CreateReplacement(CharSourceRange::getCharRange(Range), NewText); - return false; + } + + void handleV2Comment(StringRef Text, SourceRange Range) { + SmallVector Matches; + if (!TodoMatch.match(Text, &Matches)) + return; + + if (/* New style*/ (!Matches[2].empty() && !Matches[3].empty()) || + /* Old style */ !Matches[4].empty()) + return; + + // Either the component after " - " is missing (and it's something like new + // style) or it doesn't match any style, beyond starting with TODO. + Check.diag(Range.getBegin(), "TODO requires link and description"); } private: TodoCommentCheck &Check; std::string User; + TodoStyleVersion Version; llvm::Regex TodoMatch; }; TodoCommentCheck::TodoCommentCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), + UseV2Style(Options.getLocalOrGlobal("UseV2Style", false)), Handler(std::make_unique( - *this, Context->getOptions().User)) {} + *this, Context->getOptions().User, + UseV2Style ? TodoStyleVersion::V2 : TodoStyleVersion::V1)) {} TodoCommentCheck::~TodoCommentCheck() = default; diff --git a/clang-tools-extra/docs/clang-tidy/checks/google/readability-todo.rst b/clang-tools-extra/docs/clang-tidy/checks/google/readability-todo.rst --- a/clang-tools-extra/docs/clang-tidy/checks/google/readability-todo.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/google/readability-todo.rst @@ -3,9 +3,32 @@ google-readability-todo ======================= -Finds TODO comments without a username or bug number. +Finds TODO comments not conforming to Google's style guidelines. The relevant style guide section is https://google.github.io/styleguide/cppguide.html#TODO_Comments. Corresponding cpplint.py check: `readability/todo` + +Options +------- + +.. option:: UseV2Style + + Disabled by default. + + When disabled, the check enforces the style + ``` + // TODO(username/bug): description + ``` + and will suggest fixes in this style, where possible (for example, a TODO + missing the username). + + When enabled, the check also accepts TODO comments in the following style: + ``` + // TODO: link - description + ``` + It will still admit the previous style, for backwards compatability. But, it + won't suggest fixes, since mistakes are now ambiguous: given `TODO: text`, "text" + could be the description or it could be a link of some form. + diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/readability-todo-v2.cpp b/clang-tools-extra/test/clang-tidy/checkers/google/readability-todo-v2.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/google/readability-todo-v2.cpp @@ -0,0 +1,33 @@ +// RUN: %check_clang_tidy %s google-readability-todo %t -- \ +// RUN: -config="{User: 'some user', CheckOptions: \ +// RUN: [{key: google-readability-todo.UseV2Style, \ +// RUN: value: 'true'}]}" \ +// RUN: -- + +// TODOfix this1 +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: TODO requires link and description + +// TODO fix this2 +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: TODO requires link and description + +// TODO fix this3 +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: TODO requires link and description + +// TODO: fix this4 +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: TODO requires link and description + +// V2-style TODO comments: + +// TODO: https://github.com/llvm/llvm-project/issues/12345 - Some description +// TODO: link - description + +// V1-style TODO comments, supported for backwards compatability: + +// TODO(clang)fix this5 + +// TODO(foo):shave yaks +// TODO(bar): +// TODO(foo): paint bikeshed +// TODO(b/12345): find the holy grail +// TODO (b/12345): allow spaces before parentheses +// TODO(asdf) allow missing semicolon