mirror of
https://github.com/dotnet/skills.git
synced 2026-09-20 09:49:54 +08:00
Remove dotnet breaking changes work from refactoring branch
The skill and its evaluations now live on add-dotnet-breaking-changes-skill. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
committed by
Abhitej John
parent
f4376065f3
commit
24d0399816
@@ -1,98 +0,0 @@
|
||||
---
|
||||
name: dotnet-breaking-changes
|
||||
description: >
|
||||
Keep the observable .NET/C# contract intact when editing: additions count too, and the
|
||||
contract is wider than the file you are editing. Covers the public API surface and the
|
||||
DIFFERENT gates repos use for it (PublicApiAnalyzers vs ApiCompat/package validation),
|
||||
nullable/trimming/AOT annotations as API, multi-targeting and #if branches (behavior per
|
||||
target framework), source-generated and partial code, and InternalsVisibleTo.
|
||||
USE FOR: any edit to a shipped library/NuGet package or cross-assembly/multi-targeted code —
|
||||
adding a public/protected member, overload, or target framework; widening what a public method
|
||||
accepts or returns; renaming, moving, or removing a member; changing a signature, nullability,
|
||||
or attribute; touching a partial or source-generated type; or answering "is this a breaking
|
||||
change?". Applies to features, fixes, and refactors alike.
|
||||
DO NOT USE FOR: framework/SDK/NuGet upgrades (use dotnet-upgrade skills), pure formatting, or a
|
||||
single-target private app with no public/cross-assembly surface.
|
||||
license: MIT
|
||||
---
|
||||
|
||||
# .NET breaking changes: the contract is wider than the file
|
||||
|
||||
When you edit .NET code, the observable contract is usually **larger than the snippet in front of you**.
|
||||
A change that compiles and keeps tests green can still break a downstream consumer, another target
|
||||
framework, a friend assembly, or regenerate away on the next build. This applies to a feature or a bug
|
||||
fix as much as a refactor, and to **additions** — a new public member, overload, or target framework — as
|
||||
much as removals: added surface is a permanent contract obligation, and added behavior on a multi-targeted
|
||||
or partial type must stay correct on every target and survive the next regeneration. (For the
|
||||
behavior-preserving refactoring *process*, use the `csharp-refactoring` skill; this skill is the
|
||||
compatibility knowledge it — and any other change — taps into.)
|
||||
|
||||
## Inspect first — find which surfaces this repo actually has
|
||||
|
||||
The most valuable move is to **look before you leap**: find which hidden surfaces exist here, then check
|
||||
only those. Don't recite these or assume — the markers present dictate the plan; markers absent tell you a
|
||||
surface is not in play.
|
||||
|
||||
```bash
|
||||
# Public-API gate (which one? they are NOT interchangeable)
|
||||
git ls-files "**/PublicAPI.Shipped.txt" "**/PublicAPI.Unshipped.txt" # PublicApiAnalyzers
|
||||
grep -rl "EnablePackageValidation\|ApiCompat" --include=*.props --include=*.targets --include=*.csproj .
|
||||
# Multi-targeting and conditional code
|
||||
grep -rl "<TargetFrameworks>" --include=*.csproj --include=*.props . ; grep -rn "#if " --include=*.cs .
|
||||
# Generated / partial code
|
||||
git ls-files "*.g.cs" "*.generated.cs" ; grep -rln "partial class\|partial record\|partial struct" --include=*.cs .
|
||||
# Friend assemblies
|
||||
grep -rn "InternalsVisibleTo" --include=*.cs --include=*.csproj --include=*.props .
|
||||
```
|
||||
|
||||
Then read only the reference(s) for the surfaces you actually found.
|
||||
|
||||
## The hidden surfaces (depth in references/)
|
||||
|
||||
Public API (1) is the **default** surface to check on any library edit; the other three are
|
||||
**conditional** — pursue them only when the inspect-first markers above show they are in play.
|
||||
|
||||
1. **Public API.** Renaming/moving/removing/re-signing a public member — or changing nullability, a
|
||||
generic constraint, or a trimming/AOT attribute — is a breaking change a green test run will not catch.
|
||||
Repos gate this **two non-interchangeable ways**: source-level (`PublicApiAnalyzers` + `PublicAPI.*.txt`,
|
||||
which you maintain) and binary/package-level (`ApiCompat` / `<EnablePackageValidation>`). Respect the
|
||||
gate that exists; if none exists, review the surface by hand and flag it — do **not** bolt on analyzer
|
||||
infrastructure as a side effect. Move a public type via a `[TypeForwardedTo]` forwarder; a *rename*
|
||||
needs an `[Obsolete]` shim, not a forwarder. See `references/public-api.md`.
|
||||
|
||||
2. **Multi-targeting and `#if`.** Code that multi-targets (`<TargetFrameworks>`) or compiles conditionally
|
||||
(`#if NET8_0_OR_GREATER`, platform/CoreCLR/Mono/NativeAOT branches) can build for the target your editor
|
||||
shows and break one it never invoked. Inspect every branch — including inactive ones — preserve
|
||||
intentional divergence, and re-gate each TFM/RID. See `references/multi-targeting.md`.
|
||||
|
||||
3. **Source-generated and partial code.** A type is often `partial` across several files, and part may be
|
||||
**generated** (source generators, Razor, `*.g.cs`). Include every partial declaration; treat generated
|
||||
files as derived and change the generator input/template unless the repo checks the output in as source.
|
||||
See `references/source-generation.md`.
|
||||
|
||||
4. **InternalsVisibleTo.** `internal` is not private across a solution: test and friend assemblies bind to
|
||||
internals (and, when strong-named, to a public key). An internal rename/move/removal can break a
|
||||
consumer with no reference in the declaring project. See `references/internals-visible-to.md`.
|
||||
|
||||
## Stop and escalate (do not silently proceed)
|
||||
|
||||
- A **public/shipped API** would change and you cannot verify compatibility — no shim/forwarder path and
|
||||
no PublicApiAnalyzers/ApiCompat/package-validation gate to catch a break.
|
||||
- The edit changes an **observable annotation** (nullability, `[DynamicallyAccessedMembers]`,
|
||||
`[RequiresUnreferencedCode]`, generic constraints) on a public member.
|
||||
- The change can be made consistent across **some but not all** target frameworks or platforms.
|
||||
- The only way to apply it is editing **generated output** that the next build will overwrite.
|
||||
|
||||
## Reference files
|
||||
|
||||
Load a reference only for a surface the inspect-first step actually found:
|
||||
|
||||
- **[references/public-api.md](references/public-api.md)** — the two API gates and why they are not
|
||||
interchangeable, nullable/trimming/AOT as observable API, `[Obsolete]` shims and type-forwarders,
|
||||
suppression baselines.
|
||||
- **[references/multi-targeting.md](references/multi-targeting.md)** — TFMs, `#if` and platform symbols,
|
||||
RIDs, and how to inspect and re-gate every target.
|
||||
- **[references/source-generation.md](references/source-generation.md)** — partial types, generated output
|
||||
vs checked-in source, editing the generator input. (See also `dotnet-msbuild/including-generated-files`.)
|
||||
- **[references/internals-visible-to.md](references/internals-visible-to.md)** — friend/test assemblies
|
||||
and strong-name keys.
|
||||
@@ -1,42 +0,0 @@
|
||||
# InternalsVisibleTo — `internal` is not private across the solution
|
||||
|
||||
## Contents
|
||||
|
||||
- The hazard
|
||||
- Find the friend assemblies
|
||||
- Strong-named friends
|
||||
- What to do
|
||||
|
||||
## The hazard
|
||||
|
||||
`internal` limits access to the declaring assembly **unless** the assembly grants friend access via
|
||||
`[assembly: InternalsVisibleTo("Other.Assembly")]`. Test projects and split implementation assemblies use
|
||||
this constantly. So an internal rename, move, signature change, or removal can break a consumer that has
|
||||
**no project reference visible from the declaring project** — the coupling is expressed in an attribute,
|
||||
not a reference graph.
|
||||
|
||||
## Find the friend assemblies
|
||||
|
||||
```bash
|
||||
grep -rn "InternalsVisibleTo" --include=*.cs --include=*.csproj --include=*.props .
|
||||
```
|
||||
|
||||
`InternalsVisibleTo` can live in a `.cs` (`AssemblyInfo`/any file) **or** as an MSBuild
|
||||
`<InternalsVisibleTo>` item in a `.csproj`/`Directory.Build.props`. Enumerate every named friend, then
|
||||
search **those** assemblies for uses of the internal symbol you are changing — not just the declaring
|
||||
project.
|
||||
|
||||
## Strong-named friends
|
||||
|
||||
When the declaring assembly is strong-named, the `InternalsVisibleTo` string includes the friend's full
|
||||
`PublicKey=...`. Renaming or re-signing a friend assembly, or changing keys, breaks the grant. Do not alter
|
||||
the assembly name/key half of the relationship as an incidental part of another change.
|
||||
|
||||
## What to do
|
||||
|
||||
- Treat internal members that friends consume with the **same care as public API**: search all friend
|
||||
assemblies for binding references before renaming/moving/removing.
|
||||
- If the change is large, let the **compiler across the whole solution** (build the friend projects too) be
|
||||
the safety net — a missed reference becomes a build error in the friend project, not a silent break.
|
||||
- Adding a new friend (`InternalsVisibleTo`) to make a change "reachable" is itself a surface change — flag
|
||||
it rather than doing it silently.
|
||||
@@ -1,48 +0,0 @@
|
||||
# Multi-targeting and #if — satisfy every target, preserve intentional divergence
|
||||
|
||||
## Contents
|
||||
|
||||
- Why this is a hidden surface
|
||||
- Inspect every branch (including inactive ones)
|
||||
- Preserve intentional divergence
|
||||
- Re-gate every target
|
||||
|
||||
## Why this is a hidden surface
|
||||
|
||||
A project with `<TargetFrameworks>` (plural) compiles once per TFM, and code under `#if` compiles
|
||||
differently per TFM, per platform, and per custom symbol. Your editor and a default `dotnet build`
|
||||
usually show/exercise **one** active branch. An edit that is correct there can leave another target
|
||||
uncompilable or behaviorally different — and CI (which builds all of them) is where it surfaces.
|
||||
|
||||
Common conditional symbols: framework (`NET8_0_OR_GREATER`, `NETFRAMEWORK`, `NETSTANDARD2_0`), platform
|
||||
(`WINDOWS`, `LINUX`, `OSX`), runtime flavor (`CORECLR`, `MONO`, `NATIVEAOT`), and repo-defined symbols
|
||||
(`FEATURE_*`, `PRIVATE_*`) declared via `<DefineConstants>`.
|
||||
|
||||
## Inspect every branch (including inactive ones)
|
||||
|
||||
Before editing a symbol used under `#if`:
|
||||
|
||||
- Find **all** its declarations/uses across branches — including branches that are inactive for the
|
||||
current TFM. A grep for the symbol crosses `#if` boundaries; the compiler for the active TFM does not.
|
||||
- If the symbol has the same meaning in every branch, apply the equivalent change to each.
|
||||
- Watch for symbols that **only exist** in some branches (e.g. an API available on `net8.0` but polyfilled
|
||||
or absent on `netstandard2.0`).
|
||||
|
||||
## Preserve intentional divergence
|
||||
|
||||
Conditional branches often differ **on purpose** (a fast path on new runtimes, a polyfill on old ones, a
|
||||
platform-specific implementation). Do **not** homogenize them into one shape to "clean up." Preserve the
|
||||
intended per-target behavior; only unify what is genuinely duplicated with identical intent.
|
||||
|
||||
## Re-gate every target
|
||||
|
||||
After the edit, build **and** test each target, not just the default:
|
||||
|
||||
```bash
|
||||
dotnet build # builds every TargetFramework
|
||||
dotnet build -f net472 # force a specific TFM
|
||||
dotnet test -f net8.0 # test a specific TFM
|
||||
# platform/RID-specific code: build/test on (or cross-target for) each supported RID
|
||||
```
|
||||
|
||||
A green default build is **not** proof the other targets are green.
|
||||
@@ -1,65 +0,0 @@
|
||||
# Public API surface — the two gates and how not to break it
|
||||
|
||||
## Contents
|
||||
|
||||
- Two gates, not interchangeable
|
||||
- What counts as a breaking change (including annotations)
|
||||
- Deciding and validating a public change
|
||||
- Preserving identity: `[Obsolete]` shims and type-forwarders
|
||||
- Suppression baselines
|
||||
|
||||
## Two gates, not interchangeable
|
||||
|
||||
.NET repos protect the public surface in **two fundamentally different ways**. Do not treat one as a
|
||||
substitute for the other — a repo may use either, both, or neither, and each catches things the other
|
||||
does not.
|
||||
|
||||
| Gate | What it compares | Where it lives | You maintain |
|
||||
|------|------------------|----------------|--------------|
|
||||
| **PublicApiAnalyzers** (RS0016/RS0017/…) | _Declared source API_ of the current compilation | `PublicAPI.Shipped.txt` + `PublicAPI.Unshipped.txt` per project | **Yes** — you edit the `.txt` files; the analyzer only enforces they match the code |
|
||||
| **ApiCompat / package validation** | _Built assemblies / NuGet package_ against a baseline (previous version, or a contract/ref assembly) | `<EnablePackageValidation>`, `Microsoft.DotNet.ApiCompat.*` in props/targets | Baseline version + suppression file |
|
||||
|
||||
**Practical rule:** run/respect whichever gate the repo already has and update its files or baseline as
|
||||
the change legitimately requires. If **neither** exists, review the public surface by hand and flag the
|
||||
risk to the user — **do not add analyzer or package-validation infrastructure as a side effect** of an
|
||||
unrelated change (that is scope creep and its own kind of breaking change to the build).
|
||||
|
||||
## What counts as a breaking change (including annotations)
|
||||
|
||||
Beyond the obvious rename/remove/move of a public type or member, these are **also** observable and can
|
||||
break consumers or the API gate:
|
||||
|
||||
- Signature changes: parameter type/order, return type, adding a required parameter, `params`, default
|
||||
values, generic arity or **constraints**.
|
||||
- **Nullability** annotations (`string` → `string?`, `[NotNullWhen]`, `[MaybeNull]`) — these are part of
|
||||
the public contract under `#nullable enable`; changing them shifts consumer warnings and the API gate.
|
||||
- **Trimming/AOT** attributes (`[DynamicallyAccessedMembers]`, `[RequiresUnreferencedCode]`,
|
||||
`[RequiresDynamicCode]`, `[UnconditionalSuppressMessage]`) — observable API for trim/AOT consumers.
|
||||
- Accessibility widening/narrowing, `sealed`/`abstract`/`virtual`/`static` changes, `readonly`/`ref`.
|
||||
- Moving a public type to another **assembly** (identity changes even if the name does not).
|
||||
|
||||
Preserve these unless the task is explicitly to change them.
|
||||
|
||||
## Deciding and validating a public change
|
||||
|
||||
1. Determine the project's role: **shipped library/package** (public API matters) vs **app/service**
|
||||
(external contract = HTTP/config/schema, internal surface can move) vs **private/single-target**
|
||||
(behavior + tests only).
|
||||
2. If a gate exists, build/pack and let it run; update `PublicAPI.Unshipped.txt` or the ApiCompat
|
||||
baseline **intentionally**, never to silence a break you did not mean to make.
|
||||
3. If no gate exists in a library, diff the public surface manually (compare declarations, or a
|
||||
generated ref/`.txt`) and surface the delta to the user.
|
||||
|
||||
## Preserving identity: `[Obsolete]` shims and type-forwarders
|
||||
|
||||
- **`[Obsolete]` shim:** keep the old member alongside the new one, forwarding to it, so source consumers
|
||||
keep compiling. Use for renames/relocations _within_ an assembly.
|
||||
- **`[TypeForwardedTo]` type-forwarder:** preserves **binary identity** when a **public type moves to
|
||||
another assembly**. Forwarders solve _cross-assembly moves_; they do **not** help a rename (the name
|
||||
changed) and are unnecessary for moves within the same assembly.
|
||||
|
||||
## Suppression baselines
|
||||
|
||||
`ApiCompatSuppressions` / `GlobalSuppressions` and PublicApiAnalyzers baselines exist so intentional,
|
||||
reviewed changes pass. Update them deliberately with the change; do not blanket-add suppressions to make
|
||||
an edit "pass" — that hides the very break the gate exists to catch.
|
||||
@@ -1,38 +0,0 @@
|
||||
# Source-generated and partial code — edit the source, not the output
|
||||
|
||||
## Contents
|
||||
|
||||
- Partial types span files
|
||||
- Generated vs checked-in
|
||||
- How to change generated behavior
|
||||
|
||||
## Partial types span files
|
||||
|
||||
A `partial class`/`record`/`struct` (and, since C# 13, `partial` properties/methods) is one type split
|
||||
across several files. A rename, move, or member change must include **every** partial declaration, or you
|
||||
get a partial that no longer agrees with itself (duplicate/missing members, mismatched signatures).
|
||||
|
||||
- Find all parts: `grep -rln "partial .*<TypeName>" --include=*.cs` and check the containing folder.
|
||||
- One part is frequently **generated** — the same type name appears in a `*.g.cs`/`*.generated.cs` you did
|
||||
not write.
|
||||
|
||||
## Generated vs checked-in
|
||||
|
||||
Decide which kind of generated file you are looking at:
|
||||
|
||||
- **Regenerated every build** (source generators, Razor `*.razor.g.cs`, XAML, resx designer): editing the
|
||||
output is pointless — the next build overwrites it. Change the **input** (see below).
|
||||
- **Checked-in / committed generated source** (some repos commit generated `.cs`, ref assemblies, or
|
||||
`PublicAPI.*.txt`): treat it as source _for editing_, but there is almost always a **regeneration
|
||||
command** you must re-run so the checked-in copy stays in sync. Editing it by hand and skipping
|
||||
regeneration drifts it from its source of truth.
|
||||
|
||||
Look for markers: a `<auto-generated>` header, `[GeneratedCode]`, an `.editorconfig`
|
||||
`generated_code = true`, or MSBuild items adding the generator (see `dotnet-msbuild/including-generated-files`).
|
||||
|
||||
## How to change generated behavior
|
||||
|
||||
- **Source generator:** change the generator **input** — the attributes/partial declarations/`AdditionalFiles`
|
||||
it reads, or the generator itself — then rebuild and diff the regenerated output.
|
||||
- **Razor/XAML/resx:** edit the `.razor`/`.xaml`/`.resx`, not the `.g.cs`/designer file.
|
||||
- Never hand-edit regenerated output as a shortcut; the change will vanish and the diff will mislead review.
|
||||
Reference in New Issue
Block a user