9 ➡️ 10

.github/workflows/ci.yml

@@ -13,7 +13,7 @@ jobs:
         id: ci
         uses: ignition-tooling/action-ignition-ci@bionic
         with:
-          codecov-token: ${{ secrets.CODECOV_TOKEN }}
+          codecov-enabled: true
   focal-ci:
     runs-on: ubuntu-latest
     name: Ubuntu Focal CI

CMakeLists.txt

@@ -270,6 +270,27 @@ else (build_errors)
     add_subdirectory(doc)
   endif(BUILD_SDF)
 
+  ########################################
+  # Setup Codecheck
+  include (IgnCodeCheck)
+  set(CPPCHECK_DIRS
+    ${PROJECT_SOURCE_DIR}/src
+    ${PROJECT_SOURCE_DIR}/include
+    ${PROJECT_SOURCE_DIR}/test/integration
+    ${PROJECT_SOURCE_DIR}/test/performance)
+
+  set(CPPCHECK_INCLUDE_DIRS
+    ${PROJECT_BINARY_DIR}
+    ${PROJECT_SOURCE_DIR}/include
+    ${PROJECT_SOURCE_DIR}/test/integration
+    ${PROJECT_SOURCE_DIR}/test/performance)
+
+  # Ignore vendored directories.
+  file(WRITE ${PROJECT_BINARY_DIR}/cppcheck.suppress
+    "*:${PROJECT_SOURCE_DIR}/src/urdf/*\n"
+    )
+  ign_setup_target_for_codecheck()
+
   ########################################
   # Make the package config file
   configure_file(${CMAKE_SOURCE_DIR}/cmake/sdformat_pc.in

README.md

@@ -17,7 +17,8 @@ allows conversion from previous versions.
 
 Test coverage:
 
-[![codecov](https://codecov.io/gh/osrf/sdformat/branch/master/graph/badge.svg)](https://codecov.io/gh/osrf/sdformat)
+<!-- Note: The branch name in the codecov URL should be updated when forward porting -->
+[![codecov](https://codecov.io/gh/ignitionrobotics/sdformat/branch/sdf10/graph/badge.svg)](https://codecov.io/gh/ignitionrobotics/sdformat/branch/sdf10)
 
 # Installation
 

cmake/SearchForStuff.cmake

@@ -102,8 +102,8 @@ endmacro()
 
 ########################################
 # Find ignition cmake2
-# Only for using the testing macros, not really
-# being use to configure the whole project
+# Only for using the testing macros and creating the codecheck target, not 
+# really being use to configure the whole project
 find_package(ignition-cmake2 2.3 REQUIRED)
 set(IGN_CMAKE_VER ${ignition-cmake2_VERSION_MAJOR})
 

include/sdf/Console.hh

@@ -73,7 +73,7 @@ namespace sdf
       /// \brief Constructor.
       /// \param[in] _stream Pointer to an output stream operator. Can be
       /// NULL/nullptr.
-      public: ConsoleStream(std::ostream *_stream) :
+      public: explicit ConsoleStream(std::ostream *_stream) :
               stream(_stream) {}
 
       /// \brief Redirect whatever is passed in to both our ostream

include/sdf/Element.hh

@@ -206,7 +206,7 @@ namespace sdf
                               const std::string &_type,
                               const std::string &_defaultvalue,
                               bool _required,
-                              const std::string &_description="");
+                              const std::string &_description = "");
 
     /// \brief Add a value to this Element.
     /// \param[in] _type Type of data the parameter will hold.
@@ -216,7 +216,7 @@ namespace sdf
     /// \throws sdf::AssertionInternalError if an invalid type is given.
     public: void AddValue(const std::string &_type,
                           const std::string &_defaultValue, bool _required,
-                          const std::string &_description="");
+                          const std::string &_description = "");
 
     /// \brief Add a value to this Element. This override allows passing min and
     /// max values of the parameter.
@@ -514,7 +514,7 @@ namespace sdf
                                   const std::string &_type,
                                   const std::string &_defaultValue,
                                   bool _required,
-                                  const std::string &_description="");
+                                  const std::string &_description = "");
 
 
     /// \brief Private data pointer

include/sdf/Exception.hh

@@ -126,6 +126,7 @@ namespace sdf
     /// \param[in] _line Line number where the error occurred
     /// \param[in] _msg Error message
     public: InternalError(const char *_file, std::int64_t _line,
+                          // cppcheck-suppress passedByValue
                           const std::string _msg);
 
     /// \brief Destructor
