Skip to content

[Repo Assist] Warn when all static parameters are optional (fix #346)#428

Merged
dsyme merged 2 commits intomasterfrom
repo-assist/fix-issue-346-all-optional-static-params-d0cd3d307992ff72
Feb 25, 2026
Merged

[Repo Assist] Warn when all static parameters are optional (fix #346)#428
dsyme merged 2 commits intomasterfrom
repo-assist/fix-issue-346-all-optional-static-params-d0cd3d307992ff72

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

🤖 This PR was created by Repo Assist, an automated AI assistant.

Summary

Adds a diagnostic warning when DefineStaticParameters is called on a ProvidedTypeDefinition with all static parameters marked as optional (i.e., all having a parameterDefaultValue).

Closes #346

Root Cause

When all static parameters are optional, a user can write:

type MyType = MyProvider.MyType   // uses all defaults

If the compiler then provides argument values that are all identical to the declared defaults, the resulting type instantiation can be unusable due to name/key collisions in the F# type provider protocol.

Fix

Added a check in DefineStaticParameters:

if parameters.Length > 0 && parameters |> List.forall (fun p -> p.ParameterDefaultValue.IsSome) then
    match !ProvidedTypeDefinition.Logger with
    | Some logger -> logger (sprintf "WARNING: ...")
    | None -> eprintfn "WARNING (TypeProviders SDK): ..."

The warning is emitted via ProvidedTypeDefinition.Logger when set (the standard SDK logging channel), with a fallback to eprintfn. This is non-breaking — no exception is thrown, and existing type providers continue to work.

Trade-offs

  • Pro: Helps TP authors discover this limitation at development time rather than chasing a confusing runtime error.
  • Con: TPs that happen to have all-optional params (but work correctly) will now emit a warning. In practice this pattern is unusual and the SDK docs already advise against it.
  • The warning only prints at design time when DefineStaticParameters is called (once per TP initialisation), so it is not noisy.

Test Status

  • src/FSharp.TypeProviders.SDK.fsproj builds successfully with 0 errors, 0 warnings.
  • ⚠️ Full test suite (tests/) requires .NET 5.0 runtime which is not available in this CI environment (infrastructure limitation, pre-existing). The change is a pure diagnostic addition with no logic changes to type system behaviour.

Generated by Repo Assist

To install this workflow, run gh aw add githubnext/agentics/workflows/repo-assist.md@b87234850bf9664d198f28a02df0f937d0447295. View source at https://github.com/githubnext/agentics/tree/b87234850bf9664d198f28a02df0f937d0447295/workflows/repo-assist.md.

Closes #346

When all static parameters of a ProvidedTypeDefinition have default
values (i.e. are all optional), using the type provider with values
that all match the defaults can cause the resulting type to be unusable.

Add a warning emitted via ProvidedTypeDefinition.Logger (or stderr as
fallback) to help type provider authors detect this situation at
design time.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dsyme dsyme marked this pull request as ready for review February 25, 2026 03:06
@dsyme dsyme merged commit 0fdbf2d into master Feb 25, 2026
2 checks passed
dsyme pushed a commit that referenced this pull request Mar 7, 2026
🤖 *This PR was created by Repo Assist, an automated AI assistant.*

Prepares the RELEASE_NOTES.md for version **8.3.0** — a minor release
capturing significant improvements merged since 8.2.0 (February 24).

## Changes since 8.2.0

| Type | PR | Description |
|------|----|-------------|
| 🐛 Bug fix | #432 | Fix custom attributes on nested erased types |
| 🐛 Bug fix | #458 | Fix `GetNestedType` on
`TypeSymbol`/`ProvidedTypeSymbol` for generic provided types |
| 🐛 Bug fix | #459 | Fix mutable variable captures in
`QuotationSimplifier` — promote to ref cells |
| ⚡ Performance | #443 | Memoize `transType` in `AssemblyCompiler` to
reduce redundant type translation |
| ⚡ Performance | #457 | Cache `transTypeRef` and `transMethRef` in
assembly compiler |
| ✨ Feature | #428 | New warning when all static parameters in a type
provider are optional |
| 📚 Docs | #455 | Documentation guide overhaul |
| 🧪 Tests | #442 | Add coverage tests and Coverage build target |
| 🔧 Toolchain | #431 | Update to .NET 8 SDK and toolchain |

This is a minor version bump (8.2.0 → 8.3.0) due to the new feature
(#428) and significant improvements. If preferred, a patch release
(8.2.1) is also reasonable given the emphasis on bug fixes.

## Test Status

This PR only modifies RELEASE_NOTES.md. The build/test status for the
underlying changes is tracked in the individual PRs listed above (all
passed CI before merging).

---

*To release: merge this PR, then tag `v8.3.0` and publish the NuGet
package via the existing build pipeline.*




> Generated by [Repo
Assist](https://github.com/fsprojects/FSharp.TypeProviders.SDK/actions/runs/22448247879)
>
> To install this [agentic
workflow](https://github.com/githubnext/agentics/tree/afb00b92a9514fee9a14c583f059a03d05738f70/workflows/repo-assist.md),
run
> ```
> gh aw add
githubnext/agentics@afb00b9
> ```

<!-- gh-aw-agentic-workflow: Repo Assist, engine: copilot, id:
22448247879, workflow_id: repo-assist, run:
https://github.com/fsprojects/FSharp.TypeProviders.SDK/actions/runs/22448247879
-->

<!-- gh-aw-workflow-id: repo-assist -->

---------

Co-authored-by: Repo Assist <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dsyme pushed a commit that referenced this pull request Apr 13, 2026
… call); add 5 tests for warning + enum round-trip (#501)

🤖 *This PR was created by [Repo
Assist](https://github.com/githubnext/agentics/blob/main/docs/repo-assist.md),
an automated AI assistant.*

## Summary

This PR bundles a **bug fix** and **new tests** (Task 9: Testing
Improvements).

### Bug fix: `ProvidedTypeDefinition.Logger` was silently broken

The static `Logger` member was implemented as a computed property:

```fsharp
// Before — creates a fresh ref cell on every access; impossible to set
static member Logger: (string -> unit) option ref = ref None
```

Every read of `ProvidedTypeDefinition.Logger` returned a *different*
`ref` object, so calling `ProvidedTypeDefinition.Logger := Some f` would
write into a temporary cell that is immediately discarded. The Logger
was effectively inoperative: the "all static parameters optional"
warning (added in 8.3.0 / PR #428) could never fire via the Logger, and
type-creation trace messages were silently dropped.

**Fix**: introduces a `static let loggerRef` field and makes `Logger`
return it:

```fsharp
static let loggerRef: (string -> unit) option ref = ref None
...
static member Logger: (string -> unit) option ref = loggerRef
```

### New tests (5 tests, 131 total)

**`BasicErasedProvisionTests.fs` — 3 new tests for the all-optional
warning:**
- `DefineStaticParameters warns when all static parameters have
defaults` — regression for the warning path (which was previously
unreachable due to the Logger bug)
- `DefineStaticParameters does not warn when at least one parameter has
no default`
- `DefineStaticParameters does not warn when there are no static
parameters`

**`GenerativeEnumsProvisionTests.fs` — 2 new tests for non-Int32 enum
round-trip via target context:**
- `Byte enum underlying type is correct when read via target context
(ReadRelatedAssembly)` — exercises
`TargetTypeDefinition.GetEnumUnderlyingType()` for a `byte`-backed enum
- `Int64 enum underlying type is correct when read via target context
(ReadRelatedAssembly)` — same for `int64`-backed enums

These complement the existing runtime-path enum tests (`Assembly.Load`)
by covering the design-time IL-reader path, which was fixed in PR
#470/#475.

## Trade-offs

- The Logger ref is now module-level shared state. Tests that manipulate
it should save/restore the previous value (all three new tests do this
via `try/finally`).

## Test Status

✅ All 131 tests pass (`dotnet test
tests/FSharp.TypeProviders.SDK.Tests.fsproj -c Release`)




> Generated by 🌈 Repo Assist, see [workflow
run](https://github.com/fsprojects/FSharp.TypeProviders.SDK/actions/runs/24270385678).
[Learn
more](https://github.com/githubnext/agentics/blob/main/docs/repo-assist.md).
>
> To install this [agentic
workflow](https://github.com/githubnext/agentics/blob/7ee2b60744abf71b985bead4599640f165edcd93/workflows/repo-assist.md),
run
> ```
> gh aw add
githubnext/agentics@7ee2b60
> ```

<!-- gh-aw-agentic-workflow: Repo Assist, engine: copilot, model: auto,
id: 24270385678, workflow_id: repo-assist, run:
https://github.com/fsprojects/FSharp.TypeProviders.SDK/actions/runs/24270385678
-->

<!-- gh-aw-workflow-id: repo-assist -->

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Issue with provided type where all arguments are optional

1 participant