From 268ee14d4520467c4220a5cc2ed0b305d22b6392 Mon Sep 17 00:00:00 2001 From: Jeet Dekivadia Date: Fri, 29 May 2026 22:53:39 -0700 Subject: [PATCH 1/5] Fix reflection correctness edge cases --- ApiSurface/ApiMember.fs | 2 +- ApiSurface/ApiSurface.fs | 2 +- ApiSurface/DocCoverage.fs | 16 +++++++++++----- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/ApiSurface/ApiMember.fs b/ApiSurface/ApiMember.fs index d4f5c71..4f885b1 100644 --- a/ApiSurface/ApiMember.fs +++ b/ApiSurface/ApiMember.fs @@ -68,7 +68,7 @@ module ApiMember = let typeString = fieldType |> Type.toFullName if fieldInfo.IsLiteral then - let value = m.DeclaringType.GetField(m.Name).GetValue null |> string + let value = fieldInfo.GetValue null |> string // Don't print `= ` for empty strings, as many editors/Git hooks trim trailing whitespace if value = "" then diff --git a/ApiSurface/ApiSurface.fs b/ApiSurface/ApiSurface.fs index 89f93a2..e0a88c0 100644 --- a/ApiSurface/ApiSurface.fs +++ b/ApiSurface/ApiSurface.fs @@ -42,7 +42,7 @@ module ApiSurface = let frameworkNumber = Regex(@"^\.NET ([0-9]+)\.").Match desc if frameworkNumber.Success then - sprintf "SurfaceBaseline-Net%s.txt" frameworkNumber.Groups.[0].Value + sprintf "SurfaceBaseline-Net%s.txt" frameworkNumber.Groups.[1].Value else failwithf "Unknown runtime framework: %s" desc diff --git a/ApiSurface/DocCoverage.fs b/ApiSurface/DocCoverage.fs index 0211fb4..8bd211e 100644 --- a/ApiSurface/DocCoverage.fs +++ b/ApiSurface/DocCoverage.fs @@ -27,13 +27,19 @@ module DocCoverage = let isPublic (memberInfo : MemberInfo) : bool = (isNull memberInfo.DeclaringType || memberInfo.DeclaringType.IsVisible) && match memberInfo.MemberType with - | MemberTypes.Method - | MemberTypes.Constructor -> let i = memberInfo :?> MethodInfo in i.IsPublic - | MemberTypes.Event -> let i = memberInfo :?> EventInfo in i.AddMethod.IsPublic + | MemberTypes.Method -> let i = memberInfo :?> MethodInfo in i.IsPublic + | MemberTypes.Constructor -> let i = memberInfo :?> ConstructorInfo in i.IsPublic + | MemberTypes.Event -> + let i = memberInfo :?> EventInfo + (not (isNull i.AddMethod) && i.AddMethod.IsPublic) + || (not (isNull i.RemoveMethod) && i.RemoveMethod.IsPublic) | MemberTypes.Field -> let i = memberInfo :?> FieldInfo in i.IsPublic | MemberTypes.TypeInfo - | MemberTypes.NestedType -> let i = memberInfo :?> TypeInfo in i.IsPublic - | MemberTypes.Property -> let i = memberInfo :?> PropertyInfo in i.GetMethod.IsPublic + | MemberTypes.NestedType -> let i = memberInfo :?> Type in i.IsVisible + | MemberTypes.Property -> + let i = memberInfo :?> PropertyInfo + (not (isNull i.GetMethod) && i.GetMethod.IsPublic) + || (not (isNull i.SetMethod) && i.SetMethod.IsPublic) | memberType -> failwithf "Unrecognised MemberType: %O" memberType let paramInfoToString (pi : ParameterInfo) : string = From 92242b440ed1a676932d97493d5b046b9ce0e660 Mon Sep 17 00:00:00 2001 From: Jeet Dekivadia Date: Sat, 30 May 2026 03:36:53 -0700 Subject: [PATCH 2/5] Add reflection correctness regressions Signed-off-by: Jeet Dekivadia --- ApiSurface/ApiSurface.fs | 7 +- ApiSurface/DocCoverage.fs | 2 + ApiSurface/Test/ApiSurface.Test.fsproj | 1 + ApiSurface/Test/TestApiSurface.fs | 6 ++ ApiSurface/Test/TestReflectionCorrectness.fs | 96 ++++++++++++++++++++ 5 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 ApiSurface/Test/TestReflectionCorrectness.fs diff --git a/ApiSurface/ApiSurface.fs b/ApiSurface/ApiSurface.fs index e0a88c0..24ebd0f 100644 --- a/ApiSurface/ApiSurface.fs +++ b/ApiSurface/ApiSurface.fs @@ -30,9 +30,7 @@ module ApiSurface = /// In the rare case that you have several different baselines depending on what framework you are running under, /// you can use a more specific name for your baseline files. - let private frameworkBaselineFile = - let desc = RuntimeInformation.FrameworkDescription - + let internal frameworkBaselineFileFor (desc : string) = if desc.StartsWith (".NET Core", StringComparison.Ordinal) then "SurfaceBaseline-NetCore.txt" elif desc.StartsWith (".NET Framework", StringComparison.Ordinal) then @@ -46,6 +44,9 @@ module ApiSurface = else failwithf "Unknown runtime framework: %s" desc + let private frameworkBaselineFile = + RuntimeInformation.FrameworkDescription |> frameworkBaselineFileFor + let surfaces (assembly : Assembly) = assembly.GetManifestResourceNames () |> Seq.choose Option.ofObj diff --git a/ApiSurface/DocCoverage.fs b/ApiSurface/DocCoverage.fs index 8bd211e..22c4ddb 100644 --- a/ApiSurface/DocCoverage.fs +++ b/ApiSurface/DocCoverage.fs @@ -31,6 +31,7 @@ module DocCoverage = | MemberTypes.Constructor -> let i = memberInfo :?> ConstructorInfo in i.IsPublic | MemberTypes.Event -> let i = memberInfo :?> EventInfo + (not (isNull i.AddMethod) && i.AddMethod.IsPublic) || (not (isNull i.RemoveMethod) && i.RemoveMethod.IsPublic) | MemberTypes.Field -> let i = memberInfo :?> FieldInfo in i.IsPublic @@ -38,6 +39,7 @@ module DocCoverage = | MemberTypes.NestedType -> let i = memberInfo :?> Type in i.IsVisible | MemberTypes.Property -> let i = memberInfo :?> PropertyInfo + (not (isNull i.GetMethod) && i.GetMethod.IsPublic) || (not (isNull i.SetMethod) && i.SetMethod.IsPublic) | memberType -> failwithf "Unrecognised MemberType: %O" memberType diff --git a/ApiSurface/Test/ApiSurface.Test.fsproj b/ApiSurface/Test/ApiSurface.Test.fsproj index 783f233..44068f5 100644 --- a/ApiSurface/Test/ApiSurface.Test.fsproj +++ b/ApiSurface/Test/ApiSurface.Test.fsproj @@ -9,6 +9,7 @@ + diff --git a/ApiSurface/Test/TestApiSurface.fs b/ApiSurface/Test/TestApiSurface.fs index 6d59afb..8f73eda 100644 --- a/ApiSurface/Test/TestApiSurface.fs +++ b/ApiSurface/Test/TestApiSurface.fs @@ -89,3 +89,9 @@ module TestApiSurface = let expected = Sample.publicSurface |> Set.add "Bar" |> Set.add "Foo" actual |> shouldEqual expected + + [] + [] + [] + let ``Test framework baseline filename`` desc expected = + desc |> ApiSurface.frameworkBaselineFileFor |> shouldEqual expected diff --git a/ApiSurface/Test/TestReflectionCorrectness.fs b/ApiSurface/Test/TestReflectionCorrectness.fs new file mode 100644 index 0000000..f0af2d4 --- /dev/null +++ b/ApiSurface/Test/TestReflectionCorrectness.fs @@ -0,0 +1,96 @@ +namespace ApiSurface.Test + +open System +open System.Reflection +open System.Reflection.Emit +open NUnit.Framework +open FsUnitTyped +open ApiSurface + +[] +module TestReflectionCorrectness = + + let private reflectionFixture = + lazy ( + let assemblyName = AssemblyName "ApiSurface.Test.ReflectionFixture" + + let assembly = + AssemblyBuilder.DefineDynamicAssembly (assemblyName, AssemblyBuilderAccess.Run) + + let moduleBuilder = assembly.DefineDynamicModule assemblyName.Name + let typeBuilder = moduleBuilder.DefineType ("ReflectionFixture", TypeAttributes.Public) + + typeBuilder.DefineDefaultConstructor MethodAttributes.Public |> ignore + + let emitReturn (methodBuilder : MethodBuilder) = + methodBuilder.GetILGenerator().Emit OpCodes.Ret + + let propertySetter = + typeBuilder.DefineMethod ( + "set_WriteOnly", + MethodAttributes.Public ||| MethodAttributes.SpecialName ||| MethodAttributes.HideBySig, + typeof, + [| typeof |] + ) + + emitReturn propertySetter + + let property = typeBuilder.DefineProperty ("WriteOnly", PropertyAttributes.None, typeof, Type.EmptyTypes) + property.SetSetMethod propertySetter + + let eventRemover = + typeBuilder.DefineMethod ( + "remove_RemoveOnly", + MethodAttributes.Public ||| MethodAttributes.SpecialName ||| MethodAttributes.HideBySig, + typeof, + [| typeof |] + ) + + emitReturn eventRemover + + let event = typeBuilder.DefineEvent ("RemoveOnly", EventAttributes.None, typeof) + event.SetRemoveOnMethod eventRemover + + let literal = + typeBuilder.DefineField ( + "HiddenLiteral", + typeof, + FieldAttributes.Private ||| FieldAttributes.Static ||| FieldAttributes.Literal + ) + + literal.SetConstant 42 + + typeBuilder.DefineNestedType ("Nested", TypeAttributes.NestedPublic).CreateType () |> ignore + typeBuilder.CreateType () + ) + + [] + let ``isPublic handles constructors`` () = + reflectionFixture.Value.GetConstructors().[0] + |> DocCoverage.isPublic + |> shouldEqual true + + [] + let ``isPublic handles setter-only properties`` () = + reflectionFixture.Value.GetProperty "WriteOnly" + |> DocCoverage.isPublic + |> shouldEqual true + + [] + let ``isPublic handles remove-only events`` () = + reflectionFixture.Value.GetEvent "RemoveOnly" + |> DocCoverage.isPublic + |> shouldEqual true + + [] + let ``isPublic handles nested public types`` () = + reflectionFixture.Value.GetNestedType "Nested" + |> DocCoverage.isPublic + |> shouldEqual true + + [] + let ``print handles private literals`` () = + reflectionFixture.Value.GetField ("HiddenLiteral", BindingFlags.NonPublic ||| BindingFlags.Static) + |> ApiMember.ofMemberInfo + |> ApiMember.print + |> shouldEqual "ReflectionFixture.HiddenLiteral [static field]: int = 42" From 57095ef37e3bef52733146149b0f8c3e2927589d Mon Sep 17 00:00:00 2001 From: Jeet Dekivadia Date: Sat, 30 May 2026 03:39:12 -0700 Subject: [PATCH 3/5] Expose reflection helpers to focused tests Signed-off-by: Jeet Dekivadia --- ApiSurface/ApiSurface.fsi | 2 + ApiSurface/DocCoverage.fsi | 2 + ApiSurface/Test/TestReflectionCorrectness.fs | 89 +++++++++++--------- 3 files changed, 54 insertions(+), 39 deletions(-) diff --git a/ApiSurface/ApiSurface.fsi b/ApiSurface/ApiSurface.fsi index 9fc70dc..a239862 100644 --- a/ApiSurface/ApiSurface.fsi +++ b/ApiSurface/ApiSurface.fsi @@ -16,6 +16,8 @@ type internal Version = [] module ApiSurface = + val internal frameworkBaselineFileFor : string -> string + /// Read the SurfaceBaseline.txt embedded resource from the given assembly. [] val ofAssemblyBaseline : Assembly -> ApiSurface diff --git a/ApiSurface/DocCoverage.fsi b/ApiSurface/DocCoverage.fsi index 8aa51c6..cc618d3 100644 --- a/ApiSurface/DocCoverage.fsi +++ b/ApiSurface/DocCoverage.fsi @@ -9,6 +9,8 @@ type DocCoverage [] module DocCoverage = + val internal isPublic : MemberInfo -> bool + /// Map the exposed types and members of an assembly into a list of /// member names expected to be present in the corresponding .XML /// documentation file. diff --git a/ApiSurface/Test/TestReflectionCorrectness.fs b/ApiSurface/Test/TestReflectionCorrectness.fs index f0af2d4..a1cd2e2 100644 --- a/ApiSurface/Test/TestReflectionCorrectness.fs +++ b/ApiSurface/Test/TestReflectionCorrectness.fs @@ -11,58 +11,69 @@ open ApiSurface module TestReflectionCorrectness = let private reflectionFixture = - lazy ( - let assemblyName = AssemblyName "ApiSurface.Test.ReflectionFixture" + lazy + (let assemblyName = AssemblyName "ApiSurface.Test.ReflectionFixture" - let assembly = - AssemblyBuilder.DefineDynamicAssembly (assemblyName, AssemblyBuilderAccess.Run) + let assembly = + AssemblyBuilder.DefineDynamicAssembly (assemblyName, AssemblyBuilderAccess.Run) - let moduleBuilder = assembly.DefineDynamicModule assemblyName.Name - let typeBuilder = moduleBuilder.DefineType ("ReflectionFixture", TypeAttributes.Public) + let moduleBuilder = assembly.DefineDynamicModule assemblyName.Name - typeBuilder.DefineDefaultConstructor MethodAttributes.Public |> ignore + let typeBuilder = + moduleBuilder.DefineType ("ReflectionFixture", TypeAttributes.Public) - let emitReturn (methodBuilder : MethodBuilder) = - methodBuilder.GetILGenerator().Emit OpCodes.Ret + typeBuilder.DefineDefaultConstructor MethodAttributes.Public |> ignore - let propertySetter = - typeBuilder.DefineMethod ( - "set_WriteOnly", - MethodAttributes.Public ||| MethodAttributes.SpecialName ||| MethodAttributes.HideBySig, - typeof, - [| typeof |] - ) + let emitReturn (methodBuilder : MethodBuilder) = + methodBuilder.GetILGenerator().Emit OpCodes.Ret - emitReturn propertySetter + let propertySetter = + typeBuilder.DefineMethod ( + "set_WriteOnly", + MethodAttributes.Public + ||| MethodAttributes.SpecialName + ||| MethodAttributes.HideBySig, + typeof, + [| typeof |] + ) - let property = typeBuilder.DefineProperty ("WriteOnly", PropertyAttributes.None, typeof, Type.EmptyTypes) - property.SetSetMethod propertySetter + emitReturn propertySetter - let eventRemover = - typeBuilder.DefineMethod ( - "remove_RemoveOnly", - MethodAttributes.Public ||| MethodAttributes.SpecialName ||| MethodAttributes.HideBySig, - typeof, - [| typeof |] - ) + let property = + typeBuilder.DefineProperty ("WriteOnly", PropertyAttributes.None, typeof, Type.EmptyTypes) - emitReturn eventRemover + property.SetSetMethod propertySetter - let event = typeBuilder.DefineEvent ("RemoveOnly", EventAttributes.None, typeof) - event.SetRemoveOnMethod eventRemover + let eventRemover = + typeBuilder.DefineMethod ( + "remove_RemoveOnly", + MethodAttributes.Public + ||| MethodAttributes.SpecialName + ||| MethodAttributes.HideBySig, + typeof, + [| typeof |] + ) - let literal = - typeBuilder.DefineField ( - "HiddenLiteral", - typeof, - FieldAttributes.Private ||| FieldAttributes.Static ||| FieldAttributes.Literal - ) + emitReturn eventRemover - literal.SetConstant 42 + let event = + typeBuilder.DefineEvent ("RemoveOnly", EventAttributes.None, typeof) - typeBuilder.DefineNestedType ("Nested", TypeAttributes.NestedPublic).CreateType () |> ignore - typeBuilder.CreateType () - ) + event.SetRemoveOnMethod eventRemover + + let literal = + typeBuilder.DefineField ( + "HiddenLiteral", + typeof, + FieldAttributes.Private ||| FieldAttributes.Static ||| FieldAttributes.Literal + ) + + literal.SetConstant 42 + + typeBuilder.DefineNestedType ("Nested", TypeAttributes.NestedPublic).CreateTypeInfo () + |> ignore + + typeBuilder.CreateTypeInfo().AsType()) [] let ``isPublic handles constructors`` () = From de2e62ebe9114e43af5b17534b2098b4b2cee3db Mon Sep 17 00:00:00 2001 From: Jeet Dekivadia Date: Sat, 30 May 2026 03:40:39 -0700 Subject: [PATCH 4/5] Build nested reflection fixture explicitly Signed-off-by: Jeet Dekivadia --- ApiSurface/Test/TestReflectionCorrectness.fs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ApiSurface/Test/TestReflectionCorrectness.fs b/ApiSurface/Test/TestReflectionCorrectness.fs index a1cd2e2..0fc3a74 100644 --- a/ApiSurface/Test/TestReflectionCorrectness.fs +++ b/ApiSurface/Test/TestReflectionCorrectness.fs @@ -70,10 +70,10 @@ module TestReflectionCorrectness = literal.SetConstant 42 - typeBuilder.DefineNestedType ("Nested", TypeAttributes.NestedPublic).CreateTypeInfo () - |> ignore + let nestedType = typeBuilder.DefineNestedType ("Nested", TypeAttributes.NestedPublic) + nestedType.CreateTypeInfo () |> ignore - typeBuilder.CreateTypeInfo().AsType()) + typeBuilder.CreateTypeInfo().AsType ()) [] let ``isPublic handles constructors`` () = From 3f4922b3bd9b67b227ceddf915437217b3b1d8f8 Mon Sep 17 00:00:00 2001 From: Jeet Dekivadia Date: Sat, 30 May 2026 03:44:52 -0700 Subject: [PATCH 5/5] Prefer public reflection paths during coverage comparison Signed-off-by: Jeet Dekivadia --- ApiSurface/DocCoverage.fs | 5 ++++- ApiSurface/Test/TestReflectionCorrectness.fs | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/ApiSurface/DocCoverage.fs b/ApiSurface/DocCoverage.fs index 22c4ddb..95c53d3 100644 --- a/ApiSurface/DocCoverage.fs +++ b/ApiSurface/DocCoverage.fs @@ -271,7 +271,10 @@ module DocCoverage = let publicMembers = publicMembers |> Seq.map (fun (n, c, _) -> n, c) |> Set.ofSeq let nonPublicMembers = - nonPublicMembers |> Seq.map (fun (n, c, _) -> n, c) |> Set.ofSeq + nonPublicMembers + |> Seq.map (fun (n, c, _) -> n, c) + |> Set.ofSeq + |> fun members -> Set.difference members publicMembers DocCoverage (Path.GetFileName assembly.Location, Members.MixedAccess (publicMembers, nonPublicMembers)) diff --git a/ApiSurface/Test/TestReflectionCorrectness.fs b/ApiSurface/Test/TestReflectionCorrectness.fs index 0fc3a74..76dfa89 100644 --- a/ApiSurface/Test/TestReflectionCorrectness.fs +++ b/ApiSurface/Test/TestReflectionCorrectness.fs @@ -70,7 +70,9 @@ module TestReflectionCorrectness = literal.SetConstant 42 - let nestedType = typeBuilder.DefineNestedType ("Nested", TypeAttributes.NestedPublic) + let nestedType = + typeBuilder.DefineNestedType ("Nested", TypeAttributes.NestedPublic) + nestedType.CreateTypeInfo () |> ignore typeBuilder.CreateTypeInfo().AsType ())