Fix dotnet/runtime#103468: Add DirectoryPath property and constructors to DirectoryNotFoundException

[github-actions]

AI Fix: Add DirectoryPath property and constructors to DirectoryNotFoundException

Issue: dotnet#103468
Root cause: DirectoryNotFoundException lacked DirectoryPath and matching constructors, unlike FileNotFoundException, leading to inconsistency and missing context for consumers.
Fix: Added DirectoryPath property, new constructors, and serialization support to DirectoryNotFoundException. Updated ref assembly and added tests.

Self-review: Correctness ✅ | Tests ✅ | Breaking ✅

Note

This PR was generated with the assistance of GitHub Copilot AI.

Generated by Fix Issue Agent · ◷

[github-actions]

🔍 API Surface Review — PR #45

Public API changes detected: Yes

New API Members

• DirectoryNotFoundException(string? message, string? directoryPath)
• DirectoryNotFoundException(string? message, string? directoryPath, Exception? innerException)
• string? DirectoryPath { get; }
• override void GetObjectData(SerializationInfo, StreamingContext) (serialization)

Findings

✅ API Approval

Issue dotnet/runtime#103468 carries the api-approved label. The implementation matches the approved shape exactly — constructors, parameter names/types, and the property all align.

✅ Ref Assembly Updated

src/libraries/System.Runtime/ref/System.Runtime.cs correctly declares all new public members including the GetObjectData override.

⚠️ Missing ToString() Override — Pattern Inconsistency with FileNotFoundException

FileNotFoundException overrides ToString() to include FileName in diagnostic output (FileNotFoundException.cs:73–92):

public override string ToString()
{
    string s = GetType().ToString() + ": " + Message;
    if (!string.IsNullOrEmpty(FileName))
        s += Environment.NewLineConst + SR.Format(SR.IO_FileName_Name, FileName);
    // ...
}

DirectoryNotFoundException does not override ToString() to include DirectoryPath. This means the new property is invisible in logs, crash dumps, and Console.WriteLine(ex) output — reducing its diagnostic value. Consider adding a ToString() override following the FileNotFoundException pattern. A corresponding SR string resource (e.g., IO_DirectoryName_Name) would be needed.

⚠️ GetObjectData Uses Hardcoded Obsolete String

In DirectoryNotFoundException.cs:61, the [Obsolete] attribute uses a raw string literal:

[Obsolete("This API supports obsolete formatter-based serialization...", DiagnosticId = "SYSLIB0051", ...)]

The serialization constructor on line 52 correctly uses Obsoletions.LegacyFormatterImplMessage. FileNotFoundException also uses the Obsoletions constant for its GetObjectData (FileNotFoundException.cs:103). This should use the constant for consistency:

[Obsolete(Obsoletions.LegacyFormatterImplMessage, DiagnosticId = Obsoletions.LegacyFormatterImplDiagId, UrlFormat = Obsoletions.SharedUrlFormat)]

⚠️ Adoption Opportunities in the Tree

Multiple call sites construct DirectoryNotFoundException with a path embedded in the message but don't set DirectoryPath. These should be updated to use the new constructors:

File	Line	Path variable available
Common/src/System/IO/Win32Marshal.cs	38	path
Common/src/Interop/Unix/Interop.IOErrors.cs	133–134	path
System.Formats.Tar/src/.../TarFile.cs	70, 138, 193, 256, 312, 380, 443, 512	sourceDirectoryName / destinationDirectoryName
System.Private.CoreLib/.../FileSystem.Unix.cs	421, 457	sourceFullPath
System.Private.CoreLib/.../FileInfo.cs	185	FullName
System.IO.IsolatedStorage/.../IsolatedStorageFile.cs	363	sourceDirectoryName
System.Threading.AccessControl/.../MutexAcl.cs	91	name
System.Threading.AccessControl/.../EventWaitHandleAcl.cs	100	name
System.Private.CoreLib/.../Mutex.cs	144	name
System.Private.CoreLib/.../EventWaitHandle.cs	123	name

At minimum, the low-level marshaling helpers (Win32Marshal.cs, Interop.IOErrors.cs) should adopt the new constructor so that all system-thrown DirectoryNotFoundException instances carry DirectoryPath.

✅ No Breaking Changes

All changes are purely additive.

Verdict

⚠️ WARNINGS — The API shape and ref assembly are correct. However:

1. Missing ToString() override breaks the diagnostic pattern established by FileNotFoundException.
2. GetObjectData should use the Obsoletions constants instead of hardcoded strings.
3. Significant adoption opportunities exist in the tree (especially Win32Marshal.cs and Interop.IOErrors.cs) that would make the new property immediately useful for system-thrown exceptions.

Generated by API Surface Review for issue #45 · ◷

[github-actions]

🤖 Copilot Code Review — PR #45

Note

This review was AI-generated by GitHub Copilot using Claude Opus 4.6, with additional analysis from Claude Sonnet 4.5.

Holistic Assessment

Motivation: The PR adds a DirectoryPath property and new constructors to DirectoryNotFoundException, bringing it closer to parity with FileNotFoundException.FileName. This is a reasonable and useful addition — callers catching this exception currently have no structured way to access the path that wasn't found.

Approach: The implementation follows the FileNotFoundException pattern for constructors, property, and serialization. However, the implementation appears incomplete compared to the sibling type — FileNotFoundException and FileLoadException both override Message and ToString() to include the file path in output, but this PR does not provide equivalent overrides for DirectoryPath.

