diff --git a/clang-tools-extra/clang-tidy/misc/ArgumentCommentCheck.cpp b/clang-tools-extra/clang-tidy/misc/ArgumentCommentCheck.cpp new file mode 100644 index 000000000000..4ef9f8f1b821 --- /dev/null +++ b/clang-tools-extra/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> +ArgumentCommentCheck::getCommentsInRange(ASTContext *Ctx, SourceRange Range) { + std::vector> Comments; + auto &SM = Ctx->getSourceManager(); + std::pair 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 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 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 Args) { + Callee = Callee->getFirstDecl(); + for (unsigned i = 0, + e = std::min(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 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"); + if (auto Call = dyn_cast(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(E); + checkCallArgs( + Result.Context, Construct->getConstructor(), + Construct->getParenOrBraceRange().getBegin(), + llvm::makeArrayRef(Construct->getArgs(), Construct->getNumArgs())); + } +} + +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/misc/ArgumentCommentCheck.h b/clang-tools-extra/clang-tidy/misc/ArgumentCommentCheck.h new file mode 100644 index 000000000000..8180a2ce9fdc --- /dev/null +++ b/clang-tools-extra/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 Params, StringRef ArgName, + unsigned ArgIndex); + std::vector> + getCommentsInRange(ASTContext *Ctx, SourceRange Range); + void checkCallArgs(ASTContext *Ctx, const FunctionDecl *Callee, + SourceLocation ArgBeginLoc, + llvm::ArrayRef Args); +}; + +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_ARG_COMMENT_CHECK_H diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt index 5a60a35dd6d6..8c9b5dd3c0bc 100644 --- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt @@ -1,6 +1,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyMiscModule + ArgumentCommentCheck.cpp MiscTidyModule.cpp LINK_LIBS diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp index e262dc356d09..f7f657addfa3 100644 --- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "ArgumentCommentCheck.h" namespace clang { namespace tidy { @@ -17,6 +18,9 @@ namespace tidy { class MiscModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.addCheckFactory( + "misc-argument-comment", + new ClangTidyCheckFactory()); } }; diff --git a/clang-tools-extra/test/clang-tidy/arg-comments.cpp b/clang-tools-extra/test/clang-tidy/arg-comments.cpp new file mode 100644 index 000000000000..32b95767bd22 --- /dev/null +++ b/clang-tools-extra/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 diff --git a/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt b/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt index 2778571789d2..5d29b0f9e8f0 100644 --- a/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt +++ b/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt @@ -8,7 +8,8 @@ include_directories(${CLANG_LINT_SOURCE_DIR}) add_extra_unittest(ClangTidyTests LLVMModuleTest.cpp - GoogleModuleTest.cpp) + GoogleModuleTest.cpp + MiscModuleTest.cpp) target_link_libraries(ClangTidyTests clangAST @@ -18,5 +19,6 @@ target_link_libraries(ClangTidyTests clangTidy clangTidyGoogleModule clangTidyLLVMModule + clangTidyMiscModule clangTooling ) diff --git a/clang-tools-extra/unittests/clang-tidy/Makefile b/clang-tools-extra/unittests/clang-tidy/Makefile index c5b9823eb0f7..79832262c447 100644 --- a/clang-tools-extra/unittests/clang-tidy/Makefile +++ b/clang-tools-extra/unittests/clang-tidy/Makefile @@ -14,7 +14,7 @@ TESTNAME = ClangTidy LINK_COMPONENTS := asmparser bitreader support MC MCParser option \ TransformUtils USEDLIBS = clangTidy.a clangTidyLLVMModule.a clangTidyGoogleModule.a \ - clangTidy.a \ + clangTidyMiscModule.a clangTidy.a \ clangStaticAnalyzerFrontend.a clangStaticAnalyzerCheckers.a \ clangStaticAnalyzerCore.a \ clangFormat.a clangTooling.a clangFrontend.a clangSerialization.a \ diff --git a/clang-tools-extra/unittests/clang-tidy/MiscModuleTest.cpp b/clang-tools-extra/unittests/clang-tidy/MiscModuleTest.cpp new file mode 100644 index 000000000000..c26783dc7240 --- /dev/null +++ b/clang-tools-extra/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(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( + "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( + "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