fix(core): ObsoleteAttribute's three components are nullable (#2295, SR-AUD-116)

Message, DiagnosticId and UrlFormat were non-nullable std::string, so an ABSENT
value and an EMPTY one were the same state -- measured, a default attribute and
one built from std::string{} compared EQUAL. .NET's are all string?.

Route A, the faithful one, under SA-8 with SA-3 (sizeof 112 -> 136) and SA-10.

THE TICKET'S OWN OBJECTION TO ROUTE A IS DISCHARGED BY MEASUREMENT, NOT
OVERRULED. It said 'zero first-party production consumers does NOT license
option A: the header is public in Core.Base and downstream consumers exist and
were not inspected'. They have been now: mobile-eggbert mentions
ObsoleteAttribute zero times, and cna mentions it once -- inside a COMMENT, not
in code. Neither repository was modified. That is SA-2 condition 5 doing exactly
the job it exists for.

A GETTER CHANGE ALONE COULD NOT HAVE CLOSED IT, which is why route B was not
enough either: the boundary was on the way IN as well as out, since the
constructor and both setters took const std::string&, so a caller could neither
supply an absent value nor return a component to that state. A test asserts all
three directions.

The layout pin stays a RELATIONSHIP rather than a re-recorded number, and gained
a second assertion so that silently dropping the optionals back to plain strings
fails rather than passing on a stale relationship. Message and IsError stay
getter-only while DiagnosticId and UrlFormat keep their setters, matching .NET's
own { get; } / { get; set; } split.

The review's price was exact: three of four call shapes break, and only equality
against a literal survives because std::optional compares against a value of its
own type. Fixture set: 27 fixtures / 166 sites. Gate 17,279 run, 0 failed.

Author: robertvokac

CLAUDE.md

@@ -0,0 +1,72 @@
+<!-- SPDX-License-Identifier: MIT -->
+<!-- Copyright (c) Robert Vokac and contributors -->
+
+# Migration — `ObsoleteAttribute`'s three components are nullable (ticket #2295)
+
+*2026-08-18.* `Message`, `DiagnosticId` and `UrlFormat` were non-nullable `std::string`, so an
+**absent** value and an **empty** one were the same state. .NET's are all `string?`.
+
+Landed under `docs/StandingApprovals.md` **SA-8** (representation) with **SA-3** (layout) and
+**SA-10** (signatures), under SA-2's five conditions. `sizeof(ObsoleteAttribute)` grows
+**112 → 136**, so **consumers must be recompiled**.
+
+---
+
+## 1. What changed
+
+| | Was | Is |
+|---|---|---|
+| `getMessageProperty()` | `const std::string&` | **`const std::optional<std::string>&`** |
+| `getDiagnosticIdProperty()`, `getUrlFormatProperty()` | same | same change |
+| `ObsoleteAttribute(message)`, `(message, isError)` | `const std::string&` | **`std::optional<std::string>`** |
+| `setDiagnosticIdProperty`, `setUrlFormatProperty` | `const std::string&` | **`std::optional<std::string>`** |
+| `getIsErrorProperty()` | `bool` | **unchanged** — .NET's `IsError` is `bool`, not nullable |
+| `sizeof` | 112 | **136** |
+
+`Message` and `IsError` stay getter-only and `DiagnosticId`/`UrlFormat` keep their setters,
+matching .NET's own `{ get; }` / `{ get; set; }` split.
+
+## 2. Why a getter change alone could not have done it
+
+The boundary was on the way **in** as well as on the way out: the message constructor and both
+setters took `const std::string&`, so a caller could neither **supply** an absent value nor
+**return** a component to that state. Measured before the repair, `ObsoleteAttribute def;` and
+`ObsoleteAttribute empty(std::string{});` compared **equal**.
+
+A test now asserts all three directions: absent ≠ empty on the way out, an explicitly absent
+constructor argument, and a setter round-trip through value → empty → absent.
+
+## 3. The objection the ticket raised is discharged by measurement
+
+The ticket said: *"Zero first-party production consumers does NOT license option A: the header is
+public in Core.Base and downstream consumers exist and **were not inspected**."*
+
+They have been. Measured on 2026-08-18: `mobile-eggbert` mentions `ObsoleteAttribute` **zero**
+times, and `cna` mentions it **once — inside a comment**
+(`cna/modules/devices/include/Microsoft/Devices/Sensors/Accelerometer.hpp:433`), not in code.
+Neither repository was modified. That is SA-2's condition 5 doing exactly the job it exists for.
+
+## 4. To migrate
+
+```cpp
+// before
+if (attr.getMessageProperty().empty()) { ... }
+const std::string& m = attr.getMessageProperty();
+takesAString(attr.getUrlFormatProperty());
+
+// after
+if (!attr.getMessageProperty().has_value()) { ... }          // absent
+if (attr.getMessageProperty() == "") { ... }                 // present and empty — now distinct
+const std::string m = attr.getMessageProperty().value_or("");
+takesAString(attr.getUrlFormatProperty().value_or(""));
+```
+
+Equality against a string literal is the **one call shape that survives unchanged**, because
+`std::optional` compares against a value of its own type. The negative fixture pins the three
+broken shapes, a fourth that breaks *silently* (a `decltype` on the return type), and both
+survivors.
+
+## 5. Downstream, measured
+
+See §3 — **zero code sites in either consumer**. The full-rebuild requirement is recorded here for
+any future consumer.

