Fixed file path in errors when parsing sdf or urdf from string

include/sdf/Types.hh

@@ -63,6 +63,14 @@ namespace sdf
 
   const std::string kSdfScopeDelimiter = "::";
 
+  /// \brief The source path replacement if it was parsed from a string,
+  /// instead of a file.
+  constexpr char kSdfStringSource[] = "data-string";
+
+  /// \brief The source path replacement if the urdf was parsed from a string,
+  /// instead of a file.
+  constexpr char kUrdfStringSource[] = "urdf string";
+
   /// \brief Split a string using the delimiter in splitter.
   /// \param[in] str       The string to split.
   /// \param[in] splitter  The delimiter to use.

src/Element.cc

@@ -51,7 +51,7 @@ void Element::SetParent(const ElementPtr _parent)
 
   // If this element doesn't have a path, get it from the parent
   if (nullptr != _parent && (this->FilePath().empty() ||
-      this->FilePath() == "data-string"))
+      this->FilePath() == std::string(kSdfStringSource)))
   {
     this->SetFilePath(_parent->FilePath());
   }

src/parser.cc

@@ -574,7 +574,8 @@ bool readStringInternal(const std::string &_xmlString, const bool _convert,
     sdferr << "Error parsing XML from string: " << xmlDoc.ErrorStr() << '\n';
     return false;
   }
-  if (readDoc(&xmlDoc, _sdf, "data-string", _convert, _config, _errors))
+  if (readDoc(&xmlDoc, _sdf, std::string(kSdfStringSource), _convert, _config,
+              _errors))
   {
     return true;
   }
@@ -584,7 +585,8 @@ bool readStringInternal(const std::string &_xmlString, const bool _convert,
     auto doc = makeSdfDoc();
     u2g.InitModelString(_xmlString, &doc);
 
-    if (sdf::readDoc(&doc, _sdf, "urdf string", _convert, _config, _errors))
+    if (sdf::readDoc(&doc, _sdf, std::string(kUrdfStringSource), _convert,
+                    _config, _errors))
     {
       sdfdbg << "Parsing from urdf.\n";
       return true;
@@ -629,7 +631,8 @@ bool readString(const std::string &_xmlString, const ParserConfig &_config,
     sdferr << "Error parsing XML from string: " << xmlDoc.ErrorStr() << '\n';
     return false;
   }
-  if (readDoc(&xmlDoc, _sdf, "data-string", true, _config, _errors))
+  if (readDoc(&xmlDoc, _sdf, std::string(kSdfStringSource), true, _config,
+              _errors))
   {
     return true;
   }
@@ -665,7 +668,7 @@ bool readDoc(tinyxml2::XMLDocument *_xmlDoc, SDFPtr _sdf,
     return false;
   }
 
-  if (_source != "data-string")
+  if (_source != std::string(kSdfStringSource))
   {
     _sdf->SetFilePath(_source);
   }
@@ -745,7 +748,7 @@ bool readDoc(tinyxml2::XMLDocument *_xmlDoc, ElementPtr _sdf,
     return false;
   }
 
-  if (_source != "data-string")
+  if (_source != std::string(kSdfStringSource))
   {
     _sdf->SetFilePath(_source);
   }
@@ -929,6 +932,10 @@ std::string getModelFilePath(const std::string &_modelDirPath)
 bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
     const ParserConfig &_config, const std::string &_source, Errors &_errors)
 {
+  std::string sourcePath = _source;
+  if (_source == kSdfStringSource || _source == kUrdfStringSource)
+    sourcePath = "<" + _source + ">";
+
   // Check if the element pointer is deprecated.
   if (_sdf->GetRequired() == "-1")
   {
@@ -947,7 +954,7 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
       _errors.push_back({
           ErrorCode::ELEMENT_MISSING,
           "SDF Element<" + _sdf->GetName() + "> is missing",
-          _source});
+          sourcePath});
       return false;
     }
     else
@@ -1021,7 +1028,7 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
                 "'" + std::string(attribute->Value()) +
                 "' is reserved; it cannot be used as a value of "
                 "attribute [" + p->GetKey() + "]",
-                _source,
+                sourcePath,
                 attribute->GetLineNum()});
             }
         }
