From 3fbcc0cd3b38a5ca3ccb393b9bc6f3d8775dca17 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= <daniel.marjamaki@gmail.com>
Date: Thu, 20 Apr 2017 19:57:39 +0200
Subject: [PATCH] Refactor CheckFunctions::invalidFunctionUsage

---
 lib/astutils.cpp       | 19 ++++++++++++++++
 lib/astutils.h         |  6 +++++
 lib/checkfunctions.cpp | 50 +++++++++++++++++++-----------------------
 3 files changed, 47 insertions(+), 28 deletions(-)

diff --git a/lib/astutils.cpp b/lib/astutils.cpp
index a5f31d4d9..08b6d3dc8 100644
--- a/lib/astutils.cpp
+++ b/lib/astutils.cpp
@@ -452,3 +452,22 @@ int numberOfArguments(const Token *start)
     }
     return arguments;
 }
+
+static void getArgumentsRecursive(const Token *tok, std::vector<const Token *> *arguments)
+{
+    if (!tok)
+        return;
+    if (tok->str() == ",") {
+        getArgumentsRecursive(tok->astOperand1(), arguments);
+        getArgumentsRecursive(tok->astOperand2(), arguments);
+    } else {
+        arguments->push_back(tok);
+    }
+}
+
+std::vector<const Token *> getArguments(const Token *ftok)
+{
+    std::vector<const Token *> arguments;
+    getArgumentsRecursive(ftok->next()->astOperand2(), &arguments);
+    return arguments;
+}
diff --git a/lib/astutils.h b/lib/astutils.h
index c4d9254fc..ab2ed6448 100644
--- a/lib/astutils.h
+++ b/lib/astutils.h
@@ -23,6 +23,7 @@
 //---------------------------------------------------------------------------
 
 #include <string>
+#include <vector>
 
 class Settings;
 class Library;
@@ -91,4 +92,9 @@ bool isVariableChanged(const Token *start, const Token *end, const unsigned int
  */
 int numberOfArguments(const Token *start);
 
+/**
+ * Get arguments (AST)
+ */
+std::vector<const Token *> getArguments(const Token *ftok);
+
 #endif // astutilsH
diff --git a/lib/checkfunctions.cpp b/lib/checkfunctions.cpp
index b503cbd57..c18c21ca5 100644
--- a/lib/checkfunctions.cpp
+++ b/lib/checkfunctions.cpp
@@ -21,6 +21,7 @@
 //---------------------------------------------------------------------------
 
 #include "checkfunctions.h"
+#include "astutils.h"
 #include "symboldatabase.h"
 #include <cmath>
 
@@ -87,38 +88,31 @@ void CheckFunctions::invalidFunctionUsage()
     for (std::size_t i = 0; i < functions; ++i) {
         const Scope * scope = symbolDatabase->functionScopes[i];
         for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
-            if (!tok->isName() || !Token::Match(tok, "%name% ( !!)"))
+            if (!Token::Match(tok, "%name% ( !!)"))
                 continue;
             const Token * const functionToken = tok;
-            int argnr = 1;
-            const Token *argtok = tok->tokAt(2);
-            do {
-                if (Token::Match(argtok, "%num% [,)]")) {
-                    if (MathLib::isInt(argtok->str()) &&
-                        !_settings->library.isargvalid(functionToken, argnr, MathLib::toLongNumber(argtok->str())))
-                        invalidFunctionArgError(argtok, functionToken->str(), argnr, _settings->library.validarg(functionToken, argnr));
-                } else {
-                    const Token *top = argtok;
-                    while (top->astParent() && top->astParent()->str() != "," && top->astParent() != tok->next())
-                        top = top->astParent();
-                    const Token *var = top;
-                    while (Token::Match(var, ".|::"))
-                        var = var->astOperand2();
-                    if (Token::Match(top, "%comp%|%oror%|&&|!|true|false") ||
-                        (var && var->variable() && Token::Match(var->variable()->typeStartToken(), "bool|_Bool"))) {
-                        if (_settings->library.isboolargbad(functionToken, argnr))
-                            invalidFunctionArgBoolError(top, functionToken->str(), argnr);
+            const std::vector<const Token *> arguments = getArguments(tok);
+            for (unsigned int argnr = 1; argnr <= arguments.size(); ++argnr) {
+                const Token * const argtok = arguments[argnr-1];
 
-                        // Are the values 0 and 1 valid?
-                        else if (!_settings->library.isargvalid(functionToken, argnr, 0))
-                            invalidFunctionArgError(top, functionToken->str(), argnr, _settings->library.validarg(functionToken, argnr));
-                        else if (!_settings->library.isargvalid(functionToken, argnr, 1))
-                            invalidFunctionArgError(top, functionToken->str(), argnr, _settings->library.validarg(functionToken, argnr));
-                    }
+                // check <valid>...</valid>
+                if (argtok->hasKnownIntValue() &&
+                    !_settings->library.isargvalid(functionToken,argnr,argtok->values().front().intvalue)) {
+                    // TODO: Warn about possible values
+                    invalidFunctionArgError(argtok, functionToken->str(), argnr, _settings->library.validarg(functionToken, argnr));
                 }
-                argnr++;
-                argtok = argtok->nextArgument();
-            } while (argtok && argtok->str() != ")");
+
+                if (astIsBool(argtok)) {
+                    // check <not-bool>
+                    if (_settings->library.isboolargbad(functionToken, argnr))
+                        invalidFunctionArgBoolError(argtok, functionToken->str(), argnr);
+                    // Are the values 0 and 1 valid?
+                    else if (!_settings->library.isargvalid(functionToken, argnr, 0))
+                        invalidFunctionArgError(argtok, functionToken->str(), argnr, _settings->library.validarg(functionToken, argnr));
+                    else if (!_settings->library.isargvalid(functionToken, argnr, 1))
+                        invalidFunctionArgError(argtok, functionToken->str(), argnr, _settings->library.validarg(functionToken, argnr));
+                }
+            }
         }
     }
 }