Skip to content

Add obsolete compatibility shims for moved Microsoft.Extensions.Logging internal types#127194

Merged
ericstj merged 9 commits intomainfrom
copilot/add-null-scope-type-back
Apr 22, 2026
Merged

Add obsolete compatibility shims for moved Microsoft.Extensions.Logging internal types#127194
ericstj merged 9 commits intomainfrom
copilot/add-null-scope-type-back

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 21, 2026

Fixes #126995

Restores source and binary compatibility for types that were previously in *.Internal namespaces. Only [Obsolete(error: true)] + [EditorBrowsable(Never)] shims are added — no new non-obsolete public API surface.

Description

New obsolete shims (all in Microsoft.Extensions.Logging.Abstractions assembly):

Shim Redirects to
Microsoft.Extensions.Logging.Abstractions.Internal.NullScope n/a -- trivial copy of Microsoft.Extensions.Logging.NullScope
Microsoft.Extensions.Logging.Abstractions.Internal.TypeNameHelper Microsoft.Extensions.Internal.TypeNameHelper
Microsoft.Extensions.Logging.Internal.FormattedLogValues internal FormattedLogValues in same assembly
Microsoft.Extensions.Logging.Internal.LogValuesFormatter internal LogValuesFormatter in same assembly

File naming: shim files follow the *Obsolete.cs convention (NullScopeObsolete.cs, TypeNameHelperObsolete.cs, etc.).

This work is a continuation of #87480, bringing back some additional types scoped out of that first wave. We follow the same pattern as those.

Copilot AI and others added 5 commits April 20, 2026 20:42
…ractions.Internal namespace

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/5c5fb4d4-ec9e-4d85-a918-813e8b551daf

Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
…or TypeNameHelper, FormattedLogValues, LogValuesFormatter, and NullScope

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/85535733-d8cc-4425-a106-63e3ccf52b70

Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
… location conventions

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/85535733-d8cc-4425-a106-63e3ccf52b70

Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
…forwards from Microsoft.Extensions.Logging

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/6df4e960-8c24-49ae-99ed-239b9771dc2d

Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 21, 2026 00:34
Copilot AI review requested due to automatic review settings April 21, 2026 00:34
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-extensions-logging
See info in area-owners.md if you want to be subscribed.

Copilot AI review requested due to automatic review settings April 21, 2026 02:19
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR restores source/binary compatibility for older consumers by reintroducing several previously-available *.Internal-namespace types in Microsoft.Extensions.Logging.Abstractions as [Obsolete(error: true)] + [EditorBrowsable(Never)] shims.

Changes:

  • Added obsolete shim types for NullScope and TypeNameHelper under Microsoft.Extensions.Logging.Abstractions.Internal.
  • Added obsolete shim wrapper types for FormattedLogValues and LogValuesFormatter under Microsoft.Extensions.Logging.Internal.
  • Updated the ref project to include System.Collections and extended the reference surface accordingly.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/libraries/Microsoft.Extensions.Logging.Abstractions/src/TypeNameHelperObsolete.cs Adds obsolete TypeNameHelper shim forwarding to Microsoft.Extensions.Internal.TypeNameHelper.
src/libraries/Microsoft.Extensions.Logging.Abstractions/src/NullScopeObsolete.cs Adds obsolete NullScope shim type in *.Abstractions.Internal.
src/libraries/Microsoft.Extensions.Logging.Abstractions/src/LogValuesFormatterObsolete.cs Adds obsolete wrapper for Microsoft.Extensions.Logging.LogValuesFormatter.
src/libraries/Microsoft.Extensions.Logging.Abstractions/src/FormattedLogValuesObsolete.cs Adds obsolete wrapper for Microsoft.Extensions.Logging.FormattedLogValues.
src/libraries/Microsoft.Extensions.Logging.Abstractions/ref/Microsoft.Extensions.Logging.Abstractions.csproj Adds System.Collections ref dependency for the updated reference surface.
src/libraries/Microsoft.Extensions.Logging.Abstractions/ref/Microsoft.Extensions.Logging.Abstractions.cs Updates reference API surface to include the new obsolete types.

Comment thread src/libraries/Microsoft.Extensions.Logging.Abstractions/src/NullScopeObsolete.cs Outdated
@github-actions

This comment has been minimized.

Copilot AI review requested due to automatic review settings April 21, 2026 02:49
@ericstj ericstj force-pushed the copilot/add-null-scope-type-back branch from 6734d8e to 06cd5ec Compare April 21, 2026 02:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds [Obsolete(error: true)] + [EditorBrowsable(Never)] compatibility shims in Microsoft.Extensions.Logging.Abstractions to restore source/binary compatibility for previously-available *.Internal types.

