Add an EnforcementPolicy for deprecated elements (#543)

This can be used to configure whether the parser will emit a warning or an error when encountering a deprecated element.

Signed-off-by: Addisu Z. Taddese <[email protected]>

Co-authored-by: Aaron Chong <[email protected]>

Migration.md

@@ -12,10 +12,33 @@ forward programmatically.
 This document aims to contain similar information to those files
 but with improved human-readability..
 
+## libsdformat 11.0.0 to 11.x.x
+
+### Additions
+
+1. **sdf/Console.hh** Add functions to retrieve message stream objects. These 
+   can be useful for redirecting the console output to other streams.
+    + ConsoleStream &GetMsgStream()
+    + ConsoleStream &GetLogStream()
+
+1. **sdf/ParserConfig.hh**:
+    + void SetDeprecatedElementsPolicy(EnforcementPolicy _policy)
+    + void ResetDeprecatedElementsPolicy()
+    + EnforcementPolicy DeprecatedElementsPolicy() const
+
+1. **test/test_utils.hh**:
+    + ScopeExit: A utility struct that calls a function when going out of scope.
+    + RedirectConsoleStream: A utility class used for redirecting the output of 
+      sdferr, sdfwarn, etc to a more convenient stream object like a 
+      std::stringstream for testing purposes.
+
 ## libsdformat 10.x to 11.0
 
 ### Additions
 
+1. **sdf/ParserConfig.hh** A class that contains configuration options for the 
+   libsdformat parser.
+
 1. + Depend on ignition-utils1 for the ImplPtr and UniqueImplPtr.
    + [Pull request 474](https://github.com/osrf/sdformat/pull/474)
 

include/sdf/Console.hh

@@ -92,6 +92,16 @@ namespace sdf
                           const std::string &_file,
                           unsigned int _line, int _color);
 
+      /// \brief Set the stream object.
+      /// \param[in] _stream Pointer to an output stream. This can be
+      /// useful for redirecting the output, for example, to a std::stringstream
+      /// for testing.
+      public: void SetStream(std::ostream *_stream);
+
+      /// \brief Get the current stream object.
+      /// \return Pointer to current stream object.
+      public: std::ostream *GetStream();
+
       /// \brief The ostream to log to; can be NULL/nullptr.
       private: std::ostream *stream;
     };
@@ -131,6 +141,18 @@ namespace sdf
                                const std::string &file,
                                unsigned int line);
 
+    /// \brief Get the current message stream object. This can be
+    /// useful for redirecting the output, for example, to a std::stringstream
+    /// for testing.
+    /// \return Mutable reference to current message stream object.
+    public: ConsoleStream &GetMsgStream();
+
+    /// \brief Get the current log stream object. This can be
+    /// useful for redirecting the output, for example, to a std::stringstream
+    /// for testing.
+    /// \return Mutable reference to current log stream object.
+    public: ConsoleStream &GetLogStream();
+
     /// \internal
     /// \brief Pointer to private data.
     private: std::unique_ptr<ConsolePrivate> dataPtr;

include/sdf/Element.hh

@@ -101,6 +101,7 @@ namespace sdf
 
     /// \brief Set the requirement type.
     /// \param[in] _req Requirement type for this element:
+    /// -1: Deprecated.
     /// 0: Not required.
     /// 1: Exactly one element is required.
     /// +: One or more elements are required.

include/sdf/ParserConfig.hh

@@ -144,6 +144,22 @@ class SDFORMAT_VISIBLE ParserConfig
   /// \return The unrecognized elements policy enum value
   public: EnforcementPolicy UnrecognizedElementsPolicy() const;
 
+  /// \brief Set the policy for deprecated elements.
+  /// \param[in] _policy The deprecated elements enforcement policy
+  public: void SetDeprecatedElementsPolicy(EnforcementPolicy _policy);
+
+  /// \brief Resets the policy for deprecated elements so that it follows
+  /// WarningsPolicy.
+  public: void ResetDeprecatedElementsPolicy();
+
+  /// \brief Get the current deprecated elements policy. By default, the policy
+  /// is the same as the overall WarningsPolicy, but it can be overriden by
+  /// SetDeprecatedElementsPolicy. Once it is overriden, changing
+  /// SetWarningsPolicy will not change the value of DeprecatedElementsPolicy
+  /// unless ResetDeprecatedElementsPolicy is called.
+  /// \return The deperacted elements policy enum value
+  public: EnforcementPolicy DeprecatedElementsPolicy() const;
+
   /// \brief Registers a custom model parser.
   /// \param[in] _modelParser Callback as described in
   /// sdf/InterfaceElements.hh.

