Fix FN passedByValue with array access, range-based for (#4922)

* Fix FN passedByValue with array access, range-based for

* Format

* Fix/suppress new warnings

Author: chrchr-github

gui/checkthread.cpp

@@ -45,7 +45,7 @@
 #include <QSettings>
 
 // NOLINTNEXTLINE(performance-unnecessary-value-param) - used as callback so we need to preserve the signature
-static bool executeCommand(std::string exe, std::vector<std::string> args, std::string redirect, std::string &output)
+static bool executeCommand(std::string exe, std::vector<std::string> args, std::string redirect, std::string &output) // cppcheck-suppress passedByValue
 {
     output.clear();
 

gui/mainwindow.cpp

@@ -1158,7 +1158,7 @@ void MainWindow::checkConfiguration()
         analyzeProject(mProjectFile, false, true);
 }
 
-void MainWindow::reAnalyzeSelected(QStringList files)
+void MainWindow::reAnalyzeSelected(const QStringList& files)
 {
     if (files.empty())
         return;

gui/mainwindow.h

@@ -251,7 +251,7 @@ private slots:
      * @brief Reanalyze selected files
      * @param files list of selected files
      */
-    void reAnalyzeSelected(QStringList files);
+    void reAnalyzeSelected(const QStringList& files);
 
     /**
      * @brief Analyze the project.

lib/checkother.cpp

@@ -1176,6 +1176,14 @@ static int estimateSize(const Type* type, const Settings* settings, const Symbol
     });
 }
 
+static bool isConstRangeBasedFor(const Token* tok) {
+    if (astIsRangeBasedForDecl(tok)) {
+        const Variable* loopVar = tok->astParent()->astOperand1()->variable();
+        return loopVar && (!loopVar->isReference() || loopVar->isConst());
+    }
+    return false;
+}
+
 static bool canBeConst(const Variable *var, const Settings* settings)
 {
     if (!var->scope())
@@ -1201,6 +1209,8 @@ static bool canBeConst(const Variable *var, const Settings* settings)
             continue;
 
         const Token* parent = tok2->astParent();
+        while (Token::simpleMatch(parent, "["))
+            parent = parent->astParent();
         if (!parent)
             continue;
         if (Token::simpleMatch(tok2->next(), ";") && tok2->next()->isSplittedVarDeclEq()) {
@@ -1249,9 +1259,12 @@ static bool canBeConst(const Variable *var, const Settings* settings)
                    (parent->astOperand2() && settings->library.isFunctionConst(parent->astOperand2())))
             continue;
         else if (parent->isAssignmentOp()) {
-            if (parent->astOperand1() == tok2)
+            const Token* assignee = parent->astOperand1();
+            while (Token::simpleMatch(assignee, "["))
+                assignee = assignee->astOperand1();
+            if (assignee == tok2)
                 return false;
-            const Variable* assignedVar = parent->astOperand1() ? parent->astOperand1()->variable() : nullptr;
+            const Variable* assignedVar = assignee ? assignee->variable() : nullptr;
             if (assignedVar &&
                 !assignedVar->isConst() &&
                 assignedVar->isReference() &&
@@ -1263,7 +1276,9 @@ static bool canBeConst(const Variable *var, const Settings* settings)
                 continue;
             else
                 return false;
-        } else
+        } else if (isConstRangeBasedFor(tok2))
+            continue;
+        else
             return false;
     }
 

lib/importproject.cpp

@@ -274,7 +274,7 @@ static std::string unescape(const std::string &in)
     return out;
 }
 
-void ImportProject::FileSettings::parseCommand(std::string command)
+void ImportProject::FileSettings::parseCommand(const std::string& command)
 {
     std::string defs;
 

lib/importproject.h

@@ -78,7 +78,7 @@ class CPPCHECKLIB ImportProject {
         bool msc;
         bool useMfc;
 
-        void parseCommand(std::string command);
+        void parseCommand(const std::string& command);
         void setDefines(std::string defs);
         void setIncludePaths(const std::string &basepath, const std::list<std::string> &in, std::map<std::string, std::string, cppcheck::stricmp> &variables);
     };

test/testother.cpp

@@ -1993,6 +1993,36 @@ class TestOther : public TestFixture {
               "}\n");
         ASSERT_EQUALS("", errout.str());
 
+        check("int x(int);\n"
+              "void f(std::vector<int> v, int& j) {\n"
+              "    for (int i : v)\n"
+              "        j = i;\n"
+              "}\n"
+              "void fn(std::vector<int> v) {\n"
+              "    for (int& i : v)\n"
+              "        i = x(i);\n"
+              "}\n"
+              "void g(std::vector<int> v, int& j) {\n"
+              "    for (int i = 0; i < v.size(); ++i)\n"
+              "        j = v[i];\n"
+              "}\n"
+              "void gn(std::vector<int> v) {\n"
+              "    for (int i = 0; i < v.size(); ++i)\n"
+              "        v[i] = x(i);\n"
+              "}\n"
+              "void h(std::vector<std::vector<int>> v, int& j) {\n"
+              "    for (int i = 0; i < v.size(); ++i)\n"
+              "        j = v[i][0];\n"
+              "}\n"
+              "void hn(std::vector<std::vector<int>> v) {\n"
+              "    for (int i = 0; i < v.size(); ++i)\n"
+              "        v[i][0] = x(i);\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2]: (performance) Function parameter 'v' should be passed by const reference.\n"
+                      "[test.cpp:10]: (performance) Function parameter 'v' should be passed by const reference.\n"
+                      "[test.cpp:18]: (performance) Function parameter 'v' should be passed by const reference.\n",
+                      errout.str());
+
         Settings settings1;
         PLATFORM(settings1.platform, cppcheck::Platform::Type::Win64);
         check("using ui64 = unsigned __int64;\n"
@@ -2194,7 +2224,7 @@ class TestOther : public TestFixture {
               "    const int& i = x[0];\n"
               "    return i;\n"
               "}");
-        ASSERT_EQUALS("", errout.str());
+        ASSERT_EQUALS("[test.cpp:1]: (performance) Function parameter 'x' should be passed by const reference.\n", errout.str());
 
         check("int f(std::vector<int> x) {\n"
               "    static int& i = x[0];\n"