[PATCH] Add an argument comment checker to clang-tidy.
Peter Collingbourne
peter at pcc.me.uk
Sun Mar 2 04:25:45 PST 2014
Hi alexfh,
This checks that parameters named in comments that appear before arguments in
function and constructor calls match the parameter name used in the callee's
declaration. For example:
void f(int x, int y);
void g() {
f(/*y=*/0, /*z=*/0);
}
contains two violations of the policy, as the names 'x' and 'y' used in the
declaration do not match names 'y' and 'z' used at the call site.
I think there is significant value in being able to check/enforce this policy
as a way of guarding against accidental API misuse and silent breakages
caused by API changes.
Although this policy is not prescribed by the LLVM coding standards, such
marked up calls do appear somewhat frequently in the LLVM codebase, so I
decided to file this under 'llvm'. There may well be a better (new?) category
for this to live under though.
As a side note, I wrote the original version of this checker about 2 years ago
as a Clang plugin:
http://git.pcc.me.uk/?p=~peter/llvm-arg-checker.git;a=summary
but never got around to upstreaming it, mainly because there was no good place
to put it at the time. Now that clang-tidy exists I no longer have that excuse.
http://llvm-reviews.chandlerc.com/D2914
Files:
clang-tidy/llvm/LLVMTidyModule.cpp
clang-tidy/llvm/LLVMTidyModule.h
test/clang-tidy/llvm-arg-comments.cpp
Index: clang-tidy/llvm/LLVMTidyModule.cpp
===================================================================
--- clang-tidy/llvm/LLVMTidyModule.cpp
+++ clang-tidy/llvm/LLVMTidyModule.cpp
@@ -51,6 +51,109 @@
Fix);
}
+void ArgumentCommentCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(callExpr(unless(operatorCallExpr())).bind("expr"), this);
+ Finder->addMatcher(constructExpr().bind("expr"), this);
+}
+
+std::vector<std::pair<SourceLocation, StringRef>>
+ArgumentCommentCheck::getCommentsInRange(ASTContext *Ctx, SourceRange Range) {
+ std::vector<std::pair<SourceLocation, StringRef>> Comments;
+ auto &SM = Ctx->getSourceManager();
+ std::pair<FileID, unsigned> BeginLoc = SM.getDecomposedLoc(Range.getBegin()),
+ EndLoc = SM.getDecomposedLoc(Range.getEnd());
+
+ if (BeginLoc.first != EndLoc.first)
+ return Comments;
+
+ bool Invalid = false;
+ StringRef Buffer = SM.getBufferData(BeginLoc.first, &Invalid);
+ if (Invalid)
+ return Comments;
+
+ const char *StrData = Buffer.data() + BeginLoc.second;
+
+ Lexer TheLexer(SM.getLocForStartOfFile(BeginLoc.first), Ctx->getLangOpts(),
+ Buffer.begin(), StrData, Buffer.end());
+ TheLexer.SetCommentRetentionState(true);
+
+ while (1) {
+ Token Tok;
+ if (TheLexer.LexFromRawLexer(Tok))
+ break;
+ if (Tok.getLocation() == Range.getEnd() || Tok.getKind() == tok::eof)
+ break;
+
+ if (Tok.getKind() == tok::comment) {
+ std::pair<FileID, unsigned> CommentLoc =
+ SM.getDecomposedLoc(Tok.getLocation());
+ assert(CommentLoc.first == BeginLoc.first);
+ Comments.emplace_back(
+ Tok.getLocation(),
+ StringRef(Buffer.begin() + CommentLoc.second, Tok.getLength()));
+ }
+ }
+
+ return Comments;
+}
+
+void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx,
+ const FunctionDecl *Callee,
+ SourceLocation ArgBeginLoc,
+ llvm::ArrayRef<const Expr *> Args) {
+ Callee = Callee->getFirstDecl();
+ for (unsigned i = 0,
+ e = std::min<unsigned>(Args.size(), Callee->getNumParams());
+ i != e; ++i) {
+ const ParmVarDecl *PVD = Callee->getParamDecl(i);
+ IdentifierInfo *II = PVD->getIdentifier();
+ if (!II)
+ continue;
+
+ SourceLocation BeginSLoc, EndSLoc = Args[i]->getLocStart();
+ if (i == 0)
+ BeginSLoc = ArgBeginLoc;
+ else
+ BeginSLoc = Args[i - 1]->getLocEnd();
+ if (BeginSLoc.isMacroID() || EndSLoc.isMacroID())
+ continue;
+
+ auto Comments =
+ getCommentsInRange(Ctx, SourceRange(BeginSLoc, EndSLoc));
+ for (auto Comment : Comments) {
+ llvm::SmallVector<StringRef, 2> Matches;
+ if (IdentRE.match(Comment.second, &Matches)) {
+ if (Matches[1] != II->getName()) {
+ diag(Comment.first,
+ "argument name '%0' in comment does not match parameter name %1")
+ << Matches[1] << II;
+ diag(PVD->getLocation(), "%0 declared here", DiagnosticIDs::Note)
+ << II;
+ }
+ }
+ }
+ }
+}
+
+void ArgumentCommentCheck::check(const MatchFinder::MatchResult &Result) {
+ const Expr *E = Result.Nodes.getNodeAs<Expr>("expr");
+ if (auto CE = dyn_cast<CallExpr>(E)) {
+ auto FD = CE->getDirectCallee();
+ if (!FD)
+ return;
+
+ checkCallArgs(
+ Result.Context, FD, CE->getCallee()->getLocEnd(),
+ llvm::ArrayRef<const Expr *>(CE->getArgs(), CE->getNumArgs()));
+ } else {
+ auto CCE = cast<CXXConstructExpr>(E);
+ checkCallArgs(
+ Result.Context, CCE->getConstructor(),
+ CCE->getParenOrBraceRange().getBegin(),
+ llvm::ArrayRef<const Expr *>(CCE->getArgs(), CCE->getNumArgs()));
+ }
+}
+
namespace {
class IncludeOrderPPCallbacks : public PPCallbacks {
public:
@@ -81,6 +184,9 @@
void
addCheckFactories(ClangTidyCheckFactories &CheckFactories) LLVM_OVERRIDE {
CheckFactories.addCheckFactory(
+ "llvm-argument-comment",
+ new ClangTidyCheckFactory<ArgumentCommentCheck>());
+ CheckFactories.addCheckFactory(
"llvm-include-order", new ClangTidyCheckFactory<IncludeOrderCheck>());
CheckFactories.addCheckFactory(
"llvm-namespace-comment",
Index: clang-tidy/llvm/LLVMTidyModule.h
===================================================================
--- clang-tidy/llvm/LLVMTidyModule.h
+++ clang-tidy/llvm/LLVMTidyModule.h
@@ -11,6 +11,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_LLVM_TIDY_MODULE_H
#include "../ClangTidy.h"
+#include "llvm/Support/Regex.h"
namespace clang {
namespace tidy {
@@ -33,6 +34,22 @@
check(const ast_matchers::MatchFinder::MatchResult &Result) LLVM_OVERRIDE;
};
+/// \brief Checks that argument comments match parameter names.
+class ArgumentCommentCheck : public ClangTidyCheck {
+ llvm::Regex IdentRE{ "^/\\* *([_A-Za-z][_A-Za-z0-9]*) *= *\\*/$" };
+
+ std::vector<std::pair<SourceLocation, StringRef>>
+ getCommentsInRange(ASTContext *Ctx, SourceRange Range);
+ void checkCallArgs(ASTContext *Ctx, const FunctionDecl *Callee,
+ SourceLocation ArgBeginLoc,
+ llvm::ArrayRef<const Expr *> Args);
+
+public:
+ void registerMatchers(ast_matchers::MatchFinder *Finder) LLVM_OVERRIDE;
+ void
+ check(const ast_matchers::MatchFinder::MatchResult &Result) LLVM_OVERRIDE;
+};
+
} // namespace tidy
} // namespace clang
Index: test/clang-tidy/llvm-arg-comments.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/llvm-arg-comments.cpp
@@ -0,0 +1,12 @@
+// RUN: clang-tidy --checks=llvm-argument-comment %s -- > %t.cpp
+// RUN: FileCheck -input-file=%t.cpp %s
+
+void f(int x, int y);
+
+void g() {
+ // CHECK: 11:5: warning: argument name 'y' in comment does not match parameter name 'x'
+ // CHECK: 4:12: note: 'x' declared here
+ // CHECK: 11:14: warning: argument name 'z' in comment does not match parameter name 'y'
+ // CHECK: 4:19: note: 'y' declared here
+ f(/*y=*/0, /*z=*/0);
+}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2914.1.patch
Type: text/x-patch
Size: 6171 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140302/b94d04a6/attachment.bin>
More information about the cfe-commits
mailing list