Changes:

  • Added obsolete shim types for NullScope, TypeNameHelper, FormattedLogValues, and LogValuesFormatter.
  • Updated the reference assembly surface (ref/*.cs) to include the shims.
  • Updated the ref-project to reference System.Collections (needed by the new ref surface).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/libraries/Microsoft.Extensions.Logging.Abstractions/src/TypeNameHelperObsolete.cs Adds obsolete shim forwarding to Microsoft.Extensions.Internal.TypeNameHelper.
src/libraries/Microsoft.Extensions.Logging.Abstractions/src/NullScopeObsolete.cs Adds obsolete shim for prior Abstractions.Internal.NullScope.
src/libraries/Microsoft.Extensions.Logging.Abstractions/src/FormattedLogValuesObsolete.cs Adds obsolete shim wrapping internal Microsoft.Extensions.Logging.FormattedLogValues.
src/libraries/Microsoft.Extensions.Logging.Abstractions/src/LogValuesFormatterObsolete.cs Adds obsolete shim wrapping internal Microsoft.Extensions.Logging.LogValuesFormatter.
src/libraries/Microsoft.Extensions.Logging.Abstractions/ref/Microsoft.Extensions.Logging.Abstractions.csproj Adds System.Collections ref dependency for the new reference API types.
src/libraries/Microsoft.Extensions.Logging.Abstractions/ref/Microsoft.Extensions.Logging.Abstractions.cs Adds the shim types to the reference surface.

Comment thread src/libraries/Microsoft.Extensions.Logging.Abstractions/src/NullScopeObsolete.cs Outdated
@tarekgh tarekgh added this to the 11.0.0 milestone Apr 21, 2026
Copy link
Copy Markdown
Member

@tarekgh tarekgh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a few minor comments, LGTM otherwise.

It will be good if we can add some tests to cover the new exposed APIs if possible, for the sake of the code coverage.

Co-authored-by: Eric StJohn <ericstj@microsoft.com>
Copilot AI review requested due to automatic review settings April 21, 2026 19:15
@ericstj ericstj marked this pull request as ready for review April 21, 2026 19:16
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds [Obsolete(error: true)] + [EditorBrowsable(Never)] compatibility shims in Microsoft.Extensions.Logging.Abstractions to restore binary/source compatibility for several historically-public “*.Internal” logging types that were moved, addressing TypeLoadException scenarios reported in #126995.

Changes:

  • Introduces obsolete shim types for NullScope, TypeNameHelper, FormattedLogValues, and LogValuesFormatter in their legacy namespaces.
  • Updates the reference assembly (ref/) to include the restored public (obsolete) API surface.
  • Adds a missing System.Collections ref dependency needed by the updated reference assembly surface.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/libraries/Microsoft.Extensions.Logging.Abstractions/src/TypeNameHelperObsolete.cs Adds obsolete Microsoft.Extensions.Logging.Abstractions.Internal.TypeNameHelper shim forwarding to Microsoft.Extensions.Internal.TypeNameHelper.
src/libraries/Microsoft.Extensions.Logging.Abstractions/src/NullScopeObsolete.cs Adds obsolete Microsoft.Extensions.Logging.Abstractions.Internal.NullScope shim for legacy type availability.
src/libraries/Microsoft.Extensions.Logging.Abstractions/src/LogValuesFormatterObsolete.cs Adds obsolete Microsoft.Extensions.Logging.Internal.LogValuesFormatter shim that wraps the internal formatter implementation.
src/libraries/Microsoft.Extensions.Logging.Abstractions/src/FormattedLogValuesObsolete.cs Adds obsolete Microsoft.Extensions.Logging.Internal.FormattedLogValues shim that wraps the internal formatting struct.
src/libraries/Microsoft.Extensions.Logging.Abstractions/ref/Microsoft.Extensions.Logging.Abstractions.csproj Adds System.Collections ref ProjectReference required by newly exposed List<>/collection types in the ref surface.
src/libraries/Microsoft.Extensions.Logging.Abstractions/ref/Microsoft.Extensions.Logging.Abstractions.cs Adds the obsolete shim types to the reference assembly surface to restore compile-time/type identity compatibility.

@github-actions
Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #127194

Note

This review was generated by Copilot using Claude Opus 4.6 (primary) with GPT-5.3-Codex and Extensions-reviewer cross-validation. This review covers the code at commit 058c150, after the "Apply suggestions from code review" commit that addressed findings from the prior automated review.

Holistic Assessment

Motivation: Justified. Issue #126995 reports a real TypeLoadException regression — old binaries compiled against Microsoft.Extensions.Logging.Abstractions 2.1 crash when run on .NET 11 Preview 4 because four *.Internal namespace types were removed. This is a blocking compat bug.

Approach: Correct. The PR follows the established [Obsolete(error: true)] + [EditorBrowsable(Never)] shim pattern from issue #87480 / PR #85846. The *Obsolete.cs file naming, delegation to internal types, and ref assembly additions are all consistent with prior art.

Summary: ⚠️ Needs Human Review. The code is correct and all ref/src mismatches from the prior review have been resolved. Two items require human judgment: (1) whether the TypeNameHelper shim needs additional method overloads for full binary compat, and (2) whether the existing api-approved label on #87480 covers this second wave of types. Both are questions the PR author (@ericstj) is best positioned to answer, as he indicated he's running APICompat locally.


Detailed Findings

⚠️ TypeNameHelper — Verify overload completeness with APICompat

The shim exposes only GetTypeDisplayName(Type type), while the internal TypeNameHelper has a 5-parameter overload (type, fullName, includeGenericParameterNames, includeGenericParameters, nestedTypeDelimiter). GPT-5.3-Codex independently flagged this as a potential binary compat gap — if old assemblies called the overload with additional parameters, they'd get MissingMethodException.

This is likely already covered by @ericstj's local APICompat validation ("I'm doing a pass over these locally with APICompat to make sure we match what was shipped before and it's minimal"). Just confirming: was the 5-parameter overload never public in the old Abstractions assembly? If it was, it should be forwarded.

📍 src/libraries/Microsoft.Extensions.Logging.Abstractions/src/TypeNameHelperObsolete.cs:17-18


⚠️ API Approval — Human confirmation needed

This PR adds new public API surface in the ref/ assembly. The linked issue #126995 does not have the api-approved label. Issue #87480 does have api-approved, but that approval was for a first wave of types — NullScope was explicitly scoped out from that wave.

Mitigating factors: All four types are [Obsolete(error: true)] shims restoring previously-shipped surface — not genuinely new API. The runtime team's precedent for such compat shims does not typically require formal API review. @ericstj as a runtime team member is actively reviewing and shaping this PR. A brief confirmation that no separate approval is needed would close this out.


💡 FormattedLogValues._inner correctly not readonly — No action needed

The latest commit (058c150) removed readonly from the _inner field. This is correct: the inner FormattedLogValues is a struct whose ToString() mutates _cachedToString via ??=. A readonly field would cause defensive copies, making the cache ineffective. Good catch by whoever suggested this fix.

📍 src/libraries/Microsoft.Extensions.Logging.Abstractions/src/FormattedLogValuesObsolete.cs:19


Verified Correct

✅ All prior ref/src mismatches resolved

The prior review identified 3 ref/src metadata mismatches (TypeNameHelper static+signature, FormattedLogValues sealed, LogValuesFormatter sealed). All have been fixed — the ref and src now agree on all type modifiers and method signatures. Verified by reading both files at HEAD.

FormattedLogValues correctly uses class (not struct)

The Extensions-reviewer initially raised a concern about struct→class mismatch, but @ericstj confirmed that FormattedLogValues was a class in 2.1 (changed to struct afterward). Since compat targets 2.1, class is correct.

✅ Delegation patterns are correct and safe

  • FormattedLogValuesObsolete.cs: Stores internal struct in non-readonly field, delegates all IReadOnlyList<> members. Correct.
  • LogValuesFormatterObsolete.cs: readonly field for internal class, delegates all 5 public members with matching signatures. Correct.
  • TypeNameHelperObsolete.cs: Static method delegates to internal static method. Correct.
  • NullScopeObsolete.cs: Standalone singleton IDisposable — simplest correct implementation. Correct.

✅ Build dependencies correct

System.Collections ref project reference added because List<string> (used by LogValuesFormatter.ValueNames) lives in System.Collections.dll. The src csproj already had this. No new package dependencies introduced.

✅ Naming convention and obsolete messages consistent

*Obsolete.cs naming matches established convention. Obsolete messages are appropriate — FormattedLogValues/LogValuesFormatter recommend Microsoft.Extensions.Diagnostics.Testing (the modern replacement for test code that inspected log messages), while NullScope/TypeNameHelper use the generic "retained only for compatibility" form.

✅ No type forward conflicts

Verified that the Microsoft.Extensions.Logging assembly does not have type forwards for these types (correctly removed in commit c449159). No conflicts between assemblies.

✅ New files automatically included in build

The src csproj uses <EnableDefaultItems>true</EnableDefaultItems>, so all new *.cs files are automatically picked up. No manual csproj edits needed.


Models contributing: Claude Opus 4.6 (primary), GPT-5.3-Codex (cross-validation), Extensions-reviewer (domain-specific).

Generated by Code Review for issue #127194 ·

@ericstj
Copy link
Copy Markdown
Member

ericstj commented Apr 22, 2026

TypeNameHelper — Verify overload completeness with APICompat

Yes - I specifically removed overloads that weren't present when the type was made internal.

API Approval — Human confirmation needed

Discussed this with @bartonjs and he's good with it.

@ericstj ericstj added the api-approved API was approved in API review, it can be implemented label Apr 22, 2026
@ericstj
Copy link
Copy Markdown
Member

ericstj commented Apr 22, 2026

/ba-g All test failures are known. The remaining failures are all build timeouts when waiting for helix. These cannot be known issued since they are indistinguishable from build hangs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-approved API was approved in API review, it can be implemented area-Extensions-Logging

Projects

None yet

5 participants