mirror of
https://github.com/dotnet/skills.git
synced 2026-09-20 09:49:54 +08:00
Split dotnet breaking changes into separate branch
This commit is contained in:
committed by
Abhitej John
parent
2355d16e76
commit
3d98ee2b45
@@ -14,8 +14,6 @@
|
||||
/tests/dotnet/setup-local-sdk/ @dotnet/roslyn-ide @dotnet/skills-csharp-language-reviewers
|
||||
/plugins/dotnet/skills/csharp-refactoring/ @dotnet/roslyn-ide @dotnet/skills-csharp-language-reviewers
|
||||
/tests/dotnet/csharp-refactoring/ @dotnet/roslyn-ide @dotnet/skills-csharp-language-reviewers
|
||||
/plugins/dotnet/skills/dotnet-breaking-changes/ @dotnet/roslyn-ide @dotnet/skills-csharp-language-reviewers
|
||||
/tests/dotnet/dotnet-breaking-changes/ @dotnet/roslyn-ide @dotnet/skills-csharp-language-reviewers
|
||||
/plugins/dotnet/lsp.json @dotnet/roslyn-ide @dotnet/skills-csharp-language-reviewers
|
||||
|
||||
# dotnet-advanced (advanced .NET development skills)
|
||||
|
||||
@@ -42,7 +42,6 @@ github.com/dotnet/msbuild
|
||||
github.com/dotnet/roslyn
|
||||
github.com/dotnet/runtime
|
||||
github.com/dotnet/sdk
|
||||
github.com/dotnet/skills
|
||||
github.com/dotnet/templating
|
||||
github.com/microsoft/cswin32
|
||||
github.com/microsoft/cswinrt
|
||||
|
||||
@@ -20,5 +20,4 @@ Prerequisites:
|
||||
## Skills
|
||||
|
||||
- [csharp-refactoring](skills/csharp-refactoring/SKILL.md)
|
||||
- [dotnet-breaking-changes](skills/dotnet-breaking-changes/SKILL.md)
|
||||
- [setup-local-sdk](skills/setup-local-sdk/SKILL.md)
|
||||
|
||||
@@ -74,8 +74,8 @@ If — and only if — the change touches a **public** symbol, a **multi-targete
|
||||
that governs the symbol (don't assume): the public-API gate (`PublicAPI.Shipped/Unshipped.txt` for
|
||||
PublicApiAnalyzers, and/or `ApiCompat`/`<EnablePackageValidation>` — not interchangeable),
|
||||
`<TargetFrameworks>`/`#if` branches, and `InternalsVisibleTo`. Move a public type via a `[TypeForwardedTo]`
|
||||
forwarder; a *rename* needs an `[Obsolete]` shim, not a forwarder. For depth on any of these surfaces,
|
||||
load the companion **dotnet-breaking-changes** skill. For a provably local/private change, skip this.
|
||||
forwarder; a *rename* needs an `[Obsolete]` shim, not a forwarder. For a provably local/private change,
|
||||
skip these checks.
|
||||
|
||||
## Stop and ask when
|
||||
|
||||
|
||||
@@ -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/indexers) 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.
|
||||
@@ -1,24 +0,0 @@
|
||||
Microsoft Visual Studio Solution File, Format Version 12.00
|
||||
# Visual Studio Version 17
|
||||
VisualStudioVersion = 17.0.31903.59
|
||||
MinimumVisualStudioVersion = 10.0.40219.1
|
||||
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Billing", "src\Billing\Billing.csproj", "{11111111-1111-1111-1111-111111111111}"
|
||||
EndProject
|
||||
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Billing.Tests", "tests\Billing.Tests\Billing.Tests.csproj", "{22222222-2222-2222-2222-222222222222}"
|
||||
EndProject
|
||||
Global
|
||||
GlobalSection(SolutionConfigurationPlatforms) = preSolution
|
||||
Debug|Any CPU = Debug|Any CPU
|
||||
Release|Any CPU = Release|Any CPU
|
||||
EndGlobalSection
|
||||
GlobalSection(ProjectConfigurationPlatforms) = postSolution
|
||||
{11111111-1111-1111-1111-111111111111}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
|
||||
{11111111-1111-1111-1111-111111111111}.Debug|Any CPU.Build.0 = Debug|Any CPU
|
||||
{11111111-1111-1111-1111-111111111111}.Release|Any CPU.ActiveCfg = Release|Any CPU
|
||||
{11111111-1111-1111-1111-111111111111}.Release|Any CPU.Build.0 = Release|Any CPU
|
||||
{22222222-2222-2222-2222-222222222222}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
|
||||
{22222222-2222-2222-2222-222222222222}.Debug|Any CPU.Build.0 = Debug|Any CPU
|
||||
{22222222-2222-2222-2222-222222222222}.Release|Any CPU.ActiveCfg = Release|Any CPU
|
||||
{22222222-2222-2222-2222-222222222222}.Release|Any CPU.Build.0 = Release|Any CPU
|
||||
EndGlobalSection
|
||||
EndGlobal
|
||||
@@ -1,193 +0,0 @@
|
||||
name: dotnet-breaking-changes
|
||||
description: Evaluates the dotnet/dotnet-breaking-changes skill
|
||||
type: capability
|
||||
config:
|
||||
timeout: 15m
|
||||
stimuli:
|
||||
- name: Answer a public-API question before consolidating duplicated parsing
|
||||
prompt: "Is `AppSettingsHelper` part of the public API anywhere? Its int/bool setting parsing looks duplicated with `ConfigReader`. If it's safe to do, consolidate the setting parsing into one place and update the callers, without changing behavior. The solution is at Fixture.sln."
|
||||
environment:
|
||||
files:
|
||||
- src: .
|
||||
dest: .
|
||||
graders:
|
||||
- type: output-matches
|
||||
config:
|
||||
pattern: '(public API|PublicAPI\.Shipped|breaking change|shipped)'
|
||||
- type: file-contains
|
||||
config:
|
||||
path: src/Billing/PublicAPI.Shipped.txt
|
||||
value: AppSettingsHelper
|
||||
# Public-surface preservation gate: the shipped members are exercised by the
|
||||
# tests (AppSettingsHelper + ConfigReader). A "safe" consolidation keeps them
|
||||
# working; silently removing/renaming a shipped public member breaks the test
|
||||
# compile and fails here.
|
||||
- type: run-command
|
||||
config:
|
||||
command: dotnet test Fixture.sln -v:q
|
||||
expected_exit_code: 0
|
||||
stdout_contains: Passed!
|
||||
timeout: 5m
|
||||
- type: prompt
|
||||
rubric:
|
||||
- "Answers the public-API question by actually inspecting what governs the surface (finds and reads `PublicAPI.Shipped.txt`, which lists the public parsing members) rather than assuming"
|
||||
- "Recognizes that removing or renaming a shipped public member is a breaking change a green test run will not catch, and either preserves the public surface (for example keeps the members as thin forwarders) or explicitly flags the break and updates the API baseline — does not silently break the public API"
|
||||
- "If it consolidates, keeps behavior unchanged and re-runs the build and tests to prove it"
|
||||
|
||||
- name: Add behavior to a shipped public member without breaking the contract
|
||||
prompt: "Add hex support to `AppSettingsHelper.ParseIntSetting` so callers can parse values like `0x1F` in addition to plain decimals. The solution is at Fixture.sln."
|
||||
environment:
|
||||
files:
|
||||
- src: .
|
||||
dest: .
|
||||
graders:
|
||||
- type: output-matches
|
||||
config:
|
||||
pattern: (public API|PublicAPI|breaking change|shipped|signature|overload)
|
||||
- type: file-contains
|
||||
config:
|
||||
path: src/Billing/PublicAPI.Shipped.txt
|
||||
value: ParseIntSetting
|
||||
# The shipped signature must not break: existing decimal-parsing tests must
|
||||
# still pass after hex support is added.
|
||||
- type: run-command
|
||||
config:
|
||||
command: dotnet test Fixture.sln -v:q
|
||||
expected_exit_code: 0
|
||||
stdout_contains: Passed!
|
||||
timeout: 5m
|
||||
- type: prompt
|
||||
rubric:
|
||||
- "Treats this as a feature (adds new hex-parsing behavior), not a behavior-preserving refactor, and does not claim behavior is unchanged"
|
||||
- "Recognizes `AppSettingsHelper.ParseIntSetting` is a shipped public member listed in `PublicAPI.Shipped.txt`, and that changing its signature or return type would be a breaking change a green test run will not catch"
|
||||
- "Adds hex support without breaking the shipped signature — extends the body or adds an overload — and if the public surface changes, updates the API baseline (`PublicAPI.Unshipped.txt`) or explicitly flags the break rather than silently altering the shipped contract"
|
||||
- "Keeps the existing tests green and ideally adds coverage for the hex case"
|
||||
|
||||
- name: Rename a helper referenced from every #if branch of a multi-targeted type
|
||||
prompt: "The private `Label` helper in `PlatformInfo` is poorly named — rename it to `FormatLabel`. Keep behavior identical. The solution is at Fixture.sln."
|
||||
environment:
|
||||
files:
|
||||
- src: .
|
||||
dest: .
|
||||
graders:
|
||||
- type: file-contains
|
||||
config:
|
||||
path: src/Billing/PlatformInfo.cs
|
||||
value: FormatLabel
|
||||
# Multi-target gate: building the whole solution compiles BOTH net8.0 and
|
||||
# net10.0. `Label` is referenced from both #if branches, so if the rename
|
||||
# missed the branch the default build doesn't compile, the net8.0 build fails.
|
||||
- type: run-command
|
||||
config:
|
||||
command: dotnet build Fixture.sln -v:q
|
||||
expected_exit_code: 0
|
||||
timeout: 5m
|
||||
# Behavior-preservation gate on the built target.
|
||||
- type: run-command
|
||||
config:
|
||||
command: dotnet test Fixture.sln -v:q
|
||||
expected_exit_code: 0
|
||||
stdout_contains: Passed!
|
||||
timeout: 5m
|
||||
- type: output-matches
|
||||
config:
|
||||
pattern: '(multi[- ]?target|#if|net8|net10|target framework|TFM|every (branch|target))'
|
||||
- type: prompt
|
||||
rubric:
|
||||
- "Recognizes the project multi-targets (`net8.0;net10.0`) and that `Label` is referenced inside BOTH `#if` branches, so the rename must be replicated across every `#if` branch and target framework — not just the branch the default build compiles"
|
||||
- "Renames by tracking true binding references, then rebuilds ALL target frameworks and re-runs the tests to prove behavior is preserved"
|
||||
- "Keeps this a single, focused rename without bundling unrelated edits"
|
||||
|
||||
- name: Rename a member of a partial type that a generated part references
|
||||
prompt: "Rename `Coupons.Redeem` to `Apply` — the name is too generic. Keep behavior identical. The solution is at Fixture.sln."
|
||||
environment:
|
||||
files:
|
||||
- src: .
|
||||
dest: .
|
||||
graders:
|
||||
- type: file-contains
|
||||
config:
|
||||
path: src/Billing/Coupons.cs
|
||||
value: Apply
|
||||
# Positive check that the fix went to the GENERATOR INPUT (the template),
|
||||
# which is where the generated part's reference to the renamed member lives.
|
||||
# Hand-editing the obj/ output would be overwritten on the next build.
|
||||
- type: file-contains
|
||||
config:
|
||||
path: src/Billing/Coupons.g.cs.template
|
||||
value: Apply
|
||||
# Full-propagation gate across the partial type: building the solution
|
||||
# regenerates the partial from the template. If the template (or any caller)
|
||||
# still references the old name, the regenerated code fails to compile.
|
||||
- type: run-command
|
||||
config:
|
||||
command: dotnet test Fixture.sln -v:q
|
||||
expected_exit_code: 0
|
||||
stdout_contains: Passed!
|
||||
timeout: 5m
|
||||
- type: output-matches
|
||||
config:
|
||||
pattern: '(auto[- ]?generated|source generator|generated (file|output|code)|partial|template|\.g\.cs|regenerat)'
|
||||
- type: prompt
|
||||
rubric:
|
||||
- "Includes EVERY partial declaration of `Coupons` when renaming, and finds the reference in the generated part (which references the renamed member)"
|
||||
- "Recognizes the generated part is emitted at build time from `Coupons.g.cs.template`: flags that hand-editing the generated output under `obj/` is unsafe (it is overwritten on the next build) and that the correct fix edits the template (the generator input) — not the emitted file"
|
||||
- "Rebuilds and re-runs the tests, letting the compiler catch any missed reference, to prove behavior is preserved"
|
||||
|
||||
- name: Add a new public method that must be correct on every target framework
|
||||
prompt: "Add a new public method `PlatformInfo.Tag()` that returns just the short platform tag (for example `net10` or `net8`) without the `platform:` prefix. It must return the correct tag on every target framework. The solution is at Fixture.sln."
|
||||
environment:
|
||||
files:
|
||||
- src: .
|
||||
dest: .
|
||||
graders:
|
||||
- type: file-contains
|
||||
config:
|
||||
path: src/Billing/PlatformInfo.cs
|
||||
value: Tag
|
||||
# Multi-target gate: the new method must compile on BOTH net8.0 and net10.0.
|
||||
- type: run-command
|
||||
config:
|
||||
command: dotnet build Fixture.sln -v:q
|
||||
expected_exit_code: 0
|
||||
timeout: 5m
|
||||
# Existing behavior stays green after the additive change.
|
||||
- type: run-command
|
||||
config:
|
||||
command: dotnet test Fixture.sln -v:q
|
||||
expected_exit_code: 0
|
||||
stdout_contains: Passed!
|
||||
timeout: 5m
|
||||
- type: output-matches
|
||||
config:
|
||||
pattern: '(multi[- ]?target|#if|net8|net10|target framework|TFM|public API|PublicAPI)'
|
||||
- type: prompt
|
||||
rubric:
|
||||
- "Treats this as a feature addition (a new public method), not a behavior-preserving refactor"
|
||||
- "Recognizes the project multi-targets (`net8.0;net10.0`) and uses `#if` branches, and implements `Tag()` so it returns the correct value on EVERY target framework — building and testing each target, not just the one the default build compiles"
|
||||
- "Recognizes `Tag()` is a NEW public member of a shipped type (public API surface) and updates the API baseline (`PublicAPI.Unshipped.txt`) or flags the surface addition"
|
||||
- "Keeps existing tests green and adds coverage for the new method"
|
||||
|
||||
- name: Do not raise breaking-change concerns for a purely internal local rename
|
||||
prompt: "Inside `OrderProcessor.DoStuff`, the local variable `subtotal` is a bit terse. Rename that local to `runningTotal` for readability. Keep behavior identical. The solution is at Fixture.sln."
|
||||
environment:
|
||||
files:
|
||||
- src: .
|
||||
dest: .
|
||||
graders:
|
||||
- type: file-contains
|
||||
config:
|
||||
path: src/Billing/OrderProcessor.cs
|
||||
value: runningTotal
|
||||
# The local rename is behavior-identical: tests must still pass unchanged.
|
||||
- type: run-command
|
||||
config:
|
||||
command: dotnet test Fixture.sln -v:q
|
||||
expected_exit_code: 0
|
||||
stdout_contains: Passed!
|
||||
timeout: 5m
|
||||
- type: prompt
|
||||
rubric:
|
||||
- "Recognizes that renaming a method-local variable touches no public API surface, no `#if`/multi-target branch, and no generated code, so no breaking-change hazard applies"
|
||||
- "Performs the local rename directly without introducing public-API baseline edits, source-generator changes, or other breaking-change ceremony that the change does not warrant"
|
||||
- "Confirms behavior is unchanged by rebuilding and re-running the tests"
|
||||
@@ -1,21 +0,0 @@
|
||||
namespace Billing;
|
||||
|
||||
/// <summary>Parses application settings, returning a fallback when the raw value is missing or invalid.</summary>
|
||||
public static class AppSettingsHelper
|
||||
{
|
||||
public static int ParseIntSetting(string? raw, int fallback)
|
||||
=> int.TryParse(raw, out var v) ? v : fallback;
|
||||
|
||||
public static bool ParseBoolSetting(string? raw, bool fallback)
|
||||
=> bool.TryParse(raw, out var v) ? v : fallback;
|
||||
}
|
||||
|
||||
/// <summary>Reads configuration values, returning a fallback when the raw value is missing or invalid.</summary>
|
||||
public static class ConfigReader
|
||||
{
|
||||
public static int ReadInt(string? raw, int fallback)
|
||||
=> int.TryParse(raw, out var v) ? v : fallback;
|
||||
|
||||
public static bool ReadBool(string? raw, bool fallback)
|
||||
=> bool.TryParse(raw, out var v) ? v : fallback;
|
||||
}
|
||||
@@ -1,23 +0,0 @@
|
||||
<Project Sdk="Microsoft.NET.Sdk">
|
||||
|
||||
<PropertyGroup>
|
||||
<TargetFrameworks>net8.0;net10.0</TargetFrameworks>
|
||||
<Nullable>enable</Nullable>
|
||||
<ImplicitUsings>enable</ImplicitUsings>
|
||||
<IsPackable>true</IsPackable>
|
||||
</PropertyGroup>
|
||||
|
||||
<ItemGroup>
|
||||
<None Include="Coupons.g.cs.template" />
|
||||
</ItemGroup>
|
||||
|
||||
<Target Name="GenerateCouponsPartial" BeforeTargets="CoreCompile">
|
||||
<Copy SourceFiles="Coupons.g.cs.template"
|
||||
DestinationFiles="$(IntermediateOutputPath)Coupons.g.cs"
|
||||
SkipUnchangedFiles="true" />
|
||||
<ItemGroup>
|
||||
<Compile Include="$(IntermediateOutputPath)Coupons.g.cs" />
|
||||
</ItemGroup>
|
||||
</Target>
|
||||
|
||||
</Project>
|
||||
@@ -1,8 +0,0 @@
|
||||
namespace Billing;
|
||||
|
||||
/// <summary>Hand-authored part of the <c>Coupons</c> partial type.</summary>
|
||||
public partial class Coupons
|
||||
{
|
||||
public decimal Redeem(string code, decimal amount)
|
||||
=> amount - (amount * RateFor(code));
|
||||
}
|
||||
@@ -1,14 +0,0 @@
|
||||
// <auto-generated/>
|
||||
namespace Billing;
|
||||
|
||||
public partial class Coupons
|
||||
{
|
||||
private static decimal RateFor(string code) => code switch
|
||||
{
|
||||
"SAVE10" => 0.10m,
|
||||
"SAVE05" => 0.05m,
|
||||
_ => 0.00m,
|
||||
};
|
||||
|
||||
public decimal RedeemDefault(decimal amount) => Redeem("SAVE10", amount);
|
||||
}
|
||||
@@ -1,36 +0,0 @@
|
||||
namespace Billing;
|
||||
|
||||
/// <summary>A single line item on an order.</summary>
|
||||
public readonly record struct OrderLine(string Sku, decimal UnitPrice, int Quantity);
|
||||
|
||||
/// <summary>The computed result of pricing an order.</summary>
|
||||
public readonly record struct Invoice(decimal Total, decimal Quote, decimal Shipping);
|
||||
|
||||
public sealed class OrderProcessor
|
||||
{
|
||||
public Invoice DoStuff(IReadOnlyList<OrderLine> lines, string tier)
|
||||
{
|
||||
decimal subtotal = 0m;
|
||||
foreach (var line in lines)
|
||||
{
|
||||
subtotal += line.UnitPrice * line.Quantity;
|
||||
}
|
||||
|
||||
decimal shipping = subtotal >= 100m ? 0m : 9.99m;
|
||||
|
||||
decimal rateA = tier == "gold" ? 0.10m : tier == "silver" ? 0.05m : 0.00m;
|
||||
decimal discountedA = subtotal - (subtotal * rateA);
|
||||
decimal totalWithTax = discountedA + (discountedA * 0.08m);
|
||||
|
||||
decimal quoteBase = subtotal + shipping;
|
||||
|
||||
decimal rateB = tier == "gold" ? 0.10m : tier == "silver" ? 0.05m : 0.00m;
|
||||
decimal discountedB = quoteBase - (quoteBase * rateB);
|
||||
decimal quoteWithTax = discountedB + (discountedB * 0.08m);
|
||||
|
||||
return new Invoice(
|
||||
Math.Round(totalWithTax, 2),
|
||||
Math.Round(quoteWithTax, 2),
|
||||
shipping);
|
||||
}
|
||||
}
|
||||
@@ -1,19 +0,0 @@
|
||||
namespace Billing;
|
||||
|
||||
/// <summary>Reports a platform label for the current target framework.</summary>
|
||||
public static class PlatformInfo
|
||||
{
|
||||
private const string ModernTag = "net10";
|
||||
private const string LegacyTag = "net8";
|
||||
|
||||
public static string Current()
|
||||
{
|
||||
#if NET8_0
|
||||
return Label(LegacyTag);
|
||||
#else
|
||||
return Label(ModernTag);
|
||||
#endif
|
||||
}
|
||||
|
||||
private static string Label(string tag) => $"platform:{tag}";
|
||||
}
|
||||
@@ -1,25 +0,0 @@
|
||||
namespace Billing;
|
||||
|
||||
/// <summary>The canonical tax rule.</summary>
|
||||
public static class TaxRules
|
||||
{
|
||||
public static decimal Apply(decimal amount) => amount + (amount * 0.08m);
|
||||
}
|
||||
|
||||
/// <summary>Legacy tax entry point retained for older callers.</summary>
|
||||
public static class LegacyTax
|
||||
{
|
||||
public static decimal ApplyTaxWrapper(decimal amount) => TaxRules.Apply(amount);
|
||||
}
|
||||
|
||||
public sealed class GoldPricing
|
||||
{
|
||||
public decimal Rate => 0.10m;
|
||||
public string Name => "gold";
|
||||
}
|
||||
|
||||
public sealed class SilverPricing
|
||||
{
|
||||
public decimal Rate => 0.05m;
|
||||
public string Name => "silver";
|
||||
}
|
||||
@@ -1,7 +0,0 @@
|
||||
#nullable enable
|
||||
Billing.AppSettingsHelper
|
||||
static Billing.AppSettingsHelper.ParseIntSetting(string? raw, int fallback) -> int
|
||||
static Billing.AppSettingsHelper.ParseBoolSetting(string? raw, bool fallback) -> bool
|
||||
Billing.ConfigReader
|
||||
static Billing.ConfigReader.ReadInt(string? raw, int fallback) -> int
|
||||
static Billing.ConfigReader.ReadBool(string? raw, bool fallback) -> bool
|
||||
@@ -1,20 +0,0 @@
|
||||
<Project Sdk="Microsoft.NET.Sdk">
|
||||
|
||||
<PropertyGroup>
|
||||
<TargetFramework>net10.0</TargetFramework>
|
||||
<Nullable>enable</Nullable>
|
||||
<ImplicitUsings>enable</ImplicitUsings>
|
||||
<IsPackable>false</IsPackable>
|
||||
</PropertyGroup>
|
||||
|
||||
<ItemGroup>
|
||||
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.11.1" />
|
||||
<PackageReference Include="xunit" Version="2.9.2" />
|
||||
<PackageReference Include="xunit.runner.visualstudio" Version="2.8.2" />
|
||||
</ItemGroup>
|
||||
|
||||
<ItemGroup>
|
||||
<ProjectReference Include="..\..\src\Billing\Billing.csproj" />
|
||||
</ItemGroup>
|
||||
|
||||
</Project>
|
||||
@@ -1,111 +0,0 @@
|
||||
using Billing;
|
||||
using Xunit;
|
||||
|
||||
namespace Billing.Tests;
|
||||
|
||||
public class BillingTests
|
||||
{
|
||||
[Fact]
|
||||
public void Invoice_NoTier_ComputesTotalAndQuote()
|
||||
{
|
||||
var processor = new OrderProcessor();
|
||||
var lines = new[]
|
||||
{
|
||||
new OrderLine("A", 50m, 1),
|
||||
new OrderLine("B", 20m, 2),
|
||||
};
|
||||
|
||||
var invoice = processor.DoStuff(lines, "none");
|
||||
|
||||
// subtotal = 90, shipping = 9.99 (subtotal < 100)
|
||||
// total = 90 * 1.08 = 97.20
|
||||
// quote = (90 + 9.99) * 1.08 = 107.9892 -> 107.99
|
||||
Assert.Equal(97.20m, invoice.Total);
|
||||
Assert.Equal(107.99m, invoice.Quote);
|
||||
Assert.Equal(9.99m, invoice.Shipping);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Invoice_Gold_AppliesDiscountAndFreeShipping()
|
||||
{
|
||||
var processor = new OrderProcessor();
|
||||
var lines = new[] { new OrderLine("A", 100m, 2) };
|
||||
|
||||
var invoice = processor.DoStuff(lines, "gold");
|
||||
|
||||
// subtotal = 200, shipping = 0 (subtotal >= 100)
|
||||
// discounted = 200 * 0.90 = 180, total = 180 * 1.08 = 194.40
|
||||
// quote base = 200 -> same as total = 194.40
|
||||
Assert.Equal(194.40m, invoice.Total);
|
||||
Assert.Equal(194.40m, invoice.Quote);
|
||||
Assert.Equal(0m, invoice.Shipping);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Invoice_Silver_AppliesFivePercentDiscount()
|
||||
{
|
||||
var processor = new OrderProcessor();
|
||||
var lines = new[] { new OrderLine("A", 200m, 1) };
|
||||
|
||||
var invoice = processor.DoStuff(lines, "silver");
|
||||
|
||||
// subtotal = 200, shipping = 0
|
||||
// discounted = 200 * 0.95 = 190, total = 190 * 1.08 = 205.20
|
||||
Assert.Equal(205.20m, invoice.Total);
|
||||
Assert.Equal(205.20m, invoice.Quote);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Tax_Wrapper_MatchesUnderlyingRule()
|
||||
{
|
||||
Assert.Equal(TaxRules.Apply(100m), LegacyTax.ApplyTaxWrapper(100m));
|
||||
Assert.Equal(108m, LegacyTax.ApplyTaxWrapper(100m));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void PricingTiers_HaveExpectedRatesAndNames()
|
||||
{
|
||||
Assert.Equal(0.10m, new GoldPricing().Rate);
|
||||
Assert.Equal("gold", new GoldPricing().Name);
|
||||
Assert.Equal(0.05m, new SilverPricing().Rate);
|
||||
Assert.Equal("silver", new SilverPricing().Name);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void PlatformInfo_Current_ReportsModernTagUnderNet10()
|
||||
{
|
||||
// The test project targets net10.0, so the #else branch is active.
|
||||
Assert.Equal("platform:net10", PlatformInfo.Current());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void AppSettingsHelper_ParsesOrFallsBack()
|
||||
{
|
||||
Assert.Equal(42, AppSettingsHelper.ParseIntSetting("42", 0));
|
||||
Assert.Equal(7, AppSettingsHelper.ParseIntSetting("nope", 7));
|
||||
Assert.True(AppSettingsHelper.ParseBoolSetting("true", false));
|
||||
Assert.False(AppSettingsHelper.ParseBoolSetting("bad", false));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ConfigReader_MatchesAppSettingsHelper()
|
||||
{
|
||||
Assert.Equal(AppSettingsHelper.ParseIntSetting("10", 0), ConfigReader.ReadInt("10", 0));
|
||||
Assert.Equal(AppSettingsHelper.ParseBoolSetting("true", false), ConfigReader.ReadBool("true", false));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Coupons_Redeem_AppliesRate()
|
||||
{
|
||||
var coupons = new Coupons();
|
||||
Assert.Equal(90m, coupons.Redeem("SAVE10", 100m));
|
||||
Assert.Equal(95m, coupons.Redeem("SAVE05", 100m));
|
||||
Assert.Equal(100m, coupons.Redeem("UNKNOWN", 100m));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Coupons_RedeemDefault_UsesSave10()
|
||||
{
|
||||
Assert.Equal(90m, new Coupons().RedeemDefault(100m));
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user