audit/AUDIT_FINDINGS_INDEX.md

@@ -2,7 +2,9 @@
 // Copyright (c) Robert Vokac and contributors
 // Portions based on .NET runtime API (MIT License, Copyright .NET Foundation and Contributors)
 #pragma once
+#include <optional>
 #include <string>
+#include <utility>
 #include "System/Attribute.hpp"
 
 namespace System {
@@ -58,12 +60,20 @@ namespace System {
      * left derivable rather than `final` because sealing it would reject code
      * that compiles today.
      *
-     * `Message`, `DiagnosticId` and `UrlFormat` are nullable (`string?`) in
-     * .NET, so a default-constructed attribute exposes `null` there and a
-     * caller can tell that apart from an explicitly supplied `""`. This port
-     * stores three non-nullable `std::string`s, so **an absent value and an
-     * empty one are the same state here** — SR-AUD-116, whose representation is
-     * under decision at ticket #2295 and must not be assumed either way.
+     * `Message`, `DiagnosticId` and `UrlFormat` are nullable (`string?`) in .NET, so a
+     * default-constructed attribute exposes `null` and a caller can tell that apart from an
+     * explicitly supplied `""`. **Since ticket #2295 this port can too**: all three are
+     * `std::optional<std::string>`, and the constructors and setters take one, so a caller can
+     * both supply an absent value and return a component to that state.
+     *
+     * They used to be three non-nullable `std::string`s, which made an absent value and an empty
+     * one **the same state** — measured, `ObsoleteAttribute def;` and
+     * `ObsoleteAttribute empty(std::string{});` compared equal. The boundary was on the way in as
+     * well as on the way out, so the gap was not closable by a getter alone.
+     *
+     * `sizeof(ObsoleteAttribute)` grows **112 → 136** (SA-3). `Message` and `IsError` stay
+     * getter-only and `DiagnosticId`/`UrlFormat` keep their setters, matching .NET's own
+     * `{ get; }` / `{ get; set; }` split.
      *
      * The class exists so that ported code naming `System::ObsoleteAttribute`
      * still compiles and so that the .NET intent stays readable beside the
@@ -73,10 +83,10 @@ namespace System {
      * SR-AUD-115, tickets #2293 (review) and #2294.
      */
     class ObsoleteAttribute : public Attribute {
-        std::string message_;
+        std::optional<std::string> message_;
         bool isError_ = false;
-        std::string diagnosticId_;
-        std::string urlFormat_;
+        std::optional<std::string> diagnosticId_;
+        std::optional<std::string> urlFormat_;
 
     public:
         /**
@@ -89,7 +99,7 @@ namespace System {
          * @brief Constructs an ObsoleteAttribute with the given informational message.
          * @param message Human-readable explanation of the obsolescence.
          */
-        explicit ObsoleteAttribute(const std::string& message) : message_(message) {}
+        explicit ObsoleteAttribute(std::optional<std::string> message) : message_(std::move(message)) {}
         /**
          * @brief Constructs an ObsoleteAttribute with a message and an error flag.
          * @param message Human-readable explanation of the obsolescence.
@@ -99,10 +109,11 @@ namespace System {
          * stored and returned unchanged, and no value of it makes any
          * declaration harder or easier to use.
          */
-        ObsoleteAttribute(const std::string& message, bool isError) : message_(message), isError_(isError) {}
+        ObsoleteAttribute(std::optional<std::string> message, bool isError)
+            : message_(std::move(message)), isError_(isError) {}
 
         /** Returns the informational message describing the obsolescence. */
-        [[nodiscard]] const std::string& getMessageProperty()      const { return message_; }
+        [[nodiscard]] const std::optional<std::string>& getMessageProperty()      const { return message_; }
         /**
          * Returns the recorded .NET error flag. It reports what .NET's compiler
          * would do with the declaration this attribute described; it does not
@@ -113,18 +124,18 @@ namespace System {
          * Returns the diagnostic ID associated with this obsolescence, or an
          * empty string where .NET would return null (SR-AUD-116).
          */
-        [[nodiscard]] const std::string& getDiagnosticIdProperty() const { return diagnosticId_; }
+        [[nodiscard]] const std::optional<std::string>& getDiagnosticIdProperty() const { return diagnosticId_; }
         /**
          * Returns the URL format string for further documentation, or an empty
          * string where .NET would return null (SR-AUD-116). Nothing in this
          * port formats it; it is metadata for a human reader.
          */
-        [[nodiscard]] const std::string& getUrlFormatProperty()    const { return urlFormat_; }
+        [[nodiscard]] const std::optional<std::string>& getUrlFormatProperty()    const { return urlFormat_; }
 
         /** Sets the diagnostic ID associated with this obsolescence. */
-        void setDiagnosticIdProperty(const std::string& v) { diagnosticId_ = v; }
+        void setDiagnosticIdProperty(std::optional<std::string> v) { diagnosticId_ = std::move(v); }
         /** Sets the URL format string pointing to further documentation. */
-        void setUrlFormatProperty(const std::string& v)    { urlFormat_ = v; }
+        void setUrlFormatProperty(std::optional<std::string> v)    { urlFormat_ = std::move(v); }
     };
 
 } // namespace System

docs/Migration-ObsoleteAttributeNullableComponents.md

@@ -2,6 +2,7 @@
 // Copyright (c) Robert Vokac and contributors
 // Portions based on .NET runtime API (MIT License, Copyright .NET Foundation and Contributors)
 #include <gtest/gtest.h>
+#include <optional>
 #include <cstddef>
 #include <string>
 #include <type_traits>
@@ -22,8 +23,10 @@ int legacyMarkedWithDeprecated() { return 7; }
 } // namespace
 
 TEST(ObsoleteAttributeTest, DefaultCtor) {
+    // #2295: absent, not empty. The old assertion could not tell the two apart, which is the
+    // finding.
     ObsoleteAttribute a;
-    EXPECT_TRUE(a.getMessageProperty().empty());
+    EXPECT_EQ(std::nullopt, a.getMessageProperty());
     EXPECT_FALSE(a.getIsErrorProperty());
 }
 
@@ -41,14 +44,14 @@ TEST(ObsoleteAttributeTest, MessageAndErrorCtor) {
 
 TEST(ObsoleteAttributeTest, DiagnosticId) {
     ObsoleteAttribute a("msg");
-    EXPECT_TRUE(a.getDiagnosticIdProperty().empty());
+    EXPECT_EQ(std::nullopt, a.getDiagnosticIdProperty());
     a.setDiagnosticIdProperty("SYSLIB0001");
     EXPECT_EQ(a.getDiagnosticIdProperty(), "SYSLIB0001");
 }
 
 TEST(ObsoleteAttributeTest, UrlFormat) {
     ObsoleteAttribute a("msg");
-    EXPECT_TRUE(a.getUrlFormatProperty().empty());
+    EXPECT_EQ(std::nullopt, a.getUrlFormatProperty());
     a.setUrlFormatProperty("https://example.com/{0}");
     EXPECT_EQ(a.getUrlFormatProperty(), "https://example.com/{0}");
 }
@@ -63,13 +66,49 @@ TEST(ObsoleteAttributeTest, IsAttribute) {
 // constructed attribute could reach a declaration. A pointer-sized member would
 // not fit in the padding the declared members already leave, so this trips on
 // exactly the shape an attempt to "implement" the attachment would take.
-TEST(ObsoleteAttributeTest, CarriesItsFourDeclaredMembersAndNoSideChannel) {
+TEST(ObsoleteAttributeTest, Fix2295_CarriesItsFourDeclaredMembersAndNoSideChannel) {
+    // #2295 grew the three string members into std::optional<std::string>, so
+    // sizeof(ObsoleteAttribute) moves 112 -> 136 under SA-3. The pin stays a RELATIONSHIP rather
+    // than a number: it is the base plus three optionals plus the bool, rounded to alignment, so
+    // it keeps proving there is no side channel rather than re-recording a compiler's answer.
     constexpr std::size_t declared =
-        sizeof(System::Attribute) + 3 * sizeof(std::string) + sizeof(bool);
+        sizeof(System::Attribute) + 3 * sizeof(std::optional<std::string>) + sizeof(bool);
     constexpr std::size_t align = alignof(ObsoleteAttribute);
     constexpr std::size_t rounded = ((declared + align - 1) / align) * align;
     EXPECT_EQ(sizeof(ObsoleteAttribute), rounded);
     EXPECT_TRUE((std::is_base_of_v<System::Attribute, ObsoleteAttribute>));
+
+    // ...and the growth is real and was measured, so a future edit that silently drops the
+    // optionals back to plain strings fails here rather than passing on a stale relationship.
+    EXPECT_GT(sizeof(ObsoleteAttribute),
+              sizeof(System::Attribute) + 3 * sizeof(std::string) + sizeof(bool));
+}
+
+TEST(ObsoleteAttributeTest, Fix2295_AbsentAndEmptyAreDifferentStates) {
+    // THE FINDING ITSELF. Measured before #2295, these two compared EQUAL: three non-nullable
+    // std::strings cannot express .NET's `string?`, and the boundary was on the way IN as well as
+    // on the way out -- the constructor and both setters took const std::string&, so a caller
+    // could neither supply an absent value nor return a component to that state.
+    const ObsoleteAttribute absent;
+    const ObsoleteAttribute empty(std::string{});
+    EXPECT_EQ(std::nullopt, absent.getMessageProperty());
+    EXPECT_EQ(std::optional<std::string>(""), empty.getMessageProperty());
+    EXPECT_NE(absent.getMessageProperty(), empty.getMessageProperty());
+
+    // The way IN, for the two settable components: a caller can now supply absence and take it
+    // back, which .NET allows and this port could not express at all.
+    ObsoleteAttribute a("msg");
+    a.setDiagnosticIdProperty("SYSLIB0001");
+    EXPECT_EQ(std::optional<std::string>("SYSLIB0001"), a.getDiagnosticIdProperty());
+    a.setDiagnosticIdProperty(std::string{});
+    EXPECT_EQ(std::optional<std::string>(""), a.getDiagnosticIdProperty());
+    a.setDiagnosticIdProperty(std::nullopt);
+    EXPECT_EQ(std::nullopt, a.getDiagnosticIdProperty());
+
+    // The constructor too: an explicitly absent message differs from an explicitly empty one.
+    EXPECT_EQ(std::nullopt, ObsoleteAttribute(std::nullopt).getMessageProperty());
+    EXPECT_EQ(std::nullopt, ObsoleteAttribute(std::nullopt, true).getMessageProperty());
+    EXPECT_TRUE(ObsoleteAttribute(std::nullopt, true).getIsErrorProperty());
 }
 
 // SR-AUD-115 (#2294). This is a LANGUAGE-BOUNDARY DEMONSTRATION, not a
@@ -125,7 +164,7 @@ TEST(ObsoleteAttributeTest, TextComponentsAreByteTransparent) {
     a.setUrlFormatProperty("https://example.com/dokumentace/{0}?jazyk=čeština");
 
     EXPECT_EQ(a.getMessageProperty(), message);
-    EXPECT_EQ(a.getMessageProperty().size(), message.size());
+    EXPECT_EQ(a.getMessageProperty().value().size(), message.size());
     EXPECT_EQ(a.getUrlFormatProperty(),
               "https://example.com/dokumentace/{0}?jazyk=čeština");
 }

modules/core/include/System/ObsoleteAttribute.hpp

@@ -6,6 +6,7 @@
 // Attribute, AttributeTargets, AttributeUsageAttribute, CLSCompliantAttribute,
 // ObsoleteAttribute, FlagsAttribute, and other marker attributes.
 #include <gtest/gtest.h>
+#include <optional>
 #include <type_traits>
 #include <string>
 #include "System/Attribute.hpp"
@@ -242,9 +243,10 @@ TEST(CLSCompliantAttributeTests, IsCompliant_False) {
 // ObsoleteAttribute
 // ===========================================================================
 
-TEST(ObsoleteAttributeTests, DefaultCtor_MessageEmpty) {
+TEST(ObsoleteAttributeTests, DefaultCtor_MessageIsAbsent) {
+    // #2295: nullopt, not "". A default attribute has NO message in .NET, and now here too.
     ObsoleteAttribute attr;
-    EXPECT_TRUE(attr.getMessageProperty().empty());
+    EXPECT_EQ(std::nullopt, attr.getMessageProperty());
 }
 
 TEST(ObsoleteAttributeTests, MessageCtor_StoresMessage) {
@@ -267,9 +269,9 @@ TEST(ObsoleteAttributeTests, DefaultCtor_IsError_False) {
     EXPECT_FALSE(attr.getIsErrorProperty());
 }
 
-TEST(ObsoleteAttributeTests, DiagnosticId_DefaultEmpty) {
+TEST(ObsoleteAttributeTests, DiagnosticId_DefaultIsAbsent) {
     ObsoleteAttribute attr("msg");
-    EXPECT_TRUE(attr.getDiagnosticIdProperty().empty());
+    EXPECT_EQ(std::nullopt, attr.getDiagnosticIdProperty());
 }
 
 TEST(ObsoleteAttributeTests, SetDiagnosticId_Stored) {