ymandel updated this revision to Diff 523440.
ymandel added a comment.
Address reviewer comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150872/new/
https://reviews.llvm.org/D150872
Files:
clang-tools-extra/clang-tidy/google/TodoCommentCheck.cpp
clang-tools-extra/clang-tidy/google/TodoCommentCheck.h
clang-tools-extra/docs/clang-tidy/checks/google/readability-todo.rst
clang-tools-extra/test/clang-tidy/checkers/google/readability-todo-v2.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/google/readability-todo-v2.cpp
===================================================================
--- /dev/null
+++ 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
Index: clang-tools-extra/docs/clang-tidy/checks/google/readability-todo.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/google/readability-todo.rst
+++ 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.
+
Index: clang-tools-extra/clang-tidy/google/TodoCommentCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/google/TodoCommentCheck.h
+++ 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<TodoCommentHandler> Handler;
};
Index: clang-tools-extra/clang-tidy/google/TodoCommentCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/google/TodoCommentCheck.cpp
+++ 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 <optional>
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<std::string> User)
- : Check(Check), User(User ? *User : "unknown"),
- TodoMatch("^// *TODO *(\\(.*\\))?:?( )?(.*)$") {}
+ TodoCommentHandler(TodoCommentCheck &Check, std::optional<std::string> 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<StringRef, 4> 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<StringRef, 5> 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<TodoCommentHandler>(
- *this, Context->getOptions().User)) {}
+ *this, Context->getOptions().User,
+ UseV2Style ? TodoStyleVersion::V2 : TodoStyleVersion::V1)) {}
TodoCommentCheck::~TodoCommentCheck() = default;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
