Skip to content

Enable CA1852: Seal internal types#25890

Merged
daxian-dbw merged 1 commit intoPowerShell:masterfrom
xtqqczze:CA1852
Feb 23, 2026
Merged

Enable CA1852: Seal internal types#25890
daxian-dbw merged 1 commit intoPowerShell:masterfrom
xtqqczze:CA1852

Conversation

@xtqqczze
Copy link
Contributor

@xtqqczze xtqqczze commented Aug 22, 2025 •
edited
Loading

Add sealed modifier to internal types without subtypes.

https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1852

Fix #24094.

@iSazonov iSazonov added the CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log label Aug 22, 2025
@xtqqczze xtqqczze marked this pull request as draft August 22, 2025 13:28
@xtqqczze xtqqczze force-pushed the CA1852 branch 3 times, most recently from 25e6142 to ea67269 Compare August 25, 2025 09:50
@xtqqczze xtqqczze marked this pull request as ready for review August 25, 2025 09:50
@iSazonov
Copy link
Collaborator

iSazonov commented Aug 26, 2025

/azp run PowerShell-CI-linux-packaging,PowerShell-Windows-Packaging-CI

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 26, 2025

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@xtqqczze xtqqczze marked this pull request as draft August 26, 2025 12:03
@xtqqczze xtqqczze marked this pull request as ready for review August 26, 2025 12:07
@xtqqczze xtqqczze requested review from a team and jshigetomi as code owners August 26, 2025 12:07
@microsoft-github-policy-service microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Sep 2, 2025
@xtqqczze xtqqczze requested a review from iSazonov October 8, 2025 04:06
@xtqqczze xtqqczze mentioned this pull request Oct 16, 2025
Merged
@iSazonov iSazonov added the Approved-LowRisk Indicate a PR has been approved and can be merged after a quick review of another maintainer. label Feb 22, 2026
@daxian-dbw
Copy link
Member

daxian-dbw commented Feb 23, 2026

Why do we bother to do this for internal types?

@iSazonov
Copy link
Collaborator

iSazonov commented Feb 23, 2026

This allows compiler improve performance by:

  1. Devirtualization
  2. Inlining

@microsoft-github-policy-service microsoft-github-policy-service bot removed the Review - Needed The PR is being reviewed label Feb 23, 2026
@xtqqczze
Copy link
Contributor Author

xtqqczze commented Feb 23, 2026

Why do we bother to do this for internal types?

There’s no drawback to sealing internal types. Unlike public types, where removing the sealed modifier would be a breaking change, internal types don’t carry that risk.

@iSazonov
Copy link
Collaborator

iSazonov commented Feb 23, 2026

Why do we bother to do this for internal types?

There’s no drawback to sealing internal types. Unlike public types, where removing the sealed modifier would be a breaking change, internal types don’t carry that risk.

I guess you mean adding.

daxian-dbw
daxian-dbw approved these changes Feb 23, 2026
View reviewed changes
Copy link
Member

@daxian-dbw daxian-dbw left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thank both of you! I will get this one merged.

FYI, due to the limited resources of the team, we will have to deprioritize the code cleanup PRs in PR reviews. For PRs like this one, the review is easy, so I will take a quick look and merge, but there is no guarantee for those that are more complex and riskier (even with the Approve-LowRisk label). Hope you can understand.

@daxian-dbw daxian-dbw merged commit cfd5966 into PowerShell:master Feb 23, 2026
34 checks passed
@xtqqczze xtqqczze deleted the CA1852 branch February 23, 2026 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daxian-dbw daxian-dbw daxian-dbw approved these changes

@iSazonov iSazonov iSazonov approved these changes

@jshigetomi jshigetomi Awaiting requested review from jshigetomi jshigetomi is a code owner

Assignees

No one assigned

Labels

Approved-LowRisk Indicate a PR has been approved and can be merged after a quick review of another maintainer. CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Enable CA1852: Seal internal types

3 participants

@xtqqczze @iSazonov @daxian-dbw