From a9261cce48e06e1fccb30dcc240d1e8569e46c66 Mon Sep 17 00:00:00 2001 From: TIHan Date: Wed, 14 Aug 2019 17:27:58 -0700 Subject: [PATCH 1/6] Open static classes is now preview. Added fix for overloads --- src/fsharp/LanguageFeatures.fs | 2 +- src/fsharp/NameResolution.fs | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/fsharp/LanguageFeatures.fs b/src/fsharp/LanguageFeatures.fs index c4c46e4b666..5c2df791282 100644 --- a/src/fsharp/LanguageFeatures.fs +++ b/src/fsharp/LanguageFeatures.fs @@ -54,7 +54,7 @@ type LanguageVersion (specifiedVersion) = LanguageFeature.RelaxWhitespace, languageVersion47 LanguageFeature.NameOf, languageVersion47 LanguageFeature.ImplicitYield, languageVersion47 - LanguageFeature.OpenStaticClasses, languageVersion47 + LanguageFeature.OpenStaticClasses, previewVersion |] let specified = diff --git a/src/fsharp/NameResolution.fs b/src/fsharp/NameResolution.fs index 6c6cac0dd63..0bf8983799a 100644 --- a/src/fsharp/NameResolution.fs +++ b/src/fsharp/NameResolution.fs @@ -782,7 +782,16 @@ let AddStaticContentOfTyconRefToNameEnv (g:TcGlobals) (amap: Import.ImportMap) m for propInfo in propInfos do yield KeyValuePair(propName , Item.Property(propName,[propInfo])) |] - { nenv with eUnqualifiedItems = nenv.eUnqualifiedItems.AddAndMarkAsCollapsible items } + { nenv with + eUnqualifiedItems = + (nenv.eUnqualifiedItems, items) + ||> Array.fold (fun x (KeyValue(k, v)) -> + match x.TryGetValue k, v with + | (true, Item.MethodGroup (_, meths, None)), Item.MethodGroup (_, mostRecentMeths, None) -> + x.Add(k, Item.MethodGroup (k, mostRecentMeths @ meths, None)) + | _ -> + x.Add(k, v)) + } else nenv From dbf1bf628c13b5c670518bc9afad7b7db847245b Mon Sep 17 00:00:00 2001 From: TIHan Date: Thu, 15 Aug 2019 15:14:05 -0700 Subject: [PATCH 2/6] Fixing tests --- .../Compiler/Language/OpenStaticClasses.fs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/fsharp/Compiler/Language/OpenStaticClasses.fs b/tests/fsharp/Compiler/Language/OpenStaticClasses.fs index bd036d3a109..65ca748dfd2 100644 --- a/tests/fsharp/Compiler/Language/OpenStaticClasses.fs +++ b/tests/fsharp/Compiler/Language/OpenStaticClasses.fs @@ -8,7 +8,7 @@ open NUnit.Framework (* Tests in this file evaluate whether the language supports accessing functions on static classes using open - The feature was added in FSharp4.7, the test cases ensure that the original errors are reproduced when the langversion:4.6 is specified + The feature was added in preview, the test cases ensure that the original errors are reproduced when the langversion:4.6 is specified *) [] @@ -49,9 +49,9 @@ module OpenSystemMathOnce = |] [] - let ``OpenStaticClassesTests - OpenSystemMathOnce - langversion:v4.7`` () = + let ``OpenStaticClassesTests - OpenSystemMathOnce - langversion:preview`` () = CompilerAssert.TypeCheckWithErrorsAndOptions - [| "--langversion:4.7" |] + [| "--langversion:preview" |] (baseModule + """ module OpenSystemMathOnce = @@ -79,9 +79,9 @@ module OpenSystemMathTwice = |] [] - let ``OpenStaticClassesTests - OpenSystemMathTwice - langversion:v4.7`` () = + let ``OpenStaticClassesTests - OpenSystemMathTwice - langversion:preview`` () = CompilerAssert.TypeCheckWithErrorsAndOptions - [| "--langversion:4.7" |] + [| "--langversion:preview" |] (baseModule + """ module OpenSystemMathOnce = @@ -106,9 +106,9 @@ module OpenMyMathOnce = |] [] - let ``OpenStaticClassesTests - OpenMyMathOnce - langversion:v4.7`` () = + let ``OpenStaticClassesTests - OpenMyMathOnce - langversion:preview`` () = CompilerAssert.TypeCheckWithErrorsAndOptions - [| "--langversion:4.7" |] + [| "--langversion:preview" |] (baseModule + """ module OpenMyMathOnce = @@ -132,9 +132,9 @@ module DontOpenAutoMath = |] [] - let ``OpenStaticClassesTests - DontOpenAutoMath - langversion:v4.7`` () = + let ``OpenStaticClassesTests - DontOpenAutoMath - langversion:preview`` () = CompilerAssert.TypeCheckWithErrorsAndOptions - [| "--langversion:4.7" |] + [| "--langversion:preview" |] (baseModule + """ module DontOpenAutoMath = @@ -160,9 +160,9 @@ module OpenAutoMath = |] [] - let ``OpenStaticClassesTests - OpenAutoMath - langversion:v4.7`` () = + let ``OpenStaticClassesTests - OpenAutoMath - langversion:preview`` () = CompilerAssert.TypeCheckWithErrorsAndOptions - [| "--langversion:4.7" |] + [| "--langversion:preview" |] (baseModule + """ module OpenAutoMath = open AutoOpenMyMath From 9efac5337de8438182ab1f0d2278d8a6a5bbe6b4 Mon Sep 17 00:00:00 2001 From: TIHan Date: Thu, 15 Aug 2019 16:55:03 -0700 Subject: [PATCH 3/6] Fixed opening multiple times --- src/fsharp/NameResolution.fs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/fsharp/NameResolution.fs b/src/fsharp/NameResolution.fs index 0bf8983799a..d6a699c6e4a 100644 --- a/src/fsharp/NameResolution.fs +++ b/src/fsharp/NameResolution.fs @@ -787,8 +787,17 @@ let AddStaticContentOfTyconRefToNameEnv (g:TcGlobals) (amap: Import.ImportMap) m (nenv.eUnqualifiedItems, items) ||> Array.fold (fun x (KeyValue(k, v)) -> match x.TryGetValue k, v with - | (true, Item.MethodGroup (_, meths, None)), Item.MethodGroup (_, mostRecentMeths, None) -> - x.Add(k, Item.MethodGroup (k, mostRecentMeths @ meths, None)) + | (true, Item.MethodGroup (_, minfosCurrent, None)), Item.MethodGroup (_, minfosNew, None) -> + let minfos = + (minfosNew, minfosCurrent) + ||> List.fold (fun minfos minfo1 -> + // Do not add the same exact same method. + if minfos |> List.exists (fun minfo2 -> typeEquiv g minfo1.ApparentEnclosingAppType minfo2.ApparentEnclosingAppType && MethInfosEquivByNameAndSig Erasure.EraseNone false g amap m minfo1 minfo2) then + minfos + else + minfo1 :: minfos + ) + x.Add(k, Item.MethodGroup (k, minfos, None)) | _ -> x.Add(k, v)) } From 43d47f73988703107d028dff1f7838b856b1e5d1 Mon Sep 17 00:00:00 2001 From: TIHan Date: Thu, 15 Aug 2019 18:18:04 -0700 Subject: [PATCH 4/6] Shadow methods --- src/fsharp/NameResolution.fs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/fsharp/NameResolution.fs b/src/fsharp/NameResolution.fs index d6a699c6e4a..aee7990df5b 100644 --- a/src/fsharp/NameResolution.fs +++ b/src/fsharp/NameResolution.fs @@ -789,10 +789,10 @@ let AddStaticContentOfTyconRefToNameEnv (g:TcGlobals) (amap: Import.ImportMap) m match x.TryGetValue k, v with | (true, Item.MethodGroup (_, minfosCurrent, None)), Item.MethodGroup (_, minfosNew, None) -> let minfos = - (minfosNew, minfosCurrent) + (minfosCurrent, minfosNew) ||> List.fold (fun minfos minfo1 -> - // Do not add the same exact same method. - if minfos |> List.exists (fun minfo2 -> typeEquiv g minfo1.ApparentEnclosingAppType minfo2.ApparentEnclosingAppType && MethInfosEquivByNameAndSig Erasure.EraseNone false g amap m minfo1 minfo2) then + // Shadow methods with the same signature. + if minfos |> List.exists (fun minfo2 -> MethInfosEquivByNameAndSig Erasure.EraseAll true g amap m minfo1 minfo2) then minfos else minfo1 :: minfos From ab0916f5c040582d02e0e2c460c7e23109c85cb7 Mon Sep 17 00:00:00 2001 From: TIHan Date: Thu, 15 Aug 2019 22:03:00 -0700 Subject: [PATCH 5/6] few fixes --- src/fsharp/NameResolution.fs | 99 +++++++++++++++++++++++++----------- 1 file changed, 68 insertions(+), 31 deletions(-) diff --git a/src/fsharp/NameResolution.fs b/src/fsharp/NameResolution.fs index aee7990df5b..020c9939265 100644 --- a/src/fsharp/NameResolution.fs +++ b/src/fsharp/NameResolution.fs @@ -763,43 +763,80 @@ let AddStaticContentOfTyconRefToNameEnv (g:TcGlobals) (amap: Import.ImportMap) m if amap.g.langVersion.SupportsFeature LanguageFeature.OpenStaticClasses then let ty = generalizedTyconRef tcref let infoReader = InfoReader(g,amap) + + let getMethItems () = + [ let methGroups = + AllMethInfosOfTypeInScope ResultCollectionSettings.AllResults infoReader nenv None AccessorDomain.AccessibleFromSomeFSharpCode PreferOverrides m ty + |> List.groupBy (fun m -> m.LogicalName) + + for (methName, methGroup) in methGroups do + let methGroup = methGroup |> List.filter (fun m -> not m.IsInstance && not m.IsClassConstructor) + if not methGroup.IsEmpty then + yield KeyValuePair(methName, Item.MethodGroup(methName, methGroup, None)) + ] + + let getPropItems () = + [ let propInfos = + AllPropInfosOfTypeInScope ResultCollectionSettings.AllResults infoReader nenv None AccessorDomain.AccessibleFromSomeFSharpCode PreferOverrides m ty + |> List.groupBy (fun m -> m.PropertyName) + + for (propName, propInfos) in propInfos do + let propInfos = propInfos |> List.filter (fun m -> m.IsStatic) + for propInfo in propInfos do + yield KeyValuePair(propName , Item.Property(propName,[propInfo])) + ] + let items = - [| let methGroups = - AllMethInfosOfTypeInScope ResultCollectionSettings.AllResults infoReader nenv None AccessorDomain.AccessibleFromSomeFSharpCode PreferOverrides m ty - |> List.groupBy (fun m -> m.LogicalName) - - for (methName, methGroup) in methGroups do - let methGroup = methGroup |> List.filter (fun m -> not m.IsInstance && not m.IsClassConstructor) - if not methGroup.IsEmpty then - yield KeyValuePair(methName, Item.MethodGroup(methName, methGroup, None)) - - let propInfos = - AllPropInfosOfTypeInScope ResultCollectionSettings.AllResults infoReader nenv None AccessorDomain.AccessibleFromSomeFSharpCode PreferOverrides m ty - |> List.groupBy (fun m -> m.PropertyName) - - for (propName, propInfos) in propInfos do - let propInfos = propInfos |> List.filter (fun m -> m.IsStatic) - for propInfo in propInfos do - yield KeyValuePair(propName , Item.Property(propName,[propInfo])) |] + // This is to handle the current behavior with qualified extension members: + // Extension member properties will hide all extension methods that share the same name. + [| + let propItems = getPropItems () + let methItems = getMethItems () + + let itemGroups = + propItems @ methItems + |> List.groupBy (fun pair -> pair.Key) + + for (_, items) in itemGroups do + let sortedItems = + items + |> List.sortBy (fun pair -> + match pair.Value with + | Item.Property (_, [propInfo]) -> not propInfo.IsExtensionMember + | Item.MethodGroup (_, methGroup, _) -> methGroup |> List.exists (fun x -> not x.IsExtensionMember) + | _ -> false) + match sortedItems with + | item :: _ -> yield item + | _ -> () + |] { nenv with eUnqualifiedItems = (nenv.eUnqualifiedItems, items) ||> Array.fold (fun x (KeyValue(k, v)) -> - match x.TryGetValue k, v with - | (true, Item.MethodGroup (_, minfosCurrent, None)), Item.MethodGroup (_, minfosNew, None) -> - let minfos = - (minfosCurrent, minfosNew) - ||> List.fold (fun minfos minfo1 -> - // Shadow methods with the same signature. - if minfos |> List.exists (fun minfo2 -> MethInfosEquivByNameAndSig Erasure.EraseAll true g amap m minfo1 minfo2) then - minfos - else - minfo1 :: minfos - ) - x.Add(k, Item.MethodGroup (k, minfos, None)) - | _ -> - x.Add(k, v)) + match x.TryGetValue k with + | true, existingItem -> + match existingItem, v with + + // Combine methods groups. + | Item.MethodGroup (_, minfosCurrent, None), Item.MethodGroup (_, minfosNew, None) -> + let minfos = + (minfosNew, minfosCurrent) + ||> List.fold (fun minfos minfo1 -> + // Shadow methods with the same signature. + if minfos |> List.exists (fun minfo2 -> MethInfosEquivByNameAndSig Erasure.EraseAll true g amap m minfo1 minfo2) then + minfos + else + minfo1 :: minfos + ) + x.Add(k, Item.MethodGroup (k, minfos, None)) + + // Methods will hide properties, events, and fields with the same name. + | Item.MethodGroup _, Item.Property _ + | Item.MethodGroup _, Item.Event _ + | Item.MethodGroup _, Item.ILField _ -> x + | _ -> x.Add(k, v) + | _ -> x.Add(k, v)) } else nenv From 04cfcb4862bb3577d309cb8abd3c1703504e1f59 Mon Sep 17 00:00:00 2001 From: TIHan Date: Fri, 16 Aug 2019 19:33:35 -0700 Subject: [PATCH 6/6] Added events --- src/fsharp/NameResolution.fs | 46 +++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/src/fsharp/NameResolution.fs b/src/fsharp/NameResolution.fs index f0dde1a7d13..938af4b7158 100644 --- a/src/fsharp/NameResolution.fs +++ b/src/fsharp/NameResolution.fs @@ -797,18 +797,27 @@ let AddStaticContentOfTyconRefToNameEnv (g:TcGlobals) (amap: Import.ImportMap) a yield KeyValuePair(fieldName, Item.ILField(fieldInfo)) ] + let getEventItems () = + [ let events = + infoReader.GetEventInfosOfType(None, ad, m, ty) + |> List.groupBy (fun e -> e.EventName) + + for (eventName, eventInfos) in events do + let eventInfos = eventInfos |> List.filter (fun e -> e.IsStatic) + for eventInfo in eventInfos do + yield KeyValuePair(eventName, Item.Event(eventInfo)) + ] + let items = - // This is to handle the current behavior with qualified extension members: - // Extension member properties will hide all extension methods that share the same name. - [| - let propItems = getPropItems () - let methItems = getMethItems () - - let itemGroups = - propItems @ methItems + [ + let propAndMethGroups = + getPropItems () @ getMethItems () |> List.groupBy (fun pair -> pair.Key) - for (_, items) in itemGroups do + for (_, items) in propAndMethGroups do + // This is to handle the current behavior with qualified extension members: + // Extension properties will shadow all extension methods that share the same name no matter the order in which the extensions are opened from modules. + // TODO: What happens with CSharp-style extension methods? let sortedItems = items |> List.sortBy (fun pair -> @@ -820,31 +829,36 @@ let AddStaticContentOfTyconRefToNameEnv (g:TcGlobals) (amap: Import.ImportMap) a | item :: _ -> yield item | _ -> () - - |] + yield! getFieldItems () + yield! getEventItems () + ] { nenv with eUnqualifiedItems = (nenv.eUnqualifiedItems, items) - ||> Array.fold (fun x (KeyValue(k, v)) -> + ||> List.fold (fun x (KeyValue(k, v)) -> match x.TryGetValue k with | true, existingItem -> match existingItem, v with - // Combine methods groups. + // Combine methods groups. This allows overloading on methods from different classes. Mimics C# behavior. | Item.MethodGroup (_, minfosCurrent, None), Item.MethodGroup (_, minfosNew, None) -> let minfos = (minfosNew, minfosCurrent) ||> List.fold (fun minfos minfo1 -> - // Shadow methods with the same signature. - if minfos |> List.exists (fun minfo2 -> MethInfosEquivByNameAndSig Erasure.EraseAll true g amap m minfo1 minfo2) then + // Shadow methods with the same signature. + // F# specific behavior; the order of opening is significant and ambiguities are shadowed. + // In C#, an error would occur and asks the developer to resolve the ambiguity. + // We allow UoM (units of measure) to be significant in overloading here. + if minfosCurrent |> List.exists (fun minfo2 -> MethInfosEquivByNameAndSig Erasure.EraseNone true g amap m minfo1 minfo2) then minfos else minfo1 :: minfos ) x.Add(k, Item.MethodGroup (k, minfos, None)) - // Methods will hide properties, events, and fields with the same name. + // Methods will always shadow properties, events, and fields with the same name; order of opening does not matter in this case. + // This is to mimic the C# behavior of opening multiple static classes. | Item.MethodGroup _, Item.Property _ | Item.MethodGroup _, Item.Event _ | Item.MethodGroup _, Item.ILField _ -> x