varjujan created this revision.
varjujan added a reviewer: alexfh.
varjujan added subscribers: xazax.hun, cfe-commits.
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.
http://reviews.llvm.org/D20689
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/SuspiciousCallArgumentCheck.cpp
clang-tidy/misc/SuspiciousCallArgumentCheck.h
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-suspicious-call-argument.rst
test/clang-tidy/misc-suspicious-call-argument.cpp
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;
+}
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: 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: 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 <algorithm>
+
+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<CallExpr>("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<CXXMethodDecl>(CalleeFuncDecl)) {
+ if (MethodDecl->getParent()->isLambda())
+ ExtraArgIndex = 1;
+ }
+
+ // Get param identifiers.
+ SmallVector<StringRef, 8> 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<StringRef, 8> Args;
+ for (int i = 0 + ExtraArgIndex, j = MatchedCallExpr->getNumArgs(); i < j;
+ i++) {
+ if (const auto *DeclRef = dyn_cast<DeclRefExpr>(
+ MatchedCallExpr->getArg(i)->IgnoreImpCasts()->IgnoreParens())) {
+ if (const auto *Var = dyn_cast<VarDecl>(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<SmallVector<ComparisonInfo, 8>, 8> InfoMatrix;
+ for (unsigned i = 0; i < MatrixSize; i++) {
+ InfoMatrix.push_back(SmallVector<ComparisonInfo, 8>());
+ 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: 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,14 +111,18 @@
"misc-string-constructor");
CheckFactories.registerCheck<StringIntegerAssignmentCheck>(
"misc-string-integer-assignment");
+ CheckFactories.registerCheck<SuspiciousCallArgumentCheck>(
+ "misc-suspicious-call-argument");
+ CheckFactories.registerCheck<SuspiciousSemicolonCheck>(
+ "misc-suspicious-semicolon");
CheckFactories.registerCheck<StringLiteralWithEmbeddedNulCheck>(
"misc-string-literal-with-embedded-nul");
CheckFactories.registerCheck<SuspiciousMissingCommaCheck>(
"misc-suspicious-missing-comma");
CheckFactories.registerCheck<SuspiciousSemicolonCheck>(
"misc-suspicious-semicolon");
CheckFactories.registerCheck<SuspiciousStringCompareCheck>(
- "misc-suspicious-string-compare");
+ "misc-suspicious-string-compare");
CheckFactories.registerCheck<SwappedArgumentsCheck>(
"misc-swapped-arguments");
CheckFactories.registerCheck<ThrowByValueCatchByReferenceCheck>(
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
