From 1eb008312c6d1fdcd32d29a4067d5ab291f6b27f Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Sun, 16 Aug 2026 17:53:19 -0700 Subject: [PATCH] Make the Nullable.Value exception message actionable (#132367) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #126560 ## Problem `InvalidOperation_NoValue` — the message thrown by `Nullable.Value` when there's no value — reads: > Nullable object must have a value. As #126560 points out, this states an obligation on the *object's state* rather than naming the *invalid action*. It reads as "you must assign a value", when the actual fix is almost always the opposite: don't read `.Value` without checking `.HasValue` first. It also never mentions which member was accessed. ## Change ```diff -Nullable object must have a value. +Cannot read the Value property of a Nullable object that has no value. Check HasValue before reading Value. ``` This follows the wording style already used in the repo for comparable "you called this on something that has no value" failures: - `SqlMisc_NullValueMessage` (`System.Data.Common`) — *"Data is Null. This method or property cannot be called on Null values."* - `ReflectionModel_ExportNotReadable` (`System.ComponentModel.Composition`) — *"Cannot get the value of property '{0}', because the member is not readable. The property must have an accessible getter."* - `NoResultOnFailed` (`System.Text.RegularExpressions`) — *"Result cannot be called on a failed Match."* The issue also floated "Cannot read the Value property of a null Nullable object." I avoided "null Nullable object" because `Nullable` is a struct and is never `null` — "that has no value" keeps the same cadence without the imprecision. The second sentence names the check to perform, per *"DO ensure that exception messages are clear and actionable"* (`docs/coding-guidelines/framework-design-guidelines-digest.md`). The resource **name** is unchanged. ## Scope `InvalidOperation_NoValue` is not shared with any unrelated call site — all definitions and references are `Nullable.Value` semantics: | | Path | |---|---| | def | `src/libraries/System.Private.CoreLib/src/Resources/Strings.resx` | | def | `src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Resources/Strings.resx` | | use | `ThrowHelper.ThrowInvalidOperationException_InvalidOperation_NoValue` — sole caller is the `Nullable.Value` getter | | use | `CustomMethodMapper.Nullable.cs` — NativeAOT reflection-invoke shim for `Nullable.Value` | Both `.resx` definitions are updated. Mono shares the libraries CoreLib `.resx`; there is no third copy. No test asserts the text, and the NativeAOT `FrameworkStrings` string-pinning smoke test pins other resources. Per `docs/coding-guidelines/breaking-change-rules.md`, changing the text of an error message is not a breaking change ("users should not rely on these text messages, and they change anyways based on culture"). ## Validation - Built `System.Private.CoreLib` (`build.cmd clr.corelib -c Release`) — clean. - Confirmed against the produced `System.Private.CoreLib.dll` that the new string is embedded and the old one is gone, so `SR.InvalidOperation_NoValue` still resolves. - The NativeAOT `System.Private.Reflection.Execution` `.resx` was not compiled locally — that project currently fails on a pre-existing missing generated `AsmOffsets.cs` requiring a full NativeAOT native build. The edit there is identical in shape and doesn't change the resource name. CI covers it. > [!NOTE] > This pull request description was generated by GitHub Copilot. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com> --- .../src/Resources/Strings.resx | 45 ------------------- ...System.Private.Reflection.Execution.csproj | 3 -- .../src/Resources/Strings.resx | 2 +- 3 files changed, 1 insertion(+), 49 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Resources/Strings.resx b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Resources/Strings.resx index 4995f4ce280418..957cfd83bad7c7 100644 --- a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Resources/Strings.resx +++ b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/Resources/Strings.resx @@ -120,27 +120,9 @@ '{0}' is missing native code or metadata. This can happen for code that is not compatible with trimming or AOT. Inspect and fix trimming and AOT related warnings that were generated when the app was published. For more information see https://aka.ms/nativeaot-compatibility - - Cannot bind to the target method because its signature is not compatible with that of the delegate type. - - - Array may not be empty. - - - Member not found. - - - Hashtable's capacity overflowed and went negative. Check load factor, capacity and the current size of the table. - - - Nullable object must have a value. - Non-static field requires a target. - - Constructor on type '{0}' not found. - Multidimensional arrays of rank {0} are not supported. @@ -153,9 +135,6 @@ Dynamic invocation of delegate constructors is not supported on this runtime. - - An item with the same key has already been added. - An item with the same key has already been added. Key: {0} @@ -168,30 +147,6 @@ '{0}', on '{1}' violates the constraint of type '{2}'. - - At least one object must implement IComparable. - - - The stream is currently in use by a previous operation on the stream. - - - Stream was not readable. - - - Cannot read from a closed TextReader. - - - Positive number required. - - - Buffer cannot be null. - - - Non-negative number required. - - - Offset and length were out of bounds for the array or count is greater than the number of elements from index to the end of the source collection. - Cannot set a constant field. diff --git a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/System.Private.Reflection.Execution.csproj b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/System.Private.Reflection.Execution.csproj index 5648a7deed20c2..3a70b9965a124f 100644 --- a/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/System.Private.Reflection.Execution.csproj +++ b/src/coreclr/nativeaot/System.Private.Reflection.Execution/src/System.Private.Reflection.Execution.csproj @@ -67,8 +67,5 @@ Internal\LowLevelLinq\LowLevelEnumerable.ToArray.cs - - System\Collections\HashHelpers.cs - diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index 23635bf8653aab..8d6acae094adc5 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -2739,7 +2739,7 @@ Underlying type information on enumeration is not specified. - Nullable object must have a value. + Cannot read the Value property of a Nullable object that has no value. Check HasValue before reading Value. The underlying array is null.