Fixed #1388 (enum token/template parameter confusion - False positive)

lib/tokenize.cpp

@@ -5042,6 +5042,111 @@ void Tokenizer::simplifyNestedStrcat()
 
 }
 
+void Tokenizer::duplicateEnumError(const Token * tok1, const Token * tok2, const std::string & type)
+{
+    if (!(_settings && _settings->_checkCodingStyle))
+        return;
+
+    std::list<ErrorLogger::ErrorMessage::FileLocation> locationList;
+    ErrorLogger::ErrorMessage::FileLocation loc;
+    loc.line = tok1->linenr();
+    loc.file = file(tok1);
+    locationList.push_back(loc);
+    loc.line = tok2->linenr();
+    loc.file = file(tok2);
+    locationList.push_back(loc);
+
+    const ErrorLogger::ErrorMessage errmsg(locationList,
+                                           "style",
+                                           std::string(type + " '" + tok2->str() +
+                                                       "' hides enumerator of same name"),
+                                           "variableHidingEnum");
+
+    if (_errorLogger)
+        _errorLogger->reportErr(errmsg);
+    else
+        Check::reportError(errmsg);
+}
+
+// Check if this statement is a duplicate definition.  A duplicate
+// definition will hide the enumerator within it's scope so just
+// skip the entire scope of the duplicate.
+bool Tokenizer::duplicateDefinition(Token ** tokPtr, const Token * name)
+{
+    // check for an end of definition
+    const Token * tok = *tokPtr;
+    if (tok && tok->next() && Token::Match(tok->next(), ";|,|[|=|)|>"))
+    {
+        const Token * end = tok->next();
+
+        if (end->str() == "[")
+        {
+            end = end->link()->next();
+        }
+        else if (end->str() == ",")
+        {
+            // find end of definition
+            int level = 0;
+            while (end && end->next() && (!Token::Match(end->next(), ";|)|>") ||
+                                          (end->next()->str() == ")" && level == 0)))
+            {
+                if (end->next()->str() == "(")
+                    level++;
+                else if (end->next()->str() == ")")
+                    level--;
+
+                end = end->next();
+            }
+        }
+
+        if (end)
+        {
+            if (Token::Match(end, ") {")) // function parameter ?
+            {
+                // look backwards
+                if (tok->previous()->str() == "enum" ||
+                    (Token::Match(tok->previous(), "%type%") &&
+                     tok->previous()->str() != "return"))
+                {
+                    duplicateEnumError(*tokPtr, name, "Function parameter");
+                    // duplicate definition so skip entire function
+                    *tokPtr = end->next()->link();
+                    return true;
+                }
+            }
+            else if (end->str() == ">") // template parameter ?
+            {
+                // look backwards
+                if (tok->previous()->str() == "enum" ||
+                    (Token::Match(tok->previous(), "%type%") &&
+                     tok->previous()->str() != "return"))
+                {
+                    // duplicate definition so skip entire template
+                    while (end && end->str() != "{")
+                        end = end->next();
+                    if (end)
+                    {
+                        duplicateEnumError(*tokPtr, name, "Template parameter");
+                        *tokPtr = end->link();
+                        return true;
+                    }
+                }
+            }
+            else
+            {
+                // look backwards
+                if (tok->previous()->str() == "enum" ||
+                    (Token::Match(tok->previous(), "%type%") &&
+                     tok->previous()->str() != "return"))
+                {
+                    duplicateEnumError(*tokPtr, name, "Variable");
+                    return true;
+                }
+            }
+        }
+    }
+    return false;
+}
 
 void Tokenizer::simplifyEnum()
 {
@@ -5090,6 +5195,9 @@ void Tokenizer::simplifyEnum()
             Token * lastEnumValueStart = 0;
             Token * lastEnumValueEnd = 0;
 
+            // iterate over all enumerators between { and }
+            // Give each enumerator the const value specified or if not specified, 1 + the
+            // previous value or 0 if it is the first one.
             for (; tok1 && tok1 != end; tok1 = tok1->next())
             {
                 Token * enumName = 0;
@@ -5099,6 +5207,7 @@ void Tokenizer::simplifyEnum()
 
                 if (Token::Match(tok1->previous(), ",|{ %type% ,|}"))
                 {
+                    // no value specified
                     enumName = tok1;
                     lastValue++;
                     tok1->insertToken("=");
@@ -5106,11 +5215,12 @@ void Tokenizer::simplifyEnum()
 
                     if (lastEnumValueStart && lastEnumValueEnd)
                     {
+                        // previous value was an expression
                         Token * valueStart = tok1;
                         std::stack<Token *> links;
                         for (Token *tok2 = lastEnumValueStart; tok2 != lastEnumValueEnd->next(); tok2 = tok2->next())
                         {
-                            tok1->insertToken(tok2->strAt(0));
+                            tok1->insertToken(tok2->str());
                             tok1 = tok1->next();
 
                             // Check for links and fix them up
@@ -5127,21 +5237,24 @@ void Tokenizer::simplifyEnum()
                             }
                         }
 
+                        // value is previous expression + 1
                         tok1->insertToken("+");
                         tok1 = tok1->next();
-                        tok1->insertToken(MathLib::toString<long>(lastValue).c_str());
+                        tok1->insertToken(MathLib::toString<long>(lastValue));
                         enumValue = 0;
                         enumValueStart = valueStart->next();
                         enumValueEnd = tok1->next();
                     }
                     else
                     {
+                        // value is previous numeric value + 1
                         tok1->insertToken(MathLib::toString<long>(lastValue));
                         enumValue = tok1->next();
                     }
                 }
                 else if (Token::Match(tok1->previous(), ",|{ %type% = %num% ,|}"))
                 {
+                    // value is specified numeric value
                     enumName = tok1;
                     lastValue = MathLib::toLongNumber(tok1->strAt(2));
                     enumValue = tok1->tokAt(2);
@@ -5150,6 +5263,7 @@ void Tokenizer::simplifyEnum()
                 }
                 else if (Token::Match(tok1->previous(), ",|{ %type% = "))
                 {
+                    // value is specified expression
                     enumName = tok1;
                     lastValue = 0;
                     tok1 = tok1->tokAt(2);
@@ -5166,11 +5280,14 @@ void Tokenizer::simplifyEnum()
 
                         enumValueEnd = enumValueEnd->next();
                     }
+                    // remember this expression in case it needs to be incremented
                     lastEnumValueStart = enumValueStart;
                     lastEnumValueEnd = enumValueEnd;
+                    // skip over expression
                     tok1 = enumValueEnd;
                 }
 
