Backport sdf6: Move recursiveSameTypeUniqueNames from ign.cc to parser.cc

Changelog.md

@@ -2,6 +2,9 @@
 
 ### SDFormat 6.X.X (20XX-XX-XX)
 
+1. Move recursiveSameTypeUniqueNames from ign.cc to parser.cc and make public.
+    * [Pull request ]()
+
 1. Parse urdf files to sdf 1.5 instead of 1.4 to avoid `use_parent_model_frame`.
     * [BitBucket pull request 575](https://osrf-migration.github.io/sdformat-gh-pages/#!/osrf/sdformat/pull-requests/575)
 

Migration.md

@@ -14,6 +14,11 @@ but with improved human-readability..
 
 ## SDFormat 5.x to 6.x
 
+### Additions
+
+1. **sdf/parser.hh**
+   + bool recursiveSameTypeUniqueNames(sdf::ElementPtr)
+
 ### Deprecations
 
 1. **sdf/Types.hh**

include/sdf/parser.hh

@@ -141,5 +141,14 @@ namespace sdf
   SDFORMAT_VISIBLE
   bool convertString(const std::string &_sdfString,
                      const std::string &_version, SDFPtr _sdf);
+
+  /// \brief Check that all sibling elements of the same type have unique names.
+  /// This checks recursively and should check the files exhaustively
+  /// rather than terminating early when the first duplicate name is found.
+  /// \param[in] _elem sdf Element to check recursively.
+  /// \return True if all contained elements have do not share a name with
+  /// sibling elements of the same type.
+  SDFORMAT_VISIBLE
+  bool recursiveSameTypeUniqueNames(sdf::ElementPtr _elem);
 }
 #endif

src/ign.cc

@@ -23,39 +23,6 @@
 #include "sdf/parser.hh"
 #include "sdf/system_util.hh"
 
-//////////////////////////////////////////////////
-/// \brief Check that all sibling elements of the same type have unique names.
-/// This checks recursively and should check the files exhaustively
-/// rather than terminating early when the first duplicate name is found.
-/// \param[in] _elem sdf Element to check recursively.
-/// \return True if all contained elements have do not share a name with
-/// sibling elements of the same type.
-bool recursiveSameTypeUniqueNames(sdf::ElementPtr _elem)
-{
-  bool result = true;
-  auto typeNames = _elem->GetElementTypeNames();
-  for (const std::string &typeName : typeNames)
-  {
-    if (!_elem->HasUniqueChildNames(typeName))
-    {
-      std::cerr << "Non-unique names detected in type "
-                << typeName << " in\n"
-                << _elem->ToString("")
-                << std::endl;
-      result = false;
-    }
-  }
-
-  sdf::ElementPtr child = _elem->GetFirstElement();
-  while (child)
-  {
-    result = recursiveSameTypeUniqueNames(child) && result;
-    child = child->GetNextElement();
-  }
-
-  return result;
-}
-
 //////////////////////////////////////////////////
 // cppcheck-suppress unusedFunction
 extern "C" SDFORMAT_VISIBLE int cmdCheck(const char *_path)
@@ -80,7 +47,7 @@ extern "C" SDFORMAT_VISIBLE int cmdCheck(const char *_path)
     return -1;
   }
 
-  if (!recursiveSameTypeUniqueNames(sdf->Root()))
+  if (!sdf::recursiveSameTypeUniqueNames(sdf->Root()))
   {
     std::cerr << "Error: non-unique names detected.\n";
     return -1;

src/parser.cc

@@ -1208,4 +1208,31 @@ bool convertString(const std::string &_sdfString, const std::string &_version,
 
   return false;
 }
+
+//////////////////////////////////////////////////
+bool recursiveSameTypeUniqueNames(sdf::ElementPtr _elem)
+{
+  bool result = true;
+  auto typeNames = _elem->GetElementTypeNames();
+  for (const std::string &typeName : typeNames)
+  {
+    if (!_elem->HasUniqueChildNames(typeName))
+    {
+      std::cerr << "Non-unique names detected in type "
+                << typeName << " in\n"
+                << _elem->ToString("")
+                << std::endl;
+      result = false;
+    }
+  }
+
+  sdf::ElementPtr child = _elem->GetFirstElement();
+  while (child)
+  {
+    result = recursiveSameTypeUniqueNames(child) && result;
+    child = child->GetNextElement();
+  }
+
+  return result;
+}
 }

src/parser_TEST.cc

@@ -18,9 +18,10 @@
 #include <gtest/gtest.h>
 #include "sdf/parser.hh"
 #include "sdf/Element.hh"
+#include "test_config.h"
 
 /////////////////////////////////////////////////
-TEST(parser, initStringTrim)
+TEST(Parser, initStringTrim)
 {
   sdf::SDFPtr sdf(new sdf::SDF());
   std::ostringstream stream;
@@ -48,6 +49,32 @@ TEST(parser, initStringTrim)
   EXPECT_TRUE(attr->GetRequired());
 }
 
+/////////////////////////////////////////////////
+/// Tests whether the input sdf string satisfies the unique name criteria among
+/// same types
+sdf::SDFPtr InitSDF()
+{
+  sdf::SDFPtr sdf(new sdf::SDF());
+  sdf::init(sdf);
+  return sdf;
+}
+
+/////////////////////////////////////////////////
+TEST(Parser, NameUniqueness)
+{
+  std::string pathBase = PROJECT_SOURCE_PATH;
+  pathBase += "/test/sdf";
+
+  // Check an SDF file with sibling elements of the same type (world)
+  // that have duplicate names.
+  {
+    std::string path = pathBase +"/world_duplicate.sdf";
+    sdf::SDFPtr sdf = InitSDF();
+    EXPECT_TRUE(sdf::readFile(path, sdf));
+    EXPECT_FALSE(sdf::recursiveSameTypeUniqueNames(sdf->Root()));
+  }
+}
+
 /////////////////////////////////////////////////
 /// Main
 int main(int argc, char **argv)