diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 731962bb..2e433086 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -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) diff --git a/eng/known-domains.txt b/eng/known-domains.txt index 4831600c..ed66024b 100644 --- a/eng/known-domains.txt +++ b/eng/known-domains.txt @@ -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 diff --git a/plugins/dotnet/README.md b/plugins/dotnet/README.md index e845d041..fd107988 100644 --- a/plugins/dotnet/README.md +++ b/plugins/dotnet/README.md @@ -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) diff --git a/plugins/dotnet/skills/csharp-refactoring/SKILL.md b/plugins/dotnet/skills/csharp-refactoring/SKILL.md index 33a1f40f..ae017a21 100644 --- a/plugins/dotnet/skills/csharp-refactoring/SKILL.md +++ b/plugins/dotnet/skills/csharp-refactoring/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`/`` — not interchangeable), ``/`#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 diff --git a/plugins/dotnet/skills/dotnet-breaking-changes/SKILL.md b/plugins/dotnet/skills/dotnet-breaking-changes/SKILL.md deleted file mode 100644 index a84d4e53..00000000 --- a/plugins/dotnet/skills/dotnet-breaking-changes/SKILL.md +++ /dev/null @@ -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 "" --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` / ``). 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 (``) 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. diff --git a/plugins/dotnet/skills/dotnet-breaking-changes/references/internals-visible-to.md b/plugins/dotnet/skills/dotnet-breaking-changes/references/internals-visible-to.md deleted file mode 100644 index 915fdd00..00000000 --- a/plugins/dotnet/skills/dotnet-breaking-changes/references/internals-visible-to.md +++ /dev/null @@ -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 -`` 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. diff --git a/plugins/dotnet/skills/dotnet-breaking-changes/references/multi-targeting.md b/plugins/dotnet/skills/dotnet-breaking-changes/references/multi-targeting.md deleted file mode 100644 index c6b736cf..00000000 --- a/plugins/dotnet/skills/dotnet-breaking-changes/references/multi-targeting.md +++ /dev/null @@ -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 `` (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 ``. - -## 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. diff --git a/plugins/dotnet/skills/dotnet-breaking-changes/references/public-api.md b/plugins/dotnet/skills/dotnet-breaking-changes/references/public-api.md deleted file mode 100644 index 2dab2847..00000000 --- a/plugins/dotnet/skills/dotnet-breaking-changes/references/public-api.md +++ /dev/null @@ -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) | ``, `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. diff --git a/plugins/dotnet/skills/dotnet-breaking-changes/references/source-generation.md b/plugins/dotnet/skills/dotnet-breaking-changes/references/source-generation.md deleted file mode 100644 index 1ea201ac..00000000 --- a/plugins/dotnet/skills/dotnet-breaking-changes/references/source-generation.md +++ /dev/null @@ -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 .*" --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 `` 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. diff --git a/tests/dotnet/dotnet-breaking-changes/Fixture.sln b/tests/dotnet/dotnet-breaking-changes/Fixture.sln deleted file mode 100644 index 335eab49..00000000 --- a/tests/dotnet/dotnet-breaking-changes/Fixture.sln +++ /dev/null @@ -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 diff --git a/tests/dotnet/dotnet-breaking-changes/eval.yaml b/tests/dotnet/dotnet-breaking-changes/eval.yaml deleted file mode 100644 index fe71c87e..00000000 --- a/tests/dotnet/dotnet-breaking-changes/eval.yaml +++ /dev/null @@ -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" diff --git a/tests/dotnet/dotnet-breaking-changes/src/Billing/AppSettingsHelper.cs b/tests/dotnet/dotnet-breaking-changes/src/Billing/AppSettingsHelper.cs deleted file mode 100644 index f8923ed8..00000000 --- a/tests/dotnet/dotnet-breaking-changes/src/Billing/AppSettingsHelper.cs +++ /dev/null @@ -1,21 +0,0 @@ -namespace Billing; - -/// Parses application settings, returning a fallback when the raw value is missing or invalid. -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; -} - -/// Reads configuration values, returning a fallback when the raw value is missing or invalid. -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; -} diff --git a/tests/dotnet/dotnet-breaking-changes/src/Billing/Billing.csproj b/tests/dotnet/dotnet-breaking-changes/src/Billing/Billing.csproj deleted file mode 100644 index 60587d4b..00000000 --- a/tests/dotnet/dotnet-breaking-changes/src/Billing/Billing.csproj +++ /dev/null @@ -1,23 +0,0 @@ - - - - net8.0;net10.0 - enable - enable - true - - - - - - - - - - - - - - diff --git a/tests/dotnet/dotnet-breaking-changes/src/Billing/Coupons.cs b/tests/dotnet/dotnet-breaking-changes/src/Billing/Coupons.cs deleted file mode 100644 index 79ceb96d..00000000 --- a/tests/dotnet/dotnet-breaking-changes/src/Billing/Coupons.cs +++ /dev/null @@ -1,8 +0,0 @@ -namespace Billing; - -/// Hand-authored part of the Coupons partial type. -public partial class Coupons -{ - public decimal Redeem(string code, decimal amount) - => amount - (amount * RateFor(code)); -} diff --git a/tests/dotnet/dotnet-breaking-changes/src/Billing/Coupons.g.cs.template b/tests/dotnet/dotnet-breaking-changes/src/Billing/Coupons.g.cs.template deleted file mode 100644 index d247c331..00000000 --- a/tests/dotnet/dotnet-breaking-changes/src/Billing/Coupons.g.cs.template +++ /dev/null @@ -1,14 +0,0 @@ -// -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); -} diff --git a/tests/dotnet/dotnet-breaking-changes/src/Billing/OrderProcessor.cs b/tests/dotnet/dotnet-breaking-changes/src/Billing/OrderProcessor.cs deleted file mode 100644 index 78ac5748..00000000 --- a/tests/dotnet/dotnet-breaking-changes/src/Billing/OrderProcessor.cs +++ /dev/null @@ -1,36 +0,0 @@ -namespace Billing; - -/// A single line item on an order. -public readonly record struct OrderLine(string Sku, decimal UnitPrice, int Quantity); - -/// The computed result of pricing an order. -public readonly record struct Invoice(decimal Total, decimal Quote, decimal Shipping); - -public sealed class OrderProcessor -{ - public Invoice DoStuff(IReadOnlyList 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); - } -} diff --git a/tests/dotnet/dotnet-breaking-changes/src/Billing/PlatformInfo.cs b/tests/dotnet/dotnet-breaking-changes/src/Billing/PlatformInfo.cs deleted file mode 100644 index 62ab629a..00000000 --- a/tests/dotnet/dotnet-breaking-changes/src/Billing/PlatformInfo.cs +++ /dev/null @@ -1,19 +0,0 @@ -namespace Billing; - -/// Reports a platform label for the current target framework. -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}"; -} diff --git a/tests/dotnet/dotnet-breaking-changes/src/Billing/Pricing.cs b/tests/dotnet/dotnet-breaking-changes/src/Billing/Pricing.cs deleted file mode 100644 index 02b4d676..00000000 --- a/tests/dotnet/dotnet-breaking-changes/src/Billing/Pricing.cs +++ /dev/null @@ -1,25 +0,0 @@ -namespace Billing; - -/// The canonical tax rule. -public static class TaxRules -{ - public static decimal Apply(decimal amount) => amount + (amount * 0.08m); -} - -/// Legacy tax entry point retained for older callers. -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"; -} diff --git a/tests/dotnet/dotnet-breaking-changes/src/Billing/PublicAPI.Shipped.txt b/tests/dotnet/dotnet-breaking-changes/src/Billing/PublicAPI.Shipped.txt deleted file mode 100644 index 80f5318f..00000000 --- a/tests/dotnet/dotnet-breaking-changes/src/Billing/PublicAPI.Shipped.txt +++ /dev/null @@ -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 diff --git a/tests/dotnet/dotnet-breaking-changes/tests/Billing.Tests/Billing.Tests.csproj b/tests/dotnet/dotnet-breaking-changes/tests/Billing.Tests/Billing.Tests.csproj deleted file mode 100644 index 9a83c317..00000000 --- a/tests/dotnet/dotnet-breaking-changes/tests/Billing.Tests/Billing.Tests.csproj +++ /dev/null @@ -1,20 +0,0 @@ - - - - net10.0 - enable - enable - false - - - - - - - - - - - - - diff --git a/tests/dotnet/dotnet-breaking-changes/tests/Billing.Tests/BillingTests.cs b/tests/dotnet/dotnet-breaking-changes/tests/Billing.Tests/BillingTests.cs deleted file mode 100644 index 5a7f5d4f..00000000 --- a/tests/dotnet/dotnet-breaking-changes/tests/Billing.Tests/BillingTests.cs +++ /dev/null @@ -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)); - } -}