Address review comments.
Hi alexfh,
http://llvm-reviews.chandlerc.com/D2914
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D2914?vs=7584&id=7871#toc
Files:
clang-tidy/misc/ArgumentCommentCheck.cpp
clang-tidy/misc/ArgumentCommentCheck.h
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
test/clang-tidy/arg-comments.cpp
unittests/clang-tidy/CMakeLists.txt
unittests/clang-tidy/MiscModuleTest.cpp
Index: clang-tidy/misc/ArgumentCommentCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/ArgumentCommentCheck.cpp
@@ -0,0 +1,166 @@
+//===--- ArgumentCommentCheck.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 "ArgumentCommentCheck.h"
+#include "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Token.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+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 (true) {
+ 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;
+}
+
+bool
+ArgumentCommentCheck::isLikelyTypo(llvm::ArrayRef<ParmVarDecl *> Params,
+ StringRef ArgName, unsigned ArgIndex) {
+ std::string ArgNameLower = ArgName.lower();
+ unsigned UpperBound = (ArgName.size() + 2) / 3 + 1;
+ unsigned ThisED = StringRef(ArgNameLower).edit_distance(
+ Params[ArgIndex]->getIdentifier()->getName().lower(),
+ /*AllowReplacements=*/true, UpperBound);
+ if (ThisED >= UpperBound)
+ return false;
+
+ for (const auto &Param : Params) {
+ if (&Param - Params.begin() == ArgIndex)
+ continue;
+ IdentifierInfo *II = Param->getIdentifier();
+ if (!II)
+ continue;
+
+ const unsigned Threshold = 2;
+ // Other parameters must be an edit distance at least Threshold more away
+ // from this parameter. This gives us greater confidence that this is a typo
+ // of this parameter and not one with a similar name.
+ unsigned OtherED = StringRef(ArgNameLower).edit_distance(
+ II->getName().lower(),
+ /*AllowReplacements=*/true, ThisED + Threshold);
+ if (OtherED < ThisED + Threshold)
+ return false;
+ }
+
+ return true;
+}
+
+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;
+
+ for (auto Comment :
+ getCommentsInRange(Ctx, SourceRange(BeginSLoc, EndSLoc))) {
+ llvm::SmallVector<StringRef, 2> Matches;
+ if (IdentRE.match(Comment.second, &Matches)) {
+ if (Matches[2] != II->getName()) {
+ {
+ DiagnosticBuilder Diag =
+ diag(Comment.first, "argument name '%0' in comment does not "
+ "match parameter name %1")
+ << Matches[2] << II;
+ if (isLikelyTypo(Callee->parameters(), Matches[2], i)) {
+ Diag << FixItHint::CreateReplacement(
+ Comment.first,
+ (Matches[1] + II->getName() + Matches[3]).str());
+ }
+ }
+ 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 Call = dyn_cast<CallExpr>(E)) {
+ const FunctionDecl *Callee = Call->getDirectCallee();
+ if (!Callee)
+ return;
+
+ checkCallArgs(Result.Context, Callee, Call->getCallee()->getLocEnd(),
+ llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()));
+ } else {
+ auto Construct = cast<CXXConstructExpr>(E);
+ checkCallArgs(
+ Result.Context, Construct->getConstructor(),
+ Construct->getParenOrBraceRange().getBegin(),
+ llvm::makeArrayRef(Construct->getArgs(), Construct->getNumArgs()));
+ }
+}
+
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/ArgumentCommentCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/ArgumentCommentCheck.h
@@ -0,0 +1,40 @@
+//===--- ArgumentCommentCheck.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_MISC_ARG_COMMENT_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_ARG_COMMENT_CHECK_H
+
+#include "../ClangTidy.h"
+#include "llvm/Support/Regex.h"
+
+namespace clang {
+namespace tidy {
+
+/// \brief Checks that argument comments match parameter names.
+class ArgumentCommentCheck : public ClangTidyCheck {
+public:
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ llvm::Regex IdentRE{ "^(/\\* *)([_A-Za-z][_A-Za-z0-9]*)( *= *\\*/)$" };
+
+ bool isLikelyTypo(llvm::ArrayRef<ParmVarDecl *> Params, StringRef ArgName,
+ unsigned ArgIndex);
+ 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);
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_ARG_COMMENT_CHECK_H
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -1,6 +1,7 @@
set(LLVM_LINK_COMPONENTS support)
add_clang_library(clangTidyMiscModule
+ ArgumentCommentCheck.cpp
MiscTidyModule.cpp
LINK_LIBS
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -10,13 +10,17 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "ArgumentCommentCheck.h"
namespace clang {
namespace tidy {
class MiscModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.addCheckFactory(
+ "misc-argument-comment",
+ new ClangTidyCheckFactory<ArgumentCommentCheck>());
}
};
Index: test/clang-tidy/arg-comments.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/arg-comments.cpp
@@ -0,0 +1,21 @@
+// RUN: clang-tidy --checks=misc-argument-comment %s -- | FileCheck %s
+
+// FIXME: clang-tidy should provide a -verify mode to make writing these checks
+// easier and more accurate.
+
+// CHECK-NOT: warning
+
+void f(int x, int y);
+
+void ffff(int xxxx, int yyyy);
+
+void g() {
+ // CHECK: [[@LINE+5]]:5: warning: argument name 'y' in comment does not match parameter name 'x'
+ // CHECK: :8:12: note: 'x' declared here
+ // CHECK: [[@LINE+3]]:14: warning: argument name 'z' in comment does not match parameter name 'y'
+ // CHECK: :8:19: note: 'y' declared here
+ // CHECK-NOT: warning
+ f(/*y=*/0, /*z=*/0);
+}
+
+// CHECK-NOT: warning
Index: unittests/clang-tidy/CMakeLists.txt
===================================================================
--- unittests/clang-tidy/CMakeLists.txt
+++ unittests/clang-tidy/CMakeLists.txt
@@ -8,7 +8,8 @@
add_extra_unittest(ClangTidyTests
LLVMModuleTest.cpp
- GoogleModuleTest.cpp)
+ GoogleModuleTest.cpp
+ MiscModuleTest.cpp)
target_link_libraries(ClangTidyTests
clangAST
@@ -18,5 +19,6 @@
clangTidy
clangTidyGoogleModule
clangTidyLLVMModule
+ clangTidyMiscModule
clangTooling
)
Index: unittests/clang-tidy/MiscModuleTest.cpp
===================================================================
--- /dev/null
+++ unittests/clang-tidy/MiscModuleTest.cpp
@@ -0,0 +1,41 @@
+#include "ClangTidyTest.h"
+#include "google/GoogleTidyModule.h"
+#include "misc/ArgumentCommentCheck.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tidy {
+namespace test {
+
+#define EXPECT_NO_CHANGES(Check, Code) \
+ EXPECT_EQ(Code, runCheckOnCode<Check>(Code))
+
+TEST(ArgumentCommentCheckTest, CorrectComments) {
+ EXPECT_NO_CHANGES(ArgumentCommentCheck,
+ "void f(int x, int y); void g() { f(/*x=*/0, /*y=*/0); }");
+ EXPECT_NO_CHANGES(ArgumentCommentCheck,
+ "struct C { C(int x, int y); }; C c(/*x=*/0, /*y=*/0);");
+}
+
+TEST(ArgumentCommentCheckTest, ThisEditDistanceAboveThreshold) {
+ EXPECT_NO_CHANGES(ArgumentCommentCheck,
+ "void f(int xxx); void g() { f(/*xyz=*/0); }");
+}
+
+TEST(ArgumentCommentCheckTest, OtherEditDistanceAboveThreshold) {
+ EXPECT_EQ("void f(int xxx, int yyy); void g() { f(/*xxx=*/0, 0); }",
+ runCheckOnCode<ArgumentCommentCheck>(
+ "void f(int xxx, int yyy); void g() { f(/*Xxx=*/0, 0); }"));
+ EXPECT_EQ("struct C { C(int xxx, int yyy); }; C c(/*xxx=*/0, 0);",
+ runCheckOnCode<ArgumentCommentCheck>(
+ "struct C { C(int xxx, int yyy); }; C c(/*Xxx=*/0, 0);"));
+}
+
+TEST(ArgumentCommentCheckTest, OtherEditDistanceBelowThreshold) {
+ EXPECT_NO_CHANGES(ArgumentCommentCheck,
+ "void f(int xxx, int yyy); void g() { f(/*xxy=*/0, 0); }");
+}
+
+} // namespace test
+} // namespace tidy
+} // namespace clang
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