+                // find all uses of this enumerator and substitute it's value for it's name
                 if (enumName && (enumValue || (enumValueStart && enumValueEnd)))
                 {
                     const std::string pattern(className.empty() ? "" : (className + " :: " + enumName->str()).c_str());
@@ -5196,7 +5313,9 @@ void Tokenizer::simplifyEnum()
                             }
                         }
                         else if (tok2->str() == "{")
+                        {
                             ++level;
+                        }
                         else if (!pattern.empty() && Token::Match(tok2, pattern.c_str()))
                         {
                             simplifyEnum = true;
@@ -5208,11 +5327,16 @@ void Tokenizer::simplifyEnum()
                             {
                                 // Don't replace this enum if it's preceded by "::"
                             }
-                            else
+                            else if (!duplicateDefinition(&tok2, enumName))
                             {
                                 simplifyEnum = true;
                                 hasClass = false;
                             }
+                            else
+                            {
+                                // something with the same name.
+                                exitScope = level;
+                            }
                         }
 
                         if (simplifyEnum)
@@ -5228,7 +5352,7 @@ void Tokenizer::simplifyEnum()
                                 Token * nextToken = enumValueStart->next();
                                 for (; nextToken != enumValueEnd->next(); nextToken = nextToken->next())
                                 {
-                                    tok2->insertToken(nextToken->strAt(0));
+                                    tok2->insertToken(nextToken->str());
                                     tok2 = tok2->next();
 
                                     // Check for links and fix them up

lib/tokenize.h

@@ -414,6 +414,16 @@ private:
      */
     static std::string getNameForFunctionParams(const Token *start);
 
+    /**
+     * check for duplicate enum definition
+     */
+    bool duplicateDefinition(Token **tokPtr, const Token *name);
+
+    /**
+     * duplicate enum definition error
+     */
+    void duplicateEnumError(const Token *tok1, const Token *tok2, const std::string & type);
+
     /** Disable assignment operator */
     Tokenizer &operator=(const Tokenizer &);
 

test/testsimplifytokens.cpp

@@ -189,6 +189,8 @@ private:
         TEST_CASE(enum4);
         TEST_CASE(enum5);
         TEST_CASE(enum6);
+        TEST_CASE(enum7);
+        TEST_CASE(enum8);
 
         // remove "std::" on some standard functions
         TEST_CASE(removestd);
@@ -3511,6 +3513,89 @@ private:
         ASSERT_EQUALS(expected, tok(code, false));
     }
 
