From 9bdd0d43285264145424e8da4871526f17617aee Mon Sep 17 00:00:00 2001 From: Jose Luis Rivero Date: Fri, 28 May 2021 18:47:25 +0200 Subject: [PATCH 1/2] Backport sdf6: Move recursiveSameTypeUniqueNames from ign.cc to parser.cc Backport of sdf9 PR 606 https://osrf-migration.github.io/sdformat-gh-pages/#!/osrf/sdformat/pull-requests/606 Commit: https://github.com/osrf/sdformat/commit/2d1bc5627542f4072b1c2c5628074e8e1120a93d Signed-off-by: Jose Luis Rivero --- Changelog.md | 3 +++ Migration.md | 5 +++++ include/sdf/parser.hh | 9 +++++++++ src/ign.cc | 35 +---------------------------------- src/parser.cc | 27 +++++++++++++++++++++++++++ src/parser_TEST.cc | 29 ++++++++++++++++++++++++++++- 6 files changed, 73 insertions(+), 35 deletions(-) diff --git a/Changelog.md b/Changelog.md index 9b5c91a0f..2617edd67 100644 --- a/Changelog.md +++ b/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) diff --git a/Migration.md b/Migration.md index 752c94b1e..59b70dc8f 100644 --- a/Migration.md +++ b/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** diff --git a/include/sdf/parser.hh b/include/sdf/parser.hh index 1af5c4373..2884ba688 100644 --- a/include/sdf/parser.hh +++ b/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 diff --git a/src/ign.cc b/src/ign.cc index 806643119..4b7169b62 100644 --- a/src/ign.cc +++ b/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; diff --git a/src/parser.cc b/src/parser.cc index bdffa50ce..037798e32 100644 --- a/src/parser.cc +++ b/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; +} } diff --git a/src/parser_TEST.cc b/src/parser_TEST.cc index d49e0dca6..22250b916 100644 --- a/src/parser_TEST.cc +++ b/src/parser_TEST.cc @@ -18,9 +18,10 @@ #include #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) From 6b001a957344d0a45064a589b6c5fd2540512b88 Mon Sep 17 00:00:00 2001 From: Jose Luis Rivero Date: Fri, 4 Jun 2021 20:23:05 +0200 Subject: [PATCH 2/2] Update Changelog.md --- Changelog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index 2617edd67..6c13d1c5a 100644 --- a/Changelog.md +++ b/Changelog.md @@ -3,7 +3,7 @@ ### SDFormat 6.X.X (20XX-XX-XX) 1. Move recursiveSameTypeUniqueNames from ign.cc to parser.cc and make public. - * [Pull request ]() + * [Pull request 580](https://github.com/osrf/sdformat/pull/580) 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)