src/Console.cc

@@ -108,6 +108,18 @@ void Console::SetQuiet(bool _quiet)
   g_quiet = _quiet;
 }
 
+//////////////////////////////////////////////////
+sdf::Console::ConsoleStream &Console::GetMsgStream()
+{
+  return this->dataPtr->msgStream;
+}
+
+//////////////////////////////////////////////////
+sdf::Console::ConsoleStream &Console::GetLogStream()
+{
+  return this->dataPtr->logStream;
+}
+
 //////////////////////////////////////////////////
 Console::ConsoleStream &Console::ColorMsg(const std::string &lbl,
                                           const std::string &file,
@@ -160,3 +172,15 @@ void Console::ConsoleStream::Prefix(const std::string &_lbl,
       _file.substr(index , _file.size() - index)<< ":" << _line << "] ";
   }
 }
+
+//////////////////////////////////////////////////
+void Console::ConsoleStream::SetStream(std::ostream *_stream)
+{
+  this->stream = _stream;
+}
+
+//////////////////////////////////////////////////
+std::ostream *Console::ConsoleStream::GetStream()
+{
+  return this->stream;
+}

src/ParserConfig.cc

@@ -15,6 +15,8 @@
  *
  */
 
+#include <optional>
+
 #include "sdf/ParserConfig.hh"
 #include "sdf/Filesystem.hh"
 #include "sdf/Types.hh"
@@ -36,6 +38,11 @@ class sdf::ParserConfig::Implementation
   public: EnforcementPolicy unrecognizedElementsPolicy =
     EnforcementPolicy::LOG;
 
+  /// \brief Policy indicating how deprecated elements are treated.
+  /// Defaults to the value of `warningsPolicy`. It can be overriden by the user
+  /// to behave behave differently than the `warningsPolicy`.
+  public: std::optional<EnforcementPolicy> deprecatedElementsPolicy;
+
   /// \brief Collection of custom model parsers.
   public: std::vector<CustomModelParser> customParsers;
 };
@@ -116,6 +123,25 @@ EnforcementPolicy ParserConfig::UnrecognizedElementsPolicy() const
   return this->dataPtr->unrecognizedElementsPolicy;
 }
 
+/////////////////////////////////////////////////
+void ParserConfig::SetDeprecatedElementsPolicy(EnforcementPolicy _policy)
+{
+  this->dataPtr->deprecatedElementsPolicy = _policy;
+}
+
+/////////////////////////////////////////////////
+void ParserConfig::ResetDeprecatedElementsPolicy()
+{
+  this->dataPtr->deprecatedElementsPolicy.reset();
+}
+
+/////////////////////////////////////////////////
+EnforcementPolicy ParserConfig::DeprecatedElementsPolicy() const
+{
+  return this->dataPtr->deprecatedElementsPolicy.value_or(
+      this->dataPtr->warningsPolicy);
+}
+
 /////////////////////////////////////////////////
 void ParserConfig::RegisterCustomModelParser(CustomModelParser _modelParser)
 {

src/parser.cc

@@ -942,7 +942,7 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf,
     std::stringstream ss;
     ss << "SDF Element[" + _sdf->GetName() + "] is deprecated\n";
     enforceConfigurablePolicyCondition(
-        _config.WarningsPolicy(),
+        _config.DeprecatedElementsPolicy(),
         Error(ErrorCode::ELEMENT_DEPRECATED, ss.str()),
         _errors);
   }

test/integration/deprecated_specs.cc

@@ -16,8 +16,13 @@
  */
 
 #include <gtest/gtest.h>
-#include "sdf/sdf.hh"
+
+#include "sdf/Element.hh"
+#include "sdf/Error.hh"
+#include "sdf/ParserConfig.hh"
+#include "sdf/parser.hh"
 #include "test_config.h"
+#include "test_utils.hh"
 
 ////////////////////////////////////////////////////
 TEST(DeprecatedSpecs, Spec1_0)