@@ -148,8 +149,11 @@ namespace sdf
     /// \param[in] _msg Function where assertion failed
     public: AssertionInternalError(const char *_file,
                                    std::int64_t _line,
+                                   // cppcheck-suppress passedByValue
                                    const std::string _expr,
+                                   // cppcheck-suppress passedByValue
                                    const std::string _function,
+                                   // cppcheck-suppress passedByValue
                                    const std::string _msg = "");
     /// \brief Destructor
     public: virtual ~AssertionInternalError();

include/sdf/Param.hh

@@ -290,6 +290,8 @@ namespace sdf
 
     /// \def ParamVariant
     /// \brief Variant type def.
+    /// Note: When a new variant is added, add variant to functions
+    /// ParamPrivate::TypeToString and ParamPrivate::ValueFromStringImpl
     public: typedef std::variant<bool, char, std::string, int, std::uint64_t,
                                    unsigned int, double, float, sdf::Time,
                                    ignition::math::Angle,
@@ -332,6 +334,7 @@ namespace sdf
   template<typename T>
   std::string ParamPrivate::TypeToString() const
   {
+    // cppcheck-suppress syntaxError
     if constexpr (std::is_same_v<T, bool>)
       return "bool";
     else if constexpr (std::is_same_v<T, char>)

include/sdf/ParticleEmitter.hh

@@ -17,6 +17,7 @@
 #ifndef SDF_PARTICLE_EMITTER_HH_
 #define SDF_PARTICLE_EMITTER_HH_
 
+#include <memory>
 #include <string>
 
 #include <ignition/math/Pose3.hh>

sdf/1.5/model.sdf

@@ -34,6 +34,7 @@
     <element name="pose" type="pose" default="0 0 0 0 0 0" required="0">
       <description>Override the pose of the included model. A position and orientation in the global coordinate frame for the model. Position(x,y,z) and rotation (roll, pitch yaw) in the global coordinate frame.</description>
     </element>
+    <include filename="plugin.sdf" required="*"/>
 
     <element name="name" type="string" default="" required="0">
       <description>Override the name of the included model.</description>

sdf/1.5/world.sdf

@@ -28,6 +28,7 @@
     </element>
 
     <include filename="pose.sdf" required="0"/>
+    <include filename="plugin.sdf" required="*"/>
   </element>
 
 

sdf/1.6/model.sdf

@@ -34,6 +34,7 @@
     <element name="pose" type="pose" default="0 0 0 0 0 0" required="0">
       <description>Override the pose of the included model. A position and orientation in the global coordinate frame for the model. Position(x,y,z) and rotation (roll, pitch yaw) in the global coordinate frame.</description>
     </element>
+    <include filename="plugin.sdf" required="*"/>
 
     <element name="name" type="string" default="" required="0">
       <description>Override the name of the included model.</description>

sdf/1.6/world.sdf

@@ -36,6 +36,7 @@
     </element>
 
     <include filename="pose.sdf" required="0"/>
+    <include filename="plugin.sdf" required="*"/>
   </element>
 
   <element name="gravity" type="vector3" default="0 0 -9.8" required="1">

sdf/1.7/model.sdf

@@ -41,6 +41,7 @@
     </element>
 
     <include filename="pose.sdf" required="0"/>
+    <include filename="plugin.sdf" required="*"/>
 
     <element name="name" type="string" default="" required="0">
       <description>Override the name of the included model.</description>

sdf/1.7/world.sdf

@@ -36,6 +36,7 @@
     </element>
 
     <include filename="pose.sdf" required="0"/>
+    <include filename="plugin.sdf" required="*"/>
   </element>
 
   <element name="gravity" type="vector3" default="0 0 -9.8" required="1">

src/Converter_TEST.cc

@@ -1898,10 +1898,14 @@ TEST(Converter, MuchNewerVersion)
   ASSERT_TRUE(sdf::Converter::Convert(&xmlDoc, "1.6"));
 }
 
