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 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( - Method->getNextDeclInContext())) { + if (const auto *Next = + dyn_cast_or_null(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(Arg)) || + (CommentIntegerLiterals && isa(Arg)) || + (CommentFloatLiterals && isa(Arg)) || + (CommentUserDefinedLiterals && isa(Arg)) || + (CommentCharacterLiterals && isa(Arg)) || + (CommentStringLiterals && isa(Arg->IgnoreImpCasts())) || + (CommentNullPtrs && isa(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"); 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 + ` now supports + `CommentBoolLiterals`, `CommentIntegerLiterals`, `CommentFloatLiterals`, + `CommentUserDefiniedLiterals`, `CommentStringLiterals`, + `CommentCharacterLiterals` & `CommentNullPtrs` options. + Improvements to include-fixer ----------------------------- 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: 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); +}