[Repo Assist] fix: ProvidedTypeDefinition.Logger broken (new ref each 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>

Author: github-actions[bot]

src/ProvidedTypes.fs

@@ -1390,6 +1390,9 @@ and ProvidedTypeDefinition(isTgt: bool, container:TypeContainer, className: stri
         | TypeContainer.Namespace _, Some logger when not isTgt -> logger (sprintf "Creating ProvidedTypeDefinition %s [%d]" className (System.Runtime.CompilerServices.RuntimeHelpers.GetHashCode this))
         | _ -> ()
 
+    // Shared mutable logger cell; must be a static let so the same ref is returned on every access.
+    static let loggerRef: (string -> unit) option ref = ref None
+
     static let defaultAttributes (isErased, isSealed, isInterface, isAbstract, isStruct) =
         TypeAttributes.Public ||| 
         (if isInterface then TypeAttributes.Interface ||| TypeAttributes.Abstract
@@ -1940,7 +1943,7 @@ and ProvidedTypeDefinition(isTgt: bool, container:TypeContainer, className: stri
         | :? ProvidedField as l -> l.PatchDeclaringType this
         | _ -> ()
 
-    static member Logger: (string -> unit) option ref = ref None
+    static member Logger: (string -> unit) option ref = loggerRef
 
 
 //====================================================================================================

tests/BasicErasedProvisionTests.fs

@@ -811,3 +811,60 @@ let ``GetUnionCases works on option of provided type`` () =
     Assert.Equal(2, cases.Length)
     Assert.Equal("None", cases.[0].Name)
     Assert.Equal("Some", cases.[1].Name)
+
+// ---------------------------------------------------------------------------
+// Tests for DefineStaticParameters warning: all optional parameters
+// Addresses PR #428 — warn when every static param has a default value.
+// ---------------------------------------------------------------------------
+
+[<Fact>]
+let ``DefineStaticParameters warns when all static parameters have defaults``() =
+    let warnings = System.Collections.Generic.List<string>()
+    let prevLogger = !ProvidedTypeDefinition.Logger
+    ProvidedTypeDefinition.Logger := Some warnings.Add
+    try
+        let asm = Assembly.GetExecutingAssembly()
+        let t = ProvidedTypeDefinition(asm, "Test.Warning", "AllOptionalType", Some typeof<obj>)
+        warnings.Clear() // discard the "Creating..." trace message from the ctor
+        let p1 = ProvidedStaticParameter("X", typeof<int>, 0)
+        let p2 = ProvidedStaticParameter("Y", typeof<string>, "default")
+        t.DefineStaticParameters([p1; p2], fun typeName _args ->
+            ProvidedTypeDefinition(asm, "Test.Warning", typeName, Some typeof<obj>))
+        let w = warnings |> Seq.tryFind (fun msg -> msg.Contains("AllOptionalType") && msg.Contains("optional"))
+        Assert.True(w.IsSome, sprintf "Expected all-optional-params warning, got: %A" (warnings |> Seq.toList))
+    finally
+        ProvidedTypeDefinition.Logger := prevLogger
+
+[<Fact>]
+let ``DefineStaticParameters does not warn when at least one parameter has no default``() =
+    let warnings = System.Collections.Generic.List<string>()
+    let prevLogger = !ProvidedTypeDefinition.Logger
+    ProvidedTypeDefinition.Logger := Some warnings.Add
+    try
+        let asm = Assembly.GetExecutingAssembly()
+        let t = ProvidedTypeDefinition(asm, "Test.Warning", "MixedParamsType", Some typeof<obj>)
+        warnings.Clear()
+        let required = ProvidedStaticParameter("Required", typeof<int>)
+        let optional = ProvidedStaticParameter("Optional", typeof<string>, "default")
+        t.DefineStaticParameters([required; optional], fun typeName _args ->
+            ProvidedTypeDefinition(asm, "Test.Warning", typeName, Some typeof<obj>))
+        let w = warnings |> Seq.tryFind (fun msg -> msg.Contains("MixedParamsType") && msg.Contains("optional"))
+        Assert.True(w.IsNone, sprintf "No all-optional warning expected for mixed params, got: %A" (warnings |> Seq.toList))
+    finally
+        ProvidedTypeDefinition.Logger := prevLogger
+
+[<Fact>]
+let ``DefineStaticParameters does not warn when there are no static parameters``() =
+    let warnings = System.Collections.Generic.List<string>()
+    let prevLogger = !ProvidedTypeDefinition.Logger
+    ProvidedTypeDefinition.Logger := Some warnings.Add
+    try
+        let asm = Assembly.GetExecutingAssembly()
+        let t = ProvidedTypeDefinition(asm, "Test.Warning", "NoParamsType", Some typeof<obj>)
+        warnings.Clear()
+        t.DefineStaticParameters([], fun typeName _args ->
+            ProvidedTypeDefinition(asm, "Test.Warning", typeName, Some typeof<obj>))
+        let w = warnings |> Seq.tryFind (fun msg -> msg.Contains("NoParamsType") && msg.Contains("optional"))
+        Assert.True(w.IsNone, "No all-optional warning expected for empty parameter list")
+    finally
+        ProvidedTypeDefinition.Logger := prevLogger

tests/GenerativeEnumsProvisionTests.fs

@@ -145,6 +145,12 @@ let makeAssembly (tp: TypeProviderForNamespaces) =
     let assemContents = (tp :> ITypeProvider).GetGeneratedAssemblyContents(providedType.Assembly)
     Assembly.Load assemContents
 
+let makeTargetAssembly (tp: TypeProviderForNamespaces) =
+    let ns = tp.Namespaces.[0]
+    let providedType = ns.GetTypes().[0]
+    let assemContents = (tp :> ITypeProvider).GetGeneratedAssemblyContents(providedType.Assembly)
+    tp.TargetContext.ReadRelatedAssembly(assemContents)
+
 [<Fact>]
 let ``Generative enum with byte underlying type is generated correctly``() =
     let runtimeAssemblyRefs = Targets.DotNetStandard20FSharpRefs()
@@ -185,3 +191,41 @@ let ``Generative enum with int64 underlying type is generated correctly``() =
     let values = Enum.GetValues(int64Enum) |> Seq.cast<int64> |> Seq.zip (Enum.GetNames(int64Enum)) |> Seq.toList
     Assert.Equal<(string * int64) list>([("Zero", 0L); ("One", 1L); ("BigVal", 3000000000L)], values)
 
+// ---------------------------------------------------------------------------
+// Tests that read enums back via TargetContext.ReadRelatedAssembly (the IL binary
+// reader path through TargetTypeDefinition), exercising GetEnumUnderlyingType()
+// on non-Int32 enum types via the target context.
+// ---------------------------------------------------------------------------
+
+[<Fact>]
+let ``Byte enum underlying type is correct when read via target context (ReadRelatedAssembly)``() =
+    let runtimeAssemblyRefs = Targets.DotNetStandard20FSharpRefs()
+    let runtimeAssembly = runtimeAssemblyRefs.[0]
+    let cfg = Testing.MakeSimulatedTypeProviderConfig (__SOURCE_DIRECTORY__, runtimeAssembly, runtimeAssemblyRefs)
+    let tp = GenerativeByteEnumsProvider(cfg) :> TypeProviderForNamespaces
+    let targetAssem = makeTargetAssembly tp
+
+    let container = targetAssem.GetType("ByteEnums.Provided.Container")
+    Assert.NotNull(container)
+
+    let byteEnum = container.GetNestedType("ByteEnum", BindingFlags.Public ||| BindingFlags.NonPublic)
+    Assert.NotNull(byteEnum)
+    Assert.True(byteEnum.IsEnum, "ByteEnum should be recognised as an enum via target context")
+    // GetEnumUnderlyingType on a TargetTypeDefinition reads the "value__" field type from IL.
+    Assert.Equal("System.Byte", byteEnum.GetEnumUnderlyingType().FullName)
+
+[<Fact>]
+let ``Int64 enum underlying type is correct when read via target context (ReadRelatedAssembly)``() =
+    let runtimeAssemblyRefs = Targets.DotNetStandard20FSharpRefs()
+    let runtimeAssembly = runtimeAssemblyRefs.[0]
+    let cfg = Testing.MakeSimulatedTypeProviderConfig (__SOURCE_DIRECTORY__, runtimeAssembly, runtimeAssemblyRefs)
+    let tp = GenerativeInt64EnumsProvider(cfg) :> TypeProviderForNamespaces
+    let targetAssem = makeTargetAssembly tp
+
+    let container = targetAssem.GetType("Int64Enums.Provided.Container")
+    Assert.NotNull(container)
+
+    let int64Enum = container.GetNestedType("Int64Enum", BindingFlags.Public ||| BindingFlags.NonPublic)
+    Assert.NotNull(int64Enum)
+    Assert.True(int64Enum.IsEnum, "Int64Enum should be recognised as an enum via target context")
+    Assert.Equal("System.Int64", int64Enum.GetEnumUnderlyingType().FullName)