THRIFT-2089 Compiler ignores duplicate typenames

Patch: Randy Abernethy

5 files changed, 109 insertions, 5 deletions

diff --git a/compiler/cpp/src/parse/t_program.h b/compiler/cpp/src/parse/t_program.h
index dfd9d43d2..f22da0737 100644
--- a/compiler/cpp/src/parse/t_program.h
+++ b/compiler/cpp/src/parse/t_program.h
@@ -123,6 +123,103 @@ class t_program : public t_doc {
     }
   }
 
+  // Typename collision detection
+  /**
+   * Search for typename collisions
+   * @param t    the type to test for collisions
+   * @return     true if a certain collision was found, otherwise false
+   */
+  bool is_unique_typename(t_type * t) {
+    int occurances = program_typename_count(this, t);
+    for (std::vector<t_program*>::iterator it = includes_.begin();
+         it != includes_.end(); ++it) {
+      occurances += program_typename_count(*it, t);
+    }
+    return 0 == occurances;
+  }
+
+  /**
+   * Search all type collections for duplicate typenames
+   * @param prog the program to search
+   * @param t    the type to test for collisions
+   * @return     the number of certain typename collisions
+   */
+  int program_typename_count(t_program * prog, t_type * t) {
+    int occurances = 0;
+    occurances += collection_typename_count(prog, prog->typedefs_, t);
+    occurances += collection_typename_count(prog, prog->enums_, t);
+    occurances += collection_typename_count(prog, prog->objects_, t);
+    occurances += collection_typename_count(prog, prog->services_, t);
+    return occurances;
+  }
+
+  /**
+   * Search a type collection for duplicate typenames
+   * @param prog            the program to search
+   * @param type_collection the type collection to search
+   * @param t               the type to test for collisions
+   * @return                the number of certain typename collisions
+   */
+  template <class T>
+  int collection_typename_count(t_program * prog, T type_collection, t_type * t) {
+    int occurances = 0;
+    for (typename T::iterator it = type_collection.begin(); it != type_collection.end(); ++it)
+      if (t != *it && 0 == t->get_name().compare((*it)->get_name()) && is_common_namespace(prog, t))
+        ++occurances;
+    return occurances;
+  }
+
+  /**
+   * Determine whether identical typenames will collide based on namespaces.
+   *
+   * Because we do not know which languages the user will generate code for,
+   * collisions within programs (IDL files) having namespace declarations can be
+   * difficult to determine. Only guaranteed collisions return true (cause an error).
+   * Possible collisions involving explicit namespace declarations produce a warning.
+   * Other possible collisions go unreported.
+   * @param prog the program containing the preexisting typename
+   * @param t    the type containing the typename match
+   * @return     true if a collision within namespaces is found, otherwise false
+   */
+  bool is_common_namespace(t_program * prog, t_type * t) {
+    //Case 1: Typenames are in the same program [collision]
+    if (prog == t->get_program()) {
+      pwarning(1, "Duplicate typename %s found in %s",
+               t->get_name().c_str(), t->get_program()->get_name().c_str());
+      return true;
+    }
+
+    //Case 2: Both programs have identical namespace scope/name declarations [collision]
+    bool match = true;
+    for (std::map<std::string, std::string>::iterator it = prog->namespaces_.begin();
+         it != prog->namespaces_.end(); ++it) {
+      if (0 == it->second.compare(t->get_program()->get_namespace(it->first))) {
+        pwarning(1, "Duplicate typename %s found in %s,%s,%s and %s,%s,%s [file,scope,ns]",
+                 t->get_name().c_str(),
+                 t->get_program()->get_name().c_str(), it->first.c_str(), it->second.c_str(),
+                 prog->get_name().c_str(), it->first.c_str(), it->second.c_str());
+      } else {
+        match = false;
+      }
+    }
+    for (std::map<std::string, std::string>::iterator it = t->get_program()->namespaces_.begin();
+         it != t->get_program()->namespaces_.end(); ++it) {
+      if (0 == it->second.compare(prog->get_namespace(it->first))) {
+        pwarning(1, "Duplicate typename %s found in %s,%s,%s and %s,%s,%s [file,scope,ns]",
+                 t->get_name().c_str(),
+                 t->get_program()->get_name().c_str(), it->first.c_str(), it->second.c_str(),
+                 prog->get_name().c_str(), it->first.c_str(), it->second.c_str());
+      } else {
+        match = false;
+      }
+    }
+    if (0 == prog->namespaces_.size() && 0 == t->get_program()->namespaces_.size()) {
+      pwarning(1, "Duplicate typename %s found in %s and %s",
+        t->get_name().c_str(), t->get_program()->get_name().c_str(),  prog->get_name().c_str());
+    }
+    return match;
+  }
+
   // Scoping and namespacing
   void set_namespace(std::string name) {
     namespace_ = name;
diff --git a/compiler/cpp/src/thrifty.yy b/compiler/cpp/src/thrifty.yy
index 2d90f21ae..5b91a0a21 100644
--- a/compiler/cpp/src/thrifty.yy
+++ b/compiler/cpp/src/thrifty.yy
@@ -450,6 +450,10 @@ Definition:
         if (g_parent_scope != NULL) {
           g_parent_scope->add_type(g_parent_prefix + $1->get_name(), $1);
         }
+        if (! g_program->is_unique_typename($1)) {
+          yyerror("Type \"%s\" is already defined.", $1->get_name().c_str());
+          exit(1);
+        }
       }
       $$ = $1;
     }