-const std::string CONVERT_DOC_15_16 =
-  sdf::testing::SourceFile("sdf", "1.6", "1_5.convert");
-const std::string CONVERT_DOC_16_17 =
-  sdf::testing::SourceFile("sdf", "1.7", "1_6.convert");
+static std::string ConvertDoc_15_16()
+{
+  return sdf::testing::SourceFile("sdf", "1.6", "1_5.convert");
+}
+static std::string ConvertDoc_16_17()
+{
+  return sdf::testing::SourceFile("sdf", "1.7", "1_6.convert");
+}
 
 /////////////////////////////////////////////////
 /// Test conversion of imu in 1.5 to 1.6
@@ -1943,7 +1947,7 @@ TEST(Converter, IMU_15_to_16)
 
   // Convert
   tinyxml2::XMLDocument convertXmlDoc;
-  convertXmlDoc.LoadFile(CONVERT_DOC_15_16.c_str());
+  convertXmlDoc.LoadFile(ConvertDoc_15_16().c_str());
   sdf::Converter::Convert(&xmlDoc, &convertXmlDoc);
 
   // Check some basic elements
@@ -2037,7 +2041,7 @@ TEST(Converter, World_15_to_16)
 
   // Convert
   tinyxml2::XMLDocument convertXmlDoc;
-  convertXmlDoc.LoadFile(CONVERT_DOC_15_16.c_str());
+  convertXmlDoc.LoadFile(ConvertDoc_15_16().c_str());
   sdf::Converter::Convert(&xmlDoc, &convertXmlDoc);
 
   // Check some basic elements
@@ -2093,7 +2097,7 @@ TEST(Converter, Pose_16_to_17)
 
   // Convert
   tinyxml2::XMLDocument convertXmlDoc;
-  convertXmlDoc.LoadFile(CONVERT_DOC_16_17.c_str());
+  convertXmlDoc.LoadFile(ConvertDoc_16_17().c_str());
   sdf::Converter::Convert(&xmlDoc, &convertXmlDoc);
 
   // Check some basic elements

src/Error.cc

@@ -39,7 +39,6 @@ std::string Error::Message() const
 }
 
 /////////////////////////////////////////////////
