Index: clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tidy/misc/CMakeLists.txt +++ clang-tidy/misc/CMakeLists.txt @@ -30,6 +30,7 @@ StringConstructorCheck.cpp StringIntegerAssignmentCheck.cpp StringLiteralWithEmbeddedNulCheck.cpp + SuspiciousCallArgumentCheck.cpp SuspiciousMissingCommaCheck.cpp SuspiciousSemicolonCheck.cpp SuspiciousStringCompareCheck.cpp Index: clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tidy/misc/MiscTidyModule.cpp +++ clang-tidy/misc/MiscTidyModule.cpp @@ -38,6 +38,7 @@ #include "StringConstructorCheck.h" #include "StringIntegerAssignmentCheck.h" #include "StringLiteralWithEmbeddedNulCheck.h" +#include "SuspiciousCallArgumentCheck.h" #include "SuspiciousMissingCommaCheck.h" #include "SuspiciousSemicolonCheck.h" #include "SuspiciousStringCompareCheck.h" @@ -110,6 +111,10 @@ "misc-string-constructor"); CheckFactories.registerCheck( "misc-string-integer-assignment"); + CheckFactories.registerCheck( + "misc-suspicious-call-argument"); + CheckFactories.registerCheck( + "misc-suspicious-semicolon"); CheckFactories.registerCheck( "misc-string-literal-with-embedded-nul"); CheckFactories.registerCheck( @@ -117,7 +122,7 @@ CheckFactories.registerCheck( "misc-suspicious-semicolon"); CheckFactories.registerCheck( - "misc-suspicious-string-compare"); + "misc-suspicious-string-compare"); CheckFactories.registerCheck( "misc-swapped-arguments"); CheckFactories.registerCheck( Index: clang-tidy/misc/SuspiciousCallArgumentCheck.h =================================================================== --- /dev/null +++ clang-tidy/misc/SuspiciousCallArgumentCheck.h @@ -0,0 +1,37 @@ +//===--- SuspiciousCallArgumentCheck.h - clang-tidy--------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SUSPICIOUS_CALL_ARGUMENT_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SUSPICIOUS_CALL_ARGUMENT_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { + +/// This checker finds those function calls where the function arguments are +/// provided in an incorrect order. It compares the name of the given variable +/// to the argument name in the function definition. +/// It issues a message if the given variable name is similar to an another +/// function argument in a function call. It uses case insensitive comparison. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/misc-suspicious-call-argument.html +class SuspiciousCallArgumentCheck : public ClangTidyCheck { +public: + SuspiciousCallArgumentCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SUSPICIOUS_CALL_ARGUMENT_H Index: clang-tidy/misc/SuspiciousCallArgumentCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/misc/SuspiciousCallArgumentCheck.cpp @@ -0,0 +1,248 @@ +//===--- SuspiciousCallArgumentCheck.cpp - clang-tidy----------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "SuspiciousCallArgumentCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include + +using namespace clang::ast_matchers; + +namespace { + +// Struct to store information about the relation of params and args. +struct ComparisonInfo { + bool Equal; + int LevenshteinDistance; + int Prefix; + int Suffix; + int ArgLength; + int ParamLength; + + ComparisonInfo(bool Eq = false, int Ld = 32767, int Pr = 0, int Su = 0, + int Al = 0, int Pl = 0) + : Equal(Eq), LevenshteinDistance(Ld), Prefix(Pr), Suffix(Su), + ArgLength(Al), ParamLength(Pl) {} +}; + +bool checkSimilarity(ComparisonInfo LeftArgToRightParam, + ComparisonInfo LeftArgToLeftParam, + ComparisonInfo RightArgToRightParam, + ComparisonInfo RightArgToLeftParam) { + + if (LeftArgToLeftParam.Equal || RightArgToRightParam.Equal) + return false; + + // Left to right. + + // Can't compare short arguments + if (LeftArgToLeftParam.ArgLength >= 3) { + // Equality. + if (LeftArgToRightParam.Equal) + return true; + + // Prefix, suffix. + bool LeftSimilarToRight = + LeftArgToRightParam.Prefix > 0 || LeftArgToRightParam.Suffix > 0; + + bool LeftSimilarToLeft = + LeftArgToLeftParam.Prefix > 0 || LeftArgToLeftParam.Suffix > 0; + + // Levenshtein. + bool LeftArgIsNearest = LeftArgToRightParam.LevenshteinDistance < + LeftArgToLeftParam.LevenshteinDistance && + LeftArgToRightParam.LevenshteinDistance < + RightArgToRightParam.LevenshteinDistance && + LeftArgToRightParam.LevenshteinDistance < + std::min(3, LeftArgToRightParam.ArgLength / 2); + + if ((LeftSimilarToRight || LeftArgIsNearest) && !LeftSimilarToLeft) + return true; + } + + // Right to left. + + // Can't compare short arguments + if (RightArgToRightParam.ArgLength >= 3) { + // Equality. + if (RightArgToLeftParam.Equal) + return true; + + // Prefix, suffix. + bool RightSimilarToLeft = + RightArgToLeftParam.Prefix > 0 || RightArgToLeftParam.Suffix > 0; + bool RightSimilarToRight = + RightArgToRightParam.Prefix > 0 || RightArgToRightParam.Suffix > 0; + + // Levenshtein. + bool RightArgIsNearest = RightArgToLeftParam.LevenshteinDistance < + LeftArgToLeftParam.LevenshteinDistance && + RightArgToLeftParam.LevenshteinDistance < + RightArgToRightParam.LevenshteinDistance && + RightArgToLeftParam.LevenshteinDistance < + std::min(3, RightArgToLeftParam.ArgLength / 2); + + if ((RightSimilarToLeft || RightArgIsNearest) && !RightSimilarToRight) + return true; + } + + return false; +} + +int prefixMatch(StringRef Arg, StringRef Param) { + + if (!(Arg.size() >= 3 && Param.size() >= 3)) + return 0; + + StringRef Shorter = Arg.size() < Param.size() ? Arg : Param; + StringRef Longer = Arg.size() >= Param.size() ? Arg : Param; + + if (Longer.startswith_lower(Shorter)) + return Shorter.size(); + + return 0; +} + +int suffixMatch(StringRef Arg, StringRef Param) { + + if (!(Arg.size() >= 3 && Param.size() >= 3)) + return 0; + + StringRef Shorter = Arg.size() < Param.size() ? Arg : Param; + StringRef Longer = Arg.size() >= Param.size() ? Arg : Param; + + if (Longer.endswith_lower(Shorter)) + return Shorter.size(); + + return 0; +} + +} // namespace + +namespace clang { +namespace tidy { + +void SuspiciousCallArgumentCheck::registerMatchers(MatchFinder *Finder) { + // Only match calls with at least 2 argument. + Finder->addMatcher( + callExpr(unless(argumentCountIs(0)), unless(argumentCountIs(1))) + .bind("functionCall"), + this); +} + +void SuspiciousCallArgumentCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *MatchedCallExpr = + Result.Nodes.getNodeAs("functionCall"); + + const Decl *CalleeDecl = MatchedCallExpr->getCalleeDecl(); + if (!CalleeDecl) + return; + + const FunctionDecl *CalleeFuncDecl = CalleeDecl->getAsFunction(); + if (!CalleeFuncDecl) + return; + + // Lambda functions first argument are themself. + int ExtraArgIndex = 0; + + if (const auto *MethodDecl = dyn_cast(CalleeFuncDecl)) { + if (MethodDecl->getParent()->isLambda()) + ExtraArgIndex = 1; + } + + // Get param identifiers. + SmallVector ParamNames; + for (unsigned i = 0, e = CalleeFuncDecl->getNumParams(); i != e; ++i) { + const ParmVarDecl *PVD = CalleeFuncDecl->getParamDecl(i); + IdentifierInfo *II = PVD->getIdentifier(); + if (!II) + return; + + ParamNames.push_back(II->getName()); + } + + if (ParamNames.empty()) + return; + + // Get arg names. + SmallVector Args; + for (int i = 0 + ExtraArgIndex, j = MatchedCallExpr->getNumArgs(); i < j; + i++) { + if (const auto *DeclRef = dyn_cast( + MatchedCallExpr->getArg(i)->IgnoreImpCasts()->IgnoreParens())) { + if (const auto *Var = dyn_cast(DeclRef->getDecl())) { + Args.push_back(Var->getName()); + continue; + } + } + + Args.push_back(StringRef()); + } + + if (Args.empty()) + return; + + // In case of variadic functions. + unsigned MatrixSize = ParamNames.size(); + + // Create infomatrix. + SmallVector, 8> InfoMatrix; + for (unsigned i = 0; i < MatrixSize; i++) { + InfoMatrix.push_back(SmallVector()); + for (unsigned j = 0; j < MatrixSize; j++) { + if (Args[i].empty()) { + InfoMatrix[i].push_back(ComparisonInfo()); + continue; + } + + InfoMatrix[i].push_back( + ComparisonInfo(Args[i].equals_lower(ParamNames[j]), + Args[i].edit_distance(ParamNames[j]), + prefixMatch(Args[i], ParamNames[j]), + suffixMatch(Args[i], ParamNames[j]), Args[i].size(), + ParamNames[j].size())); + } + } + + // Check similarity. + for (unsigned i = 0; i < MatrixSize; i++) { + for (unsigned j = i + 1; j < MatrixSize; j++) { + if (checkSimilarity(InfoMatrix[i][j], InfoMatrix[i][i], InfoMatrix[j][j], + InfoMatrix[j][i])) { + StringRef WarningMessages[3] = { + StringRef("%0 (%1) is swapped with %2 (%3)."), + StringRef("literal (%1) is potentially swapped with %2 (%3)."), + StringRef("%0 (%1) is potentially swapped with literal (%3).")}; + StringRef Message = WarningMessages[0]; + + if (Args[i].empty()) { + Message = WarningMessages[1]; + } else if (Args[j].empty()) { + Message = WarningMessages[2]; + } + + // TODO: Underline swapped arguments. + // Warning at the function call. + diag(MatchedCallExpr->getLocStart(), Message.str()) + << Args[i] << ParamNames[i] << Args[j] << ParamNames[j]; + + // Note at the functions declaration. + diag(CalleeFuncDecl->getLocStart(), "%0 is declared here:", + DiagnosticIDs::Note) + << CalleeFuncDecl->getNameInfo().getName().getAsString(); + + return; + } + } + } +} + +} // namespace tidy +} // namespace clang Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -75,6 +75,7 @@ misc-string-constructor misc-string-integer-assignment misc-string-literal-with-embedded-nul + misc-suspicious-call-argument misc-suspicious-missing-comma misc-suspicious-semicolon misc-suspicious-string-compare Index: docs/clang-tidy/checks/misc-suspicious-call-argument.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/misc-suspicious-call-argument.rst @@ -0,0 +1,9 @@ +misc-suspicious-call-argument +============================= + +This checker finds those function calls where the function arguments are +provided in an incorrect order. It compares the name of the given variable +to the argument name in the function definition. + +It issues a message if the given variable name is similar to an another +function argument in a function call. It uses case insensitive comparison. Index: test/clang-tidy/misc-suspicious-call-argument.cpp =================================================================== --- /dev/null +++ test/clang-tidy/misc-suspicious-call-argument.cpp @@ -0,0 +1,106 @@ +// RUN: %check_clang_tidy %s misc-suspicious-call-argument %t -- -- -std=c++11 + +// For equality test. +void set(int *buff, int value, int count) { + while (count > 0) { + *buff = value; + ++buff; + --count; + } +} + +// For equality test. +void foo(int *array, int count) { + // Equality test. + set(array, count, 10); + // CHECK-MESSAGES: :[[@LINE-1]]:4: warning: count (value) is potentially swapped with literal (count). [misc-suspicious-call-argument] +} + +// For equality test. +void f(int b, int aaa) {} + +// For prefix/suffix test. +int uselessFoo(int valToRet, int uselessParam) { return valToRet; } + +// For variadic function test. +double average(int count, int fixParam1, int fixParam2, ...) { return 0.0; } + +void fooo(int asdasdasd, int bbb, int ccc = 1, int ddd = 0) {} + +class TestClass { +public: + void thisFunction(int integerParam, int thisIsPARAM) {} +}; + +int main() { + int asdasdasd, qweqweqweq, cccccc, ddd = 0; + + fooo(asdasdasd, cccccc); + // CHECK-MESSAGES: :[[@LINE-1]]:2: warning: cccccc (bbb) is potentially swapped with literal (ccc). [misc-suspicious-call-argument] + + const int MAX = 15; + int *array = new int[MAX]; + int count = 5; + + // Equality test 1. + foo(array, count); + + // Equality test 2. + double avg = average(1, 2, count, 3, 4, 5, 6, + 7); // "count" should be the first argument + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: literal (count) is potentially swapped with count (fixParam2). [misc-suspicious-call-argument] + + // Equality test 3. + int aa, aaa = 0; + f(aa, aaa); // should pass (0. arg is similar to 1. param, but 1. arg is equal to 1. param) + + // Equality test 4. + int fixParam1 = 5; + avg = average(count, 1, 2, 3, 4, fixParam1, 6, + 7); // should pass, we only check params and args before "..." + + // Levenshtein test 1. + int cooundt = 4; + set(array, cooundt, 10); // "cooundt" is similar to "count" + // CHECK-MESSAGES: :[[@LINE-1]]:2: warning: cooundt (value) is potentially swapped with literal (count). [misc-suspicious-call-argument] + + // Levenshtein test 2. + int _value = 3; + set(array, 5, _value); // "_value" is similar to "value" + // CHECK-MESSAGES: :[[@LINE-1]]:2: warning: literal (value) is potentially swapped with _value (count). [misc-suspicious-call-argument] + + // Prefix test 1. + int val = 1; + int tmp = uselessFoo(5, val); // "val" is a prefix of "valToRet" + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: literal (valToRet) is potentially swapped with val (uselessParam). [misc-suspicious-call-argument] + + // Prefix test 2. + int VALtoretAwesome = 1; + tmp = uselessFoo( + 5, VALtoretAwesome); // "valToRet" is a prefix of "VALtoretAwesome" + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: literal (valToRet) is potentially swapped with VALtoretAwesome (uselessParam). [misc-suspicious-call-argument] + + // Suffix test 1. + int param = 1; + int randomValue = 5; + tmp = uselessFoo(param, randomValue); // "param" is a suffix of "uselessParam" + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: param (valToRet) is swapped with randomValue (uselessParam). [misc-suspicious-call-argument] + + // Suffix test 2. + int reallyUselessParam = 1; + tmp = uselessFoo(reallyUselessParam, + 5); // "uselessParam" is a suffix of "reallyUselessParam" + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: reallyUselessParam (valToRet) is potentially swapped with literal (uselessParam). [misc-suspicious-call-argument] + + // Test lambda. + auto testMethod = [&](int method, int randomParam) { return 0; }; + int method = 0; + testMethod(method, 0); // Should pass. + + // Member function test + TestClass test; + int integ, thisIsAnArg = 0; + test.thisFunction(integ, thisIsAnArg); // Should pass. + + return 0; +}