+    void enum7()
+    {
+        {
+            // ticket 1388
+            const char code[] = "enum FOO {A,B,C};\n"
+                                "int main()\n"
+                                "{\n"
+                                "  int A = B;\n"
+                                "  { float A = C; }\n"
+                                "}";
+            const char expected[] = "; "
+                                    "int main ( ) "
+                                    "{ "
+                                    "int A ; A = 1 ; "
+                                    "{ float A ; A = 2 ; } "
+                                    "}";
+            ASSERT_EQUALS(expected, tok(code, false));
+        }
+
+        {
+            const char code[] = "enum FOO {A,B,C};\n"
+                                "void f(int A, float B, char C) { }";
+            const char expected[] = "; "
+                                    "void f ( int A , float B , char C ) { }";
+            ASSERT_EQUALS(expected, tok(code, false));
+        }
+    }
+
+    // Check simplifyEnum
+    void checkSimplifyEnum(const char code[])
+    {
+        // Tokenize..
+        Settings settings;
+        settings._showAll = true;
+        settings._checkCodingStyle = true;
+        Tokenizer tokenizer(&settings, this);
+        std::istringstream istr(code);
+        errout.str("");
+        tokenizer.tokenize(istr, "test.cpp");
+    }
+
+    void enum8()
+    {
+        // ticket 1388
+        checkSimplifyEnum("enum Direction {N=100,E,S,W,ALL};\n"
+                          "template<class T,int S> class EF_Vector{\n"
+                          "  T v_v[S];\n"
+                          "\n"
+                          "public:\n"
+                          "  EF_Vector();\n"
+                          "  explicit EF_Vector(const T &);\n"
+                          "  explicit EF_Vector(const T arr[S]);\n"
+                          "};\n"
+                          "\n"
+                          "template<class T,int S>\n"
+                          "EF_Vector<T,S>::EF_Vector()\n"
+                          "{\n"
+                          "}\n"
+                          "\n"
+                          "template<class T,int S>\n"
+                          "EF_Vector<T,S>::EF_Vector(const T &t)\n"
+                          "{\n"
+                          "  for(int i=0;i<S;i++)\n"
+                          "    v_v[i]=t;\n"
+                          "}\n"
+                          "\n"
+                          "template<class T,int S>\n"
+                          "EF_Vector<T,S>::EF_Vector(const T arr[S])\n"
+                          "{\n"
+                          "  for(int i=0;i<S;i++)\n"
+                          "    v_v[i]=arr[i];\n"
+                          "}\n"
+                          "\n"
+                          "void initialize()\n"
+                          "{\n"
+                          "   EF_Vector<float,6> d;\n"
+                          "}");
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:1]: (style) Template parameter 'S' hides enumerator of same name\n"
+                      "[test.cpp:11] -> [test.cpp:1]: (style) Template parameter 'S' hides enumerator of same name\n"
+                      "[test.cpp:16] -> [test.cpp:1]: (style) Template parameter 'S' hides enumerator of same name\n"
+                      "[test.cpp:23] -> [test.cpp:1]: (style) Template parameter 'S' hides enumerator of same name\n", errout.str());
+    }
+
     void removestd()
     {
         ASSERT_EQUALS("; strcpy ( a , b ) ;", tok("; std::strcpy(a,b);"));