mirror of
https://github.com/capstone-engine/llvm-capstone.git
synced 2024-12-15 04:00:56 +00:00
[clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals
bugprone-argument-comment only supports identifying those comments which do not match the function parameter name This revision add 3 options to adding missing argument comments to literals (granularity on type is added to control verbosity of fixit) ``` CheckOptions: - key: bugprone-argument-comment.CommentBoolLiterals value: '1' - key: bugprone-argument-comment.CommentFloatLiterals value: '1' - key: bugprone-argument-comment.CommentIntegerLiterals value: '1' - key: bugprone-argument-comment.CommentStringLiterals value: '1' - key: bugprone-argument-comment.CommentCharacterLiterals value: '1' - key: bugprone-argument-comment.CommentUserDefinedLiterals value: '1' - key: bugprone-argument-comment.CommentNullPtrs value: '1' ``` After applying these options, literal arguments will be preceded with /*ParameterName=*/ Reviewers: JonasToth, Eugene.Zelenko, alexfh, hokein, aaron.ballman Reviewed By: aaron.ballman, Eugene.Zelenko Differential Revision: https://reviews.llvm.org/D57674 llvm-svn: 353535
This commit is contained in:
parent
68457c1e52
commit
6bfd721571
@ -11,6 +11,7 @@
|
||||
#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 +24,37 @@ ArgumentCommentCheck::ArgumentCommentCheck(StringRef Name,
|
||||
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 +147,8 @@ static bool isLikelyTypo(llvm::ArrayRef<ParmVarDecl *> Params,
|
||||
|
||||
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 +201,8 @@ static const CXXMethodDecl *findMockedMethod(const CXXMethodDecl *Method) {
|
||||
}
|
||||
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 +227,21 @@ static const FunctionDecl *resolveMocks(const FunctionDecl *Func) {
|
||||
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 {
|
||||
if (Arg->getExprLoc().isMacroID())
|
||||
return false;
|
||||
Arg = Arg->IgnoreImpCasts();
|
||||
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)) ||
|
||||
(CommentNullPtrs && isa<CXXNullPtrLiteralExpr>(Arg));
|
||||
}
|
||||
|
||||
void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx,
|
||||
const FunctionDecl *OriginalCallee,
|
||||
SourceLocation ArgBeginLoc,
|
||||
@ -219,7 +255,7 @@ void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx,
|
||||
if (NumArgs == 0)
|
||||
return;
|
||||
|
||||
auto makeFileCharRange = [Ctx](SourceLocation Begin, SourceLocation End) {
|
||||
auto MakeFileCharRange = [Ctx](SourceLocation Begin, SourceLocation End) {
|
||||
return Lexer::makeFileCharRange(CharSourceRange::getCharRange(Begin, End),
|
||||
Ctx->getSourceManager(),
|
||||
Ctx->getLangOpts());
|
||||
@ -242,7 +278,7 @@ void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx,
|
||||
}
|
||||
|
||||
CharSourceRange BeforeArgument =
|
||||
makeFileCharRange(ArgBeginLoc, Args[I]->getBeginLoc());
|
||||
MakeFileCharRange(ArgBeginLoc, Args[I]->getBeginLoc());
|
||||
ArgBeginLoc = Args[I]->getEndLoc();
|
||||
|
||||
std::vector<std::pair<SourceLocation, StringRef>> Comments;
|
||||
@ -250,7 +286,7 @@ void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx,
|
||||
Comments = getCommentsInRange(Ctx, BeforeArgument);
|
||||
} else {
|
||||
// Fall back to parsing back from the start of the argument.
|
||||
CharSourceRange ArgsRange = makeFileCharRange(
|
||||
CharSourceRange ArgsRange = MakeFileCharRange(
|
||||
Args[I]->getBeginLoc(), Args[NumArgs - 1]->getEndLoc());
|
||||
Comments = getCommentsBeforeLoc(Ctx, ArgsRange.getBegin());
|
||||
}
|
||||
@ -277,8 +313,19 @@ void ArgumentCommentCheck::checkCallArgs(ASTContext *Ctx,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// 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");
|
||||
|
@ -26,7 +26,8 @@ namespace bugprone {
|
||||
///
|
||||
/// ...
|
||||
/// 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.
|
||||
@ -39,12 +40,21 @@ public:
|
||||
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
|
||||
|
||||
private:
|
||||
const bool StrictMode;
|
||||
const unsigned StrictMode : 1;
|
||||
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
|
||||
|
@ -92,6 +92,12 @@ Improvements to clang-tidy
|
||||
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
|
||||
-----------------------------
|
||||
|
||||
|
@ -27,3 +27,158 @@ Options
|
||||
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);
|
||||
|
@ -0,0 +1,124 @@
|
||||
// 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);
|
||||
};
|
||||
|
||||
#define FOO 1
|
||||
|
||||
void g(int a);
|
||||
void h(double b);
|
||||
void i(const char *c);
|
||||
|
||||
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');
|
||||
|
||||
g(FOO);
|
||||
h(1.0f);
|
||||
// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument comment missing for literal argument 'b' [bugprone-argument-comment]
|
||||
// CHECK-FIXES: h(/*b=*/1.0f);
|
||||
i(__FILE__);
|
||||
|
||||
// FIXME Would like the below to add argument comments.
|
||||
g((1));
|
||||
// FIXME But we should not add argument comments here.
|
||||
g(_Generic(0, int : 0));
|
||||
}
|
||||
|
||||
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);
|
||||
}
|
Loading…
Reference in New Issue
Block a user