@@ -1031,7 +1038,7 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
           _errors.push_back({
               ErrorCode::ATTRIBUTE_INVALID,
               "Unable to read attribute[" + p->GetKey() + "]",
-              _source,
+              sourcePath,
               attribute->GetLineNum()});
           return false;
         }
@@ -1050,7 +1057,7 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
           Error(
               ErrorCode::ATTRIBUTE_INCORRECT_TYPE,
               ss.str(),
-              _source,
+              sourcePath,
               _xml->GetLineNum()),
           _errors);
     }
@@ -1068,7 +1075,7 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
           ErrorCode::ATTRIBUTE_MISSING,
           "Required attribute[" + p->GetKey() + "] in element[" + _xml->Value()
           + "] is not specified in SDF.",
-          _source,
+          sourcePath,
           _xml->GetLineNum()});
       return false;
     }
@@ -1104,7 +1111,7 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
             _errors.push_back({
                 ErrorCode::URI_LOOKUP,
                 "Unable to find uri[" + uri + "]",
-                _source,
+                sourcePath,
                 uriElement->GetLineNum()});
             continue;
           }
@@ -1122,7 +1129,7 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
                     "Unable to resolve uri[" + uri + "] to model path [" +
                     modelPath + "] since it does not contain a model.config " +
                     "file.",
-                    _source,
+                    sourcePath,
                     uriElement->GetLineNum()});
                 continue;
               }
@@ -1141,7 +1148,7 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
           _errors.push_back({
               ErrorCode::ATTRIBUTE_MISSING,
               "<include> element missing 'uri' attribute",
-              _source,
+              sourcePath,
               elemXml->GetLineNum()});
           continue;
         }
@@ -1170,7 +1177,7 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
             _errors.push_back({
                 ErrorCode::FILE_READ,
                 "Unable to read file[" + filename + "]",
-                _source,
+                sourcePath,
                 uriElement->GetLineNum()});
             return false;
           }
@@ -1212,7 +1219,7 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
                 ErrorCode::ELEMENT_MISSING,
                 "Failed to find top level <model> / <actor> / <light> for "
                 "<include>\n",
-                _source,
+                sourcePath,
                 uriElement->GetLineNum()});
             continue;
           }
@@ -1287,7 +1294,7 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
                   ErrorCode::MODEL_PLACEMENT_FRAME_INVALID,
                   "<pose> is required when specifying the placement_frame "
                   "element",
-                  _source,
+                  sourcePath,
                   elemXml->GetLineNum()});
               return false;
             }
@@ -1301,7 +1308,7 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
                   "'" + placementFrameVal +
                   "' is reserved; it cannot be used as a value of "
                   "element [placement_frame]",
-                  _source,
+                  sourcePath,
                   placementFrameElem->GetLineNum()});
             }
             topLevelElem->GetAttribute("placement_frame")
@@ -1320,12 +1327,12 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
                 pluginElem = topLevelElem->AddElement("plugin");
 
                 if (!readXml(
-                    childElemXml, pluginElem, _config, _source, _errors))
+                    childElemXml, pluginElem, _config, sourcePath, _errors))
                 {
                   _errors.push_back({
                       ErrorCode::ELEMENT_INVALID,
                       "Error reading plugin element",
-                      _source,
+                      sourcePath,
                       childElemXml->GetLineNum()});
                   return false;
                 }
@@ -1360,7 +1367,7 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
         {
           ElementPtr element = elemDesc->Clone();
           element->SetParent(_sdf);
-          if (readXml(elemXml, element, _config, _source, _errors))
+          if (readXml(elemXml, element, _config, sourcePath, _errors))
           {
             _sdf->InsertElement(element);
           }
@@ -1370,7 +1377,7 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
                 ErrorCode::ELEMENT_INVALID,
                 std::string("Error reading element <") +
                 elemXml->Value() + ">",
-                _source,
+                sourcePath,
                 elemXml->GetLineNum()});
             return false;
           }
@@ -1392,7 +1399,7 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
             Error(
                 ErrorCode::ELEMENT_INCORRECT_TYPE,
                 ss.str(),
-                _source,
+                sourcePath,
                 elemXml->GetLineNum()),
             _errors);
 