-// cppcheck-suppress unusedFunction
 Error::operator bool() const
 {
   return this->code != ErrorCode::NONE;
@@ -58,7 +57,6 @@ namespace sdf
 inline namespace SDF_VERSION_NAMESPACE {
 
 /////////////////////////////////////////////////
-// cppcheck-suppress unusedFunction
 std::ostream &operator<<(std::ostream &_out, const sdf::Error &_err)
 {
   _out << "Error Code "

src/FrameSemantics.cc

@@ -15,6 +15,7 @@
  *
 */
 #include <string>
+#include <set>
 #include <utility>
 #include <vector>
 

src/Joint.cc

@@ -67,7 +67,6 @@ class sdf::JointPrivate
   public: double threadPitch = 1.0;
 
   /// \brief Joint axis
-  // cppcheck-suppress
   public: std::array<std::unique_ptr<JointAxis>, 2> axis;
 
   /// \brief The SDF element pointer used during load.

src/Light.cc

@@ -122,7 +122,7 @@ Light &Light::operator=(Light &&_light)
 //////////////////////////////////////////////////
 void Light::CopyFrom(const Light &_light)
 {
-  this->dataPtr->name= _light.dataPtr->name;
+  this->dataPtr->name = _light.dataPtr->name;
   this->dataPtr->pose = _light.dataPtr->pose;
   this->dataPtr->poseRelativeTo = _light.dataPtr->poseRelativeTo;
   this->dataPtr->type = _light.dataPtr->type;
@@ -251,7 +251,7 @@ Errors Light::Load(ElementPtr _sdf)
     this->SetSpotInnerAngle(doubleValue.first);
 
     // Check for and set outer_angle
-    doubleValue =spotElem->Get<double>(
+    doubleValue = spotElem->Get<double>(
         "outer_angle", this->dataPtr->spotOuterAngle.Radian());
     if (!doubleValue.second)
     {
@@ -261,7 +261,7 @@ Errors Light::Load(ElementPtr _sdf)
     this->SetSpotOuterAngle(doubleValue.first);
 
     // Check for and set falloff
-    doubleValue =spotElem->Get<double>("falloff", this->dataPtr->spotFalloff);
+    doubleValue = spotElem->Get<double>("falloff", this->dataPtr->spotFalloff);
     if (!doubleValue.second)
     {
       errors.push_back({ErrorCode::ELEMENT_MISSING,

src/Link.cc

@@ -524,5 +524,5 @@ bool Link::EnableWind() const
 /////////////////////////////////////////////////
 void Link::SetEnableWind(const bool _enableWind)
 {
-  this->dataPtr->enableWind =_enableWind;
+  this->dataPtr->enableWind = _enableWind;
 }

src/Model.cc

@@ -495,7 +495,7 @@ bool Model::EnableWind() const
 /////////////////////////////////////////////////
 void Model::SetEnableWind(const bool _enableWind)
 {
-  this->dataPtr->enableWind =_enableWind;
+  this->dataPtr->enableWind = _enableWind;
 }
 
 /////////////////////////////////////////////////

src/Noise.cc

@@ -123,7 +123,7 @@ Errors Noise::Load(ElementPtr _sdf)
   else if (typeLower == "gaussian")
     this->dataPtr->type = NoiseType::GAUSSIAN;
   else if (typeLower == "gaussian_quantized")
-    this->dataPtr->type =NoiseType::GAUSSIAN_QUANTIZED;
+    this->dataPtr->type = NoiseType::GAUSSIAN_QUANTIZED;
   else
   {
     errors.push_back({ErrorCode::ELEMENT_MISSING,

src/Param.cc

@@ -632,6 +632,7 @@ bool Param::ValidateValue() const
       {
         using T = std::decay_t<decltype(_val)>;
         // cppcheck-suppress syntaxError
+        // cppcheck-suppress unmatchedSuppression
         if constexpr (std::is_scalar_v<T>)
         {
           if (this->dataPtr->minValue.has_value())

src/ParamPassing.cc

@@ -15,6 +15,7 @@
  *
  */
 #include <cstring>
+#include <memory>
 #include <vector>
 
 #include "sdf/Filesystem.hh"

src/Pbr.cc

@@ -206,7 +206,7 @@ Errors PbrWorkflow::Load(sdf::ElementPtr _sdf)
   this->dataPtr->albedoMap = _sdf->Get<std::string>("albedo_map",
       this->dataPtr->albedoMap).first;
 
-  this->dataPtr->normalMap= _sdf->Get<std::string>("normal_map",
+  this->dataPtr->normalMap = _sdf->Get<std::string>("normal_map",
     this->dataPtr->normalMap).first;
 
   this->dataPtr->environmentMap = _sdf->Get<std::string>("environment_map",

src/SDF.cc

@@ -44,10 +44,11 @@ static URIPathMap g_uriPathMap;
 
 static std::function<std::string(const std::string &)> g_findFileCB;
 
-std::string SDF::version = SDF_VERSION;
+// TODO(azeey) This violates the Google style guide. Change to a function that
+// returns the version string when possible.
+std::string SDF::version = SDF_VERSION;  // NOLINT(runtime/string)
 
 /////////////////////////////////////////////////
-// cppcheck-suppress passedByValue
 void setFindCallback(std::function<std::string(const std::string &)> _cb)
 {
   g_findFileCB = _cb;
@@ -65,7 +66,6 @@ std::string findFile(const std::string &_filename, bool _searchLocalPath,
   {
     // Check to see if the URI in the global map is the first part of the
     // given filename
-    // cppcheck-suppress stlIfStrFind
     if (_filename.find(iter->first) == 0)
     {
       std::string suffix = _filename;
@@ -323,9 +323,7 @@ void SDF::PrintDoc()
 
   std::cout << "</div>\n";
 
-  std::cout << "\
-    </body>\
-    </html>\n";
+  std::cout << "    </body>    </html>\n";
 }
 
 /////////////////////////////////////////////////

src/SDFExtension.hh

@@ -34,7 +34,7 @@ namespace sdf
   // Inline bracket to help doxygen filtering.
   inline namespace SDF_VERSION_NAMESPACE {
   //
-  using XMLDocumentPtr= std::shared_ptr<tinyxml2::XMLDocument>;
+  using XMLDocumentPtr = std::shared_ptr<tinyxml2::XMLDocument>;
   using XMLElementPtr = std::shared_ptr<tinyxml2::XMLElement>;
 
   /// \internal

src/Scene.cc

@@ -128,7 +128,7 @@ Errors Scene::Load(ElementPtr _sdf)
   }
 
   // Get the ambient property
-  this->dataPtr->ambient= _sdf->Get<ignition::math::Color>("ambient",
+  this->dataPtr->ambient = _sdf->Get<ignition::math::Color>("ambient",
       this->dataPtr->ambient).first;
 
   // Get the background color property