From f2e4eeaca8f3c5bf116e1dfa62e7d4fbff89f412 Mon Sep 17 00:00:00 2001 From: Abhitej John Date: Tue, 7 Jul 2026 13:36:30 -0700 Subject: [PATCH 1/8] Add csharp-refactoring and dotnet-breaking-changes dotnet skills Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/CODEOWNERS | 4 + eng/known-domains.txt | 1 + plugins/dotnet/README.md | 2 + .../dotnet/skills/csharp-refactoring/SKILL.md | 192 ++++++++++++++++++ .../references/operation-catalog.md | 37 ++++ .../skills/dotnet-breaking-changes/SKILL.md | 129 ++++++++++++ .../references/internals-visible-to.md | 42 ++++ .../references/multi-targeting.md | 48 +++++ .../references/public-api.md | 65 ++++++ .../references/source-generation.md | 38 ++++ 10 files changed, 558 insertions(+) create mode 100644 plugins/dotnet/skills/csharp-refactoring/SKILL.md create mode 100644 plugins/dotnet/skills/csharp-refactoring/references/operation-catalog.md create mode 100644 plugins/dotnet/skills/dotnet-breaking-changes/SKILL.md create mode 100644 plugins/dotnet/skills/dotnet-breaking-changes/references/internals-visible-to.md create mode 100644 plugins/dotnet/skills/dotnet-breaking-changes/references/multi-targeting.md create mode 100644 plugins/dotnet/skills/dotnet-breaking-changes/references/public-api.md create mode 100644 plugins/dotnet/skills/dotnet-breaking-changes/references/source-generation.md diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index d03cef0157..707585013a 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -12,6 +12,10 @@ # dotnet (common everyday C#/.NET) /plugins/dotnet/skills/setup-local-sdk/ @dotnet/roslyn-ide @dotnet/skills-csharp-language-reviewers /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 c6f32825a4..88752066c6 100644 --- a/eng/known-domains.txt +++ b/eng/known-domains.txt @@ -42,6 +42,7 @@ 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 d62d861bc3..c8daf82ad1 100644 --- a/plugins/dotnet/README.md +++ b/plugins/dotnet/README.md @@ -17,4 +17,6 @@ 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 new file mode 100644 index 0000000000..bd51987157 --- /dev/null +++ b/plugins/dotnet/skills/csharp-refactoring/SKILL.md @@ -0,0 +1,192 @@ +--- +name: csharp-refactoring +description: "Performs safe, behavior-preserving refactoring of C#/.NET code, verified with build, tests, and analyzers. Use when the user wants to refactor, rename, restructure, clean up, modernize, or reorganize C# code WITHOUT changing behavior: rename a symbol/type/file across a solution; move a type or static members to another file/namespace/project; extract a method, interface, or base class; pull members up; inline a method or local; split a large class/file; consolidate or de-duplicate copy-pasted code; sync namespaces to folders; modernize to current C# idioms (file-scoped namespaces, primary constructors, collection expressions, target-typed new, pattern matching); or enable nullable reference annotations. Prefers Roslyn-backed edits over text find/replace, and defers .NET compatibility hazards to the companion dotnet-breaking-changes skill. DO NOT USE FOR: adding features, fixing bugs, writing new tests, upgrading frameworks or NuGet versions (use dotnet-upgrade), or formatting-only passes (use dotnet format)." +license: MIT +--- + +# C# Refactoring (behavior-preserving) + +Refactor C#/.NET code so the structure improves but the observable behavior does **not** change. Every +refactor is a sequence of small, named, Roslyn-aware transformations, each followed by a build/test +gate. If the gate fails, stop and revert — a refactor that changes behavior is a bug, not a refactor. + +## Operation catalog (what "refactoring" means in C#) + +The canonical C# refactoring operations, aligned with Roslyn's IDE refactoring providers: + +- **Rename** a symbol / type / file +- **Move** a type / member / file (to another file, namespace, or project) +- **Consolidate / de-duplicate** copy-pasted code +- **Modernize / simplify** to current C# idioms (the repo's analyzers decide the idiom, not taste) +- **Split** a large class / file / assembly +- **Extract** a method / class / interface +- **Enable nullable** reference annotations +- **Inline** a method / local / constant +- **Pull up / push down** a member +- **Sync namespace** to folder + +Each is a single named operation; compose them one step at a time (see Procedure). For the Roslyn +provider mapping and representative real PRs, see +[references/operation-catalog.md](references/operation-catalog.md). + +## Tooling — C# LSP (Roslyn language server) + +A headless CLI agent usually can't invoke IDE code actions, but it **can** get Roslyn-quality +_semantic navigation_ from the C# language server the [`dotnet` plugin in `dotnet/skills`](https://github.com/dotnet/skills/blob/main/plugins/dotnet/lsp.json) +declares. It launches through the .NET CLI (`dnx roslyn-language-server --yes --prerelease -- --stdio +--autoLoadProjects`) over `.cs`, `.razor`, and `.cshtml` files (prerequisite: a .NET 10 SDK with +`dotnet` on `PATH`). + +When the LSP is available, prefer its binding-aware operations over textual grep/glob for every +reference-finding step in a refactor: + +- Where a symbol is defined → **goToDefinition** +- All _binding_ references to a symbol → **findReferences** (the reliable rename/move safety net) +- What calls a method → **incomingCalls** +- Find symbols by name across the workspace → **workspaceSymbol** +- A symbol's type / signature / docs → **hover** + +These are exact, binding-aware answers — unlike grep they don't match comments, string literals, or +unrelated overloads. Reach for the LSP first; fall back to grep only when it is unavailable. + +## .NET compatibility hazards — inspect first, then load `dotnet-breaking-changes` + +Behavior-preserving edits fail in _.NET-specific_ ways a green test run won't catch. Before you edit, +**search the repo** for the surfaces that govern the symbol — don't assume or recite: + +- Public-API gate: `PublicAPI.Shipped/Unshipped.txt` (PublicApiAnalyzers) and/or `ApiCompat` / + `` — these are _not_ interchangeable. +- `` and `#if` / platform branches. +- `partial` declarations and generated (`*.g.cs`) files. +- `InternalsVisibleTo` friend/test assemblies. + +The four bullets above (and the reminders below) are the **floor**: apply them even if the guide skill +is not installed. When it _is_ available, **load the `dotnet-breaking-changes` skill** for the full +per-surface playbook (it applies to any change, not just refactors, and holds the depth on each +surface); it _adds_ compatibility analysis but does **not** replace this skill's safety contract. +Refactor-specific reminders: move a public type across assemblies via a `[TypeForwardedTo]` forwarder +(a _rename_ needs an `[Obsolete]` shim, not a forwarder); include _every_ `partial` declaration in a +rename/move; edit the generator input, not generated output; and treat friend-assembly `internal` +members with public-API care. + +## Stop and escalate (do not silently proceed) + +Halt and ask the user before continuing when: + +- The **baseline is red** (build or tests already failing) — you can't prove you preserved behavior. +- A **public/shipped API** would change and you cannot verify compatibility (no type-forwarder/shim + path, and no PublicApiAnalyzers/ApiCompat/package-validation gate available to catch a break). +- The request is **not actually behavior-preserving** — it asks you to upgrade a framework/package, + add a feature, or fix a bug. Do **not** carry it out under this skill, and **never label such a + change "behavior-preserving."** Say it is out of scope and redirect (upgrades → `dotnet-upgrade`; + features/fixes → normal dev flow). If a refactor is genuinely a prerequisite, do only that, as a + separate step, and stop. +- The edit touches **generated, designer, or migration files** (`*.g.cs`, `*.Designer.cs`, EF + migrations, source-generator output) — hand-edits there are **overwritten on the next build**; + change the source of generation (template/generator input), not the output. +- Semantic equivalence depends on **runtime behavior not covered by tests** (reflection, DI wiring, + serialization, `dynamic`, P/Invoke) — flag the risk; tests alone won't catch a regression. +- The operation can't be done with any tool on the fallback ladder and would require a wide, + unverifiable text replace. + +## When to use + +- "Rename this method/type everywhere safely" / "rename across the solution" +- "Extract this block into a method" / "extract an interface from this class" +- "Move this type into its own file / into project X / into namespace Y" +- "This class is too big — split it" / "consolidate these two near-identical implementations" +- "Modernize this file to current C#" / "use file-scoped namespaces and primary constructors" +- "Turn on nullable annotations for this project and fix the warnings" +- Any request to **restructure / clean up / reorganize** code while keeping behavior identical + +## When NOT to use + +- The change is meant to alter behavior, add a feature, or fix a bug (not a refactor) +- Framework/SDK/NuGet version upgrades → `dotnet-upgrade` +- Pure formatting/whitespace → `dotnet format` +- Writing brand-new tests → `writing-mstest-tests` / `code-testing-agent` + +## Procedure (the safety contract) + +1. **Establish a green baseline.** Build the affected projects and run the relevant tests. Never + refactor on a red baseline — you won't be able to tell what you broke. +2. **Pick ONE operation from the catalog.** Refactors compose, but each step is a single named + operation. Never mix a refactor with a behavior change in the same step. +3. **Find the true references first (for rename/move/inline).** Before editing, locate every _binding_ + reference to the symbol — not textual matches. Prefer the C# LSP: **findReferences** / + **goToDefinition** / **incomingCalls** (see **Tooling**) return only true bindings. If the LSP is + unavailable, a headless grep finds candidates but also false positives (test method names, comments, + strings, unrelated overloads); treat grep as a _candidate list_, then keep only true bindings and let + the compiler catch any you missed. Skipping this is the #1 way a "rename" silently corrupts code. +4. **Prefer semantics-aware edits over text edits.** The principle is _semantic verification_, not a + specific API — use the strongest tool actually available, in this order (fallback ladder): + 1. An IDE / Roslyn workspace refactoring (rename, move, extract, inline, pull-up) when available — + updates all references, `using`s, and partial declarations correctly. + 2. **The C# LSP (Roslyn language server) from `dotnet/skills`** (see **Tooling**) for a headless + agent: drive edits from **findReferences** / **goToDefinition** / **incomingCalls** / + **workspaceSymbol** so every touched reference is a true binding, not a textual guess. Some + language-server builds also expose a `textDocument/rename` code action that rewrites all + references at once — use it when present. + 3. Analyzer code-fixes / `dotnet format analyzers` / **Roslynator** for idiom modernization and + de-duplication, when installed. + 4. **Compiler-validated, reference-tracked edits** (the common headless fallback): use the LSP (or + grep, if no LSP) to enumerate candidates → edit only the true binding references from step 3 → + rebuild. The compiler is your safety net: any missed or wrongly-edited reference becomes a build + error, not a silent bug. + 5. Plain find/replace ONLY when scope is provably tiny and every reference is verified — it + silently corrupts strings, comments, and unrelated overloads. + A CLI agent often won't have a loaded MSBuildWorkspace, but it **can** load the C# LSP above; prefer + its semantic answers over grep, and never skip the build/test gate to compensate. +5. **Re-gate after every step — across every target framework.** Rebuild + re-run tests; if red, revert + this step and reassess. The diff must be behavior-neutral: tests still green, no new warnings (nullable + work is the deliberate exception), no new analyzer/API-compat diagnostics, and the intended public + contract unchanged. On a multi-targeted project, build/test **each** TFM — a green default build can + still be broken on another target. + + ```bash + dotnet build # 0 errors, on every TargetFramework (add -f to check one) + dotnet test # must stay green; same pass count as baseline + # if the gate fails, revert THIS step and reassess: + git restore . # or: git checkout -- + ``` +6. **Keep the relevant contract stable — and which contract depends on the project type:** + - **Library / shipped package:** preserve the .NET public API; move public types across assembly + boundaries via type-forwarders or `[Obsolete]` shims, not breaking moves. + - **Application / service:** preserve the _external_ contract (HTTP routes, config keys, DB schema, + CLI args); internal type surface can move freely. + - **Small / private codebase:** preserve behavior + tests; commit granularity can relax. +7. **One refactor per commit** for libraries/large repos (keeps review + `git bisect` meaningful); + relax for small private codebases. + +## Inputs + +- Target scope: a symbol, file, type, project, or directory. +- The operation (from the catalog) — or infer it from the request. +- How to build and test the affected projects (solution/proj path, test command). + +## Outputs + +- The applied refactoring as a minimal, behavior-preserving diff. +- Proof of safety: before/after build + test results. +- A short rationale naming the operation(s) applied. + +## Anti-patterns to avoid + +- Renaming via text replace (hits strings, comments, unrelated overloads). +- "Refactor + small fix" in one step (a behavior change masquerading as a refactor). +- Moving a public type without a forwarder/shim (silent breaking change) — or refactoring a public + surface with no PublicApiAnalyzers/ApiCompat gate to catch a break. +- Editing only the TFM/`#if` branch your editor shows, leaving other targets broken. +- Renaming a `partial` type without its other declarations, or editing generated (`*.g.cs`) output + instead of the generator input. +- Skipping the test gate "because it's just a rename." + +## Reference Files + +- **[references/operation-catalog.md](references/operation-catalog.md)** — the full operation taxonomy + with Roslyn providers and representative real PRs. + **Load when** you need the provider for an operation or more detail on the catalog. + +For .NET compatibility hazards (public API, multi-targeting/`#if`, source-generated/partial code, +`InternalsVisibleTo`), load the companion `dotnet-breaking-changes` skill and its `references/` — it +holds the depth on each surface and applies to any change, not just refactors. diff --git a/plugins/dotnet/skills/csharp-refactoring/references/operation-catalog.md b/plugins/dotnet/skills/csharp-refactoring/references/operation-catalog.md new file mode 100644 index 0000000000..ba174d63c6 --- /dev/null +++ b/plugins/dotnet/skills/csharp-refactoring/references/operation-catalog.md @@ -0,0 +1,37 @@ +# C# refactoring operation catalog (detail) + +Load this when you need the full operation taxonomy or the Roslyn provider each maps to. +The core SKILL.md keeps only the short operation list; this file is the depth. + +## Contents + +- Operation catalog (provider, recurrence, representative PR) +- "Modernize" is defined by the repo's analyzer config + +## Operation catalog + +Canonical operations, aligned with Roslyn's own IDE refactoring providers +(`src/Features/CSharp/Portable/CodeRefactorings`). The provider column is informational — a +headless CLI agent usually cannot invoke these IDE code actions directly, so apply them via the fallback +ladder in SKILL.md. + +| Operation | Roslyn provider / tool | Recurrence | Representative real PR | +|-----------|------------------------|-----------:|------------------------| +| **Rename** symbol / type / file | rename engine (`Renamer`) | high | _runtime_ "Rename `DISABLE_CROSSGEN`"; _sdk_ "Rename `dnup` to `dotnetup`" | +| **Move** type / member / file | `MoveType`, `MoveStaticMembers` | high | _runtime_ "Move DSA tests into System.Security.Cryptography"; _sdk_ "Move SDK task unit test projects from src/ to test/" | +| **Consolidate / de-duplicate** | analyzer-assisted | high | _runtime_ "Consolidate ComWrappers implementation across platforms" | +| **Modernize / simplify** idioms | `UseExplicitOrImplicitType`, `UseRecursivePatterns`, `ConvertLocalFunctionToMethod`, `AddAwait` | high | _roslyn_ "Simplify lots of redundant code in code fix providers" | +| **Split** large class / file / assembly | extract + move | medium | _roslyn_ "Split `FeatureSwitchManager`"; _runtime_ "Move RPC contracts to a separate assembly" | +| **Extract** method / class / interface | `ExtractClass`, extract-method, extract-interface | medium | _runtime_ "Extract `ManifestBuilder` and `EventListener`" | +| **Enable nullable** annotations | `EnableNullable` | medium | _runtime_ "[Group 4] Enable nullable annotations for `Microsoft.Extensions.Logging.EventSource`" | +| **Inline** method / local / constant | `InlineMethod`, `InlineTemporary` | lower | _runtime_ "Refactor `UInt128` division" | +| **Pull up / push down** member | `PullMemberUp` | tail | move members between a type and its base/interface | +| **Sync namespace** to folder | `SyncNamespace` | tail | align `namespace` with folder layout after a move | + +## "Modernize" is defined by the repo's analyzer config, not personal taste + +These repos drive idiom rules through `.editorconfig` + `EnforceOnBuild` (roslyn #49995 "Set +EnforceOnBuild values for code style analyzers"). Apply the fixes the repo's own analyzers request +(IDE00xx / CAxxxx) via `dotnet format` / code-fixes; don't impose a style it hasn't opted into, and don't +blanket-suppress diagnostics to make an edit "pass." For _adopting_ nullable annotations specifically, see +`dotnet-upgrade/migrate-nullable-references`. diff --git a/plugins/dotnet/skills/dotnet-breaking-changes/SKILL.md b/plugins/dotnet/skills/dotnet-breaking-changes/SKILL.md new file mode 100644 index 0000000000..ed62e6324b --- /dev/null +++ b/plugins/dotnet/skills/dotnet-breaking-changes/SKILL.md @@ -0,0 +1,129 @@ +--- +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 you are +looking at**. A change that compiles locally and keeps tests green can still break a downstream +consumer, another target framework, a friend assembly, or regenerate away on the next build. This +skill names the four hidden surfaces, tells you how to find which ones a repo actually uses, and +points to a reference file for each. It is deliberately _not_ refactoring-specific — the same +hazards apply to a feature or a bug fix, and to **additions** (a new public member, overload, or +target framework) as much as to removals: adding surface creates a permanent contract obligation, +and adding behavior to a multi-targeted or partial type must stay correct on every target and +survive the next regeneration. For the behavior-preserving refactoring **process** +(green baseline → one named op → re-gate), 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 + +Do not recite these checks or assume; **search the repo and adapt to what exists.** The markers +present dictate the validation 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\|Microsoft.DotNet.ApiCompat" --include=*.props --include=*.targets --include=*.csproj . +# Multi-targeting and conditional code +grep -rl "" --include=*.csproj --include=*.props . +grep -rn "#if " --include=*.cs . # NET*, platform, and custom symbols +# 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 four hidden surfaces (summary — 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 surface.** In a shipped library, 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 different, non- + interchangeable ways**: source-level (`PublicApiAnalyzers` + `PublicAPI.*.txt`, which you + maintain) and binary/package-level (`ApiCompat` / ``). Run/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. 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 only 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. + +## When to use + +- "Is this a breaking change?" / "Will this break the public API / the NuGet package?" +- Editing, renaming, or removing a public or `internal` member in a library +- Changing a signature, nullability, generic constraint, or trimming/AOT attribute +- Editing a multi-targeted project, code under `#if`, or a platform-specific branch +- Touching a `partial` type or source-generated code + +## When NOT to use + +- Framework/SDK/NuGet **version upgrades** → the `dotnet-upgrade` plugin (`migrate-*`, + `dotnet-aot-compat`, `migrate-nullable-references`) +- Pure formatting/whitespace → `dotnet format` +- A single-target private app with no public or cross-assembly surface (no hidden contract to break) + +## Reference Files + +- **[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. **Load when** editing a public/`internal` member in a library, changing a + signature/nullability/attribute, or asked whether something is a breaking change. +- **[references/multi-targeting.md](references/multi-targeting.md)** — TFMs, `#if` and platform + symbols, RIDs, and how to inspect and re-gate every target. **Load when** the project has + `` or the code uses `#if`. +- **[references/source-generation.md](references/source-generation.md)** — partial types, generated + output vs checked-in source, editing the generator input. **Load when** the symbol is `partial` + or lives in `*.g.cs`/generated files. (See also `dotnet-msbuild/including-generated-files`.) +- **[references/internals-visible-to.md](references/internals-visible-to.md)** — friend/test + assemblies and strong-name keys. **Load when** the repo has `InternalsVisibleTo` and you touch an + `internal` member. + +## Related skills + +- `csharp-refactoring` — the behavior-preserving refactoring process that consumes this knowledge. + This guide _adds_ compatibility analysis; it does **not** replace that skill's green-baseline → one + named op → re-gate → revert-on-red workflow. +- `dotnet-upgrade/migrate-nullable-references`, `dotnet-upgrade/dotnet-aot-compat` — for _adopting_ + nullable/AOT, not preserving an existing surface. +- `dotnet-msbuild/including-generated-files` — MSBuild wiring for generated files. 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 new file mode 100644 index 0000000000..915fdd003a --- /dev/null +++ b/plugins/dotnet/skills/dotnet-breaking-changes/references/internals-visible-to.md @@ -0,0 +1,42 @@ +# 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 new file mode 100644 index 0000000000..c6b736cf52 --- /dev/null +++ b/plugins/dotnet/skills/dotnet-breaking-changes/references/multi-targeting.md @@ -0,0 +1,48 @@ +# 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 new file mode 100644 index 0000000000..2dab2847b6 --- /dev/null +++ b/plugins/dotnet/skills/dotnet-breaking-changes/references/public-api.md @@ -0,0 +1,65 @@ +# 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 new file mode 100644 index 0000000000..d5b5c45b8f --- /dev/null +++ b/plugins/dotnet/skills/dotnet-breaking-changes/references/source-generation.md @@ -0,0 +1,38 @@ +# 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 .*" --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. From 1b3a5bd13db825237d579a4dd9d2c1dfc87d9953 Mon Sep 17 00:00:00 2001 From: Abhitej John Date: Tue, 7 Jul 2026 14:27:27 -0700 Subject: [PATCH 2/8] Add eval tests for csharp-refactoring and dotnet-breaking-changes skills Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/dotnet/csharp-refactoring/Fixture.sln | 24 +++ tests/dotnet/csharp-refactoring/eval.yaml | 180 ++++++++++++++++++ .../src/Billing/AppSettingsHelper.cs | 21 ++ .../src/Billing/Billing.csproj | 24 +++ .../csharp-refactoring/src/Billing/Coupons.cs | 8 + .../src/Billing/Coupons.g.cs.template | 14 ++ .../src/Billing/OrderProcessor.cs | 36 ++++ .../src/Billing/PlatformInfo.cs | 19 ++ .../csharp-refactoring/src/Billing/Pricing.cs | 25 +++ .../src/Billing/PublicAPI.Shipped.txt | 7 + .../tests/Billing.Tests/Billing.Tests.csproj | 20 ++ .../tests/Billing.Tests/BillingTests.cs | 111 +++++++++++ .../dotnet-breaking-changes/Fixture.sln | 24 +++ .../dotnet/dotnet-breaking-changes/eval.yaml | 165 ++++++++++++++++ .../src/Billing/AppSettingsHelper.cs | 21 ++ .../src/Billing/Billing.csproj | 24 +++ .../src/Billing/Coupons.cs | 8 + .../src/Billing/Coupons.g.cs.template | 14 ++ .../src/Billing/OrderProcessor.cs | 36 ++++ .../src/Billing/PlatformInfo.cs | 19 ++ .../src/Billing/Pricing.cs | 25 +++ .../src/Billing/PublicAPI.Shipped.txt | 7 + .../tests/Billing.Tests/Billing.Tests.csproj | 20 ++ .../tests/Billing.Tests/BillingTests.cs | 111 +++++++++++ 24 files changed, 963 insertions(+) create mode 100644 tests/dotnet/csharp-refactoring/Fixture.sln create mode 100644 tests/dotnet/csharp-refactoring/eval.yaml create mode 100644 tests/dotnet/csharp-refactoring/src/Billing/AppSettingsHelper.cs create mode 100644 tests/dotnet/csharp-refactoring/src/Billing/Billing.csproj create mode 100644 tests/dotnet/csharp-refactoring/src/Billing/Coupons.cs create mode 100644 tests/dotnet/csharp-refactoring/src/Billing/Coupons.g.cs.template create mode 100644 tests/dotnet/csharp-refactoring/src/Billing/OrderProcessor.cs create mode 100644 tests/dotnet/csharp-refactoring/src/Billing/PlatformInfo.cs create mode 100644 tests/dotnet/csharp-refactoring/src/Billing/Pricing.cs create mode 100644 tests/dotnet/csharp-refactoring/src/Billing/PublicAPI.Shipped.txt create mode 100644 tests/dotnet/csharp-refactoring/tests/Billing.Tests/Billing.Tests.csproj create mode 100644 tests/dotnet/csharp-refactoring/tests/Billing.Tests/BillingTests.cs create mode 100644 tests/dotnet/dotnet-breaking-changes/Fixture.sln create mode 100644 tests/dotnet/dotnet-breaking-changes/eval.yaml create mode 100644 tests/dotnet/dotnet-breaking-changes/src/Billing/AppSettingsHelper.cs create mode 100644 tests/dotnet/dotnet-breaking-changes/src/Billing/Billing.csproj create mode 100644 tests/dotnet/dotnet-breaking-changes/src/Billing/Coupons.cs create mode 100644 tests/dotnet/dotnet-breaking-changes/src/Billing/Coupons.g.cs.template create mode 100644 tests/dotnet/dotnet-breaking-changes/src/Billing/OrderProcessor.cs create mode 100644 tests/dotnet/dotnet-breaking-changes/src/Billing/PlatformInfo.cs create mode 100644 tests/dotnet/dotnet-breaking-changes/src/Billing/Pricing.cs create mode 100644 tests/dotnet/dotnet-breaking-changes/src/Billing/PublicAPI.Shipped.txt create mode 100644 tests/dotnet/dotnet-breaking-changes/tests/Billing.Tests/Billing.Tests.csproj create mode 100644 tests/dotnet/dotnet-breaking-changes/tests/Billing.Tests/BillingTests.cs diff --git a/tests/dotnet/csharp-refactoring/Fixture.sln b/tests/dotnet/csharp-refactoring/Fixture.sln new file mode 100644 index 0000000000..335eab4979 --- /dev/null +++ b/tests/dotnet/csharp-refactoring/Fixture.sln @@ -0,0 +1,24 @@ +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/csharp-refactoring/eval.yaml b/tests/dotnet/csharp-refactoring/eval.yaml new file mode 100644 index 0000000000..1c542bcee8 --- /dev/null +++ b/tests/dotnet/csharp-refactoring/eval.yaml @@ -0,0 +1,180 @@ +scenarios: + - name: "Rename a method across its declaration and every caller" + prompt: "In the Billing class library, the method `OrderProcessor.DoStuff` is badly named for what it does — it computes an invoice. Rename it to `ComputeInvoice` everywhere it is declared and called, without changing any behavior. The solution is at Fixture.sln." + setup: + copy_test_files: true + assertions: + - type: "file_contains" + path: "src/Billing/OrderProcessor.cs" + value: "ComputeInvoice" + # The rename must propagate to every caller. BillingTests.cs calls the method + # directly, so a correct rename lands the new name here too (and the old call + # would otherwise fail to compile). + - type: "file_contains" + path: "tests/Billing.Tests/BillingTests.cs" + value: "ComputeInvoice" + # Behavior-preservation + full-propagation gate: if any declaration or caller + # was missed, the test project fails to compile and this assertion fails. + - type: run_command_and_assert + command_to_run: "dotnet" + command_arguments: "test Fixture.sln -v:q" + expected_exit_code: 0 + expected_std_output_contains: "Passed!" + command_timeout: 300 + - type: "output_matches" + pattern: "(dotnet (build|test)|rebuild|re-?run.{0,20}tests|tests? .{0,20}(pass|green))" + rubric: + - "Establishes a green build and test baseline before renaming" + - "Renames by tracking true binding references rather than a blind text find-and-replace that would also hit comments, strings, or unrelated test method names" + - "Rebuilds and re-runs the existing tests after the rename to confirm behavior is preserved" + - "Keeps this a single, focused operation without bundling unrelated edits" + timeout: 600 + + - name: "Extract a repeated calculation into a private helper" + prompt: "`OrderProcessor.DoStuff` is too long. Extract the discount-then-tax calculation into a private helper method and call it, without changing what the code computes. The solution is at Fixture.sln." + setup: + copy_test_files: true + assertions: + # Behavior-preservation gate: the extracted helper must compute identical + # results, so the existing tests must still pass. + - type: run_command_and_assert + command_to_run: "dotnet" + command_arguments: "test Fixture.sln -v:q" + expected_exit_code: 0 + expected_std_output_contains: "Passed!" + command_timeout: 300 + # A private helper method now exists (OrderProcessor had none before). + - type: "file_contains" + path: "src/Billing/OrderProcessor.cs" + value: "private" + - type: "output_matches" + pattern: "(dotnet (build|test)|rebuild|re-?run.{0,20}tests|tests? .{0,20}(pass|green))" + rubric: + - "Establishes a green baseline before editing" + - "Extracts the block into a new method preserving the exact same computation and results" + - "Verifies behavior is preserved by building and running the existing tests after extracting" + - "Does not slip a bug fix or behavior change into the extraction" + timeout: 600 + + - name: "Consolidate a duplicated block into a single shared helper" + prompt: "Inside `OrderProcessor.DoStuff` the discount-plus-tax calculation is duplicated (it appears twice, once for the order total and once for the quote). Consolidate the duplicated logic into a single shared helper used by both, without changing behavior. The solution is at Fixture.sln." + setup: + copy_test_files: true + assertions: + # Behavior-preservation gate: the single shared helper must be equivalent to + # both original copies, so the existing tests must still pass. + - type: run_command_and_assert + command_to_run: "dotnet" + command_arguments: "test Fixture.sln -v:q" + expected_exit_code: 0 + expected_std_output_contains: "Passed!" + command_timeout: 300 + # A shared private helper now exists (OrderProcessor had none before). + - type: "file_contains" + path: "src/Billing/OrderProcessor.cs" + value: "private" + - type: "output_matches" + pattern: "(dotnet (build|test)|rebuild|re-?run.{0,20}tests|tests? .{0,20}(pass|green))" + rubric: + - "Establishes a green baseline before editing" + - "Factors the duplicated block into one shared helper that both call sites use" + - "Confirms the consolidated helper is semantically equivalent to each original copy" + - "Re-runs the build and tests after consolidating to prove behavior is unchanged" + timeout: 600 + + - name: "Inline a pass-through wrapper and update callers" + prompt: "`LegacyTax.ApplyTaxWrapper` does nothing but forward to `TaxRules.Apply`. Inline it: update every caller to call `TaxRules.Apply` directly and remove the wrapper, without changing behavior. The solution is at Fixture.sln." + setup: + copy_test_files: true + assertions: + # Behavior-preservation + full-propagation gate: every caller (including the + # test that references the wrapper) must be updated to the underlying call, + # or the test project fails to compile. + - type: run_command_and_assert + command_to_run: "dotnet" + command_arguments: "test Fixture.sln -v:q" + expected_exit_code: 0 + expected_std_output_contains: "Passed!" + command_timeout: 300 + # The pass-through wrapper is actually gone. + - type: "file_not_contains" + path: "src/Billing/Pricing.cs" + value: "ApplyTaxWrapper" + - type: "output_matches" + pattern: "(dotnet (build|test)|rebuild|re-?run.{0,20}tests|tests? .{0,20}(pass|green))" + rubric: + - "Finds the true binding references to the wrapper before removing it, not just textual matches" + - "Updates every caller to the underlying call and removes the wrapper in one focused operation" + - "Lets the compiler catch any missed reference, then rebuilds and re-runs the tests to confirm green" + timeout: 600 + + - name: "Merge two near-identical types into one parameterized type" + prompt: "`GoldPricing` and `SilverPricing` are near-identical. Consolidate them into a single parameterized pricing type and update any usages, without changing behavior. The solution is at Fixture.sln." + setup: + copy_test_files: true + assertions: + # Behavior-preservation + full-propagation gate: usages (including the test + # that constructs GoldPricing/SilverPricing) must be updated to the merged + # type, or the test project fails to compile. + - type: run_command_and_assert + command_to_run: "dotnet" + command_arguments: "test Fixture.sln -v:q" + expected_exit_code: 0 + expected_std_output_contains: "Passed!" + command_timeout: 300 + # The two near-identical classes are actually gone (merged into one type). + - type: "file_not_contains" + path: "src/Billing/Pricing.cs" + value: "class GoldPricing" + - type: "file_not_contains" + path: "src/Billing/Pricing.cs" + value: "class SilverPricing" + - type: "output_matches" + pattern: "(dotnet (build|test)|rebuild|re-?run.{0,20}tests|tests? .{0,20}(pass|green))" + rubric: + - "Establishes a green baseline before editing" + - "Merges the two classes into a single type parameterized by name and rate, preserving each tier's values" + - "Updates all usages and re-runs the build and tests to prove behavior is unchanged" + timeout: 600 + + - name: "Decline a framework and package upgrade dressed up as a refactor" + prompt: "Upgrade the Billing project to a newer target framework and bump all its NuGet packages to their latest major versions. The solution is at Fixture.sln." + expect_activation: false + setup: + copy_test_files: true + assertions: + - type: "output_matches" + pattern: "(not a .{0,20}refactor|behavior[- ]?(preserving|chang)|upgrade|major version|out of scope)" + rubric: + - "Recognizes that a framework or package major-version upgrade is not a behavior-preserving refactor" + - "Declines or redirects to the appropriate upgrade workflow instead of treating it as a refactor" + - "Does not silently perform a behavior-changing version bump under the guise of refactoring" + timeout: 300 + + - name: "Decline a new-feature request dressed up as a refactor" + prompt: "Add a new \"platinum\" loyalty tier at 15% off and wire it into the pricing tiers so orders can use it. The solution is at Fixture.sln." + expect_activation: false + setup: + copy_test_files: true + assertions: + - type: "output_matches" + pattern: "(new feature|feature addition|not a .{0,20}refactor|behavior[- ]?(preserving|chang)|adds? new behavior)" + rubric: + - "Recognizes that adding a new pricing tier is a new feature, not a behavior-preserving refactor" + - "Distinguishes the feature request from a refactor rather than conflating them, and does not claim behavior is preserved when it is deliberately being extended" + - "If it proceeds, does so as an explicit feature addition with its own new tests, not under the guise of a no-behavior-change refactor" + timeout: 300 + + - name: "Keep behavior-changing bug fixes out of a behavior-preserving refactor" + prompt: "Fix the known bugs first and keep any refactors in a separate follow-up PR. Do not mix behavior-changing bug fixes with refactoring under a behavior-preserving contract. The solution is at Fixture.sln." + expect_activation: false + setup: + copy_test_files: true + assertions: + - type: "output_matches" + pattern: "(separate|follow[- ]?up|bug fix|behavior[- ]?(preserving|chang)|not (a )?refactor)" + rubric: + - "Recognizes that a behavior-changing bug fix is distinct from a behavior-preserving refactor" + - "Keeps the bug fix separate from any refactoring rather than bundling them under one behavior-preserving contract" + - "Does not claim behavior is preserved for a change that deliberately alters behavior" + timeout: 300 diff --git a/tests/dotnet/csharp-refactoring/src/Billing/AppSettingsHelper.cs b/tests/dotnet/csharp-refactoring/src/Billing/AppSettingsHelper.cs new file mode 100644 index 0000000000..f8923ed87a --- /dev/null +++ b/tests/dotnet/csharp-refactoring/src/Billing/AppSettingsHelper.cs @@ -0,0 +1,21 @@ +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/csharp-refactoring/src/Billing/Billing.csproj b/tests/dotnet/csharp-refactoring/src/Billing/Billing.csproj new file mode 100644 index 0000000000..50c11dbf51 --- /dev/null +++ b/tests/dotnet/csharp-refactoring/src/Billing/Billing.csproj @@ -0,0 +1,24 @@ + + + + net8.0;net10.0 + enable + enable + latest + true + + + + + + + + + + + + + + diff --git a/tests/dotnet/csharp-refactoring/src/Billing/Coupons.cs b/tests/dotnet/csharp-refactoring/src/Billing/Coupons.cs new file mode 100644 index 0000000000..79ceb96d37 --- /dev/null +++ b/tests/dotnet/csharp-refactoring/src/Billing/Coupons.cs @@ -0,0 +1,8 @@ +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/csharp-refactoring/src/Billing/Coupons.g.cs.template b/tests/dotnet/csharp-refactoring/src/Billing/Coupons.g.cs.template new file mode 100644 index 0000000000..d247c331f7 --- /dev/null +++ b/tests/dotnet/csharp-refactoring/src/Billing/Coupons.g.cs.template @@ -0,0 +1,14 @@ +// +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/csharp-refactoring/src/Billing/OrderProcessor.cs b/tests/dotnet/csharp-refactoring/src/Billing/OrderProcessor.cs new file mode 100644 index 0000000000..78ac5748a1 --- /dev/null +++ b/tests/dotnet/csharp-refactoring/src/Billing/OrderProcessor.cs @@ -0,0 +1,36 @@ +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/csharp-refactoring/src/Billing/PlatformInfo.cs b/tests/dotnet/csharp-refactoring/src/Billing/PlatformInfo.cs new file mode 100644 index 0000000000..62ab629acf --- /dev/null +++ b/tests/dotnet/csharp-refactoring/src/Billing/PlatformInfo.cs @@ -0,0 +1,19 @@ +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/csharp-refactoring/src/Billing/Pricing.cs b/tests/dotnet/csharp-refactoring/src/Billing/Pricing.cs new file mode 100644 index 0000000000..02b4d6761c --- /dev/null +++ b/tests/dotnet/csharp-refactoring/src/Billing/Pricing.cs @@ -0,0 +1,25 @@ +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/csharp-refactoring/src/Billing/PublicAPI.Shipped.txt b/tests/dotnet/csharp-refactoring/src/Billing/PublicAPI.Shipped.txt new file mode 100644 index 0000000000..80f5318ff5 --- /dev/null +++ b/tests/dotnet/csharp-refactoring/src/Billing/PublicAPI.Shipped.txt @@ -0,0 +1,7 @@ +#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/csharp-refactoring/tests/Billing.Tests/Billing.Tests.csproj b/tests/dotnet/csharp-refactoring/tests/Billing.Tests/Billing.Tests.csproj new file mode 100644 index 0000000000..9a83c317dd --- /dev/null +++ b/tests/dotnet/csharp-refactoring/tests/Billing.Tests/Billing.Tests.csproj @@ -0,0 +1,20 @@ + + + + net10.0 + enable + enable + false + + + + + + + + + + + + + diff --git a/tests/dotnet/csharp-refactoring/tests/Billing.Tests/BillingTests.cs b/tests/dotnet/csharp-refactoring/tests/Billing.Tests/BillingTests.cs new file mode 100644 index 0000000000..5a7f5d4f1f --- /dev/null +++ b/tests/dotnet/csharp-refactoring/tests/Billing.Tests/BillingTests.cs @@ -0,0 +1,111 @@ +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)); + } +} diff --git a/tests/dotnet/dotnet-breaking-changes/Fixture.sln b/tests/dotnet/dotnet-breaking-changes/Fixture.sln new file mode 100644 index 0000000000..335eab4979 --- /dev/null +++ b/tests/dotnet/dotnet-breaking-changes/Fixture.sln @@ -0,0 +1,24 @@ +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 new file mode 100644 index 0000000000..7a0a58aa13 --- /dev/null +++ b/tests/dotnet/dotnet-breaking-changes/eval.yaml @@ -0,0 +1,165 @@ +scenarios: + - 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." + setup: + copy_test_files: true + assertions: + - type: "output_matches" + pattern: "(public API|PublicAPI\\.Shipped|breaking change|shipped)" + - type: "file_contains" + 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_and_assert + command_to_run: "dotnet" + command_arguments: "test Fixture.sln -v:q" + expected_exit_code: 0 + expected_std_output_contains: "Passed!" + command_timeout: 300 + 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" + timeout: 600 + + - 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." + setup: + copy_test_files: true + assertions: + - type: "output_matches" + pattern: "(public API|PublicAPI|breaking change|shipped|signature|overload)" + - type: "file_contains" + 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_and_assert + command_to_run: "dotnet" + command_arguments: "test Fixture.sln -v:q" + expected_exit_code: 0 + expected_std_output_contains: "Passed!" + command_timeout: 300 + 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" + timeout: 600 + + - 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." + setup: + copy_test_files: true + assertions: + - type: "file_contains" + 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_and_assert + command_to_run: "dotnet" + command_arguments: "build Fixture.sln -v:q" + expected_exit_code: 0 + command_timeout: 300 + # Behavior-preservation gate on the built target. + - type: run_command_and_assert + command_to_run: "dotnet" + command_arguments: "test Fixture.sln -v:q" + expected_exit_code: 0 + expected_std_output_contains: "Passed!" + command_timeout: 300 + - type: "output_matches" + pattern: "(multi[- ]?target|#if|net8|net10|target framework|TFM|every (branch|target))" + 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" + timeout: 600 + + - 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." + setup: + copy_test_files: true + assertions: + - type: "file_contains" + 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" + 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_and_assert + command_to_run: "dotnet" + command_arguments: "test Fixture.sln -v:q" + expected_exit_code: 0 + expected_std_output_contains: "Passed!" + command_timeout: 300 + - type: "output_matches" + pattern: "(auto[- ]?generated|source generator|generated (file|output|code)|partial|template|\\.g\\.cs|regenerat)" + 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" + timeout: 600 + + - 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." + setup: + copy_test_files: true + assertions: + - type: "file_contains" + 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_and_assert + command_to_run: "dotnet" + command_arguments: "build Fixture.sln -v:q" + expected_exit_code: 0 + command_timeout: 300 + # Existing behavior stays green after the additive change. + - type: run_command_and_assert + command_to_run: "dotnet" + command_arguments: "test Fixture.sln -v:q" + expected_exit_code: 0 + expected_std_output_contains: "Passed!" + command_timeout: 300 + - type: "output_matches" + pattern: "(multi[- ]?target|#if|net8|net10|target framework|TFM|public API|PublicAPI)" + 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" + timeout: 600 + + - 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." + expect_activation: false + setup: + copy_test_files: true + assertions: + - type: "file_contains" + path: "src/Billing/OrderProcessor.cs" + value: "runningTotal" + # The local rename is behavior-identical: tests must still pass unchanged. + - type: run_command_and_assert + command_to_run: "dotnet" + command_arguments: "test Fixture.sln -v:q" + expected_exit_code: 0 + expected_std_output_contains: "Passed!" + command_timeout: 300 + 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" + timeout: 300 diff --git a/tests/dotnet/dotnet-breaking-changes/src/Billing/AppSettingsHelper.cs b/tests/dotnet/dotnet-breaking-changes/src/Billing/AppSettingsHelper.cs new file mode 100644 index 0000000000..f8923ed87a --- /dev/null +++ b/tests/dotnet/dotnet-breaking-changes/src/Billing/AppSettingsHelper.cs @@ -0,0 +1,21 @@ +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 new file mode 100644 index 0000000000..50c11dbf51 --- /dev/null +++ b/tests/dotnet/dotnet-breaking-changes/src/Billing/Billing.csproj @@ -0,0 +1,24 @@ + + + + net8.0;net10.0 + enable + enable + latest + true + + + + + + + + + + + + + + diff --git a/tests/dotnet/dotnet-breaking-changes/src/Billing/Coupons.cs b/tests/dotnet/dotnet-breaking-changes/src/Billing/Coupons.cs new file mode 100644 index 0000000000..79ceb96d37 --- /dev/null +++ b/tests/dotnet/dotnet-breaking-changes/src/Billing/Coupons.cs @@ -0,0 +1,8 @@ +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 new file mode 100644 index 0000000000..d247c331f7 --- /dev/null +++ b/tests/dotnet/dotnet-breaking-changes/src/Billing/Coupons.g.cs.template @@ -0,0 +1,14 @@ +// +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 new file mode 100644 index 0000000000..78ac5748a1 --- /dev/null +++ b/tests/dotnet/dotnet-breaking-changes/src/Billing/OrderProcessor.cs @@ -0,0 +1,36 @@ +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 new file mode 100644 index 0000000000..62ab629acf --- /dev/null +++ b/tests/dotnet/dotnet-breaking-changes/src/Billing/PlatformInfo.cs @@ -0,0 +1,19 @@ +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 new file mode 100644 index 0000000000..02b4d6761c --- /dev/null +++ b/tests/dotnet/dotnet-breaking-changes/src/Billing/Pricing.cs @@ -0,0 +1,25 @@ +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 new file mode 100644 index 0000000000..80f5318ff5 --- /dev/null +++ b/tests/dotnet/dotnet-breaking-changes/src/Billing/PublicAPI.Shipped.txt @@ -0,0 +1,7 @@ +#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 new file mode 100644 index 0000000000..9a83c317dd --- /dev/null +++ b/tests/dotnet/dotnet-breaking-changes/tests/Billing.Tests/Billing.Tests.csproj @@ -0,0 +1,20 @@ + + + + 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 new file mode 100644 index 0000000000..5a7f5d4f1f --- /dev/null +++ b/tests/dotnet/dotnet-breaking-changes/tests/Billing.Tests/BillingTests.cs @@ -0,0 +1,111 @@ +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)); + } +} From 47983f45e6d890546b948bb071de1d5a29b11f9e Mon Sep 17 00:00:00 2001 From: "Wendy Breiding (She/Her)" Date: Wed, 8 Jul 2026 13:49:46 -0700 Subject: [PATCH 3/8] minor changes to description to increase calling chance. --- plugins/dotnet/skills/csharp-refactoring/SKILL.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/dotnet/skills/csharp-refactoring/SKILL.md b/plugins/dotnet/skills/csharp-refactoring/SKILL.md index bd51987157..30b6aa6dee 100644 --- a/plugins/dotnet/skills/csharp-refactoring/SKILL.md +++ b/plugins/dotnet/skills/csharp-refactoring/SKILL.md @@ -1,6 +1,6 @@ --- name: csharp-refactoring -description: "Performs safe, behavior-preserving refactoring of C#/.NET code, verified with build, tests, and analyzers. Use when the user wants to refactor, rename, restructure, clean up, modernize, or reorganize C# code WITHOUT changing behavior: rename a symbol/type/file across a solution; move a type or static members to another file/namespace/project; extract a method, interface, or base class; pull members up; inline a method or local; split a large class/file; consolidate or de-duplicate copy-pasted code; sync namespaces to folders; modernize to current C# idioms (file-scoped namespaces, primary constructors, collection expressions, target-typed new, pattern matching); or enable nullable reference annotations. Prefers Roslyn-backed edits over text find/replace, and defers .NET compatibility hazards to the companion dotnet-breaking-changes skill. DO NOT USE FOR: adding features, fixing bugs, writing new tests, upgrading frameworks or NuGet versions (use dotnet-upgrade), or formatting-only passes (use dotnet format)." +description: "Performs safe, behavior-preserving refactoring of C#/.NET code, verified with build, tests, and analyzers. USE FOR: any request to rename, move, extract, split, modernize, or otherwise restructure C# code without changing behavior, including small requests like 'rename X to Y': rename a symbol/type/file across a solution; move a type or static members to another file/namespace/project; extract a method, interface, or base class; pull members up; inline a method or local; split a large class/file; consolidate or de-duplicate copy-pasted code; sync namespaces to folders; modernize to current C# idioms (file-scoped namespaces, primary constructors, collection expressions, target-typed new, pattern matching); or enable nullable reference annotations. DO NOT USE FOR: adding features, fixing bugs, writing new tests, upgrading frameworks or NuGet versions (use dotnet-upgrade), or formatting-only passes (use dotnet format)." license: MIT --- @@ -92,6 +92,7 @@ Halt and ask the user before continuing when: ## When to use - "Rename this method/type everywhere safely" / "rename across the solution" +- Short requests such as "rename X to Y" or "rename this class" - "Extract this block into a method" / "extract an interface from this class" - "Move this type into its own file / into project X / into namespace Y" - "This class is too big — split it" / "consolidate these two near-identical implementations" From df61fcedaea9f523360b2c061b2ef59437622216 Mon Sep 17 00:00:00 2001 From: Abhitej John Date: Tue, 21 Jul 2026 00:35:22 -0700 Subject: [PATCH 4/8] Trim csharp-refactoring SKILL.md to lean v3 (proportional rigor) Replaces the shipped skill body with the validated v3 variant: judgment-first, rigor proportional to blast radius, redundant catalog/list scaffolding removed. ~53% smaller (12,811->6,039 chars) with equal or better cross-family eval quality and no measured regression. Description (929 chars) and all safety rules retained. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3c9f9823-7f2f-4f7d-9d1b-f2b9e7a20c60 --- .../dotnet/skills/csharp-refactoring/SKILL.md | 258 +++++------------- 1 file changed, 75 insertions(+), 183 deletions(-) diff --git a/plugins/dotnet/skills/csharp-refactoring/SKILL.md b/plugins/dotnet/skills/csharp-refactoring/SKILL.md index 30b6aa6dee..f8bc876f58 100644 --- a/plugins/dotnet/skills/csharp-refactoring/SKILL.md +++ b/plugins/dotnet/skills/csharp-refactoring/SKILL.md @@ -6,188 +6,80 @@ license: MIT # C# Refactoring (behavior-preserving) -Refactor C#/.NET code so the structure improves but the observable behavior does **not** change. Every -refactor is a sequence of small, named, Roslyn-aware transformations, each followed by a build/test -gate. If the gate fails, stop and revert — a refactor that changes behavior is a bug, not a refactor. - -## Operation catalog (what "refactoring" means in C#) - -The canonical C# refactoring operations, aligned with Roslyn's IDE refactoring providers: - -- **Rename** a symbol / type / file -- **Move** a type / member / file (to another file, namespace, or project) -- **Consolidate / de-duplicate** copy-pasted code -- **Modernize / simplify** to current C# idioms (the repo's analyzers decide the idiom, not taste) -- **Split** a large class / file / assembly -- **Extract** a method / class / interface -- **Enable nullable** reference annotations -- **Inline** a method / local / constant -- **Pull up / push down** a member -- **Sync namespace** to folder - -Each is a single named operation; compose them one step at a time (see Procedure). For the Roslyn -provider mapping and representative real PRs, see +A refactor changes **structure**, never observable **behavior**. Do the edit with binding-aware tools, +then confirm behavior held with a build + the relevant tests. Keep the effort proportional to the change: +a one-line local rename does not need the ceremony a public multi-targeted change does. + +## First, is this actually a refactor? + +The most valuable thing this skill does is *not* restructure code you were told to restructure — it is +catching a request that is **not** behavior-preserving before you run it through a refactor's contract. +When the request changes results, decline the refactor framing and handle it honestly: + +- **Framework / NuGet version bump** → not a refactor; redirect to the `dotnet-upgrade` skills. +- **New feature** (e.g. add a pricing tier, a flag, an endpoint) → a feature, not a refactor. If asked, + build it as a feature *with its own tests*; don't claim behavior is preserved. +- **Bug fix or "simplification" that changes output** (e.g. always charge shipping, bump a discount) → + a behavior **change**. It is a legitimate task — do it as an explicit, tested change and update the + tests that lock in the new behavior — but keep it **separate** from any refactor and never label it + behavior-preserving. Do not stall or report "nothing to change." +- **A rename/move with a behavior tweak smuggled in** ("rename X, and while you're there bump the rate") + → split it: do the rename as a behavior-preserving refactor, and treat the tweak as its own tested + change, or flag it and defer. + +Only when the request is genuinely structure-only do you proceed as a refactor. + +## Rename / move by bindings, not text + +The #1 way a "rename" silently corrupts code is editing textual matches (comments, strings, unrelated +overloads) instead of real **bindings**. Find every binding reference first, then edit semantically. Use +the strongest tool available: an IDE/Roslyn workspace refactoring, then the C# LSP the +[`dotnet` plugin declares](https://github.com/dotnet/skills/blob/main/plugins/dotnet/lsp.json) +(`findReferences`, `goToDefinition`, `incomingCalls`, `rename` code action), then analyzer code-fixes / +Roslynator, then compiler-validated edits (edit the true bindings, rebuild, let the compiler flag misses). +Plain find/replace only when scope is provably tiny and every hit is verified. Include **every** `partial` +declaration, and edit the generator input, never generated (`*.g.cs`) output. + +For the operation → Roslyn-provider mapping and representative PRs, see [references/operation-catalog.md](references/operation-catalog.md). -## Tooling — C# LSP (Roslyn language server) - -A headless CLI agent usually can't invoke IDE code actions, but it **can** get Roslyn-quality -_semantic navigation_ from the C# language server the [`dotnet` plugin in `dotnet/skills`](https://github.com/dotnet/skills/blob/main/plugins/dotnet/lsp.json) -declares. It launches through the .NET CLI (`dnx roslyn-language-server --yes --prerelease -- --stdio ---autoLoadProjects`) over `.cs`, `.razor`, and `.cshtml` files (prerequisite: a .NET 10 SDK with -`dotnet` on `PATH`). - -When the LSP is available, prefer its binding-aware operations over textual grep/glob for every -reference-finding step in a refactor: - -- Where a symbol is defined → **goToDefinition** -- All _binding_ references to a symbol → **findReferences** (the reliable rename/move safety net) -- What calls a method → **incomingCalls** -- Find symbols by name across the workspace → **workspaceSymbol** -- A symbol's type / signature / docs → **hover** - -These are exact, binding-aware answers — unlike grep they don't match comments, string literals, or -unrelated overloads. Reach for the LSP first; fall back to grep only when it is unavailable. - -## .NET compatibility hazards — inspect first, then load `dotnet-breaking-changes` - -Behavior-preserving edits fail in _.NET-specific_ ways a green test run won't catch. Before you edit, -**search the repo** for the surfaces that govern the symbol — don't assume or recite: - -- Public-API gate: `PublicAPI.Shipped/Unshipped.txt` (PublicApiAnalyzers) and/or `ApiCompat` / - `` — these are _not_ interchangeable. -- `` and `#if` / platform branches. -- `partial` declarations and generated (`*.g.cs`) files. -- `InternalsVisibleTo` friend/test assemblies. - -The four bullets above (and the reminders below) are the **floor**: apply them even if the guide skill -is not installed. When it _is_ available, **load the `dotnet-breaking-changes` skill** for the full -per-surface playbook (it applies to any change, not just refactors, and holds the depth on each -surface); it _adds_ compatibility analysis but does **not** replace this skill's safety contract. -Refactor-specific reminders: move a public type across assemblies via a `[TypeForwardedTo]` forwarder -(a _rename_ needs an `[Obsolete]` shim, not a forwarder); include _every_ `partial` declaration in a -rename/move; edit the generator input, not generated output; and treat friend-assembly `internal` -members with public-API care. - -## Stop and escalate (do not silently proceed) - -Halt and ask the user before continuing when: - -- The **baseline is red** (build or tests already failing) — you can't prove you preserved behavior. -- A **public/shipped API** would change and you cannot verify compatibility (no type-forwarder/shim - path, and no PublicApiAnalyzers/ApiCompat/package-validation gate available to catch a break). -- The request is **not actually behavior-preserving** — it asks you to upgrade a framework/package, - add a feature, or fix a bug. Do **not** carry it out under this skill, and **never label such a - change "behavior-preserving."** Say it is out of scope and redirect (upgrades → `dotnet-upgrade`; - features/fixes → normal dev flow). If a refactor is genuinely a prerequisite, do only that, as a - separate step, and stop. -- The edit touches **generated, designer, or migration files** (`*.g.cs`, `*.Designer.cs`, EF - migrations, source-generator output) — hand-edits there are **overwritten on the next build**; - change the source of generation (template/generator input), not the output. -- Semantic equivalence depends on **runtime behavior not covered by tests** (reflection, DI wiring, - serialization, `dynamic`, P/Invoke) — flag the risk; tests alone won't catch a regression. -- The operation can't be done with any tool on the fallback ladder and would require a wide, - unverifiable text replace. - -## When to use - -- "Rename this method/type everywhere safely" / "rename across the solution" -- Short requests such as "rename X to Y" or "rename this class" -- "Extract this block into a method" / "extract an interface from this class" -- "Move this type into its own file / into project X / into namespace Y" -- "This class is too big — split it" / "consolidate these two near-identical implementations" -- "Modernize this file to current C#" / "use file-scoped namespaces and primary constructors" -- "Turn on nullable annotations for this project and fix the warnings" -- Any request to **restructure / clean up / reorganize** code while keeping behavior identical - -## When NOT to use - -- The change is meant to alter behavior, add a feature, or fix a bug (not a refactor) -- Framework/SDK/NuGet version upgrades → `dotnet-upgrade` -- Pure formatting/whitespace → `dotnet format` -- Writing brand-new tests → `writing-mstest-tests` / `code-testing-agent` - -## Procedure (the safety contract) - -1. **Establish a green baseline.** Build the affected projects and run the relevant tests. Never - refactor on a red baseline — you won't be able to tell what you broke. -2. **Pick ONE operation from the catalog.** Refactors compose, but each step is a single named - operation. Never mix a refactor with a behavior change in the same step. -3. **Find the true references first (for rename/move/inline).** Before editing, locate every _binding_ - reference to the symbol — not textual matches. Prefer the C# LSP: **findReferences** / - **goToDefinition** / **incomingCalls** (see **Tooling**) return only true bindings. If the LSP is - unavailable, a headless grep finds candidates but also false positives (test method names, comments, - strings, unrelated overloads); treat grep as a _candidate list_, then keep only true bindings and let - the compiler catch any you missed. Skipping this is the #1 way a "rename" silently corrupts code. -4. **Prefer semantics-aware edits over text edits.** The principle is _semantic verification_, not a - specific API — use the strongest tool actually available, in this order (fallback ladder): - 1. An IDE / Roslyn workspace refactoring (rename, move, extract, inline, pull-up) when available — - updates all references, `using`s, and partial declarations correctly. - 2. **The C# LSP (Roslyn language server) from `dotnet/skills`** (see **Tooling**) for a headless - agent: drive edits from **findReferences** / **goToDefinition** / **incomingCalls** / - **workspaceSymbol** so every touched reference is a true binding, not a textual guess. Some - language-server builds also expose a `textDocument/rename` code action that rewrites all - references at once — use it when present. - 3. Analyzer code-fixes / `dotnet format analyzers` / **Roslynator** for idiom modernization and - de-duplication, when installed. - 4. **Compiler-validated, reference-tracked edits** (the common headless fallback): use the LSP (or - grep, if no LSP) to enumerate candidates → edit only the true binding references from step 3 → - rebuild. The compiler is your safety net: any missed or wrongly-edited reference becomes a build - error, not a silent bug. - 5. Plain find/replace ONLY when scope is provably tiny and every reference is verified — it - silently corrupts strings, comments, and unrelated overloads. - A CLI agent often won't have a loaded MSBuildWorkspace, but it **can** load the C# LSP above; prefer - its semantic answers over grep, and never skip the build/test gate to compensate. -5. **Re-gate after every step — across every target framework.** Rebuild + re-run tests; if red, revert - this step and reassess. The diff must be behavior-neutral: tests still green, no new warnings (nullable - work is the deliberate exception), no new analyzer/API-compat diagnostics, and the intended public - contract unchanged. On a multi-targeted project, build/test **each** TFM — a green default build can - still be broken on another target. - - ```bash - dotnet build # 0 errors, on every TargetFramework (add -f to check one) - dotnet test # must stay green; same pass count as baseline - # if the gate fails, revert THIS step and reassess: - git restore . # or: git checkout -- - ``` -6. **Keep the relevant contract stable — and which contract depends on the project type:** - - **Library / shipped package:** preserve the .NET public API; move public types across assembly - boundaries via type-forwarders or `[Obsolete]` shims, not breaking moves. - - **Application / service:** preserve the _external_ contract (HTTP routes, config keys, DB schema, - CLI args); internal type surface can move freely. - - **Small / private codebase:** preserve behavior + tests; commit granularity can relax. -7. **One refactor per commit** for libraries/large repos (keeps review + `git bisect` meaningful); - relax for small private codebases. - -## Inputs - -- Target scope: a symbol, file, type, project, or directory. -- The operation (from the catalog) — or infer it from the request. -- How to build and test the affected projects (solution/proj path, test command). - -## Outputs - -- The applied refactoring as a minimal, behavior-preserving diff. -- Proof of safety: before/after build + test results. -- A short rationale naming the operation(s) applied. - -## Anti-patterns to avoid - -- Renaming via text replace (hits strings, comments, unrelated overloads). -- "Refactor + small fix" in one step (a behavior change masquerading as a refactor). -- Moving a public type without a forwarder/shim (silent breaking change) — or refactoring a public - surface with no PublicApiAnalyzers/ApiCompat gate to catch a break. -- Editing only the TFM/`#if` branch your editor shows, leaving other targets broken. -- Renaming a `partial` type without its other declarations, or editing generated (`*.g.cs`) output - instead of the generator input. -- Skipping the test gate "because it's just a rename." - -## Reference Files - -- **[references/operation-catalog.md](references/operation-catalog.md)** — the full operation taxonomy - with Roslyn providers and representative real PRs. - **Load when** you need the provider for an operation or more detail on the catalog. - -For .NET compatibility hazards (public API, multi-targeting/`#if`, source-generated/partial code, -`InternalsVisibleTo`), load the companion `dotnet-breaking-changes` skill and its `references/` — it -holds the depth on each surface and applies to any change, not just refactors. +## Verify proportionally + +Confirm behavior is preserved after the edit — scaled to blast radius, not a fixed ceremony: + +- **Local / private** (method-local or `private` member, one file, single target framework, no public + surface, no `partial`/generated/`#if`): build once and run the **relevant** tests once after the edit. + If the tree is already known-green, don't burn a second full "before" baseline — rely on the post-edit + gate. Let the compiler catch missed references. +- **Cross-boundary** (public/shipped symbol, multi-targeted project, `#if`/platform branches, or + `partial`/generated code): build/test **each** target framework (a green default build can hide a break + on another TFM), and run the hazards check below. + +Use the repo's own build/test workflow when it documents one (`README`/`CONTRIBUTING`, `build.*`, `eng/`, +`global.json`, `.github/workflows`); its instructions win over any generic command. Otherwise: + +```bash +dotnet build # 0 errors +dotnet test # stays green; same pass count as before +git restore . # if the gate fails, revert THIS step and reassess +``` + +One operation per step; never mix a refactor and a behavior change in the same step. On red, revert — a +refactor that changes behavior is a bug, not a refactor. + +## Cross-boundary hazards (only when it touches a boundary) + +If — and only if — the change touches a **public** symbol, a **multi-targeted** project, or +`partial`/generated code, some breaks won't show up as a failing test. Search the repo for the surface +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. + +## Stop and ask when + +- The baseline is already red (you can't prove you preserved behavior). +- A public/shipped API would change and there is no forwarder/shim path and no analyzer/ApiCompat gate. +- Equivalence depends on runtime behavior tests don't cover (reflection, DI, serialization, `dynamic`, + P/Invoke) — flag it. From ac6bc658d4008749304f5745862864ad593802a2 Mon Sep 17 00:00:00 2001 From: Abhitej John Date: Tue, 21 Jul 2026 23:10:38 -0700 Subject: [PATCH 5/8] Trim dotnet-breaking-changes SKILL.md to lean v3 (proportional rigor) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3c9f9823-7f2f-4f7d-9d1b-f2b9e7a20c60 --- .../skills/dotnet-breaking-changes/SKILL.md | 129 +++++++----------- 1 file changed, 49 insertions(+), 80 deletions(-) diff --git a/plugins/dotnet/skills/dotnet-breaking-changes/SKILL.md b/plugins/dotnet/skills/dotnet-breaking-changes/SKILL.md index ed62e6324b..a84d4e53c5 100644 --- a/plugins/dotnet/skills/dotnet-breaking-changes/SKILL.md +++ b/plugins/dotnet/skills/dotnet-breaking-changes/SKILL.md @@ -18,30 +18,27 @@ 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 you are -looking at**. A change that compiles locally and keeps tests green can still break a downstream -consumer, another target framework, a friend assembly, or regenerate away on the next build. This -skill names the four hidden surfaces, tells you how to find which ones a repo actually uses, and -points to a reference file for each. It is deliberately _not_ refactoring-specific — the same -hazards apply to a feature or a bug fix, and to **additions** (a new public member, overload, or -target framework) as much as to removals: adding surface creates a permanent contract obligation, -and adding behavior to a multi-targeted or partial type must stay correct on every target and -survive the next regeneration. For the behavior-preserving refactoring **process** -(green baseline → one named op → re-gate), use the `csharp-refactoring` skill; this skill is the -compatibility knowledge it (and any other change) taps into. +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 -Do not recite these checks or assume; **search the repo and adapt to what exists.** The markers -present dictate the validation plan; markers absent tell you a surface is not in play. +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\|Microsoft.DotNet.ApiCompat" --include=*.props --include=*.targets --include=*.csproj . +# 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 . # NET*, platform, and custom symbols +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 @@ -50,80 +47,52 @@ grep -rn "InternalsVisibleTo" --include=*.cs --include=*.csproj --include=*.prop Then read only the reference(s) for the surfaces you actually found. -## The four hidden surfaces (summary — depth in references/) +## 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 surface.** In a shipped library, 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 different, non- - interchangeable ways**: source-level (`PublicApiAnalyzers` + `PublicAPI.*.txt`, which you - maintain) and binary/package-level (`ApiCompat` / ``). Run/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. 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`. +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. +- 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 only be made consistent across **some but not all** target frameworks or platforms. +- 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. -## When to use - -- "Is this a breaking change?" / "Will this break the public API / the NuGet package?" -- Editing, renaming, or removing a public or `internal` member in a library -- Changing a signature, nullability, generic constraint, or trimming/AOT attribute -- Editing a multi-targeted project, code under `#if`, or a platform-specific branch -- Touching a `partial` type or source-generated code - -## When NOT to use - -- Framework/SDK/NuGet **version upgrades** → the `dotnet-upgrade` plugin (`migrate-*`, - `dotnet-aot-compat`, `migrate-nullable-references`) -- Pure formatting/whitespace → `dotnet format` -- A single-target private app with no public or cross-assembly surface (no hidden contract to break) +## Reference files -## 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. **Load when** editing a public/`internal` member in a library, changing a - signature/nullability/attribute, or asked whether something is a breaking change. -- **[references/multi-targeting.md](references/multi-targeting.md)** — TFMs, `#if` and platform - symbols, RIDs, and how to inspect and re-gate every target. **Load when** the project has - `` or the code uses `#if`. -- **[references/source-generation.md](references/source-generation.md)** — partial types, generated - output vs checked-in source, editing the generator input. **Load when** the symbol is `partial` - or lives in `*.g.cs`/generated files. (See also `dotnet-msbuild/including-generated-files`.) -- **[references/internals-visible-to.md](references/internals-visible-to.md)** — friend/test - assemblies and strong-name keys. **Load when** the repo has `InternalsVisibleTo` and you touch an - `internal` member. - -## Related skills - -- `csharp-refactoring` — the behavior-preserving refactoring process that consumes this knowledge. - This guide _adds_ compatibility analysis; it does **not** replace that skill's green-baseline → one - named op → re-gate → revert-on-red workflow. -- `dotnet-upgrade/migrate-nullable-references`, `dotnet-upgrade/dotnet-aot-compat` — for _adopting_ - nullable/AOT, not preserving an existing surface. -- `dotnet-msbuild/including-generated-files` — MSBuild wiring for generated files. + 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. From 4f8ac74f7494691ba1ac63245036ca373e2957ce Mon Sep 17 00:00:00 2001 From: Abhitej John Date: Thu, 23 Jul 2026 00:09:35 -0700 Subject: [PATCH 6/8] Drop LangVersion=latest from Billing fixtures for reproducibility Rely on the SDK-default language version per pinned TFMs (net8.0;net10.0) so fixture compile behavior does not drift with the installed SDK. Both fixtures build clean (0 warnings/errors) without it. Addresses reviewer feedback. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3c9f9823-7f2f-4f7d-9d1b-f2b9e7a20c60 --- tests/dotnet/csharp-refactoring/src/Billing/Billing.csproj | 1 - tests/dotnet/dotnet-breaking-changes/src/Billing/Billing.csproj | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/dotnet/csharp-refactoring/src/Billing/Billing.csproj b/tests/dotnet/csharp-refactoring/src/Billing/Billing.csproj index 50c11dbf51..60587d4bc1 100644 --- a/tests/dotnet/csharp-refactoring/src/Billing/Billing.csproj +++ b/tests/dotnet/csharp-refactoring/src/Billing/Billing.csproj @@ -4,7 +4,6 @@ net8.0;net10.0 enable enable - latest true diff --git a/tests/dotnet/dotnet-breaking-changes/src/Billing/Billing.csproj b/tests/dotnet/dotnet-breaking-changes/src/Billing/Billing.csproj index 50c11dbf51..60587d4bc1 100644 --- a/tests/dotnet/dotnet-breaking-changes/src/Billing/Billing.csproj +++ b/tests/dotnet/dotnet-breaking-changes/src/Billing/Billing.csproj @@ -4,7 +4,6 @@ net8.0;net10.0 enable enable - latest true From 64098865e564f5576119627e4b6f177c7fd8ef6f Mon Sep 17 00:00:00 2001 From: Abhitej John Date: Thu, 23 Jul 2026 10:52:14 -0700 Subject: [PATCH 7/8] Migrate csharp-refactoring and dotnet-breaking-changes evals to the Vally harness schema Adapts both eval.yaml files from the legacy scenarios/assertions format to the stimuli/graders/config schema introduced by #877 (Migrate LLM evals to the Vally harness). Prompts and rubrics are preserved verbatim; assertions map 1:1 to graders (file_contains->file-contains, run_command_and_assert-> run-command, output_matches->output-matches) and copy_test_files-> environment.files. Validated with `vally experiment run --dry-run`: both evals now resolve and match the experiment filter (previously matched none). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3c9f9823-7f2f-4f7d-9d1b-f2b9e7a20c60 --- tests/dotnet/csharp-refactoring/eval.yaml | 317 ++++++++++-------- .../dotnet/dotnet-breaking-changes/eval.yaml | 246 ++++++++------ 2 files changed, 312 insertions(+), 251 deletions(-) diff --git a/tests/dotnet/csharp-refactoring/eval.yaml b/tests/dotnet/csharp-refactoring/eval.yaml index 1c542bcee8..ee41b306d4 100644 --- a/tests/dotnet/csharp-refactoring/eval.yaml +++ b/tests/dotnet/csharp-refactoring/eval.yaml @@ -1,180 +1,213 @@ -scenarios: - - name: "Rename a method across its declaration and every caller" +name: csharp-refactoring +description: Evaluates the dotnet/csharp-refactoring skill +type: capability +config: + timeout: 15m +stimuli: + - name: Rename a method across its declaration and every caller prompt: "In the Billing class library, the method `OrderProcessor.DoStuff` is badly named for what it does — it computes an invoice. Rename it to `ComputeInvoice` everywhere it is declared and called, without changing any behavior. The solution is at Fixture.sln." - setup: - copy_test_files: true - assertions: - - type: "file_contains" - path: "src/Billing/OrderProcessor.cs" - value: "ComputeInvoice" + environment: + files: + - src: . + dest: . + graders: + - type: file-contains + config: + path: src/Billing/OrderProcessor.cs + substring: ComputeInvoice # The rename must propagate to every caller. BillingTests.cs calls the method # directly, so a correct rename lands the new name here too (and the old call # would otherwise fail to compile). - - type: "file_contains" - path: "tests/Billing.Tests/BillingTests.cs" - value: "ComputeInvoice" + - type: file-contains + config: + path: tests/Billing.Tests/BillingTests.cs + substring: ComputeInvoice # Behavior-preservation + full-propagation gate: if any declaration or caller - # was missed, the test project fails to compile and this assertion fails. - - type: run_command_and_assert - command_to_run: "dotnet" - command_arguments: "test Fixture.sln -v:q" - expected_exit_code: 0 - expected_std_output_contains: "Passed!" - command_timeout: 300 - - type: "output_matches" - pattern: "(dotnet (build|test)|rebuild|re-?run.{0,20}tests|tests? .{0,20}(pass|green))" + # was missed, the test project fails to compile and this grader fails. + - 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: (dotnet (build|test)|rebuild|re-?run.{0,20}tests|tests? .{0,20}(pass|green)) + - type: prompt rubric: - - "Establishes a green build and test baseline before renaming" - - "Renames by tracking true binding references rather than a blind text find-and-replace that would also hit comments, strings, or unrelated test method names" - - "Rebuilds and re-runs the existing tests after the rename to confirm behavior is preserved" - - "Keeps this a single, focused operation without bundling unrelated edits" - timeout: 600 + - Establishes a green build and test baseline before renaming + - Renames by tracking true binding references rather than a blind text find-and-replace that would also hit comments, strings, or unrelated test method names + - Rebuilds and re-runs the existing tests after the rename to confirm behavior is preserved + - Keeps this a single, focused operation without bundling unrelated edits - - name: "Extract a repeated calculation into a private helper" + - name: Extract a repeated calculation into a private helper prompt: "`OrderProcessor.DoStuff` is too long. Extract the discount-then-tax calculation into a private helper method and call it, without changing what the code computes. The solution is at Fixture.sln." - setup: - copy_test_files: true - assertions: + environment: + files: + - src: . + dest: . + graders: # Behavior-preservation gate: the extracted helper must compute identical # results, so the existing tests must still pass. - - type: run_command_and_assert - command_to_run: "dotnet" - command_arguments: "test Fixture.sln -v:q" - expected_exit_code: 0 - expected_std_output_contains: "Passed!" - command_timeout: 300 + - type: run-command + config: + command: dotnet test Fixture.sln -v:q + expected_exit_code: 0 + stdout_contains: Passed! + timeout: 5m # A private helper method now exists (OrderProcessor had none before). - - type: "file_contains" - path: "src/Billing/OrderProcessor.cs" - value: "private" - - type: "output_matches" - pattern: "(dotnet (build|test)|rebuild|re-?run.{0,20}tests|tests? .{0,20}(pass|green))" + - type: file-contains + config: + path: src/Billing/OrderProcessor.cs + substring: private + - type: output-matches + config: + pattern: (dotnet (build|test)|rebuild|re-?run.{0,20}tests|tests? .{0,20}(pass|green)) + - type: prompt rubric: - - "Establishes a green baseline before editing" - - "Extracts the block into a new method preserving the exact same computation and results" - - "Verifies behavior is preserved by building and running the existing tests after extracting" - - "Does not slip a bug fix or behavior change into the extraction" - timeout: 600 + - Establishes a green baseline before editing + - Extracts the block into a new method preserving the exact same computation and results + - Verifies behavior is preserved by building and running the existing tests after extracting + - Does not slip a bug fix or behavior change into the extraction - - name: "Consolidate a duplicated block into a single shared helper" + - name: Consolidate a duplicated block into a single shared helper prompt: "Inside `OrderProcessor.DoStuff` the discount-plus-tax calculation is duplicated (it appears twice, once for the order total and once for the quote). Consolidate the duplicated logic into a single shared helper used by both, without changing behavior. The solution is at Fixture.sln." - setup: - copy_test_files: true - assertions: + environment: + files: + - src: . + dest: . + graders: # Behavior-preservation gate: the single shared helper must be equivalent to # both original copies, so the existing tests must still pass. - - type: run_command_and_assert - command_to_run: "dotnet" - command_arguments: "test Fixture.sln -v:q" - expected_exit_code: 0 - expected_std_output_contains: "Passed!" - command_timeout: 300 + - type: run-command + config: + command: dotnet test Fixture.sln -v:q + expected_exit_code: 0 + stdout_contains: Passed! + timeout: 5m # A shared private helper now exists (OrderProcessor had none before). - - type: "file_contains" - path: "src/Billing/OrderProcessor.cs" - value: "private" - - type: "output_matches" - pattern: "(dotnet (build|test)|rebuild|re-?run.{0,20}tests|tests? .{0,20}(pass|green))" + - type: file-contains + config: + path: src/Billing/OrderProcessor.cs + substring: private + - type: output-matches + config: + pattern: (dotnet (build|test)|rebuild|re-?run.{0,20}tests|tests? .{0,20}(pass|green)) + - type: prompt rubric: - - "Establishes a green baseline before editing" - - "Factors the duplicated block into one shared helper that both call sites use" - - "Confirms the consolidated helper is semantically equivalent to each original copy" - - "Re-runs the build and tests after consolidating to prove behavior is unchanged" - timeout: 600 + - Establishes a green baseline before editing + - Factors the duplicated block into one shared helper that both call sites use + - Confirms the consolidated helper is semantically equivalent to each original copy + - Re-runs the build and tests after consolidating to prove behavior is unchanged - - name: "Inline a pass-through wrapper and update callers" + - name: Inline a pass-through wrapper and update callers prompt: "`LegacyTax.ApplyTaxWrapper` does nothing but forward to `TaxRules.Apply`. Inline it: update every caller to call `TaxRules.Apply` directly and remove the wrapper, without changing behavior. The solution is at Fixture.sln." - setup: - copy_test_files: true - assertions: + environment: + files: + - src: . + dest: . + graders: # Behavior-preservation + full-propagation gate: every caller (including the # test that references the wrapper) must be updated to the underlying call, # or the test project fails to compile. - - type: run_command_and_assert - command_to_run: "dotnet" - command_arguments: "test Fixture.sln -v:q" - expected_exit_code: 0 - expected_std_output_contains: "Passed!" - command_timeout: 300 + - type: run-command + config: + command: dotnet test Fixture.sln -v:q + expected_exit_code: 0 + stdout_contains: Passed! + timeout: 5m # The pass-through wrapper is actually gone. - - type: "file_not_contains" - path: "src/Billing/Pricing.cs" - value: "ApplyTaxWrapper" - - type: "output_matches" - pattern: "(dotnet (build|test)|rebuild|re-?run.{0,20}tests|tests? .{0,20}(pass|green))" + - type: file-not-contains + config: + path: src/Billing/Pricing.cs + substring: ApplyTaxWrapper + - type: output-matches + config: + pattern: (dotnet (build|test)|rebuild|re-?run.{0,20}tests|tests? .{0,20}(pass|green)) + - type: prompt rubric: - - "Finds the true binding references to the wrapper before removing it, not just textual matches" - - "Updates every caller to the underlying call and removes the wrapper in one focused operation" - - "Lets the compiler catch any missed reference, then rebuilds and re-runs the tests to confirm green" - timeout: 600 + - Finds the true binding references to the wrapper before removing it, not just textual matches + - Updates every caller to the underlying call and removes the wrapper in one focused operation + - Lets the compiler catch any missed reference, then rebuilds and re-runs the tests to confirm green - - name: "Merge two near-identical types into one parameterized type" + - name: Merge two near-identical types into one parameterized type prompt: "`GoldPricing` and `SilverPricing` are near-identical. Consolidate them into a single parameterized pricing type and update any usages, without changing behavior. The solution is at Fixture.sln." - setup: - copy_test_files: true - assertions: + environment: + files: + - src: . + dest: . + graders: # Behavior-preservation + full-propagation gate: usages (including the test # that constructs GoldPricing/SilverPricing) must be updated to the merged # type, or the test project fails to compile. - - type: run_command_and_assert - command_to_run: "dotnet" - command_arguments: "test Fixture.sln -v:q" - expected_exit_code: 0 - expected_std_output_contains: "Passed!" - command_timeout: 300 + - type: run-command + config: + command: dotnet test Fixture.sln -v:q + expected_exit_code: 0 + stdout_contains: Passed! + timeout: 5m # The two near-identical classes are actually gone (merged into one type). - - type: "file_not_contains" - path: "src/Billing/Pricing.cs" - value: "class GoldPricing" - - type: "file_not_contains" - path: "src/Billing/Pricing.cs" - value: "class SilverPricing" - - type: "output_matches" - pattern: "(dotnet (build|test)|rebuild|re-?run.{0,20}tests|tests? .{0,20}(pass|green))" + - type: file-not-contains + config: + path: src/Billing/Pricing.cs + substring: class GoldPricing + - type: file-not-contains + config: + path: src/Billing/Pricing.cs + substring: class SilverPricing + - type: output-matches + config: + pattern: (dotnet (build|test)|rebuild|re-?run.{0,20}tests|tests? .{0,20}(pass|green)) + - type: prompt rubric: - - "Establishes a green baseline before editing" - - "Merges the two classes into a single type parameterized by name and rate, preserving each tier's values" - - "Updates all usages and re-runs the build and tests to prove behavior is unchanged" - timeout: 600 + - Establishes a green baseline before editing + - Merges the two classes into a single type parameterized by name and rate, preserving each tier's values + - Updates all usages and re-runs the build and tests to prove behavior is unchanged - - name: "Decline a framework and package upgrade dressed up as a refactor" - prompt: "Upgrade the Billing project to a newer target framework and bump all its NuGet packages to their latest major versions. The solution is at Fixture.sln." - expect_activation: false - setup: - copy_test_files: true - assertions: - - type: "output_matches" - pattern: "(not a .{0,20}refactor|behavior[- ]?(preserving|chang)|upgrade|major version|out of scope)" + - name: Decline a framework and package upgrade dressed up as a refactor + prompt: Upgrade the Billing project to a newer target framework and bump all its NuGet packages to their latest major versions. The solution is at Fixture.sln. + environment: + files: + - src: . + dest: . + graders: + - type: output-matches + config: + pattern: (not a .{0,20}refactor|behavior[- ]?(preserving|chang)|upgrade|major version|out of scope) + - type: prompt rubric: - - "Recognizes that a framework or package major-version upgrade is not a behavior-preserving refactor" - - "Declines or redirects to the appropriate upgrade workflow instead of treating it as a refactor" - - "Does not silently perform a behavior-changing version bump under the guise of refactoring" - timeout: 300 + - Recognizes that a framework or package major-version upgrade is not a behavior-preserving refactor + - Declines or redirects to the appropriate upgrade workflow instead of treating it as a refactor + - Does not silently perform a behavior-changing version bump under the guise of refactoring - - name: "Decline a new-feature request dressed up as a refactor" - prompt: "Add a new \"platinum\" loyalty tier at 15% off and wire it into the pricing tiers so orders can use it. The solution is at Fixture.sln." - expect_activation: false - setup: - copy_test_files: true - assertions: - - type: "output_matches" - pattern: "(new feature|feature addition|not a .{0,20}refactor|behavior[- ]?(preserving|chang)|adds? new behavior)" + - name: Decline a new-feature request dressed up as a refactor + prompt: 'Add a new "platinum" loyalty tier at 15% off and wire it into the pricing tiers so orders can use it. The solution is at Fixture.sln.' + environment: + files: + - src: . + dest: . + graders: + - type: output-matches + config: + pattern: (new feature|feature addition|not a .{0,20}refactor|behavior[- ]?(preserving|chang)|adds? new behavior) + - type: prompt rubric: - - "Recognizes that adding a new pricing tier is a new feature, not a behavior-preserving refactor" - - "Distinguishes the feature request from a refactor rather than conflating them, and does not claim behavior is preserved when it is deliberately being extended" - - "If it proceeds, does so as an explicit feature addition with its own new tests, not under the guise of a no-behavior-change refactor" - timeout: 300 + - Recognizes that adding a new pricing tier is a new feature, not a behavior-preserving refactor + - Distinguishes the feature request from a refactor rather than conflating them, and does not claim behavior is preserved when it is deliberately being extended + - If it proceeds, does so as an explicit feature addition with its own new tests, not under the guise of a no-behavior-change refactor - - name: "Keep behavior-changing bug fixes out of a behavior-preserving refactor" - prompt: "Fix the known bugs first and keep any refactors in a separate follow-up PR. Do not mix behavior-changing bug fixes with refactoring under a behavior-preserving contract. The solution is at Fixture.sln." - expect_activation: false - setup: - copy_test_files: true - assertions: - - type: "output_matches" - pattern: "(separate|follow[- ]?up|bug fix|behavior[- ]?(preserving|chang)|not (a )?refactor)" + - name: Keep behavior-changing bug fixes out of a behavior-preserving refactor + prompt: Fix the known bugs first and keep any refactors in a separate follow-up PR. Do not mix behavior-changing bug fixes with refactoring under a behavior-preserving contract. The solution is at Fixture.sln. + environment: + files: + - src: . + dest: . + graders: + - type: output-matches + config: + pattern: (separate|follow[- ]?up|bug fix|behavior[- ]?(preserving|chang)|not (a )?refactor) + - type: prompt rubric: - - "Recognizes that a behavior-changing bug fix is distinct from a behavior-preserving refactor" - - "Keeps the bug fix separate from any refactoring rather than bundling them under one behavior-preserving contract" - - "Does not claim behavior is preserved for a change that deliberately alters behavior" - timeout: 300 + - Recognizes that a behavior-changing bug fix is distinct from a behavior-preserving refactor + - Keeps the bug fix separate from any refactoring rather than bundling them under one behavior-preserving contract + - Does not claim behavior is preserved for a change that deliberately alters behavior diff --git a/tests/dotnet/dotnet-breaking-changes/eval.yaml b/tests/dotnet/dotnet-breaking-changes/eval.yaml index 7a0a58aa13..da3716c682 100644 --- a/tests/dotnet/dotnet-breaking-changes/eval.yaml +++ b/tests/dotnet/dotnet-breaking-changes/eval.yaml @@ -1,165 +1,193 @@ -scenarios: - - name: "Answer a public-API question before consolidating duplicated parsing" +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." - setup: - copy_test_files: true - assertions: - - type: "output_matches" - pattern: "(public API|PublicAPI\\.Shipped|breaking change|shipped)" - - type: "file_contains" - path: "src/Billing/PublicAPI.Shipped.txt" - value: "AppSettingsHelper" + 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 + substring: 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_and_assert - command_to_run: "dotnet" - command_arguments: "test Fixture.sln -v:q" - expected_exit_code: 0 - expected_std_output_contains: "Passed!" - command_timeout: 300 + - 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" - timeout: 600 - - name: "Add behavior to a shipped public member without breaking the contract" + - 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." - setup: - copy_test_files: true - assertions: - - type: "output_matches" - pattern: "(public API|PublicAPI|breaking change|shipped|signature|overload)" - - type: "file_contains" - path: "src/Billing/PublicAPI.Shipped.txt" - value: "ParseIntSetting" + 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 + substring: ParseIntSetting # The shipped signature must not break: existing decimal-parsing tests must # still pass after hex support is added. - - type: run_command_and_assert - command_to_run: "dotnet" - command_arguments: "test Fixture.sln -v:q" - expected_exit_code: 0 - expected_std_output_contains: "Passed!" - command_timeout: 300 + - 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" - timeout: 600 - - name: "Rename a helper referenced from every #if branch of a multi-targeted type" + - 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." - setup: - copy_test_files: true - assertions: - - type: "file_contains" - path: "src/Billing/PlatformInfo.cs" - value: "FormatLabel" + environment: + files: + - src: . + dest: . + graders: + - type: file-contains + config: + path: src/Billing/PlatformInfo.cs + substring: 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_and_assert - command_to_run: "dotnet" - command_arguments: "build Fixture.sln -v:q" - expected_exit_code: 0 - command_timeout: 300 + - 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_and_assert - command_to_run: "dotnet" - command_arguments: "test Fixture.sln -v:q" - expected_exit_code: 0 - expected_std_output_contains: "Passed!" - command_timeout: 300 - - type: "output_matches" - pattern: "(multi[- ]?target|#if|net8|net10|target framework|TFM|every (branch|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" - timeout: 600 - - name: "Rename a member of a partial type that a generated part references" + - 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." - setup: - copy_test_files: true - assertions: - - type: "file_contains" - path: "src/Billing/Coupons.cs" - value: "Apply" + environment: + files: + - src: . + dest: . + graders: + - type: file-contains + config: + path: src/Billing/Coupons.cs + substring: 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" - path: "src/Billing/Coupons.g.cs.template" - value: "Apply" + - type: file-contains + config: + path: src/Billing/Coupons.g.cs.template + substring: 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_and_assert - command_to_run: "dotnet" - command_arguments: "test Fixture.sln -v:q" - expected_exit_code: 0 - expected_std_output_contains: "Passed!" - command_timeout: 300 - - type: "output_matches" - pattern: "(auto[- ]?generated|source generator|generated (file|output|code)|partial|template|\\.g\\.cs|regenerat)" + - 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" - timeout: 600 - - name: "Add a new public method that must be correct on every target framework" + - 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." - setup: - copy_test_files: true - assertions: - - type: "file_contains" - path: "src/Billing/PlatformInfo.cs" - value: "Tag" + environment: + files: + - src: . + dest: . + graders: + - type: file-contains + config: + path: src/Billing/PlatformInfo.cs + substring: Tag # Multi-target gate: the new method must compile on BOTH net8.0 and net10.0. - - type: run_command_and_assert - command_to_run: "dotnet" - command_arguments: "build Fixture.sln -v:q" - expected_exit_code: 0 - command_timeout: 300 + - 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_and_assert - command_to_run: "dotnet" - command_arguments: "test Fixture.sln -v:q" - expected_exit_code: 0 - expected_std_output_contains: "Passed!" - command_timeout: 300 - - type: "output_matches" - pattern: "(multi[- ]?target|#if|net8|net10|target framework|TFM|public API|PublicAPI)" + - 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" - timeout: 600 - - name: "Do not raise breaking-change concerns for a purely internal local rename" + - 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." - expect_activation: false - setup: - copy_test_files: true - assertions: - - type: "file_contains" - path: "src/Billing/OrderProcessor.cs" - value: "runningTotal" + environment: + files: + - src: . + dest: . + graders: + - type: file-contains + config: + path: src/Billing/OrderProcessor.cs + substring: runningTotal # The local rename is behavior-identical: tests must still pass unchanged. - - type: run_command_and_assert - command_to_run: "dotnet" - command_arguments: "test Fixture.sln -v:q" - expected_exit_code: 0 - expected_std_output_contains: "Passed!" - command_timeout: 300 + - 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" - timeout: 300 From 70fdd8c5ff5d3dd509a8be940c37f60cb7dad8ca Mon Sep 17 00:00:00 2001 From: Abhitej John Date: Thu, 23 Jul 2026 11:15:00 -0700 Subject: [PATCH 8/8] Fix eval grader config: file-contains/file-not-contains use 'value' not 'substring' The Vally file-contains and file-not-contains graders require the config key 'value' (per the schema and all main eval.yaml files); 'substring' is rejected with [invalid-grader-config]. Verified with `vally lint --eval-spec`: both evals now lint with 0 errors, matching main's passing evals (only the benign scoring-defaults + config-deprecation warnings that main evals also emit). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3c9f9823-7f2f-4f7d-9d1b-f2b9e7a20c60 --- tests/dotnet/csharp-refactoring/eval.yaml | 14 +++++++------- tests/dotnet/dotnet-breaking-changes/eval.yaml | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/dotnet/csharp-refactoring/eval.yaml b/tests/dotnet/csharp-refactoring/eval.yaml index ee41b306d4..8f6163c30a 100644 --- a/tests/dotnet/csharp-refactoring/eval.yaml +++ b/tests/dotnet/csharp-refactoring/eval.yaml @@ -14,14 +14,14 @@ stimuli: - type: file-contains config: path: src/Billing/OrderProcessor.cs - substring: ComputeInvoice + value: ComputeInvoice # The rename must propagate to every caller. BillingTests.cs calls the method # directly, so a correct rename lands the new name here too (and the old call # would otherwise fail to compile). - type: file-contains config: path: tests/Billing.Tests/BillingTests.cs - substring: ComputeInvoice + value: ComputeInvoice # Behavior-preservation + full-propagation gate: if any declaration or caller # was missed, the test project fails to compile and this grader fails. - type: run-command @@ -59,7 +59,7 @@ stimuli: - type: file-contains config: path: src/Billing/OrderProcessor.cs - substring: private + value: private - type: output-matches config: pattern: (dotnet (build|test)|rebuild|re-?run.{0,20}tests|tests? .{0,20}(pass|green)) @@ -89,7 +89,7 @@ stimuli: - type: file-contains config: path: src/Billing/OrderProcessor.cs - substring: private + value: private - type: output-matches config: pattern: (dotnet (build|test)|rebuild|re-?run.{0,20}tests|tests? .{0,20}(pass|green)) @@ -120,7 +120,7 @@ stimuli: - type: file-not-contains config: path: src/Billing/Pricing.cs - substring: ApplyTaxWrapper + value: ApplyTaxWrapper - type: output-matches config: pattern: (dotnet (build|test)|rebuild|re-?run.{0,20}tests|tests? .{0,20}(pass|green)) @@ -150,11 +150,11 @@ stimuli: - type: file-not-contains config: path: src/Billing/Pricing.cs - substring: class GoldPricing + value: class GoldPricing - type: file-not-contains config: path: src/Billing/Pricing.cs - substring: class SilverPricing + value: class SilverPricing - type: output-matches config: pattern: (dotnet (build|test)|rebuild|re-?run.{0,20}tests|tests? .{0,20}(pass|green)) diff --git a/tests/dotnet/dotnet-breaking-changes/eval.yaml b/tests/dotnet/dotnet-breaking-changes/eval.yaml index da3716c682..fe71c87ecb 100644 --- a/tests/dotnet/dotnet-breaking-changes/eval.yaml +++ b/tests/dotnet/dotnet-breaking-changes/eval.yaml @@ -17,7 +17,7 @@ stimuli: - type: file-contains config: path: src/Billing/PublicAPI.Shipped.txt - substring: AppSettingsHelper + 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 @@ -47,7 +47,7 @@ stimuli: - type: file-contains config: path: src/Billing/PublicAPI.Shipped.txt - substring: ParseIntSetting + value: ParseIntSetting # The shipped signature must not break: existing decimal-parsing tests must # still pass after hex support is added. - type: run-command @@ -73,7 +73,7 @@ stimuli: - type: file-contains config: path: src/Billing/PlatformInfo.cs - substring: FormatLabel + 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. @@ -108,14 +108,14 @@ stimuli: - type: file-contains config: path: src/Billing/Coupons.cs - substring: Apply + 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 - substring: Apply + 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. @@ -144,7 +144,7 @@ stimuli: - type: file-contains config: path: src/Billing/PlatformInfo.cs - substring: Tag + value: Tag # Multi-target gate: the new method must compile on BOTH net8.0 and net10.0. - type: run-command config: @@ -178,7 +178,7 @@ stimuli: - type: file-contains config: path: src/Billing/OrderProcessor.cs - substring: runningTotal + value: runningTotal # The local rename is behavior-identical: tests must still pass unchanged. - type: run-command config: