Trade: disallow empty MaterialAttributeData names.

Overlooked during the initial design -- it'd sort before " LayerName",
which is undesirable.

src/Magnum/Trade/MaterialData.cpp

@@ -130,6 +130,9 @@ std::size_t materialAttributeTypeSize(const MaterialAttributeType type) {
 }
 
 MaterialAttributeData::MaterialAttributeData(const Containers::StringView name, const MaterialAttributeType type, const std::size_t size, const void* const value) noexcept: _data{} /* zero-initialized */ {
+    /* It would sort before " LayerName" and that's not desirable */
+    CORRADE_ASSERT(!name.isEmpty(), "Trade::MaterialAttributeData: name is not allowed to be empty", );
+
     /* Special handling for strings */
     if(type == MaterialAttributeType::String) {
         const auto& stringValue = *static_cast<const Containers::StringView*>(value);

src/Magnum/Trade/MaterialData.h

@@ -2906,20 +2906,26 @@ template<class T
     , class
     #endif
 > constexpr MaterialAttributeData::MaterialAttributeData(const Containers::StringView name, const T& value) noexcept:
-    _data{Implementation::MaterialAttributeTypeFor<T>::type(),
+    _data{Implementation::MaterialAttributeTypeFor<T>::type(), (
+        /* It would sort before " LayerName" and that's not desirable */
+        CORRADE_CONSTEXPR_ASSERT(!name.isEmpty(), "Trade::MaterialAttributeData: name is not allowed to be empty"),
         /* MSVC 2015 complains about "error C2065: 'T': undeclared identifier"
            in the lambda inside this macro. Sorry, the assert will be less
            useful on that stupid thing. */
         #ifndef CORRADE_MSVC2015_COMPATIBILITY
-        (CORRADE_CONSTEXPR_ASSERT(name.size() + sizeof(T) + 2 <= Implementation::MaterialAttributeDataSize, "Trade::MaterialAttributeData: name" << name << "too long, expected at most" << Implementation::MaterialAttributeDataSize - sizeof(T) - 2 << "bytes for" << Implementation::MaterialAttributeTypeFor<T>::type() << "but got" << name.size()), name)
+        CORRADE_CONSTEXPR_ASSERT(name.size() + sizeof(T) + 2 <= Implementation::MaterialAttributeDataSize, "Trade::MaterialAttributeData: name" << name << "too long, expected at most" << Implementation::MaterialAttributeDataSize - sizeof(T) - 2 << "bytes for" << Implementation::MaterialAttributeTypeFor<T>::type() << "but got" << name.size()),
         #else
-        (CORRADE_CONSTEXPR_ASSERT(name.size() + sizeof(T) + 2 <= Implementation::MaterialAttributeDataSize, "Trade::MaterialAttributeData: name" << name << "too long, got" << name.size() << "bytes"), name)
+        CORRADE_CONSTEXPR_ASSERT(name.size() + sizeof(T) + 2 <= Implementation::MaterialAttributeDataSize, "Trade::MaterialAttributeData: name" << name << "too long, got" << name.size() << "bytes"),
         #endif
-    , value} {}
+    name), value} {}
 
 /* The 4 extra bytes are for a null byte after both the name and value, a type
    and a string size */
-constexpr MaterialAttributeData::MaterialAttributeData(const Containers::StringView name, const Containers::StringView value) noexcept: _data{(CORRADE_CONSTEXPR_ASSERT(name.size() + value.size() + 4 <= Implementation::MaterialAttributeDataSize, "Trade::MaterialAttributeData: name" << name << "and value" << value << "too long, expected at most" << Implementation::MaterialAttributeDataSize - 4 << "bytes in total but got" << name.size() + value.size()), name), value} {}
+constexpr MaterialAttributeData::MaterialAttributeData(const Containers::StringView name, const Containers::StringView value) noexcept: _data{(
+        /* It would sort before " LayerName" and that's not desirable */
+        CORRADE_CONSTEXPR_ASSERT(!name.isEmpty(), "Trade::MaterialAttributeData: name is not allowed to be empty"),
+        CORRADE_CONSTEXPR_ASSERT(name.size() + value.size() + 4 <= Implementation::MaterialAttributeDataSize, "Trade::MaterialAttributeData: name" << name << "and value" << value << "too long, expected at most" << Implementation::MaterialAttributeDataSize - 4 << "bytes in total but got" << name.size() + value.size()),
+    name), value} {}
 
 template<class T> T MaterialAttributeData::value() const {
     CORRADE_ASSERT(Implementation::MaterialAttributeTypeFor<T>::type() == _data.type,

src/Magnum/Trade/Test/MaterialDataTest.cpp

@@ -73,6 +73,8 @@ class MaterialDataTest: public TestSuite::Tester {
         void constructAttributeInvalidLayerName();
         void constructAttributeWrongTypeForName();
         void constructAttributeInvalidType();
+        void constructAttributeEmptyName();
+        void constructAttributeEmptyNameString();
         void constructAttributeTooLarge();
         void constructAttributeTooLargeString();
         void constructAttributeTooLargeNameString();
@@ -214,6 +216,8 @@ MaterialDataTest::MaterialDataTest() {
               &MaterialDataTest::constructAttributeInvalidLayerName,
               &MaterialDataTest::constructAttributeWrongTypeForName,
               &MaterialDataTest::constructAttributeInvalidType,
+              &MaterialDataTest::constructAttributeEmptyName,
+              &MaterialDataTest::constructAttributeEmptyNameString,
               &MaterialDataTest::constructAttributeTooLarge,
               &MaterialDataTest::constructAttributeTooLargeString,
               &MaterialDataTest::constructAttributeTooLargeNameString,
@@ -707,6 +711,42 @@ void MaterialDataTest::constructAttributeInvalidType() {
         "Trade::materialAttributeTypeSize(): invalid type Trade::MaterialAttributeType(0xfe)\n");
 }
 
+void MaterialDataTest::constructAttributeEmptyName() {
+    #ifdef CORRADE_NO_ASSERT
+    CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
+    #endif
+
+    std::ostringstream out;
+    Error redirectError{&out};
+    MaterialAttributeData{"", Int{}};
+    /* Constexpr variant has the same assert, but in the header. It should have
+       the same output. */
+    /*constexpr*/ MaterialAttributeData{""_s, Int{}};
+    CORRADE_COMPARE(out.str(),
+        "Trade::MaterialAttributeData: name is not allowed to be empty\n"
+        "Trade::MaterialAttributeData: name is not allowed to be empty\n");
+}
+
+void MaterialDataTest::constructAttributeEmptyNameString() {
+    #ifdef CORRADE_NO_ASSERT
+    CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
+    #endif
+
+    /* This has no reason to not be allowed */
+    MaterialAttributeData{"hello this string is empty", ""};
+    MaterialAttributeData{"hello this string is empty"_s, ""_s};
+
+    std::ostringstream out;
+    Error redirectError{&out};
+    MaterialAttributeData{"", "hello"};
+    /* Constexpr variant has the same assert, but in the header. It should have
+       the same output. */
+    /*constexpr*/ MaterialAttributeData{""_s, "hello"_s};
+    CORRADE_COMPARE(out.str(),
+        "Trade::MaterialAttributeData: name is not allowed to be empty\n"
+        "Trade::MaterialAttributeData: name is not allowed to be empty\n");
+}
+
 void MaterialDataTest::constructAttributeTooLarge() {
     #ifdef CORRADE_NO_ASSERT
     CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");