Merge pull request #38993 from mantidproject/38989-follow-up

Remove two CppCheck suppressions on ExperimentInfo

Author: rosswhitfield

Framework/API/inc/MantidAPI/FileLoaderRegistry.h

@@ -92,25 +92,22 @@ class MANTID_API_DLL FileLoaderRegistryImpl {
       case LegacyNexus:
         if (!std::is_base_of<IFileLoader<Kernel::LegacyNexusDescriptor>, T>::value) {
           throw std::runtime_error(std::string("FileLoaderRegistryImpl::subscribe - Class '") + typeid(T).name() +
-                                   "' registered as Nexus loader but it does not "
-                                   "inherit from "
-                                   "API::IFileLoader<Kernel::NexusDescriptor>");
+                                   "' registered as LegacyNexus loader but it does not inherit from "
+                                   "API::IFileLoader<Kernel::LegacyNexusDescriptor>");
         }
         break;
       case Nexus:
         if (!std::is_base_of<IFileLoader<Kernel::NexusDescriptor>, T>::value) {
-          throw std::runtime_error(std::string("FileLoaderRegistryImpl::subscribe - Class '") + typeid(T).name() +
-                                   "' registered as NexusHDF5 loader but it does not "
-                                   "inherit from "
-                                   "API::IFileLoader<Kernel::NexusDescriptor>");
+          throw std::runtime_error(
+              std::string("FileLoaderRegistryImpl::subscribe - Class '") + typeid(T).name() +
+              "' registered as Nexus loader but it does not inherit from API::IFileLoader<Kernel::NexusDescriptor>");
         }
         break;
       case Generic:
         if (!std::is_base_of<IFileLoader<Kernel::FileDescriptor>, T>::value) {
-          throw std::runtime_error(std::string("FileLoaderRegistryImpl::subscribe - Class '") + typeid(T).name() +
-                                   "' registered as Generic loader but it does "
-                                   "not inherit from "
-                                   "API::IFileLoader<Kernel::FileDescriptor>");
+          throw std::runtime_error(
+              std::string("FileLoaderRegistryImpl::subscribe - Class '") + typeid(T).name() +
+              "' registered as Generic loader but it does not inherit from API::IFileLoader<Kernel::FileDescriptor>");
         }
         break;
       default:

Framework/API/src/ExperimentInfo.cpp

@@ -1224,7 +1224,7 @@ void ExperimentInfo::readParameterMap(const std::string &parameterStr) {
     else {                      // defined, the paramValue has one too many entries, -1 to remove also the semicolon
       paramVisibility =
           paramVisibility.substr(paramVisibility.find(visibilityKey) + visibilityKey.size(), paramVisibility.size());
-      paramValue = paramValue.substr(0, paramValue.find(visibilityKey) - 1);
+      paramValue.erase(paramValue.find(visibilityKey) - 1, paramValue.size());
     }
     const auto paramDescr = std::string("");
     if (paramName == "masked") {
@@ -1350,7 +1350,7 @@ IPropertyManager::getValue<Mantid::API::ExperimentInfo_sptr>(const std::string &
 template <>
 MANTID_API_DLL Mantid::API::ExperimentInfo_const_sptr
 IPropertyManager::getValue<Mantid::API::ExperimentInfo_const_sptr>(const std::string &name) const {
-  auto *prop = dynamic_cast<PropertyWithValue<Mantid::API::ExperimentInfo_sptr> *>(getPointerToProperty(name));
+  auto const *prop = dynamic_cast<PropertyWithValue<Mantid::API::ExperimentInfo_sptr> *>(getPointerToProperty(name));
   if (prop) {
     return prop->operator()();
   } else {

Framework/API/test/ExperimentInfoTest.h

@@ -680,6 +680,27 @@ class ExperimentInfoTest : public CxxTest::TestSuite {
     auto &pmap = expInfo.instrumentParameters();
     auto det = expInfo.getInstrument()->getDetector(1);
     auto value = pmap.getString(det.get(), "par");
+    TS_ASSERT_EQUALS(pmap.get(det.get(), "par")->visible(), true);
+    TS_ASSERT_EQUALS(value, "11;22;33;44");
+  }
+
+  void test_readParameterMap_visibility_key() {
+    auto inst = ComponentCreationHelper::createMinimalInstrument(V3D{-2, 0, 0} /*source*/, V3D{10, 0, 0} /*sample*/,
+                                                                 V3D{12, 0, 0} /*detector*/);
+    ExperimentInfo expInfo;
+    expInfo.setInstrument(inst);
+    expInfo.readParameterMap("detID:1;string;par;11;22;33;44;visible:false");
+    auto &pmap = expInfo.instrumentParameters();
+    auto det = expInfo.getInstrument()->getDetector(1);
+    auto value = pmap.getString(det.get(), "par");
+    TS_ASSERT_EQUALS(pmap.get(det.get(), "par")->visible(), false);
+    TS_ASSERT_EQUALS(value, "11;22;33;44");
+
+    expInfo.readParameterMap("detID:1;string;par;11;22;33;44;visible:true");
+    auto &pmap2 = expInfo.instrumentParameters();
+    det = expInfo.getInstrument()->getDetector(1);
+    value = pmap2.getString(det.get(), "par");
+    TS_ASSERT_EQUALS(pmap2.get(det.get(), "par")->visible(), true);
     TS_ASSERT_EQUALS(value, "11;22;33;44");
   }
 

buildconfig/CMake/CppCheck_Suppressions.txt.in

@@ -123,8 +123,6 @@ constVariablePointer:${CMAKE_SOURCE_DIR}/Framework/API/src/CompositeFunction.cpp
 constVariableReference:${CMAKE_SOURCE_DIR}/Framework/API/src/ExperimentInfo.cpp:140
 constVariablePointer:${CMAKE_SOURCE_DIR}/Framework/API/src/ExperimentInfo.cpp:579
 nullPointerRedundantCheck:${CMAKE_SOURCE_DIR}/Framework/API/src/ExperimentInfo.cpp:889
-uselessCallsSubstr:${CMAKE_SOURCE_DIR}/Framework/API/src/ExperimentInfo.cpp:1227
-constVariablePointer:${CMAKE_SOURCE_DIR}/Framework/API/src/ExperimentInfo.cpp:1353
 stlIfStrFind:${CMAKE_SOURCE_DIR}/Framework/API/src/FileFinder.cpp:148
 stlIfStrFind:${CMAKE_SOURCE_DIR}/Framework/API/src/FileFinder.cpp:160
 knownConditionTrueFalse:${CMAKE_SOURCE_DIR}/Framework/API/src/FunctionFactory.cpp:267