[clang-tidy] Add a checker that warns on TODO comments without username.

[bkramer]

Summary

It also suggests a fix-it, taking the name from $USER. This will be made
configurable eventually.

[alexfh]

Looks good provided that you fix the issues.

clang-tidy/google/TodoCommentCheck.cpp

54

This could be a reference as well.

test/clang-tidy/google-readability-todo.cpp

5

It should be CHECK-MESSAGES:

6

Please make the comments distinct, so that they can be matched reliably. Currently, the test can pass even if one of the replacement is not done, as there's a matching string in line 24.

Closed by commit rL218049 (authored by d0k).

[kimgr]

See inline.

clang-tools-extra/trunk/clang-tidy/google/TodoCommentCheck.cpp

42 ↗

(On Diff #13829)

This doesn't work on Windows. The variable there is USERNAME; would it make sense to check both?

[alexfh]

clang-tools-extra/trunk/clang-tidy/google/TodoCommentCheck.cpp

42 ↗

(On Diff #13829)

Thanks for noting. Some further work on this is happening in D5408, and I'll add USERNAME variable handling there.

clang-tidy/google/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyGoogleModule
   AvoidCStyleCastsCheck.cpp
   ExplicitConstructorCheck.cpp
   ExplicitMakePairCheck.cpp
   GoogleTidyModule.cpp
   IntegerTypesCheck.cpp
   MemsetZeroLengthCheck.cpp
   NamedParameterCheck.cpp
   OverloadedUnaryAndCheck.cpp
   StringReferenceMemberCheck.cpp
+  TodoCommentCheck.cpp
   UnnamedNamespaceInHeaderCheck.cpp
   UsingNamespaceDirectiveCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   )

clang-tidy/google/GoogleTidyModule.cpp

 #include "AvoidCStyleCastsCheck.h"
 #include "ExplicitConstructorCheck.h"
 #include "ExplicitMakePairCheck.h"
 #include "IntegerTypesCheck.h"
 #include "MemsetZeroLengthCheck.h"
 #include "NamedParameterCheck.h"
 #include "OverloadedUnaryAndCheck.h"
 #include "StringReferenceMemberCheck.h"
+#include "TodoCommentCheck.h"
 #include "UnnamedNamespaceInHeaderCheck.h"
 #include "UsingNamespaceDirectiveCheck.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<runtime::StringReferenceMemberCheck>(
         "google-runtime-member-string-references");
     CheckFactories.registerCheck<runtime::MemsetZeroLengthCheck>(
         "google-runtime-memset");
     CheckFactories.registerCheck<readability::AvoidCStyleCastsCheck>(
         "google-readability-casting");
     CheckFactories.registerCheck<readability::NamedParameterCheck>(
         "google-readability-function");
+    CheckFactories.registerCheck<readability::TodoCommentCheck>(
+        "google-readability-todo");
   }
 };
 
 // Register the GoogleTidyModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add<GoogleModule> X("google-module",
                                                     "Adds Google lint checks.");
 
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the GoogleModule.
 volatile int GoogleModuleAnchorSource = 0;
 
 } // namespace tidy
 } // namespace clang

clang-tidy/google/TodoCommentCheck.h

+//===--- TodoCommentCheck.h - clang-tidy ------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_TODOCOMMENTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_TODOCOMMENTCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// \brief Finds TODO comments without a username or bug number.
+///
+/// Corresponding cpplint.py check: readability/todo
+class TodoCommentCheck : public ClangTidyCheck {
+public:
+  TodoCommentCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerPPCallbacks(CompilerInstance &Compiler) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_TODOCOMMENTCHECK_H

clang-tidy/google/TodoCommentCheck.cpp

+//===--- TodoCommentCheck.cpp - clang-tidy --------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "TodoCommentCheck.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Preprocessor.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+namespace {
+class TodoCommentHandler : public CommentHandler {
+public:
+  explicit TodoCommentHandler(TodoCommentCheck *Check)
+      : Check(Check), TodoMatch("^// *TODO(\\(.*\\))?:?( )?(.*)$") {}
+
+  bool HandleComment(Preprocessor &PP, SourceRange Range) override {
+    StringRef Text =
+        Lexer::getSourceText(CharSourceRange::getCharRange(Range),
+                             PP.getSourceManager(), PP.getLangOpts());
+
+    SmallVector<StringRef, 4> Matches;
+    if (!TodoMatch.match(Text, &Matches))
+      return false;
+
+    StringRef Username = Matches[1];
+    StringRef Comment = Matches[3];
+
+    if (!Username.empty())
+      return false;
+
+    // If the username is missing put in the current user's name. Not ideal but
+    // works for running tidy locally.
+    // FIXME: Can we get this from a more reliable source?
+    const char *User = std::getenv("USER");
+    if (!User)
+      User = "unknown";
+    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;
+  }
+
+private:
+  TodoCommentCheck *Check;
+  llvm::Regex TodoMatch;
+};
+} // namespace
+
+void TodoCommentCheck::registerPPCallbacks(CompilerInstance &Compiler) {
+  Compiler.getPreprocessor().addCommentHandler(new TodoCommentHandler(this));
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang

test/clang-tidy/google-readability-todo.cpp

+// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s google-readability-todo %t
+// REQUIRES: shell
+
+//   TODOfix this
+// CHECK-WARNINGS: [[@LINE-1]]:1: warning: missing username/bug in TODO
+// CHECK-FIXES: // TODO({{[^)]+}}): fix this
+
+//   TODO fix this
+// CHECK-WARNINGS: [[@LINE-1]]:1: warning: missing username/bug in TODO
+// CHECK-FIXES: // TODO({{[^)]+}}): fix this
+
+// TODO fix this
+// CHECK-WARNINGS: [[@LINE-1]]:1: warning: missing username/bug in TODO
+// CHECK-FIXES: // TODO({{[^)]+}}): fix this
+
+// TODO: fix this
+// CHECK-WARNINGS: [[@LINE-1]]:1: warning: missing username/bug in TODO
+// CHECK-FIXES: // TODO({{[^)]+}}): fix this
+
+//   TODO(clang)fix this
+
+// TODO(foo):Fix this
+// TODO(bar):
+// TODO(foo): fix this
+// TODO(b/12345): Fix this