MyDeveloperDay updated this revision to Diff 185385.
MyDeveloperDay marked 6 inline comments as done.
MyDeveloperDay added a comment.
Address review comments
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57674/new/
https://reviews.llvm.org/D57674
Files:
clang-tidy/bugprone/ArgumentCommentCheck.cpp
clang-tidy/bugprone/ArgumentCommentCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/bugprone-argument-comment.rst
test/clang-tidy/bugprone-argument-comment-literals.cpp
Index: test/clang-tidy/bugprone-argument-comment-literals.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-argument-comment-literals.cpp
@@ -0,0 +1,107 @@
+// RUN: %check_clang_tidy %s bugprone-argument-comment %t -- \
+// RUN: -config="{CheckOptions: [{key: CommentBoolLiterals, value: 1},{key: CommentIntegerLiterals, value: 1}, {key: CommentFloatLiterals, value: 1}, {key: CommentUserDefinedLiterals, value: 1}, {key: CommentStringLiterals, value: 1}, {key: CommentNullPtrs, value: 1}, {key: CommentCharacterLiterals, value: 1}]}" --
+
+struct A {
+ void foo(bool abc);
+ void foo(bool abc, bool cde);
+ void foo(const char *, bool abc);
+ void foo(int iabc);
+ void foo(float fabc);
+ void foo(double dabc);
+ void foo(const char *strabc);
+ void fooW(const wchar_t *wstrabc);
+ void fooPtr(A *ptrabc);
+ void foo(char chabc);
+};
+
+double operator"" _km(long double);
+
+void test() {
+ A a;
+
+ a.foo(true);
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+ // CHECK-FIXES: a.foo(/*abc=*/true);
+
+ a.foo(false);
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+ // CHECK-FIXES: a.foo(/*abc=*/false);
+
+ a.foo(true, false);
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+ // CHECK-MESSAGES: [[@LINE-2]]:15: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+ // CHECK-FIXES: a.foo(/*abc=*/true, /*cde=*/false);
+
+ a.foo(false, true);
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+ // CHECK-MESSAGES: [[@LINE-2]]:16: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+ // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+ a.foo(/*abc=*/false, true);
+ // CHECK-MESSAGES: [[@LINE-1]]:24: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+ // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+ a.foo(false, /*cde=*/true);
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+ // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+ bool val1 = true;
+ bool val2 = false;
+ a.foo(val1, val2);
+
+ a.foo("", true);
+ // CHECK-MESSAGES: [[@LINE-1]]:13: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+ // CHECK-FIXES: a.foo("", /*abc=*/true);
+
+ a.foo(0);
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'iabc' [bugprone-argument-comment]
+ // CHECK-FIXES: a.foo(/*iabc=*/0);
+
+ a.foo(1.0f);
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'fabc' [bugprone-argument-comment]
+ // CHECK-FIXES: a.foo(/*fabc=*/1.0f);
+
+ a.foo(1.0);
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'dabc' [bugprone-argument-comment]
+ // CHECK-FIXES: a.foo(/*dabc=*/1.0);
+
+ int val3 = 10;
+ a.foo(val3);
+
+ float val4 = 10.0;
+ a.foo(val4);
+
+ double val5 = 10.0;
+ a.foo(val5);
+
+ a.foo("Hello World");
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'strabc' [bugprone-argument-comment]
+ // CHECK-FIXES: a.foo(/*strabc=*/"Hello World");
+ //
+ a.fooW(L"Hello World");
+ // CHECK-MESSAGES: [[@LINE-1]]:10: warning: argument comment missing for literal argument 'wstrabc' [bugprone-argument-comment]
+ // CHECK-FIXES: a.fooW(/*wstrabc=*/L"Hello World");
+
+ a.fooPtr(nullptr);
+ // CHECK-MESSAGES: [[@LINE-1]]:12: warning: argument comment missing for literal argument 'ptrabc' [bugprone-argument-comment]
+ // CHECK-FIXES: a.fooPtr(/*ptrabc=*/nullptr);
+
+ a.foo(402.0_km);
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'dabc' [bugprone-argument-comment]
+ // CHECK-FIXES: a.foo(/*dabc=*/402.0_km);
+
+ a.foo('A');
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'chabc' [bugprone-argument-comment]
+ // CHECK-FIXES: a.foo(/*chabc=*/'A');
+}
+
+void f(bool _with_underscores_);
+void ignores_underscores() {
+ f(false);
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument comment missing for literal argument '_with_underscores_' [bugprone-argument-comment]
+ // CHECK-FIXES: f(/*_with_underscores_=*/false);
+
+ f(true);
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument comment missing for literal argument
+ // CHECK-FIXES: f(/*_with_underscores_=*/true);
+}
Index: docs/clang-tidy/checks/bugprone-argument-comment.rst
===================================================================
--- docs/clang-tidy/checks/bugprone-argument-comment.rst
+++ docs/clang-tidy/checks/bugprone-argument-comment.rst
@@ -27,3 +27,158 @@
When zero (default value), the check will ignore leading and trailing
underscores and case when comparing names -- otherwise they are taken into
account.
+
+.. option:: CommentBoolLiterals
+
+ When true, the check will add argument comments in the format
+ ``/*ParameterName=*/`` right before the boolean literal argument.
+
+Before:
+
+.. code-block:: c++
+
+ void foo(bool TurnKey, bool PressButton);
+
+ foo(true, false);
+
+After:
+
+.. code-block:: c++
+
+ void foo(bool TurnKey, bool PressButton);
+
+ foo(/*TurnKey=*/true, /*PressButton=*/false);
+
+.. option:: CommentIntegerLiterals
+
+ When true, the check will add argument comments in the format
+ ``/*ParameterName=*/`` right before the integer literal argument.
+
+Before:
+
+.. code-block:: c++
+
+ void foo(int MeaningOfLife);
+
+ foo(42);
+
+After:
+
+.. code-block:: c++
+
+ void foo(int MeaningOfLife);
+
+ foo(/*MeaningOfLife=*/42);
+
+.. option:: CommentFloatLiterals
+
+ When true, the check will add argument comments in the format
+ ``/*ParameterName=*/`` right before the float/double literal argument.
+
+Before:
+
+.. code-block:: c++
+
+ void foo(float Pi);
+
+ foo(3.14159);
+
+After:
+
+.. code-block:: c++
+
+ void foo(float Pi);
+
+ foo(/*Pi=*/3.14159);
+
+.. option:: CommentStringLiterals
+
+ When true, the check will add argument comments in the format
+ ``/*ParameterName=*/`` right before the string literal argument.
+
+Before:
+
+.. code-block:: c++
+
+ void foo(const char *String);
+ void foo(const wchar_t *WideString);
+
+ foo("Hello World");
+ foo(L"Hello World");
+
+After:
+
+.. code-block:: c++
+
+ void foo(const char *String);
+ void foo(const wchar_t *WideString);
+
+ foo(/*String=*/"Hello World");
+ foo(/*WideString=*/L"Hello World");
+
+.. option:: CommentCharacterLiterals
+
+ When true, the check will add argument comments in the format
+ ``/*ParameterName=*/`` right before the character literal argument.
+
+Before:
+
+.. code-block:: c++
+
+ void foo(char *Character);
+
+ foo('A');
+
+After:
+
+.. code-block:: c++
+
+ void foo(char *Character);
+
+ foo(/*Character=*/'A');
+
+.. option:: CommentUserDefinedLiterals
+
+ When true, the check will add argument comments in the format
+ ``/*ParameterName=*/`` right before the user defined literal argument.
+
+Before:
+
+.. code-block:: c++
+
+ void foo(double Distance);
+
+ double operator"" _km(long double);
+
+ foo(402.0_km);
+
+After:
+
+.. code-block:: c++
+
+ void foo(double Distance);
+
+ double operator"" _km(long double);
+
+ foo(/*Distance=*/402.0_km);
+
+.. option:: CommentNullPtrs
+
+ When true, the check will add argument comments in the format
+ ``/*ParameterName=*/`` right before the nullptr literal argument.
+
+Before:
+
+.. code-block:: c++
+
+ void foo(A* Value);
+
+ foo(nullptr);
+
+After:
+
+.. code-block:: c++
+
+ void foo(A* Value);
+
+ foo(/*Value=*/nullptr);
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -86,6 +86,12 @@
Checks whether there are underscores in googletest test and test case names in
test macros, which is prohibited by the Googletest FAQ.
+- The :doc:`bugprone-argument-comment
+ <clang-tidy/checks/bugprone-argument-comment>` now supports
+ `CommentBoolLiterals`, `CommentIntegerLiterals`, `CommentFloatLiterals`,
+ `CommentUserDefiniedLiterals`, `CommentStringLiterals`,
+ `CommentCharacterLiterals` & `CommentNullPtrs` options.
+
Improvements to include-fixer
-----------------------------
Index: clang-tidy/bugprone/ArgumentCommentCheck.h
===================================================================
--- clang-tidy/bugprone/ArgumentCommentCheck.h
+++ clang-tidy/bugprone/ArgumentCommentCheck.h
@@ -26,7 +26,8 @@
///
/// ...
/// f(/*bar=*/true);
-/// // warning: argument name 'bar' in comment does not match parameter name 'foo'
+/// // warning: argument name 'bar' in comment does not match parameter name
+/// 'foo'
/// \endcode
///
/// The check tries to detect typos and suggest automated fixes for them.
@@ -40,11 +41,20 @@
private:
const bool StrictMode;
+ const unsigned CommentBoolLiterals : 1;
+ const unsigned CommentIntegerLiterals : 1;
+ const unsigned CommentFloatLiterals : 1;
+ const unsigned CommentStringLiterals : 1;
+ const unsigned CommentUserDefinedLiterals : 1;
+ const unsigned CommentCharacterLiterals : 1;
+ const unsigned CommentNullPtrs : 1;
llvm::Regex IdentRE;
void checkCallArgs(ASTContext *Ctx, const FunctionDecl *Callee,
SourceLocation ArgBeginLoc,
llvm::ArrayRef<const Expr *> Args);
+
+ bool shouldAddComment(const Expr *Arg) const;
};
} // namespace bugprone
Index: clang-tidy/bugprone/ArgumentCommentCheck.cpp
===================================================================
--- clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -1,4 +1,3 @@
-//===--- ArgumentCommentCheck.cpp - clang-tidy ----------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -7,11 +6,11 @@
//===----------------------------------------------------------------------===//
#include "ArgumentCommentCheck.h"
+#include "../utils/LexerUtils.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
#include "clang/Lex/Token.h"
-#include "../utils/LexerUtils.h"
using namespace clang::ast_matchers;
@@ -23,17 +22,37 @@
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
StrictMode(Options.getLocalOrGlobal("StrictMode", 0) != 0),
+ CommentBoolLiterals(Options.getLocalOrGlobal("CommentBoolLiterals", 0) !=
+ 0),
+ CommentIntegerLiterals(
+ Options.getLocalOrGlobal("CommentIntegerLiterals", 0) != 0),
+ CommentFloatLiterals(
+ Options.getLocalOrGlobal("CommentFloatLiterals", 0) != 0),
+ CommentStringLiterals(
+ Options.getLocalOrGlobal("CommentStringLiterals", 0) != 0),
+ CommentUserDefinedLiterals(
+ Options.getLocalOrGlobal("CommentUserDefinedLiterals", 0) != 0),
+ CommentCharacterLiterals(
+ Options.getLocalOrGlobal("CommentCharacterLiterals", 0) != 0),
+ CommentNullPtrs(Options.getLocalOrGlobal("CommentNullPtrs", 0) != 0),
IdentRE("^(/\\* *)([_A-Za-z][_A-Za-z0-9]*)( *= *\\*/)$") {}
void ArgumentCommentCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "StrictMode", StrictMode);
+ Options.store(Opts, "CommentBoolLiterals", CommentBoolLiterals);
+ Options.store(Opts, "CommentIntegerLiterals", CommentIntegerLiterals);
+ Options.store(Opts, "CommentFloatLiterals", CommentFloatLiterals);
+ Options.store(Opts, "CommentStringLiterals", CommentStringLiterals);
+ Options.store(Opts, "CommentUserDefinedLiterals", CommentUserDefinedLiterals);
+ Options.store(Opts, "CommentCharacterLiterals", CommentCharacterLiterals);
+ Options.store(Opts, "CommentNullPtrs", CommentNullPtrs);
}
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.
+ // NewCallback's arguments relate to the pointed function,
+ // don't check them against NewCallback's parameter names.
// FIXME: Make this configurable.
unless(hasDeclaration(functionDecl(
hasAnyName("NewCallback", "NewPermanentCallback")))))
@@ -126,8 +145,8 @@
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.
+ // 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 = ArgNameLower.edit_distance(II->getName().lower(),
/*AllowReplacements=*/true,
ThisED + Threshold);
@@ -180,8 +199,8 @@
}
return nullptr;
}
- if (const auto *Next = dyn_cast_or_null<CXXMethodDecl>(
- Method->getNextDeclInContext())) {
+ if (const auto *Next =
+ dyn_cast_or_null<CXXMethodDecl>(Method->getNextDeclInContext())) {
if (looksLikeExpectMethod(Next) && areMockAndExpectMethods(Method, Next))
return Method;
}
@@ -206,6 +225,18 @@
return Func;
}
+// Given the argument type and the options determine if we should
+// be adding an argument comment.
+bool ArgumentCommentCheck::shouldAddComment(const Expr *Arg) const {
+ return (CommentBoolLiterals && isa<CXXBoolLiteralExpr>(Arg)) ||
+ (CommentIntegerLiterals && isa<IntegerLiteral>(Arg)) ||
+ (CommentFloatLiterals && isa<FloatingLiteral>(Arg)) ||
+ (CommentUserDefinedLiterals && isa<UserDefinedLiteral>(Arg)) ||
+ (CommentCharacterLiterals && isa<CharacterLiteral>(Arg)) ||
+ (CommentStringLiterals && isa<StringLiteral>(Arg->IgnoreImpCasts())) ||
+ (CommentNullPtrs && isa<CXXNullPtrLiteralExpr>(Arg->IgnoreImpCasts()));
+}
+
void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx,
const FunctionDecl *OriginalCallee,
SourceLocation ArgBeginLoc,
@@ -277,8 +308,19 @@
}
}
}
+
+ // If the argument comments are missing for literals add them.
+ if (Comments.empty() && shouldAddComment(Args[I])) {
+ std::string ArgComment =
+ (llvm::Twine("/*") + II->getName() + "=*/").str();
+ DiagnosticBuilder Diag =
+ diag(Args[I]->getBeginLoc(),
+ "argument comment missing for literal argument %0")
+ << II
+ << FixItHint::CreateInsertion(Args[I]->getBeginLoc(), ArgComment);
+ }
}
-}
+} // namespace bugprone
void ArgumentCommentCheck::check(const MatchFinder::MatchResult &Result) {
const auto *E = Result.Nodes.getNodeAs<Expr>("expr");
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
