dotnet-fsharp: Adding Basic F# Skills - #753
Conversation
|
Note This PR is from a fork and modifies infrastructure files ( Changes to infrastructure typically need to be submitted from a branch in Please consider recreating this PR from an upstream branch. If you don't have push access to |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new dotnet-fsharp plugin that provides curated skills for writing idiomatic F#, testing, error handling, domain modeling, async/task usage, type providers, scripts, formatting with Fantomas, and .NET interop—along with evaluation scenarios and marketplace registration.
Changes:
- Introduces the
dotnet-fsharpplugin manifest and registers it in repository/plugin marketplaces. - Adds a set of F# skill guides (
SKILL.md) plus an idiom rewrite reference doc. - Adds evaluation scenarios (
tests/dotnet-fsharp/**/eval.yaml) covering each skill area.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/dotnet-fsharp/writing-idiomatic-fsharp/eval.yaml | Adds eval scenarios for idiomatic refactors and null-to-Option guidance |
| tests/dotnet-fsharp/testing-fsharp/eval.yaml | Adds eval scenarios for property-based testing recommendations |
| tests/dotnet-fsharp/fsharp-units-of-measure/eval.yaml | Adds eval scenarios for units-of-measure modeling and conversions |
| tests/dotnet-fsharp/fsharp-type-providers/eval.yaml | Adds eval scenarios for JSON type provider usage vs trivial parsing |
| tests/dotnet-fsharp/fsharp-scripts/eval.yaml | Adds eval scenarios for .fsx scripting and #r "nuget:" usage |
| tests/dotnet-fsharp/fsharp-project-organization/eval.yaml | Adds eval scenarios for compile-order issues and namespace vs module guidance |
| tests/dotnet-fsharp/fsharp-error-handling/eval.yaml | Adds eval scenarios for Result-based errors and applicative validation |
| tests/dotnet-fsharp/fsharp-domain-modeling/eval.yaml | Adds eval scenarios for smart constructors and DU-based state modeling |
| tests/dotnet-fsharp/fsharp-async-and-tasks/eval.yaml | Adds eval scenarios for removing .Result and running async work in parallel |
| tests/dotnet-fsharp/fsharp-active-patterns/eval.yaml | Adds eval scenarios for partial/parameterized active patterns |
| tests/dotnet-fsharp/format-fsharp-with-fantomas/eval.yaml | Adds eval scenarios for Fantomas install/check and .editorconfig configuration |
| tests/dotnet-fsharp/design-fsharp-for-dotnet-interop/eval.yaml | Adds eval scenarios for C#-friendly public API shaping |
| tests/dotnet-fsharp/convert-csharp-to-fsharp/eval.yaml | Adds eval scenarios for idiomatic ports from C# to F# |
| tests/dotnet-fsharp/authoring-computation-expressions/eval.yaml | Adds eval scenarios for authoring a Result computation expression |
| plugins/dotnet-fsharp/skills/writing-idiomatic-fsharp/references/csharpism-rewrites.md | Adds before/after catalog of common “C#-isms” and idiomatic rewrites |
| plugins/dotnet-fsharp/skills/writing-idiomatic-fsharp/SKILL.md | Adds “writing idiomatic F#” skill definition and workflow |
| plugins/dotnet-fsharp/skills/testing-fsharp/SKILL.md | Adds testing skill definition including FsCheck/Expecto guidance |
| plugins/dotnet-fsharp/skills/fsharp-units-of-measure/SKILL.md | Adds units-of-measure skill definition and examples |
| plugins/dotnet-fsharp/skills/fsharp-type-providers/SKILL.md | Adds type provider skill definition and FSharp.Data examples |
| plugins/dotnet-fsharp/skills/fsharp-scripts/SKILL.md | Adds .fsx/dotnet fsi scripting skill definition and directives workflow |
| plugins/dotnet-fsharp/skills/fsharp-project-organization/SKILL.md | Adds project ordering and namespace/module guidance |
| plugins/dotnet-fsharp/skills/fsharp-error-handling/SKILL.md | Adds Result/Option vs exceptions guidance and validation patterns |
| plugins/dotnet-fsharp/skills/fsharp-domain-modeling/SKILL.md | Adds “illegal states unrepresentable” modeling patterns |
| plugins/dotnet-fsharp/skills/fsharp-async-and-tasks/SKILL.md | Adds async/task guidance, bridging patterns, and cancellation notes |
| plugins/dotnet-fsharp/skills/fsharp-active-patterns/SKILL.md | Adds active patterns guidance and examples |
| plugins/dotnet-fsharp/skills/format-fsharp-with-fantomas/SKILL.md | Adds Fantomas install/config/CI check guidance |
| plugins/dotnet-fsharp/skills/design-fsharp-for-dotnet-interop/SKILL.md | Adds public API interop rules and patterns |
| plugins/dotnet-fsharp/skills/convert-csharp-to-fsharp/SKILL.md | Adds mapping/workflow for idiomatic conversions from C# |
| plugins/dotnet-fsharp/skills/authoring-computation-expressions/SKILL.md | Adds CE authoring guidance and a minimal Result builder |
| plugins/dotnet-fsharp/plugin.json | Introduces the plugin manifest for dotnet-fsharp |
| eng/known-domains.txt | Adds F# ecosystem domains for link allowlisting |
| README.md | Adds dotnet-fsharp to the plugin list |
| .github/plugin/marketplace.json | Registers dotnet-fsharp in GitHub marketplace config |
| .cursor-plugin/marketplace.json | Registers dotnet-fsharp in Cursor marketplace config |
| .claude-plugin/marketplace.json | Registers dotnet-fsharp in Claude marketplace config |
| .agents/plugins/marketplace.json | Registers dotnet-fsharp in agents marketplace config |
Comments suppressed due to low confidence (1)
tests/dotnet-fsharp/testing-fsharp/eval.yaml:1
- This scenario asks the agent to 'run it' but doesn’t assert
exit_successor specifyexpect_tools(like other scenarios that requiredotnet fsi). Addingexit_successandexpect_tools: [\"bash\"](and optionally an output assertion indicating the test actually executed) will make the evaluation enforce the 'run it' requirement rather than only checking for framework keywords.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| match status with | ||
| | Active -> "on" | ||
| | _ -> "off" |
There was a problem hiding this comment.
False positive |, not ||. The syntax is valid as-written.
| match status with | ||
| | Active -> "on" | ||
| | Suspended -> "off" | ||
| | Closed -> "off" |
There was a problem hiding this comment.
False positive — same as above, these cases use a single |. Valid F# syntax.
| | `if x <> null then f x else d` | `x \|> Option.map f \|> Option.defaultValue d` | | ||
| | `let r = if c then a else b` (statement style) | `let r = if c then a else b` (used as a value, single expression) | | ||
| | `g(f(x))` | `x \|> f \|> g` | |
| - name: "Accumulate validation errors instead of stopping at the first" | ||
| prompt: | | ||
| In F#, validate a form with a Name (non-empty) and an Age (between 0 and 130). When BOTH | ||
| are invalid, the caller must receive BOTH error messages, not just the first one. Implement | ||
| it and demonstrate the both-invalid case with dotnet fsi. | ||
| assertions: | ||
| - type: "exit_success" | ||
| expect_tools: ["bash"] | ||
| rubric: | ||
| - "Uses applicative validation (and! / a Validation type) so errors accumulate" | ||
| - "Does NOT use Result.bind / let! chaining that would short-circuit on the first error" | ||
| - "The both-invalid run reports two errors" | ||
| timeout: 180 |
There was a problem hiding this comment.
Added an output_matches assertion requiring both the Name and Age errors to appear. Skipped the let! guard: applicative validation in F# uses let! ... and!, so it would reject correct code. The no-short-circuit check stays in the rubric.
|
@dotnet-policy-service agree |
| @@ -0,0 +1,211 @@ | |||
| # C#-ism to idiomatic F# rewrites | |||
|
|
|||
| A before/after catalog for the most common ways C# habits leak into F#. Each pair compiles. | |||
There was a problem hiding this comment.
Softened the intro: the pairs are minimal fragments, not full programs. (The || part is a false positive — these cases already use a single |.)
| match status with | ||
| | Active -> "on" | ||
| | _ -> "off" |
There was a problem hiding this comment.
Softened the intro: the pairs are minimal fragments, not full programs. (The || part is a false positive — these cases already use a single |.)
| match status with | ||
| | Active -> "on" | ||
| | Suspended -> "off" | ||
| | Closed -> "off" |
There was a problem hiding this comment.
Softened the intro: the pairs are minimal fragments, not full programs. (The || part is a false positive — these cases already use a single |.)
| `result { }` ships in libraries such as FsToolkit.ErrorHandling, or can be authored with | ||
| `authoring-computation-expressions`. |
There was a problem hiding this comment.
Added attribution: validation { } and List.sequenceResultM now noted as FsToolkit.ErrorHandling, not FSharp.Core.
| validation { | ||
| let! name = validateName input | ||
| and! age = validateAge input | ||
| return { Name = name; Age = age } | ||
| } |
There was a problem hiding this comment.
Added attribution: validation { } and List.sequenceResultM now noted as FsToolkit.ErrorHandling, not FSharp.Core.
| let parseAll lines = | ||
| lines | ||
| |> List.map parseLine // string list -> Result<Row,string> list | ||
| |> List.sequenceResultM // -> Result<Row list, string> |
There was a problem hiding this comment.
Added attribution: validation { } and List.sequenceResultM now noted as FsToolkit.ErrorHandling, not FSharp.Core.
|
@T-Gro : Gentle ping to help review. |
T-Gro
left a comment
There was a problem hiding this comment.
Thank you for this contribution.
There is massive overlap with existing F# docs and style guide.
I would like you to research one of two options also w.r.t. to test results on the provided scenarios:
- Make the skills a lot slimmer and just point them to docs
- Just use the mslearn MCP
- If there are practical reasons for not using any of the above, let embed metadata so that .md here can evolve as docs are evolving. (= I am after future maintenance of this content. Someone will have to do it). i.e. admit this is a copy, list the practical reasons why a copy is better than MCP/URL, and allow automatic sync.
If there is legit content not covered in the docs, do keep it in here, I am definitely happy for any of that.
Also, to make review easier, it would be better if you shared original prompt that went into generating this - so that we can review the methodology of assembling this instead of reading the full set.
It is also great you did the eval scenarios - they will be helpful in comparing the options I listed above, great job 👍 . It should also show which skills are not providing much value over nothing (just leaning on model's training data), MsLearn MCP or slim skills just pointing to docs.
|
One more thought, maybe consider porting #236 over to this PR as well ? |
|
This PR has been automatically marked as stale because it has no activity for 30 days. It will be closed if no further activity occurs within another 7 days of this comment. If it is closed, you may reopen it anytime when you're ready again. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
|
This pull request has been automatically closed because it has been open for more than 30 days with no recent activity. If you believe this work is still relevant, please feel free to reopen or create a new pull request. Thank you for your contribution! |
Adds a set of dotnet-fsharp skills closes: #752