@@ -462,6 +466,10 @@ Definition:
           g_parent_scope->add_service(g_parent_prefix + $1->get_name(), $1);
         }
         g_program->add_service($1);
+        if (! g_program->is_unique_typename($1)) {
+          yyerror("Type \"%s\" is already defined.", $1->get_name().c_str());
+          exit(1);
+        }
       }
       $$ = $1;
     }
diff --git a/lib/cpp/test/TFileTransportTest.cpp b/lib/cpp/test/TFileTransportTest.cpp
index 7691c73fc..20c3b5c27 100644..100755
--- a/lib/cpp/test/TFileTransportTest.cpp
+++ b/lib/cpp/test/TFileTransportTest.cpp
@@ -20,9 +20,8 @@
 #define _GNU_SOURCE // needed for getopt_long
 #endif
 
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
+#include <thrift/thrift-config.h>
+
 #ifdef HAVE_SYS_TIME_H
 #include <sys/time.h>
 #endif
diff --git a/lib/cpp/test/concurrency/ThreadFactoryTests.h b/lib/cpp/test/concurrency/ThreadFactoryTests.h
index b7e873ff1..fda6c9e6c 100644..100755
--- a/lib/cpp/test/concurrency/ThreadFactoryTests.h
+++ b/lib/cpp/test/concurrency/ThreadFactoryTests.h
@@ -17,7 +17,7 @@
  * under the License.
  */
 
-#include <config.h>
+#include <thrift/thrift-config.h>
 #include <thrift/concurrency/Thread.h>
 #include <thrift/concurrency/PlatformThreadFactory.h>
 #include <thrift/concurrency/Monitor.h>
diff --git a/lib/cpp/test/concurrency/ThreadManagerTests.h b/lib/cpp/test/concurrency/ThreadManagerTests.h
index b734f7a0f..4e53a2d12 100644..100755
--- a/lib/cpp/test/concurrency/ThreadManagerTests.h
+++ b/lib/cpp/test/concurrency/ThreadManagerTests.h
@@ -17,7 +17,7 @@
  * under the License.
  */
 
-#include <config.h>
+#include <thrift/thrift-config.h>
 #include <thrift/concurrency/ThreadManager.h>
 #include <thrift/concurrency/PlatformThreadFactory.h>
 #include <thrift/concurrency/Monitor.h>