Skip to content

Commit 5385f41

Browse files
qukhanqukhan
authored
[clang-tidy] Update google todo checker with style guide changes. (#165565)
The [Google style guide] now allows (and recommends) writing TODOs with the following format: ```cpp // TODO: bug reference - details about what needs to be done. ``` With this change the checker accepts the new style and suggests in in the fix-it hint. The previous style is still accepted. [Google style guide]: https://google.github.io/styleguide/cppguide.html#TODO_Comments
1 parent 0a86635 commit 5385f41

File tree

6 files changed

+147
-12
lines changed

6 files changed

+147
-12
lines changed

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

Lines changed: 75 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,42 +11,102 @@
1111
#include "clang/Lex/Preprocessor.h"
1212
#include <optional>
1313

14-
namespace clang::tidy::google::readability {
14+
namespace clang::tidy {
15+
16+
namespace google::readability {
17+
18+
enum class StyleKind { Parentheses, Hyphen };
19+
20+
} // namespace google::readability
21+
22+
template <> struct OptionEnumMapping<google::readability::StyleKind> {
23+
static ArrayRef<std::pair<google::readability::StyleKind, StringRef>>
24+
getEnumMapping() {
25+
static constexpr std::pair<google::readability::StyleKind, StringRef>
26+
Mapping[] = {
27+
{google::readability::StyleKind::Hyphen, "Hyphen"},
28+
{google::readability::StyleKind::Parentheses, "Parentheses"},
29+
};
30+
return {Mapping};
31+
}
32+
};
33+
34+
} // namespace clang::tidy
1535

36+
namespace clang::tidy::google::readability {
1637
class TodoCommentCheck::TodoCommentHandler : public CommentHandler {
1738
public:
1839
TodoCommentHandler(TodoCommentCheck &Check, std::optional<std::string> User)
1940
: Check(Check), User(User ? *User : "unknown"),
20-
TodoMatch("^// *TODO *(\\(.*\\))?:?( )?(.*)$") {}
41+
TodoMatch(R"(^// *TODO *((\((.*)\))?:?( )?|: *(.*) *- *)?(.*)$)") {
42+
const llvm::StringRef TodoStyleString =
43+
Check.Options.get("Style", "Hyphen");
44+
for (const auto &[Value, Name] :
45+
OptionEnumMapping<StyleKind>::getEnumMapping()) {
46+
if (Name == TodoStyleString) {
47+
TodoStyle = Value;
48+
return;
49+
}
50+
}
51+
Check.configurationDiag(
52+
"invalid value '%0' for "
53+
"google-readability-todo.Style; valid values are "
54+
"'Parentheses' and 'Hyphen'. Defaulting to 'Hyphen'")
55+
<< TodoStyleString;
56+
}
2157

2258
bool HandleComment(Preprocessor &PP, SourceRange Range) override {
2359
const StringRef Text =
2460
Lexer::getSourceText(CharSourceRange::getCharRange(Range),
2561
PP.getSourceManager(), PP.getLangOpts());
2662

27-
SmallVector<StringRef, 4> Matches;
63+
SmallVector<StringRef, 7> Matches;
2864
if (!TodoMatch.match(Text, &Matches))
2965
return false;
3066

31-
const StringRef Username = Matches[1];
32-
const StringRef Comment = Matches[3];
67+
const StyleKind ParsedStyle =
68+
!Matches[3].empty() ? StyleKind::Parentheses : StyleKind::Hyphen;
69+
const StringRef Username =
70+
ParsedStyle == StyleKind::Parentheses ? Matches[3] : Matches[5];
71+
const StringRef Comment = Matches[6];
3372

34-
if (!Username.empty())
73+
if (!Username.empty() &&
74+
(ParsedStyle == StyleKind::Parentheses || !Comment.empty())) {
3575
return false;
76+
}
3677

37-
const std::string NewText =
38-
("// TODO(" + Twine(User) + "): " + Comment).str();
78+
if (Username.empty()) {
79+
Check.diag(Range.getBegin(), "missing username/bug in TODO")
80+
<< FixItHint::CreateReplacement(
81+
CharSourceRange::getCharRange(Range),
82+
createReplacementString(Username, Comment));
83+
}
84+
85+
if (Comment.empty())
86+
Check.diag(Range.getBegin(), "missing details in TODO");
3987

40-
Check.diag(Range.getBegin(), "missing username/bug in TODO")
41-
<< FixItHint::CreateReplacement(CharSourceRange::getCharRange(Range),
42-
NewText);
4388
return false;
4489
}
4590

91+
std::string createReplacementString(const StringRef Username,
92+
const StringRef Comment) const {
93+
if (TodoStyle == StyleKind::Parentheses) {
94+
return ("// TODO(" + Twine(User) +
95+
"): " + (Comment.empty() ? "some details" : Comment))
96+
.str();
97+
}
98+
return ("// TODO: " + Twine(User) + " - " +
99+
(Comment.empty() ? "some details" : Comment))
100+
.str();
101+
}
102+
103+
StyleKind getTodoStyle() const { return TodoStyle; }
104+
46105
private:
47106
TodoCommentCheck &Check;
48107
std::string User;
49108
llvm::Regex TodoMatch;
109+
StyleKind TodoStyle = StyleKind::Hyphen;
50110
};
51111

52112
TodoCommentCheck::TodoCommentCheck(StringRef Name, ClangTidyContext *Context)
@@ -62,4 +122,8 @@ void TodoCommentCheck::registerPPCallbacks(const SourceManager &SM,
62122
PP->addCommentHandler(Handler.get());
63123
}
64124

125+
void TodoCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
126+
Options.store(Opts, "Style", Handler->getTodoStyle());
127+
}
128+
65129
} // namespace clang::tidy::google::readability

‎clang-tools-extra/clang-tidy/google/TodoCommentCheck.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ class TodoCommentCheck : public ClangTidyCheck {
2727
void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
2828
Preprocessor *ModuleExpanderPP) override;
2929

30+
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
31+
3032
private:
3133
class TodoCommentHandler;
3234
std::unique_ptr<TodoCommentHandler> Handler;

‎clang-tools-extra/docs/ReleaseNotes.rst‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,10 @@ Changes in existing checks
406406
<clang-tidy/checks/google/readability-casting>` check by adding fix-it
407407
notes for downcasts.
408408

409+
- Improved :doc:`google-readability-todo
410+
<clang-tidy/checks/google/readability-todo>` check to accept the new TODO
411+
format from the Google Style Guide.
412+
409413
- Improved :doc:`llvm-prefer-isa-or-dyn-cast-in-conditionals
410414
<clang-tidy/checks/llvm/prefer-isa-or-dyn-cast-in-conditionals>` check:
411415

‎clang-tools-extra/docs/clang-tidy/checks/google/readability-todo.rst‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,14 @@ The relevant style guide section is
99
https://google.github.io/styleguide/cppguide.html#TODO_Comments.
1010

1111
Corresponding cpplint.py check: `readability/todo`
12+
13+
Options
14+
-------
15+
16+
.. option:: Style
17+
18+
A string specifying the TODO style for fix-it hints. Accepted values are
19+
`Hyphen` and `Parentheses`. Default is `Hyphen`.
20+
21+
* `Hyphen` will format the fix-it as: ``// TODO: username - details``.
22+
* `Parentheses` will format the fix-it as: ``// TODO(username): details``.

‎clang-tools-extra/test/clang-tidy/checkers/google/readability-todo-hyphen.cpp‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// RUN: %check_clang_tidy %s google-readability-todo %t -- -config="{User: 'some user'}" --
2+
3+
// TODOfix this1
4+
// CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing username/bug in TODO
5+
// CHECK-FIXES: // TODO: some user - fix this1
6+
7+
// TODO fix this2
8+
// CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing username/bug in TODO
9+
// CHECK-FIXES: // TODO: some user - fix this2
10+
11+
// TODO fix this3
12+
// CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing username/bug in TODO
13+
// CHECK-FIXES: // TODO: some user - fix this3
14+
15+
// TODO: fix this4
16+
// CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing username/bug in TODO
17+
// CHECK-FIXES: // TODO: some user - fix this4
18+
19+
// TODO: bug 12345 -
20+
// CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing details in TODO
21+
22+
// TODO: a message without a reference
23+
// CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing username/bug in TODO
24+
// CHECK-FIXES: // TODO: some user - a message without a reference
25+
26+
// TODO(clang)fix this5
27+
28+
// TODO: foo - shave yaks
29+
// TODO:foo - no space bewteen semicolon and username
30+
// TODO: foo- no space bewteen username and dash
31+
// TODO: foo - extra spaces between semicolon and username
32+
// TODO: foo - extra spaces between username and dash
33+
// TODO: b/12345 - use a b/ prefix
34+
// TODO: bug 12345 - use a space in username/bug reference
35+
// TODO(foo):shave yaks
36+
// TODO(bar):
37+
// TODO(foo): paint bikeshed
38+
// TODO(b/12345): find the holy grail
39+
// TODO (b/12345): allow spaces before parentheses
40+
// TODO(asdf) allow missing semicolon

‎clang-tools-extra/test/clang-tidy/checkers/google/readability-todo.cpp‎ renamed to ‎clang-tools-extra/test/clang-tidy/checkers/google/readability-todo-parentheses.cpp‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %check_clang_tidy %s google-readability-todo %t -- -config="{User: 'some user'}" --
1+
// RUN: %check_clang_tidy %s google-readability-todo %t -- -config="{User: 'some user', CheckOptions: {google-readability-todo.Style: 'Parentheses'}}" --
22

33
// TODOfix this1
44
// CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing username/bug in TODO
@@ -16,8 +16,22 @@
1616
// CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing username/bug in TODO
1717
// CHECK-FIXES: // TODO(some user): fix this4
1818

19+
// TODO: bug 12345 -
20+
// CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing details in TODO
21+
22+
// TODO: a message without a reference
23+
// CHECK-MESSAGES: [[@LINE-1]]:1: warning: missing username/bug in TODO
24+
// CHECK-FIXES: // TODO(some user): a message without a reference
25+
1926
// TODO(clang)fix this5
2027

28+
// TODO: foo - shave yaks
29+
// TODO:foo - no space bewteen semicolon and username
30+
// TODO: foo- no space bewteen username and dash
31+
// TODO: foo - extra spaces between semicolon and username
32+
// TODO: foo - extra spaces between username and dash
33+
// TODO: b/12345 - use a b/ prefix
34+
// TODO: bug 12345 - use a space in username/bug reference
2135
// TODO(foo):shave yaks
2236
// TODO(bar):
2337
// TODO(foo): paint bikeshed

0 commit comments

Comments
 (0)