Stop importing operator* and operator-> into Nullable. (project-chip#…

…34353)

These are serious footguns because:

1.  They don't do checks that there is actually a value, and will silently
    produce garbage.
2.  They are really easy to accidentally use without a IsNull() check
    (especially operator->).

I've now reviewed several PRs where people were misusing these and helped a few
other people who were misusing them and then not getting expected behavior, so
it would be better if we just didn't have these at all.  Nullable is not a
pointer and people shouldn't treat it like one.

src/app/cluster-building-blocks/QuieterReporting.h

@@ -178,9 +178,9 @@ class QuieterReportingAttribute
         bool isChangeOfNull       = newValue.IsNull() ^ mValue.IsNull();
         bool areBothValuesNonNull = !newValue.IsNull() && !mValue.IsNull();
 
-        bool changeToFromZero = areBothValuesNonNull && (*newValue == 0 || *mValue == 0);
-        bool isIncrement      = areBothValuesNonNull && (*newValue > *mValue);
-        bool isDecrement      = areBothValuesNonNull && (*newValue < *mValue);
+        bool changeToFromZero = areBothValuesNonNull && (newValue.Value() == 0 || mValue.Value() == 0);
+        bool isIncrement      = areBothValuesNonNull && (newValue.Value() > mValue.Value());
+        bool isDecrement      = areBothValuesNonNull && (newValue.Value() < mValue.Value());
 
         bool isNewlyDirty = isChangeOfNull;
         isNewlyDirty =

src/app/cluster-building-blocks/tests/TestQuieterReporting.cpp

@@ -98,7 +98,7 @@ TEST(TestQuieterReporting, ChangeOnIncrementPolicyWorks)
     QuieterReportingAttribute<int> attribute{ MakeNullable<int>(10) };
 
     // Always start not dirty (because first sub priming always just read value anyway).
-    ASSERT_EQ(*attribute.value(), 10);
+    ASSERT_EQ(attribute.value().Value(), 10);
 
     auto now = fakeClock.now();
 
@@ -149,7 +149,7 @@ TEST(TestQuieterReporting, ChangeOnDecrementPolicyWorks)
     QuieterReportingAttribute<int> attribute{ MakeNullable<int>(9) };
 
     // Always start not dirty (because first sub priming always just read value anyway).
-    ASSERT_EQ(*attribute.value(), 9);
+    ASSERT_EQ(attribute.value().Value(), 9);
 
     auto now = fakeClock.now();
 
@@ -202,7 +202,7 @@ TEST(TestQuieterReporting, SufficientChangePredicateWorks)
     QuieterReportingAttribute<int> attribute{ MakeNullable<int>(9) };
 
     // Always start not dirty (because first sub priming always just read value anyway).
-    ASSERT_EQ(*attribute.value(), 9);
+    ASSERT_EQ(attribute.value().Value(), 9);
 
     auto now = fakeClock.now();
 

src/app/codegen-data-model/CodegenDataModel_Write.cpp

@@ -166,8 +166,8 @@ CHIP_ERROR DecodeIntoEmberBuffer(AttributeValueDecoder & decoder, bool isNullabl
             // Nullable<uint8_t>(0xFF) is not representable because 0xFF is the encoding of NULL in ember
             // as well as odd-sized integers (e.g. full 32-bit value like 0x11223344 cannot be written
             // to a 3-byte odd-sized integger).
-            VerifyOrReturnError(Traits::CanRepresentValue(isNullable, *workingValue), CHIP_ERROR_INVALID_ARGUMENT);
-            Traits::WorkingToStorage(*workingValue, storageValue);
+            VerifyOrReturnError(Traits::CanRepresentValue(isNullable, workingValue.Value()), CHIP_ERROR_INVALID_ARGUMENT);
+            Traits::WorkingToStorage(workingValue.Value(), storageValue);
         }
 
         VerifyOrReturnError(out.size() >= sizeof(storageValue), CHIP_ERROR_INVALID_ARGUMENT);

src/app/data-model/Nullable.h

@@ -45,8 +45,11 @@ struct Nullable : protected std::optional<T>
     // all constructors of the base class within this derived class.
     //
     using std::optional<T>::optional;
-    using std::optional<T>::operator*;
-    using std::optional<T>::operator->;
+
+    // Do NOT pull in optional::operator* or optional::operator->, because that
+    // leads people to write code that looks like it should work, and compiles,
+    // but does not do the right things with TLV encoding and decoding, when
+    // nullable data model objects are involved.
 
     Nullable(NullOptionalType) : std::optional<T>(std::nullopt) {}