Merge pull request #36284 from mantidproject/patch-enumerated-string2

type security to enumerated string, extra test of non-class enum

Framework/Kernel/inc/MantidKernel/EnumeratedString.h

@@ -8,6 +8,7 @@
 #include <boost/algorithm/string.hpp>
 #include <sstream>
 #include <string>
+#include <type_traits>
 #include <vector>
 
 namespace Mantid {
@@ -38,6 +39,9 @@ class EnumeratedString {
  *
  * @tparam an optional pointer to a statically defined string comparator.
  */
+
+ static_assert(std::is_enum_v<E>); // cause a compiler error if E is not an enum
+
 public:
  EnumeratedString() { ensureCompatibleSize(); }
 

Framework/Kernel/test/EnumeratedStringTest.h

@@ -35,10 +35,12 @@ class EnumeratedStringTest : public CxxTest::TestSuite {
 
  // test constructor from enumerator
  COOLGUY dude1(CoolGuys::Fred);
+ TS_ASSERT_EQUALS(dude1, "Frederic");
  TS_ASSERT_EQUALS(dude1, CoolGuys(0));
  TS_ASSERT_EQUALS(dude1, CoolGuys::Fred);
  TS_ASSERT_EQUALS(dude1, coolGuyNames[0]);
  TS_ASSERT_DIFFERS(dude1, CoolGuys::Bill);
+ TS_ASSERT_DIFFERS(dude1, "Jeremy");
 
  // test constructor from string name
  COOLGUY dude2(coolGuyNames[1]);
@@ -211,7 +213,7 @@ class EnumeratedStringTest : public CxxTest::TestSuite {
  // try removing enum_count -- should give a compiler error
  enum class Letters : size_t { a, b, enum_count };
  static const std::vector<std::string> letters{"a", "b"};
- enum Graphia : size_t { alpha, beta, enum_count };
+ enum class Graphia : size_t { alpha, beta, enum_count };
  static const std::vector<std::string> graphia{"alpha", "beta"};
  EnumeratedString<Letters, &letters> l1("a");
  EnumeratedString<Graphia, &graphia> l2("alpha");
@@ -232,8 +234,8 @@ class EnumeratedStringTest : public CxxTest::TestSuite {
  void testSwitchAndIf() {
  g_log.notice("\ntestSwitchAndIf...");
 
- const size_t index = 3;
- CAKE tasty = Cakes(index), scrumptious = Cakes(index);
+ const char index = 3;
+ CAKE tasty = Cakes(index);
 
  // test enumerated string against string
  if (tasty == cakeNames[index]) {
@@ -262,6 +264,29 @@ class EnumeratedStringTest : public CxxTest::TestSuite {
  }
  }
 
+ void testUnderlyingType() {
+ g_log.notice("\ntestUnderlyingType...");
+
+ // use a non-class enum to compare against chars
+ enum LetterA : char { a, enum_count };
+ static const std::vector<std::string> letterA = {"a"};
+ typedef EnumeratedString<LetterA, &letterA> LETTERA;
+ LETTERA a1 = a; // note that a is in namespace, vars cannot be named this
+ TS_ASSERT_EQUALS(a1, a); // compare against enum
+ TS_ASSERT_EQUALS(a1, "a"); // compares against string
+ TS_ASSERT_EQUALS(a1, 0); // compare against literal
+ TS_ASSERT_EQUALS(a1, '\x00'); // char literal at 0
+ TS_ASSERT_DIFFERS(a1, 'a'); // Letters::a corresponds to 0, not to 'a'= 97
+ TS_ASSERT_DIFFERS(a1, '\x01'); // char literal at 1
+ TS_ASSERT_DIFFERS(a1, 1);
+ switch (a1) {
+ case '\x00':
+ break;
+ default:
+ TS_FAIL("EnumeratedString in 'SWITCH' failed to match to underlying type");
+ }
+ }
+
  void testCaseInsensitiveNameComparison() {
  g_log.notice("\ntestCaseInsensitiveNameComparison...");
 

dev-docs/source/EnumeratedString.rst

@@ -19,7 +19,7 @@ would be something like the following:
 
 However, this is not allowed under C++.
 
-The ``EnumeratedString`` objects allow for binding an ``enum class`` to a C-style static array of strings, allowing for much
+The ``EnumeratedString`` objects allow for binding an ``enum`` or ``enum class`` to a vector of strings, allowing for much
 of the same behavior. This allows for easy-to-read ``if`` and ``switch`` statements, as well as easy conversions and assignments
 with strings from the allowed set. This further adds an additional layer of validation for string properties, in additon to the
 ``StringListValidator`` used in the property declaration.
@@ -69,6 +69,11 @@ Notice that the final element of the ``enum`` is called :code:`enum_count`. Thi
 number of elements inside the ``enum``, and used for verifying compatibility with the vector of strings. A compiler error
 will be triggered if this is not included.
 
+Further, the ``enum`` *must* have elements in order from 0 to :code:`enum_count`. That is, you *CANNOT* set them like so:
+.. code-block:: cpp
+ enum class CakeTypeEnum : char {LEMON='l', BUNDT='b', POUND='p', enum_count=3}; // NOT ALLOWED
+as this will break validation features inside the class.
+
 Notice the use of the reference operator, :code:`&cakeTypeNames`, and *not* :code:`cakeTypeNames`.
 
 In the above code, a :code:`CAKETYPE` object can be created either from a :code:`CakeTypeEnum`, or from one of the strings
@@ -104,7 +109,7 @@ An example of where this might be used inside an algorithm is shown below:
 
  void BakeCake::exec() {
  // this will assign cakeType from the string property
- CAKETYPE cakeType = getProperty("CakeType");
+ CAKETYPE cakeType = getPropertyValue("CakeType");
 
  // logic can branch on cakeType comparing to the enum
  switch(cakeType){