@@ -38,3 +43,89 @@ TEST(DeprecatedSpecs, Spec1_2)
   sdf::SDFPtr sdf(new sdf::SDF());
   EXPECT_FALSE(sdf::initFile(filename, sdf));
 }
+
+////////////////////////////////////////////////////
+TEST(DeprecatedElements, CanEmitErrors)
+{
+  sdf::SDFPtr sdf(new sdf::SDF());
+  sdf::init(sdf);
+  auto elem = std::make_shared<sdf::Element>();
+  elem->SetRequired("-1");
+  elem->SetName("testElem");
+  // Change the root of sdf so we can test "testElem" in isolation
+  sdf->Root(elem);
+  auto config = sdf::ParserConfig::GlobalConfig();
+  config.SetWarningsPolicy(sdf::EnforcementPolicy::ERR);
+  sdf::Errors errors;
+  EXPECT_TRUE(sdf::readString(
+      "<sdf version='1.8'><testElem/></sdf>", config, sdf, errors));
+  ASSERT_FALSE(errors.empty());
+  EXPECT_EQ(sdf::ErrorCode::ELEMENT_DEPRECATED, errors[0].Code());
+}
+
+bool contains(const std::string &_a, const std::string &_b)
+{
+  return _a.find(_b) != std::string::npos;
+}
+
+////////////////////////////////////////////////////
+TEST(DeprecatedElements, CanEmitWarningWithErrorEnforcmentPolicy)
+{
+  // Redirect sdfwarn output
+  std::stringstream buffer;
+  sdf::testing::RedirectConsoleStream redir(
+      sdf::Console::Instance()->GetMsgStream(), &buffer);
+
+#ifdef _WIN32
+  sdf::Console::Instance()->SetQuiet(false);
+  sdf::testing::ScopeExit revertSetQuiet(
+      []
+      {
+        sdf::Console::Instance()->SetQuiet(true);
+      });
+#endif
+
+  sdf::SDFPtr sdf(new sdf::SDF());
+  sdf::init(sdf);
+  auto elem = std::make_shared<sdf::Element>();
+  elem->SetRequired("-1");
+  elem->SetName("testElem");
+  // Change the root of sdf so we can test "testElem" in isolation
+  sdf->Root(elem);
+  {
+    auto config = sdf::ParserConfig::GlobalConfig();
+    config.SetWarningsPolicy(sdf::EnforcementPolicy::ERR);
+    config.SetDeprecatedElementsPolicy(sdf::EnforcementPolicy::WARN);
+    sdf::Errors errors;
+    EXPECT_TRUE(sdf::readString(
+          "<sdf version='1.8'><testElem/></sdf>", config, sdf, errors));
+    EXPECT_TRUE(errors.empty());
+    EXPECT_PRED2(contains, buffer.str(), "SDF Element[testElem] is deprecated");
+  }
+  // Flip the order of calling SetDeprecatedElementsPolicy and
+  // SetWarningsPolicy.
+  {
+    // clear the contents of the buffer
+    buffer.str("");
+    auto config = sdf::ParserConfig::GlobalConfig();
+    config.SetDeprecatedElementsPolicy(sdf::EnforcementPolicy::WARN);
+    config.SetWarningsPolicy(sdf::EnforcementPolicy::ERR);
+    sdf::Errors errors;
+    EXPECT_TRUE(sdf::readString(
+          "<sdf version='1.8'><testElem/></sdf>", config, sdf, errors));
+    EXPECT_TRUE(errors.empty());
+    EXPECT_PRED2(contains, buffer.str(), "SDF Element[testElem] is deprecated");
+  }
+  // Test ResetDeprecatedElementsPolicy
+  {
+    auto config = sdf::ParserConfig::GlobalConfig();
+    config.SetDeprecatedElementsPolicy(sdf::EnforcementPolicy::WARN);
+    config.SetWarningsPolicy(sdf::EnforcementPolicy::ERR);
+    config.ResetDeprecatedElementsPolicy();
+    sdf::Errors errors;
+    EXPECT_TRUE(sdf::readString(
+          "<sdf version='1.8'><testElem/></sdf>", config, sdf, errors));
+    ASSERT_FALSE(errors.empty());
+    EXPECT_EQ(sdf::ErrorCode::ELEMENT_DEPRECATED, errors[0].Code());
+  }
+}