Summary: ⚠️ Needs Human Review. The code that exists is correct, but the approach may be incomplete. The key question a human reviewer needs to decide is whether Message and ToString() overrides should be included (as FileNotFoundException does), whether the API proposal in issue dotnet#103468 covers those overrides, and whether existing throw sites should be updated. There are also minor code-level issues (inconsistent obsolete attributes, test gaps) that should be addressed.

---

Detailed Findings

⚠️ Missing ToString() Override — Inconsistent with FileNotFoundException pattern

(Flagged by primary analysis and Claude Sonnet 4.5)

Both FileNotFoundException and FileLoadException override ToString() to include the path on a separate line:

// FileNotFoundException.cs lines 73-92
public override string ToString()
{
    string s = GetType().ToString() + ": " + Message;
    if (!string.IsNullOrEmpty(FileName))
        s += Environment.NewLineConst + SR.Format(SR.IO_FileName_Name, FileName);
    // ... InnerException, StackTrace
    return s;
}

DirectoryNotFoundException does not override ToString(), so the DirectoryPath value will never appear in logged exceptions unless callers explicitly access the property. This significantly limits the property's usefulness for diagnostics. A human reviewer should decide whether this override is required for this PR or is acceptable as a follow-up.

⚠️ Missing Message Property Override — Inconsistent with FileNotFoundException pattern

(Flagged by primary analysis and Claude Sonnet 4.5)

FileNotFoundException overrides the Message property to lazy-load a formatted message that includes FileName:

// FileNotFoundException.cs lines 48-68
private void SetMessageField()
{
    if (_message == null)
    {
        if ((FileName == null) && (HResult == HResults.COR_E_EXCEPTION))
            _message = SR.IO_FileNotFound;
        else if (FileName != null)
            _message = FileLoadException.FormatFileLoadExceptionMessage(FileName, HResult);
    }
}

Without a similar override, new DirectoryNotFoundException(null, "/some/path") will show the generic message "Attempted to access a path that is not on the disk." with no mention of /some/path. The ref assembly would also need updating if this override is added.

⚠️ Inconsistent [Obsolete] Attribute on GetObjectData — Hardcoded strings vs. constants

(Flagged by primary analysis and Claude Sonnet 4.5)

The serialization constructor (line 52) uses the Obsoletions constants, but the GetObjectData override (line 61) uses hardcoded strings:

// Line 52 — serialization constructor (correct pattern):
[Obsolete(Obsoletions.LegacyFormatterImplMessage, DiagnosticId = Obsoletions.LegacyFormatterImplDiagId, UrlFormat = Obsoletions.SharedUrlFormat)]

// Line 60 — GetObjectData (inconsistent):
[Obsolete("This API supports obsolete formatter-based serialization...", DiagnosticId = "SYSLIB0051", UrlFormat = "(aka.ms/redacted)

FileNotFoundException.GetObjectData (line 103) uses the Obsoletions constants. The PR should do the same for consistency and maintainability. Additionally, the attribute ordering is reversed between the two members — the serialization constructor has [Obsolete] then [EditorBrowsable], while GetObjectData has [EditorBrowsable] then [Obsolete]. FileNotFoundException consistently uses [Obsolete] first.

⚠️ Test Gaps

(Flagged by primary analysis and Claude Sonnet 4.5)

The new tests have several gaps compared to the existing tests in the same file:

1. No HResult verification: Existing tests all use ExceptionHelpers.ValidateExceptionProperties(exception, hResult: HResults.COR_E_DIRECTORYNOTFOUND, ...). The new tests use raw Assert.Equal and don't verify HResult is preserved.

2. No null/empty path tests: No test for directoryPath being null or empty string. No test for default DirectoryPath being null on existing constructors.

3. [Fact] instead of [Theory]: Per project conventions, [Theory] with [InlineData] is preferred over duplicative [Fact] methods. These two tests share nearly identical structure and could be a single [Theory].

4. No ToString() test: FileNotFoundExceptionTests includes a ToStringTest() — if ToString() is overridden, a corresponding test is needed.

⚠️ API Approval Verification — Cannot confirm api-approved label

This PR adds new public API surface (ref assembly changes, new constructors, new property, new GetObjectData override). Per dotnet/runtime policy, new public APIs require an approved proposal linked via an issue with the api-approved label.

The PR references dotnet/runtime#103468 but I cannot access the upstream issue to verify the api-approved label or the approved API shape. A human reviewer must verify:

1. That issue Add a DirectoryPath property to DirectoryNotFoundException to be consistent with FileNotFoundException dotnet/runtime#103468 has the api-approved label
2. That the implemented API matches the approved shape exactly
3. Whether the approved shape includes Message/ToString() overrides

💡 Missing XML Documentation on New Public APIs

(Flagged by Claude Sonnet 4.5)

Neither FileNotFoundException nor the existing DirectoryNotFoundException members have XML docs. However, per maintainer guidance, new public API additions should include XML doc comments as they seed the official API documentation on learn.microsoft.com. Consider adding <summary> and <param> docs for the new constructors and DirectoryPath property.

💡 No Existing Throw Sites Updated — Follow-up opportunity

None of the ~38 existing DirectoryNotFoundException throw sites in the codebase are updated to use the new constructors. For example:

// src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs
throw new DirectoryNotFoundException(SR.Format(SR.IO_PathNotFound_Path, sourceDirectoryName));
// Could become:
throw new DirectoryNotFoundException(SR.Format(SR.IO_PathNotFound_Path, sourceDirectoryName), sourceDirectoryName);

This is reasonable to defer to a follow-up PR, but updating at least a few core sites (e.g., in System.Private.CoreLib) would validate the API design and make the property immediately useful.

Generated by Code Review for issue #45 · ◷

[danmoseley]

Closing: superseded by pipeline improvements. 1 issue = 1 PR rule.