[clang-tools-extra] r277729 - [clang-tidy] misc-argument-comment non-strict mode
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 4 07:54:54 PDT 2016
Author: alexfh
Date: Thu Aug 4 09:54:54 2016
New Revision: 277729
URL: http://llvm.org/viewvc/llvm-project?rev=277729&view=rev
Log:
[clang-tidy] misc-argument-comment non-strict mode
Summary:
The misc-argument-comment check now ignores leading and trailing underscores and
case. The new `StrictMode` local/global option can be used to switch back to
strict checking.
Add getLocalOrGlobal version for integral types, minor cleanups.
Reviewers: hokein, aaron.ballman
Subscribers: aaron.ballman, Prazek, cfe-commits
Differential Revision: https://reviews.llvm.org/D23135
Added:
clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment-strict.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/ClangTidy.h
clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-argument-comment.rst
clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment.cpp
clang-tools-extra/trunk/unittests/clang-tidy/MiscModuleTest.cpp
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.h?rev=277729&r1=277728&r2=277729&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.h (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.h Thu Aug 4 09:54:54 2016
@@ -73,6 +73,23 @@ public:
return Result;
}
+ /// \brief Read a named option from the ``Context`` and parse it as an
+ /// integral type ``T``.
+ ///
+ /// Reads the option with the check-local name \p LocalName from local or
+ /// global ``CheckOptions``. Gets local option first. If local is not present,
+ /// falls back to get global option. If global option is not present either,
+ /// returns Default.
+ template <typename T>
+ typename std::enable_if<std::is_integral<T>::value, T>::type
+ getLocalOrGlobal(StringRef LocalName, T Default) const {
+ std::string Value = getLocalOrGlobal(LocalName, "");
+ T Result = Default;
+ if (!Value.empty())
+ StringRef(Value).getAsInteger(10, Result);
+ return Result;
+ }
+
/// \brief Stores an option with the check-local name \p LocalName with string
/// value \p Value to \p Options.
void store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
Modified: clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp?rev=277729&r1=277728&r2=277729&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp Thu Aug 4 09:54:54 2016
@@ -22,16 +22,21 @@ namespace misc {
ArgumentCommentCheck::ArgumentCommentCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
+ StrictMode(Options.getLocalOrGlobal("StrictMode", 0) != 0),
IdentRE("^(/\\* *)([_A-Za-z][_A-Za-z0-9]*)( *= *\\*/)$") {}
+void ArgumentCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "StrictMode", StrictMode);
+}
+
void ArgumentCommentCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
callExpr(unless(cxxOperatorCallExpr()),
// NewCallback's arguments relate to the pointed function, don't
// check them against NewCallback's parameter names.
// FIXME: Make this configurable.
- unless(hasDeclaration(functionDecl(anyOf(
- hasName("NewCallback"), hasName("NewPermanentCallback"))))))
+ unless(hasDeclaration(functionDecl(
+ hasAnyName("NewCallback", "NewPermanentCallback")))))
.bind("expr"),
this);
Finder->addMatcher(cxxConstructExpr().bind("expr"), this);
@@ -78,12 +83,13 @@ getCommentsInRange(ASTContext *Ctx, Char
return Comments;
}
-bool
-ArgumentCommentCheck::isLikelyTypo(llvm::ArrayRef<ParmVarDecl *> Params,
- StringRef ArgName, unsigned ArgIndex) {
- std::string ArgNameLower = ArgName.lower();
+bool ArgumentCommentCheck::isLikelyTypo(llvm::ArrayRef<ParmVarDecl *> Params,
+ StringRef ArgName, unsigned ArgIndex) {
+ std::string ArgNameLowerStr = ArgName.lower();
+ StringRef ArgNameLower = ArgNameLowerStr;
+ // The threshold is arbitrary.
unsigned UpperBound = (ArgName.size() + 2) / 3 + 1;
- unsigned ThisED = StringRef(ArgNameLower).edit_distance(
+ unsigned ThisED = ArgNameLower.edit_distance(
Params[ArgIndex]->getIdentifier()->getName().lower(),
/*AllowReplacements=*/true, UpperBound);
if (ThisED >= UpperBound)
@@ -100,9 +106,9 @@ ArgumentCommentCheck::isLikelyTypo(llvm:
// 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);
+ unsigned OtherED = ArgNameLower.edit_distance(II->getName().lower(),
+ /*AllowReplacements=*/true,
+ ThisED + Threshold);
if (OtherED < ThisED + Threshold)
return false;
}
@@ -110,15 +116,24 @@ ArgumentCommentCheck::isLikelyTypo(llvm:
return true;
}
+static bool sameName(StringRef InComment, StringRef InDecl, bool StrictMode) {
+ if (StrictMode)
+ return InComment == InDecl;
+ InComment = InComment.trim('_');
+ InDecl = InDecl.trim('_');
+ // FIXME: compare_lower only works for ASCII.
+ return InComment.compare_lower(InDecl) == 0;
+}
+
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);
+ 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;
@@ -126,28 +141,28 @@ void ArgumentCommentCheck::checkCallArgs
// Don't warn on arguments for parameters instantiated from template
// parameter packs. If we find more arguments than the template definition
// has, it also means that they correspond to a parameter pack.
- if (Template->getNumParams() <= i ||
- Template->getParamDecl(i)->isParameterPack()) {
+ if (Template->getNumParams() <= I ||
+ Template->getParamDecl(I)->isParameterPack()) {
continue;
}
}
CharSourceRange BeforeArgument = CharSourceRange::getCharRange(
- i == 0 ? ArgBeginLoc : Args[i - 1]->getLocEnd(),
- Args[i]->getLocStart());
+ I == 0 ? ArgBeginLoc : Args[I - 1]->getLocEnd(),
+ Args[I]->getLocStart());
BeforeArgument = Lexer::makeFileCharRange(
BeforeArgument, Ctx->getSourceManager(), Ctx->getLangOpts());
for (auto Comment : getCommentsInRange(Ctx, BeforeArgument)) {
llvm::SmallVector<StringRef, 2> Matches;
if (IdentRE.match(Comment.second, &Matches)) {
- if (Matches[2] != II->getName()) {
+ if (!sameName(Matches[2], II->getName(), StrictMode)) {
{
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)) {
+ if (isLikelyTypo(Callee->parameters(), Matches[2], I)) {
Diag << FixItHint::CreateReplacement(
Comment.first,
(Matches[1] + II->getName() + Matches[3]).str());
@@ -163,7 +178,7 @@ void ArgumentCommentCheck::checkCallArgs
void ArgumentCommentCheck::check(const MatchFinder::MatchResult &Result) {
const Expr *E = Result.Nodes.getNodeAs<Expr>("expr");
- if (auto Call = dyn_cast<CallExpr>(E)) {
+ if (const auto *Call = dyn_cast<CallExpr>(E)) {
const FunctionDecl *Callee = Call->getDirectCallee();
if (!Callee)
return;
@@ -171,7 +186,7 @@ void ArgumentCommentCheck::check(const M
checkCallArgs(Result.Context, Callee, Call->getCallee()->getLocEnd(),
llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()));
} else {
- auto Construct = cast<CXXConstructExpr>(E);
+ const auto *Construct = cast<CXXConstructExpr>(E);
checkCallArgs(
Result.Context, Construct->getConstructor(),
Construct->getParenOrBraceRange().getBegin(),
Modified: clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.h?rev=277729&r1=277728&r2=277729&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.h Thu Aug 4 09:54:54 2016
@@ -37,8 +37,10 @@ public:
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap& Opts) override;
private:
+ const bool StrictMode;
llvm::Regex IdentRE;
bool isLikelyTypo(llvm::ArrayRef<ParmVarDecl *> Params, StringRef ArgName,
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-argument-comment.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/misc-argument-comment.rst?rev=277729&r1=277728&r2=277729&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-argument-comment.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-argument-comment.rst Thu Aug 4 09:54:54 2016
@@ -18,3 +18,7 @@ that are placed right before the argumen
// warning: argument name 'bar' in comment does not match parameter name 'foo'
The check tries to detect typos and suggest automated fixes for them.
+
+Supported options:
+ - `StrictMode` (local or global): when non-zero, the check will ignore leading
+ and trailing underscores and case when comparing parameter names.
Added: clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment-strict.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment-strict.cpp?rev=277729&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment-strict.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment-strict.cpp Thu Aug 4 09:54:54 2016
@@ -0,0 +1,19 @@
+// RUN: %check_clang_tidy %s misc-argument-comment %t -- \
+// RUN: -config="{CheckOptions: [{key: StrictMode, value: 1}]}" --
+
+void f(int _with_underscores_);
+void g(int x_);
+void ignores_underscores() {
+ f(/*With_Underscores=*/0);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name 'With_Underscores' in comment does not match parameter name '_with_underscores_'
+// CHECK-FIXES: f(/*_with_underscores_=*/0);
+ f(/*with_underscores=*/1);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name 'with_underscores' in comment does not match parameter name '_with_underscores_'
+// CHECK-FIXES: f(/*_with_underscores_=*/1);
+ f(/*_With_Underscores_=*/2);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name '_With_Underscores_' in comment does not match parameter name '_with_underscores_'
+// CHECK-FIXES: f(/*_with_underscores_=*/2);
+ g(/*X=*/3);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name 'X' in comment does not match parameter name 'x_'
+// CHECK-FIXES: g(/*x_=*/3);
+}
Modified: clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment.cpp?rev=277729&r1=277728&r2=277729&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment.cpp Thu Aug 4 09:54:54 2016
@@ -36,8 +36,8 @@ void variadic2(int zzz, Args&&... args);
void templates() {
variadic(/*xxx=*/0, /*yyy=*/1);
- variadic2(/*zzZ=*/0, /*xxx=*/1, /*yyy=*/2);
- // CHECK-MESSAGES: [[@LINE-1]]:13: warning: argument name 'zzZ' in comment does not match parameter name 'zzz'
+ variadic2(/*zzU=*/0, /*xxx=*/1, /*yyy=*/2);
+ // CHECK-MESSAGES: [[@LINE-1]]:13: warning: argument name 'zzU' in comment does not match parameter name 'zzz'
// CHECK-FIXES: variadic2(/*zzz=*/0, /*xxx=*/1, /*yyy=*/2);
}
@@ -46,3 +46,8 @@ void qqq(bool aaa);
void f() { qqq(/*bbb=*/FALSE); }
// CHECK-MESSAGES: [[@LINE-1]]:16: warning: argument name 'bbb' in comment does not match parameter name 'aaa'
// CHECK-FIXES: void f() { qqq(/*bbb=*/FALSE); }
+
+void f(bool _with_underscores_);
+void ignores_underscores() {
+ f(/*With_Underscores=*/false);
+}
Modified: clang-tools-extra/trunk/unittests/clang-tidy/MiscModuleTest.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/MiscModuleTest.cpp?rev=277729&r1=277728&r2=277729&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clang-tidy/MiscModuleTest.cpp (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/MiscModuleTest.cpp Thu Aug 4 09:54:54 2016
@@ -23,10 +23,10 @@ TEST(ArgumentCommentCheckTest, ThisEditD
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); }"));
+ "void f(int xxx, int yyy); void g() { f(/*Zxx=*/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);"));
+ "struct C { C(int xxx, int yyy); }; C c(/*Zxx=*/0, 0);"));
}
TEST(ArgumentCommentCheckTest, OtherEditDistanceBelowThreshold) {
More information about the cfe-commits
mailing list