@@ -1420,7 +1427,7 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
                 ErrorCode::ELEMENT_MISSING,
                 "XML Missing required element[" + elemDesc->GetName() +
                 "], child of element[" + _sdf->GetName() + "]",
-                _source,
+                sourcePath,
                 elemXml->GetLineNum()});
             return false;
           }
@@ -1595,8 +1602,8 @@ bool convertString(const std::string &_sdfString, const std::string &_version,
     if (sdf::Converter::Convert(&xmlDoc, _version, true))
     {
       Errors errors;
-      bool result =
-          sdf::readDoc(&xmlDoc, _sdf, "data-string", false, _config, errors);
+      bool result = sdf::readDoc(&xmlDoc, _sdf, std::string(kSdfStringSource),
+                                 false, _config, errors);
 
       // Output errors
       for (auto const &e : errors)

src/parser_TEST.cc

@@ -630,6 +630,84 @@ TEST_F(ValueConstraintsFixture, ElementMinMaxValues)
   }
 }
 
+/////////////////////////////////////////////////
+/// Check for parsing errors while reading strings
+TEST(Parser, ReadSingleLineStringError)
+{
+  sdf::SDFPtr sdf = InitSDF();
+  std::ostringstream stream;
+  stream
+    << "<sdf version='1.8'>"
+    << "<model name=\"test\">"
+    << "  <link name=\"test1\">"
+    << "    <visual name=\"good\">"
+    << "      <geometry>"
+    << "        <box><size>1 1 1</size></box>"
+    << "      </geometry>"
+    << "    </visual>"
+    << "  </link>"
+    << "  <link>"
+    << "    <visual name=\"good2\">"
+    << "      <geometry>"
+    << "        <box><size>1 1 1</size></box>"
+    << "      </geometry>"
+    << "    </visual>"
+    << "  </link>"
+    << "</model>"
+    << "</sdf>";
+  sdf::Errors errors;
+  EXPECT_FALSE(sdf::readString(stream.str(), sdf, errors));
+  ASSERT_NE(errors.size(), 0u);
+
+  std::cerr << errors[0] << std::endl;
+
+  EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::ATTRIBUTE_MISSING);
+  ASSERT_TRUE(errors[0].FilePath().has_value());
+  EXPECT_EQ(errors[0].FilePath().value(),
+      "<" + std::string(sdf::kSdfStringSource) + ">");
+  ASSERT_TRUE(errors[0].LineNumber().has_value());
+  EXPECT_EQ(errors[0].LineNumber().value(), 1);
+}
+
+/////////////////////////////////////////////////
+/// Check for parsing errors while reading strings
+TEST(Parser, ReadMultiLineStringError)
+{
+  sdf::SDFPtr sdf = InitSDF();
+  std::ostringstream stream;
+  stream
+    << "<sdf version='1.8'>\n"
+    << "<model name=\"test\">\n"
+    << "  <link name=\"test1\">\n"
+    << "    <visual name=\"good\">\n"
+    << "      <geometry>\n"
+    << "        <box><size>1 1 1</size></box>\n"
+    << "      </geometry>\n"
+    << "    </visual>\n"
+    << "  </link>\n"
+    << "  <link>\n"
+    << "    <visual name=\"good2\">\n"
+    << "      <geometry>\n"
+    << "        <box><size>1 1 1</size></box>\n"
+    << "      </geometry>\n"
+    << "    </visual>\n"
+    << "  </link>\n"
+    << "</model>\n"
+    << "</sdf>\n";
+  sdf::Errors errors;
+  EXPECT_FALSE(sdf::readString(stream.str(), sdf, errors));
+  ASSERT_NE(errors.size(), 0u);
+
+  std::cerr << errors[0] << std::endl;
+
+  EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::ATTRIBUTE_MISSING);
+  ASSERT_TRUE(errors[0].FilePath().has_value());
+  EXPECT_EQ(errors[0].FilePath().value(),
+      "<" + std::string(sdf::kSdfStringSource) + ">");
+  ASSERT_TRUE(errors[0].LineNumber().has_value());
+  EXPECT_EQ(errors[0].LineNumber().value(), 10);
+}
+
 /////////////////////////////////////////////////
 /// Main
 int main(int argc, char **argv)