alexfh updated this revision to Diff 66806.
alexfh added a comment.
- Added a FIXME.
https://reviews.llvm.org/D23135
Files:
clang-tidy/ClangTidy.h
clang-tidy/misc/ArgumentCommentCheck.cpp
clang-tidy/misc/ArgumentCommentCheck.h
docs/clang-tidy/checks/misc-argument-comment.rst
test/clang-tidy/misc-argument-comment-strict.cpp
test/clang-tidy/misc-argument-comment.cpp
unittests/clang-tidy/MiscModuleTest.cpp
Index: unittests/clang-tidy/MiscModuleTest.cpp
===================================================================
--- unittests/clang-tidy/MiscModuleTest.cpp
+++ unittests/clang-tidy/MiscModuleTest.cpp
@@ -23,10 +23,10 @@
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) {
Index: test/clang-tidy/misc-argument-comment.cpp
===================================================================
--- test/clang-tidy/misc-argument-comment.cpp
+++ test/clang-tidy/misc-argument-comment.cpp
@@ -36,13 +36,18 @@
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);
}
#define FALSE 0
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);
+}
Index: test/clang-tidy/misc-argument-comment-strict.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-argument-comment-strict.cpp
@@ -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);
+}
Index: docs/clang-tidy/checks/misc-argument-comment.rst
===================================================================
--- docs/clang-tidy/checks/misc-argument-comment.rst
+++ docs/clang-tidy/checks/misc-argument-comment.rst
@@ -18,3 +18,7 @@
// 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.
Index: clang-tidy/misc/ArgumentCommentCheck.h
===================================================================
--- clang-tidy/misc/ArgumentCommentCheck.h
+++ clang-tidy/misc/ArgumentCommentCheck.h
@@ -37,8 +37,10 @@
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,
Index: clang-tidy/misc/ArgumentCommentCheck.cpp
===================================================================
--- clang-tidy/misc/ArgumentCommentCheck.cpp
+++ clang-tidy/misc/ArgumentCommentCheck.cpp
@@ -22,16 +22,21 @@
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 @@
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,54 +106,63 @@
// 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;
}
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;
if (auto Template = Callee->getTemplateInstantiationPattern()) {
// 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,15 +178,15 @@
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;
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(),
Index: clang-tidy/ClangTidy.h
===================================================================
--- clang-tidy/ClangTidy.h
+++ clang-tidy/ClangTidy.h
@@ -73,6 +73,23 @@
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,
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
