diff --git a/clang-tools-extra/clang-tidy/google/TodoCommentCheck.cpp b/clang-tools-extra/clang-tidy/google/TodoCommentCheck.cpp index 8554870287c81..0372981fa819e 100644 --- a/clang-tools-extra/clang-tidy/google/TodoCommentCheck.cpp +++ b/clang-tools-extra/clang-tidy/google/TodoCommentCheck.cpp @@ -15,37 +15,84 @@ namespace clang::tidy::google::readability { class TodoCommentCheck::TodoCommentHandler : public CommentHandler { public: + enum class StyleKind { Parentheses, Hyphen }; + TodoCommentHandler(TodoCommentCheck &Check, std::optional User) : Check(Check), User(User ? *User : "unknown"), - TodoMatch("^// *TODO *(\\(.*\\))?:?( )?(.*)$") {} + TodoMatch("^// *TODO *((\\((.*)\\))?:?( )?|: *(.*) *- *)?(.*)$") { + llvm::StringRef TodoStyleString = Check.Options.get("Style", "Hyphen"); + if (TodoStyleString == "Parentheses") { + TodoStyle = StyleKind::Parentheses; + } else if (TodoStyleString == "Hyphen") { + TodoStyle = StyleKind::Hyphen; + } else { + Check.configurationDiag( + "invalid value '%0' for " + "google-readability-todo.Style; valid values are " + "'Parentheses' and 'Hyphen'. Defaulting to 'Hyphen'") + << TodoStyleString; + } + } bool HandleComment(Preprocessor &PP, SourceRange Range) override { StringRef Text = Lexer::getSourceText(CharSourceRange::getCharRange(Range), PP.getSourceManager(), PP.getLangOpts()); - SmallVector Matches; + SmallVector Matches; if (!TodoMatch.match(Text, &Matches)) return false; - StringRef Username = Matches[1]; - StringRef Comment = Matches[3]; + const StyleKind ParsedStyle = + !Matches[3].empty() ? StyleKind::Parentheses : StyleKind::Hyphen; + const StringRef Username = + ParsedStyle == StyleKind::Parentheses ? Matches[3] : Matches[5]; + const StringRef Comment = Matches[6]; - if (!Username.empty()) + if (!Username.empty() && + (ParsedStyle == StyleKind::Parentheses || !Comment.empty())) { return false; + } + + if (Username.empty()) { + Check.diag(Range.getBegin(), "missing username/bug in TODO") + << FixItHint::CreateReplacement( + CharSourceRange::getCharRange(Range), + createReplacementString(Username, Comment)); + } - std::string NewText = ("// TODO(" + Twine(User) + "): " + Comment).str(); + if (Comment.empty()) + Check.diag(Range.getBegin(), "missing details in TODO"); - Check.diag(Range.getBegin(), "missing username/bug in TODO") - << FixItHint::CreateReplacement(CharSourceRange::getCharRange(Range), - NewText); return false; } + std::string createReplacementString(const StringRef Username, + const StringRef Comment) const { + if (TodoStyle == StyleKind::Parentheses) { + return ("// TODO(" + Twine(User) + + "): " + (Comment.empty() ? "some details" : Comment)) + .str(); + } + return ("// TODO: " + Twine(User) + " - " + + (Comment.empty() ? "some details" : Comment)) + .str(); + } + + std::string getTodoStyleString() const { + switch (TodoStyle) { + case StyleKind::Parentheses: + return "Parentheses"; + case StyleKind::Hyphen: + return "Hyphen"; + } + } + private: TodoCommentCheck &Check; std::string User; llvm::Regex TodoMatch; + StyleKind TodoStyle = StyleKind::Hyphen; }; TodoCommentCheck::TodoCommentCheck(StringRef Name, ClangTidyContext *Context) @@ -61,4 +108,8 @@ void TodoCommentCheck::registerPPCallbacks(const SourceManager &SM, PP->addCommentHandler(Handler.get()); } +void TodoCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "Style", Handler->getTodoStyleString()); +} + } // namespace clang::tidy::google::readability diff --git a/clang-tools-extra/clang-tidy/google/TodoCommentCheck.h b/clang-tools-extra/clang-tidy/google/TodoCommentCheck.h index 05f9cc6618eb1..854b9cea673c2 100644 --- a/clang-tools-extra/clang-tidy/google/TodoCommentCheck.h +++ b/clang-tools-extra/clang-tidy/google/TodoCommentCheck.h @@ -27,6 +27,8 @@ class TodoCommentCheck : public ClangTidyCheck { void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + private: class TodoCommentHandler; std::unique_ptr Handler; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 8a0151f567c24..68e2cf3a4656f 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -329,6 +329,10 @@ Changes in existing checks adding an option to allow pointer arithmetic via prefix/postfix increment or decrement operators. +- Improved :doc:`google-readability-todo + ` check to accept the new todo + format from the Google Style Guide. + - Improved :doc:`llvm-prefer-isa-or-dyn-cast-in-conditionals ` check: 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 index 159d2b4adccac..fccfa81166436 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/google/readability-todo.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/google/readability-todo.rst @@ -9,3 +9,14 @@ The relevant style guide section is https://google.github.io/styleguide/cppguide.html#TODO_Comments. Corresponding cpplint.py check: `readability/todo` + +Options +------- + +.. option:: Style + + A string specifying the TODO style for fix-it hints. Accepted values are + `Hyphen` and `Parentheses`. Default is `Hyphen`. + + * `Hyphen` will format the fix-it as: ``// TODO: username - details``. + * `Parentheses` will format the fix-it as: ``// TODO(username): details``. diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/readability-todo-hyphen.cpp b/clang-tools-extra/test/clang-tidy/checkers/google/readability-todo-hyphen.cpp new file mode 100644 index 0000000000000..5701b30bef395 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/google/readability-todo-hyphen.cpp @@ -0,0 +1,40 @@ +// RUN: %check_clang_tidy %s google-readability-todo %t -- -config="{User: 'some user'}" -- + +// TODOfix this1 +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing username/bug in TODO +// CHECK-FIXES: // TODO: some user - fix this1 + +// TODO fix this2 +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing username/bug in TODO +// CHECK-FIXES: // TODO: some user - fix this2 + +// TODO fix this3 +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing username/bug in TODO +// CHECK-FIXES: // TODO: some user - fix this3 + +// TODO: fix this4 +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing username/bug in TODO +// CHECK-FIXES: // TODO: some user - fix this4 + +// TODO: bug 12345 - +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing details in TODO + +// TODO: a message without a reference +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing username/bug in TODO +// CHECK-FIXES: // TODO: some user - a message without a reference + +// TODO(clang)fix this5 + +// TODO: foo - shave yaks +// TODO:foo - no space bewteen semicolon and username +// TODO: foo- no space bewteen username and dash +// TODO: foo - extra spaces between semicolon and username +// TODO: foo - extra spaces between username and dash +// TODO: b/12345 - use a b/ prefix +// TODO: bug 12345 - use a space in username/bug reference +// 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 diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/readability-todo.cpp b/clang-tools-extra/test/clang-tidy/checkers/google/readability-todo-parentheses.cpp similarity index 54% rename from clang-tools-extra/test/clang-tidy/checkers/google/readability-todo.cpp rename to clang-tools-extra/test/clang-tidy/checkers/google/readability-todo-parentheses.cpp index 6b900aa92150e..f97e395d8bf48 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/google/readability-todo.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/google/readability-todo-parentheses.cpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s google-readability-todo %t -- -config="{User: 'some user'}" -- +// RUN: %check_clang_tidy %s google-readability-todo %t -- -config="{User: 'some user', CheckOptions: {google-readability-todo.Style: 'Parentheses'}}" -- // TODOfix this1 // CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing username/bug in TODO @@ -16,8 +16,22 @@ // CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing username/bug in TODO // CHECK-FIXES: // TODO(some user): fix this4 +// TODO: bug 12345 - +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing details in TODO + +// TODO: a message without a reference +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing username/bug in TODO +// CHECK-FIXES: // TODO(some user): a message without a reference + // TODO(clang)fix this5 +// TODO: foo - shave yaks +// TODO:foo - no space bewteen semicolon and username +// TODO: foo- no space bewteen username and dash +// TODO: foo - extra spaces between semicolon and username +// TODO: foo - extra spaces between username and dash +// TODO: b/12345 - use a b/ prefix +// TODO: bug 12345 - use a space in username/bug reference // TODO(foo):shave yaks // TODO(bar): // TODO(foo): paint bikeshed