From 4b54853fadbd762644058f3529574ae58e6f0b8c Mon Sep 17 00:00:00 2001 From: kerams Date: Tue, 1 Aug 2023 18:55:13 +0200 Subject: [PATCH 1/3] Implement completions for overrides --- src/Compiler/Service/FSharpCheckerResults.fs | 65 ++++++++++++++++++- src/Compiler/Service/ServiceParsedInputOps.fs | 44 ++++++++++++- .../Service/ServiceParsedInputOps.fsi | 3 + ...ervice.SurfaceArea.netstandard20.debug.bsl | 9 +++ ...vice.SurfaceArea.netstandard20.release.bsl | 9 +++ 5 files changed, 128 insertions(+), 2 deletions(-) diff --git a/src/Compiler/Service/FSharpCheckerResults.fs b/src/Compiler/Service/FSharpCheckerResults.fs index 4a791a9ec10..7619b6f7b42 100644 --- a/src/Compiler/Service/FSharpCheckerResults.fs +++ b/src/Compiler/Service/FSharpCheckerResults.fs @@ -1008,6 +1008,63 @@ type internal TypeCheckInfo |> List.prependIfSome (CreateCompletionItemForSuggestedPatternName caseIdPos fieldName)) |> Option.defaultValue completions + /// Gets all methods that a type can, but has not yet, overriden. Object methods are not included. + let GetOverridableMethods pos enclosingTypeNameRange inheritTypeNameRange = + let methodNotAlreadyOverridden (alreadyOverriddenMethods: MethInfo list) (candidate: MethInfo) g = + alreadyOverriddenMethods + |> List.forall (fun x -> + if candidate.DisplayName <> x.DisplayName then + true + else + match x, candidate with + | FSMeth (valRef = vref1), FSMeth (valRef = vref2) -> + match stripTyEqns g vref1.Type, stripTyEqns g vref2.Type with + | TType_fun (rangeType = ty1), TType_fun (rangeType = ty2) -> not (typeEquiv g ty1 ty2) + | _ -> not (typeEquiv g vref1.Type vref2.Type) + | _ -> false) + + let (nenv, ad), m = GetBestEnvForPos pos + + let alreadyOverridden = + sResolutions.CapturedNameResolutions + |> ResizeArray.tryPick (fun r -> + match r.Item with + | Item.Types (_, ty :: _) when equals r.Range enclosingTypeNameRange -> + GetImmediateIntrinsicMethInfosOfType (None, ad) g amap inheritTypeNameRange ty + |> List.filter (fun x -> x.IsDefiniteFSharpOverride) + |> Some + | _ -> None) + |> Option.defaultValue [] + + sResolutions.CapturedNameResolutions + |> ResizeArray.tryPick (fun r -> + match r.Item with + | Item.Types (_, ty :: _) when equals r.Range inheritTypeNameRange -> + let meths = + GetIntrinsicMethInfosOfType + infoReader + None + ad + TypeHierarchy.AllowMultiIntfInstantiations.No + FindMemberFlag.IgnoreOverrides + inheritTypeNameRange + ty + |> List.choose (fun x -> + if + not x.IsFinal + && not (tyconRefEq g x.DeclaringTyconRef g.system_Object_tcref) + && methodNotAlreadyOverridden alreadyOverridden x g + then + Item.MethodGroup(x.DisplayName, [ x ], None) + |> ItemWithNoInst + |> CompletionItem ValueNone ValueNone + |> Some + else + None) + + Some(meths, nenv.DisplayEnv, m) + | _ -> None) + let getItem (x: ItemWithInst) = x.Item let GetDeclaredItems @@ -1559,6 +1616,9 @@ type internal TypeCheckInfo | _ -> None) |> Option.orElse declaredItems + | Some (CompletionContext.MethodOverride (enclosingTypeNameRange, inheritTypeNameRange)) -> + GetOverridableMethods pos enclosingTypeNameRange inheritTypeNameRange + // Other completions | cc -> match residueOpt |> Option.bind Seq.tryHead with @@ -1664,7 +1724,10 @@ type internal TypeCheckInfo |> Option.map (fun parsedInput -> ParsedInput.GetFullNameOfSmallestModuleOrNamespaceAtPoint(mkPos line 0, parsedInput)) - let isAttributeApplication = ctx = Some CompletionContext.AttributeApplication + let isAttributeApplication = + match ctx with + | Some CompletionContext.AttributeApplication -> true + | _ -> false DeclarationListInfo.Create( infoReader, diff --git a/src/Compiler/Service/ServiceParsedInputOps.fs b/src/Compiler/Service/ServiceParsedInputOps.fs index d7774643d08..561f7b1c3e2 100644 --- a/src/Compiler/Service/ServiceParsedInputOps.fs +++ b/src/Compiler/Service/ServiceParsedInputOps.fs @@ -9,6 +9,7 @@ open System.Text.RegularExpressions open Internal.Utilities.Library open FSharp.Compiler.Diagnostics open FSharp.Compiler.Syntax +open FSharp.Compiler.SyntaxTrivia open FSharp.Compiler.Syntax.PrettyNaming open FSharp.Compiler.SyntaxTreeOps open FSharp.Compiler.Text @@ -97,6 +98,9 @@ type CompletionContext = /// Completing a pattern in a match clause, member/let binding or lambda | Pattern of context: PatternContext + /// Completing a method override (e.g. override this.ToStr|) + | MethodOverride of enclosingTypeNameRange: range * inheritTypeNameRange: range + type ShortIdent = string type ShortIdents = ShortIdent[] @@ -1412,9 +1416,45 @@ module ParsedInput = | _ -> None - member _.VisitBinding(_, defaultTraverse, (SynBinding (headPat = headPat) as synBinding)) = + member _.VisitBinding(path, defaultTraverse, (SynBinding (headPat = headPat; trivia = trivia) as synBinding)) = + + let isOverride leadingKeyword = + match leadingKeyword with + | SynLeadingKeyword.Override _ -> true + | _ -> false + + let overrideContextOrInvalid path = + let overrideCtx = + match path with + | _ :: SyntaxNode.SynTypeDefn (SynTypeDefn (typeInfo = SynComponentInfo(longId = [ enclosingType ]) + typeRepr = SynTypeDefnRepr.ObjectModel (members = members))) :: _ -> + members + |> List.tryPick (fun memb -> + match memb with + | SynMemberDefn.ImplicitInherit (inheritType = inheritType) -> + Some(CompletionContext.MethodOverride(enclosingType.idRange, inheritType.Range)) + | _ -> None) + | _ -> None + + overrideCtx |> Option.orElseWith (fun () -> Some CompletionContext.Invalid) match headPat with + + // override _.| + | SynPat.FromParseError _ when isOverride trivia.LeadingKeyword -> overrideContextOrInvalid path + + // override this.| + | SynPat.Named(ident = SynIdent (ident = thisId)) when + isOverride trivia.LeadingKeyword && thisId.idRange.End.IsAdjacentTo pos + -> + overrideContextOrInvalid path + + // override this.ToStr| + | SynPat.LongIdent(longDotId = SynLongIdent(id = [ _; methodId ])) when + isOverride trivia.LeadingKeyword && rangeContainsPos methodId.idRange pos + -> + overrideContextOrInvalid path + | SynPat.LongIdent (longDotId = lidwd; argPats = SynArgPats.Pats pats; range = m) when rangeContainsPos m pos -> if rangeContainsPos lidwd.Range pos then // let fo|o x = () @@ -1423,10 +1463,12 @@ module ParsedInput = pats |> List.tryPick (fun pat -> TryGetCompletionContextInPattern true pat None pos) |> Option.orElseWith (fun () -> defaultTraverse synBinding) + | SynPat.Named (range = range) | SynPat.As (_, SynPat.Named (range = range), _) when rangeContainsPos range pos -> // let fo|o = 1 Some CompletionContext.Invalid + | _ -> defaultTraverse synBinding member _.VisitHashDirective(_, _directive, range) = diff --git a/src/Compiler/Service/ServiceParsedInputOps.fsi b/src/Compiler/Service/ServiceParsedInputOps.fsi index 0f70080f1ff..208ac9d4ca9 100644 --- a/src/Compiler/Service/ServiceParsedInputOps.fsi +++ b/src/Compiler/Service/ServiceParsedInputOps.fsi @@ -70,6 +70,9 @@ type public CompletionContext = /// Completing a pattern in a match clause, member/let binding or lambda | Pattern of context: PatternContext + /// Completing a method override (e.g. override this.ToStr|) + | MethodOverride of enclosingTypeNameRange: range * inheritTypeNameRange: range + type public ModuleKind = { IsAutoOpen: bool HasModuleSuffix: bool } diff --git a/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.debug.bsl b/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.debug.bsl index 6a555b4aed0..639629290bc 100644 --- a/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.debug.bsl +++ b/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.debug.bsl @@ -2536,6 +2536,10 @@ FSharp.Compiler.EditorServices.CompletionContext+Inherit: FSharp.Compiler.Editor FSharp.Compiler.EditorServices.CompletionContext+Inherit: FSharp.Compiler.EditorServices.InheritanceContext get_context() FSharp.Compiler.EditorServices.CompletionContext+Inherit: System.Tuple`2[Microsoft.FSharp.Collections.FSharpList`1[System.String],Microsoft.FSharp.Core.FSharpOption`1[System.String]] get_path() FSharp.Compiler.EditorServices.CompletionContext+Inherit: System.Tuple`2[Microsoft.FSharp.Collections.FSharpList`1[System.String],Microsoft.FSharp.Core.FSharpOption`1[System.String]] path +FSharp.Compiler.EditorServices.CompletionContext+MethodOverride: FSharp.Compiler.Text.Range enclosingTypeNameRange +FSharp.Compiler.EditorServices.CompletionContext+MethodOverride: FSharp.Compiler.Text.Range get_enclosingTypeNameRange() +FSharp.Compiler.EditorServices.CompletionContext+MethodOverride: FSharp.Compiler.Text.Range get_inheritTypeNameRange() +FSharp.Compiler.EditorServices.CompletionContext+MethodOverride: FSharp.Compiler.Text.Range inheritTypeNameRange FSharp.Compiler.EditorServices.CompletionContext+OpenDeclaration: Boolean get_isOpenType() FSharp.Compiler.EditorServices.CompletionContext+OpenDeclaration: Boolean isOpenType FSharp.Compiler.EditorServices.CompletionContext+ParameterList: FSharp.Compiler.Text.Position Item1 @@ -2549,6 +2553,7 @@ FSharp.Compiler.EditorServices.CompletionContext+RecordField: FSharp.Compiler.Ed FSharp.Compiler.EditorServices.CompletionContext+Tags: Int32 AttributeApplication FSharp.Compiler.EditorServices.CompletionContext+Tags: Int32 Inherit FSharp.Compiler.EditorServices.CompletionContext+Tags: Int32 Invalid +FSharp.Compiler.EditorServices.CompletionContext+Tags: Int32 MethodOverride FSharp.Compiler.EditorServices.CompletionContext+Tags: Int32 OpenDeclaration FSharp.Compiler.EditorServices.CompletionContext+Tags: Int32 ParameterList FSharp.Compiler.EditorServices.CompletionContext+Tags: Int32 Pattern @@ -2563,6 +2568,7 @@ FSharp.Compiler.EditorServices.CompletionContext: Boolean Equals(System.Object, FSharp.Compiler.EditorServices.CompletionContext: Boolean IsAttributeApplication FSharp.Compiler.EditorServices.CompletionContext: Boolean IsInherit FSharp.Compiler.EditorServices.CompletionContext: Boolean IsInvalid +FSharp.Compiler.EditorServices.CompletionContext: Boolean IsMethodOverride FSharp.Compiler.EditorServices.CompletionContext: Boolean IsOpenDeclaration FSharp.Compiler.EditorServices.CompletionContext: Boolean IsParameterList FSharp.Compiler.EditorServices.CompletionContext: Boolean IsPattern @@ -2574,6 +2580,7 @@ FSharp.Compiler.EditorServices.CompletionContext: Boolean IsUnionCaseFieldsDecla FSharp.Compiler.EditorServices.CompletionContext: Boolean get_IsAttributeApplication() FSharp.Compiler.EditorServices.CompletionContext: Boolean get_IsInherit() FSharp.Compiler.EditorServices.CompletionContext: Boolean get_IsInvalid() +FSharp.Compiler.EditorServices.CompletionContext: Boolean get_IsMethodOverride() FSharp.Compiler.EditorServices.CompletionContext: Boolean get_IsOpenDeclaration() FSharp.Compiler.EditorServices.CompletionContext: Boolean get_IsParameterList() FSharp.Compiler.EditorServices.CompletionContext: Boolean get_IsPattern() @@ -2585,6 +2592,7 @@ FSharp.Compiler.EditorServices.CompletionContext: Boolean get_IsUnionCaseFieldsD FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext AttributeApplication FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext Invalid FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewInherit(FSharp.Compiler.EditorServices.InheritanceContext, System.Tuple`2[Microsoft.FSharp.Collections.FSharpList`1[System.String],Microsoft.FSharp.Core.FSharpOption`1[System.String]]) +FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewMethodOverride(FSharp.Compiler.Text.Range, FSharp.Compiler.Text.Range) FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewOpenDeclaration(Boolean) FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewParameterList(FSharp.Compiler.Text.Position, System.Collections.Generic.HashSet`1[System.String]) FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewPattern(FSharp.Compiler.EditorServices.PatternContext) @@ -2600,6 +2608,7 @@ FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext get_TypeAbbreviationOrSingleCaseUnion() FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext get_UnionCaseFieldsDeclaration() FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext+Inherit +FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext+MethodOverride FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext+OpenDeclaration FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext+ParameterList FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext+Pattern diff --git a/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.release.bsl b/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.release.bsl index 6a555b4aed0..639629290bc 100644 --- a/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.release.bsl +++ b/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.release.bsl @@ -2536,6 +2536,10 @@ FSharp.Compiler.EditorServices.CompletionContext+Inherit: FSharp.Compiler.Editor FSharp.Compiler.EditorServices.CompletionContext+Inherit: FSharp.Compiler.EditorServices.InheritanceContext get_context() FSharp.Compiler.EditorServices.CompletionContext+Inherit: System.Tuple`2[Microsoft.FSharp.Collections.FSharpList`1[System.String],Microsoft.FSharp.Core.FSharpOption`1[System.String]] get_path() FSharp.Compiler.EditorServices.CompletionContext+Inherit: System.Tuple`2[Microsoft.FSharp.Collections.FSharpList`1[System.String],Microsoft.FSharp.Core.FSharpOption`1[System.String]] path +FSharp.Compiler.EditorServices.CompletionContext+MethodOverride: FSharp.Compiler.Text.Range enclosingTypeNameRange +FSharp.Compiler.EditorServices.CompletionContext+MethodOverride: FSharp.Compiler.Text.Range get_enclosingTypeNameRange() +FSharp.Compiler.EditorServices.CompletionContext+MethodOverride: FSharp.Compiler.Text.Range get_inheritTypeNameRange() +FSharp.Compiler.EditorServices.CompletionContext+MethodOverride: FSharp.Compiler.Text.Range inheritTypeNameRange FSharp.Compiler.EditorServices.CompletionContext+OpenDeclaration: Boolean get_isOpenType() FSharp.Compiler.EditorServices.CompletionContext+OpenDeclaration: Boolean isOpenType FSharp.Compiler.EditorServices.CompletionContext+ParameterList: FSharp.Compiler.Text.Position Item1 @@ -2549,6 +2553,7 @@ FSharp.Compiler.EditorServices.CompletionContext+RecordField: FSharp.Compiler.Ed FSharp.Compiler.EditorServices.CompletionContext+Tags: Int32 AttributeApplication FSharp.Compiler.EditorServices.CompletionContext+Tags: Int32 Inherit FSharp.Compiler.EditorServices.CompletionContext+Tags: Int32 Invalid +FSharp.Compiler.EditorServices.CompletionContext+Tags: Int32 MethodOverride FSharp.Compiler.EditorServices.CompletionContext+Tags: Int32 OpenDeclaration FSharp.Compiler.EditorServices.CompletionContext+Tags: Int32 ParameterList FSharp.Compiler.EditorServices.CompletionContext+Tags: Int32 Pattern @@ -2563,6 +2568,7 @@ FSharp.Compiler.EditorServices.CompletionContext: Boolean Equals(System.Object, FSharp.Compiler.EditorServices.CompletionContext: Boolean IsAttributeApplication FSharp.Compiler.EditorServices.CompletionContext: Boolean IsInherit FSharp.Compiler.EditorServices.CompletionContext: Boolean IsInvalid +FSharp.Compiler.EditorServices.CompletionContext: Boolean IsMethodOverride FSharp.Compiler.EditorServices.CompletionContext: Boolean IsOpenDeclaration FSharp.Compiler.EditorServices.CompletionContext: Boolean IsParameterList FSharp.Compiler.EditorServices.CompletionContext: Boolean IsPattern @@ -2574,6 +2580,7 @@ FSharp.Compiler.EditorServices.CompletionContext: Boolean IsUnionCaseFieldsDecla FSharp.Compiler.EditorServices.CompletionContext: Boolean get_IsAttributeApplication() FSharp.Compiler.EditorServices.CompletionContext: Boolean get_IsInherit() FSharp.Compiler.EditorServices.CompletionContext: Boolean get_IsInvalid() +FSharp.Compiler.EditorServices.CompletionContext: Boolean get_IsMethodOverride() FSharp.Compiler.EditorServices.CompletionContext: Boolean get_IsOpenDeclaration() FSharp.Compiler.EditorServices.CompletionContext: Boolean get_IsParameterList() FSharp.Compiler.EditorServices.CompletionContext: Boolean get_IsPattern() @@ -2585,6 +2592,7 @@ FSharp.Compiler.EditorServices.CompletionContext: Boolean get_IsUnionCaseFieldsD FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext AttributeApplication FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext Invalid FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewInherit(FSharp.Compiler.EditorServices.InheritanceContext, System.Tuple`2[Microsoft.FSharp.Collections.FSharpList`1[System.String],Microsoft.FSharp.Core.FSharpOption`1[System.String]]) +FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewMethodOverride(FSharp.Compiler.Text.Range, FSharp.Compiler.Text.Range) FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewOpenDeclaration(Boolean) FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewParameterList(FSharp.Compiler.Text.Position, System.Collections.Generic.HashSet`1[System.String]) FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewPattern(FSharp.Compiler.EditorServices.PatternContext) @@ -2600,6 +2608,7 @@ FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext get_TypeAbbreviationOrSingleCaseUnion() FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext get_UnionCaseFieldsDeclaration() FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext+Inherit +FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext+MethodOverride FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext+OpenDeclaration FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext+ParameterList FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext+Pattern From 3164eef7a80e481203d5e2b5abb1c4f7649efac8 Mon Sep 17 00:00:00 2001 From: kerams Date: Tue, 1 Aug 2023 21:23:13 +0200 Subject: [PATCH 2/3] Fix override matching --- src/Compiler/Service/FSharpCheckerResults.fs | 63 +++++++++++++------- 1 file changed, 40 insertions(+), 23 deletions(-) diff --git a/src/Compiler/Service/FSharpCheckerResults.fs b/src/Compiler/Service/FSharpCheckerResults.fs index 7619b6f7b42..ecf45e82531 100644 --- a/src/Compiler/Service/FSharpCheckerResults.fs +++ b/src/Compiler/Service/FSharpCheckerResults.fs @@ -1010,18 +1010,38 @@ type internal TypeCheckInfo /// Gets all methods that a type can, but has not yet, overriden. Object methods are not included. let GetOverridableMethods pos enclosingTypeNameRange inheritTypeNameRange = - let methodNotAlreadyOverridden (alreadyOverriddenMethods: MethInfo list) (candidate: MethInfo) g = + let areSlotParamsEqual slotParams candidateSlotParams = + List.forall2 + (fun (list1: SlotParam list) (list2: SlotParam list) -> + list1.Length = list2.Length + && List.forall2 (fun (TSlotParam (paramType = ty1)) (TSlotParam (paramType = ty2)) -> typeEquiv g ty1 ty2) list1 list2) + + slotParams + candidateSlotParams + + let methodNotAlreadyOverridden (alreadyOverriddenMethods: MethInfo list) (candidateMeth: MethInfo) = + let candidateSlot = candidateMeth.GetSlotSig(amap, range0) + alreadyOverriddenMethods |> List.forall (fun x -> - if candidate.DisplayName <> x.DisplayName then - true - else - match x, candidate with - | FSMeth (valRef = vref1), FSMeth (valRef = vref2) -> - match stripTyEqns g vref1.Type, stripTyEqns g vref2.Type with - | TType_fun (rangeType = ty1), TType_fun (rangeType = ty2) -> not (typeEquiv g ty1 ty2) - | _ -> not (typeEquiv g vref1.Type vref2.Type) - | _ -> false) + match x with + | FSMeth (valRef = valref) -> + x.ImplementedSlotSignatures + |> List.forall (fun slot -> + if + slot.Name <> candidateSlot.Name + || slot.FormalParams.Length <> candidateSlot.FormalParams.Length + then + true + else + let slot = ReparentSlotSigToUseMethodTypars g range0 valref slot + + match slot.FormalReturnType, candidateSlot.FormalReturnType with + | None, None -> not (areSlotParamsEqual slot.FormalParams candidateSlot.FormalParams) + | Some returnTy1, Some returnTy2 when typeEquiv g returnTy1 returnTy2 -> + not (areSlotParamsEqual slot.FormalParams candidateSlot.FormalParams) + | _ -> true) + | _ -> false) let (nenv, ad), m = GetBestEnvForPos pos @@ -1030,7 +1050,7 @@ type internal TypeCheckInfo |> ResizeArray.tryPick (fun r -> match r.Item with | Item.Types (_, ty :: _) when equals r.Range enclosingTypeNameRange -> - GetImmediateIntrinsicMethInfosOfType (None, ad) g amap inheritTypeNameRange ty + GetImmediateIntrinsicMethInfosOfType (None, ad) g amap enclosingTypeNameRange ty |> List.filter (fun x -> x.IsDefiniteFSharpOverride) |> Some | _ -> None) @@ -1049,18 +1069,15 @@ type internal TypeCheckInfo FindMemberFlag.IgnoreOverrides inheritTypeNameRange ty - |> List.choose (fun x -> - if - not x.IsFinal - && not (tyconRefEq g x.DeclaringTyconRef g.system_Object_tcref) - && methodNotAlreadyOverridden alreadyOverridden x g - then - Item.MethodGroup(x.DisplayName, [ x ], None) - |> ItemWithNoInst - |> CompletionItem ValueNone ValueNone - |> Some - else - None) + |> List.filter (fun x -> + not x.IsFinal + && not (tyconRefEq g x.DeclaringTyconRef g.system_Object_tcref) + && methodNotAlreadyOverridden alreadyOverridden x) + |> List.groupBy (fun x -> x.DisplayName) + |> List.map (fun (name, overloads) -> + Item.MethodGroup(name, overloads, None) + |> ItemWithNoInst + |> CompletionItem ValueNone ValueNone) Some(meths, nenv.DisplayEnv, m) | _ -> None) From 72352dfe450811fc0f6c89c62cee091e063951d8 Mon Sep 17 00:00:00 2001 From: kerams Date: Wed, 2 Aug 2023 15:06:09 +0200 Subject: [PATCH 3/3] Refactor, add tests --- src/Compiler/Service/FSharpCheckerResults.fs | 80 +++++---------- src/Compiler/Service/ServiceParsedInputOps.fs | 31 +++--- .../Service/ServiceParsedInputOps.fsi | 2 +- ...ervice.SurfaceArea.netstandard20.debug.bsl | 4 +- ...vice.SurfaceArea.netstandard20.release.bsl | 4 +- .../CompletionProviderTests.fs | 98 +++++++++++++++++++ 6 files changed, 135 insertions(+), 84 deletions(-) diff --git a/src/Compiler/Service/FSharpCheckerResults.fs b/src/Compiler/Service/FSharpCheckerResults.fs index ecf45e82531..819921750a3 100644 --- a/src/Compiler/Service/FSharpCheckerResults.fs +++ b/src/Compiler/Service/FSharpCheckerResults.fs @@ -1008,78 +1008,45 @@ type internal TypeCheckInfo |> List.prependIfSome (CreateCompletionItemForSuggestedPatternName caseIdPos fieldName)) |> Option.defaultValue completions - /// Gets all methods that a type can, but has not yet, overriden. Object methods are not included. - let GetOverridableMethods pos enclosingTypeNameRange inheritTypeNameRange = - let areSlotParamsEqual slotParams candidateSlotParams = - List.forall2 - (fun (list1: SlotParam list) (list2: SlotParam list) -> - list1.Length = list2.Length - && List.forall2 (fun (TSlotParam (paramType = ty1)) (TSlotParam (paramType = ty2)) -> typeEquiv g ty1 ty2) list1 list2) - - slotParams - candidateSlotParams - - let methodNotAlreadyOverridden (alreadyOverriddenMethods: MethInfo list) (candidateMeth: MethInfo) = - let candidateSlot = candidateMeth.GetSlotSig(amap, range0) - - alreadyOverriddenMethods - |> List.forall (fun x -> - match x with - | FSMeth (valRef = valref) -> - x.ImplementedSlotSignatures - |> List.forall (fun slot -> - if - slot.Name <> candidateSlot.Name - || slot.FormalParams.Length <> candidateSlot.FormalParams.Length - then - true - else - let slot = ReparentSlotSigToUseMethodTypars g range0 valref slot - - match slot.FormalReturnType, candidateSlot.FormalReturnType with - | None, None -> not (areSlotParamsEqual slot.FormalParams candidateSlot.FormalParams) - | Some returnTy1, Some returnTy2 when typeEquiv g returnTy1 returnTy2 -> - not (areSlotParamsEqual slot.FormalParams candidateSlot.FormalParams) - | _ -> true) - | _ -> false) + /// Gets all methods that a type can override, but has not yet done so. + let GetOverridableMethods pos typeNameRange = + let isMethodOverridable alreadyOverridden (candidate: MethInfo) = + not candidate.IsFinal + && not ( + alreadyOverridden + |> List.exists (MethInfosEquivByNameAndSig EraseNone true g amap range0 candidate) + ) let (nenv, ad), m = GetBestEnvForPos pos - let alreadyOverridden = - sResolutions.CapturedNameResolutions - |> ResizeArray.tryPick (fun r -> - match r.Item with - | Item.Types (_, ty :: _) when equals r.Range enclosingTypeNameRange -> - GetImmediateIntrinsicMethInfosOfType (None, ad) g amap enclosingTypeNameRange ty - |> List.filter (fun x -> x.IsDefiniteFSharpOverride) - |> Some - | _ -> None) - |> Option.defaultValue [] - sResolutions.CapturedNameResolutions |> ResizeArray.tryPick (fun r -> match r.Item with - | Item.Types (_, ty :: _) when equals r.Range inheritTypeNameRange -> - let meths = + | Item.Types (_, ty :: _) when equals r.Range typeNameRange && isAppTy g ty -> + let superTy = + (tcrefOfAppTy g ty).TypeContents.tcaug_super |> Option.defaultValue g.obj_ty + + let overriddenMethods = + GetImmediateIntrinsicMethInfosOfType (None, ad) g amap typeNameRange ty + |> List.filter (fun x -> x.IsDefiniteFSharpOverride) + + let overridableMethods = GetIntrinsicMethInfosOfType infoReader None ad TypeHierarchy.AllowMultiIntfInstantiations.No - FindMemberFlag.IgnoreOverrides - inheritTypeNameRange - ty - |> List.filter (fun x -> - not x.IsFinal - && not (tyconRefEq g x.DeclaringTyconRef g.system_Object_tcref) - && methodNotAlreadyOverridden alreadyOverridden x) + FindMemberFlag.PreferOverrides + range0 + superTy + |> List.filter (isMethodOverridable overriddenMethods) |> List.groupBy (fun x -> x.DisplayName) |> List.map (fun (name, overloads) -> Item.MethodGroup(name, overloads, None) |> ItemWithNoInst |> CompletionItem ValueNone ValueNone) - Some(meths, nenv.DisplayEnv, m) + Some(overridableMethods, nenv.DisplayEnv, m) | _ -> None) let getItem (x: ItemWithInst) = x.Item @@ -1633,8 +1600,7 @@ type internal TypeCheckInfo | _ -> None) |> Option.orElse declaredItems - | Some (CompletionContext.MethodOverride (enclosingTypeNameRange, inheritTypeNameRange)) -> - GetOverridableMethods pos enclosingTypeNameRange inheritTypeNameRange + | Some (CompletionContext.MethodOverride enclosingTypeNameRange) -> GetOverridableMethods pos enclosingTypeNameRange // Other completions | cc -> diff --git a/src/Compiler/Service/ServiceParsedInputOps.fs b/src/Compiler/Service/ServiceParsedInputOps.fs index 561f7b1c3e2..a9e188fef73 100644 --- a/src/Compiler/Service/ServiceParsedInputOps.fs +++ b/src/Compiler/Service/ServiceParsedInputOps.fs @@ -99,7 +99,7 @@ type CompletionContext = | Pattern of context: PatternContext /// Completing a method override (e.g. override this.ToStr|) - | MethodOverride of enclosingTypeNameRange: range * inheritTypeNameRange: range + | MethodOverride of enclosingTypeNameRange: range type ShortIdent = string @@ -1423,37 +1423,28 @@ module ParsedInput = | SynLeadingKeyword.Override _ -> true | _ -> false - let overrideContextOrInvalid path = - let overrideCtx = - match path with - | _ :: SyntaxNode.SynTypeDefn (SynTypeDefn (typeInfo = SynComponentInfo(longId = [ enclosingType ]) - typeRepr = SynTypeDefnRepr.ObjectModel (members = members))) :: _ -> - members - |> List.tryPick (fun memb -> - match memb with - | SynMemberDefn.ImplicitInherit (inheritType = inheritType) -> - Some(CompletionContext.MethodOverride(enclosingType.idRange, inheritType.Range)) - | _ -> None) - | _ -> None - - overrideCtx |> Option.orElseWith (fun () -> Some CompletionContext.Invalid) + let overrideContext path = + match path with + | _ :: SyntaxNode.SynTypeDefn (SynTypeDefn(typeInfo = SynComponentInfo(longId = [ enclosingType ]))) :: _ -> + Some(CompletionContext.MethodOverride enclosingType.idRange) + | _ -> Some CompletionContext.Invalid match headPat with // override _.| - | SynPat.FromParseError _ when isOverride trivia.LeadingKeyword -> overrideContextOrInvalid path + | SynPat.FromParseError _ when isOverride trivia.LeadingKeyword -> overrideContext path // override this.| - | SynPat.Named(ident = SynIdent (ident = thisId)) when - isOverride trivia.LeadingKeyword && thisId.idRange.End.IsAdjacentTo pos + | SynPat.Named(ident = SynIdent (ident = selfId)) when + isOverride trivia.LeadingKeyword && selfId.idRange.End.IsAdjacentTo pos -> - overrideContextOrInvalid path + overrideContext path // override this.ToStr| | SynPat.LongIdent(longDotId = SynLongIdent(id = [ _; methodId ])) when isOverride trivia.LeadingKeyword && rangeContainsPos methodId.idRange pos -> - overrideContextOrInvalid path + overrideContext path | SynPat.LongIdent (longDotId = lidwd; argPats = SynArgPats.Pats pats; range = m) when rangeContainsPos m pos -> if rangeContainsPos lidwd.Range pos then diff --git a/src/Compiler/Service/ServiceParsedInputOps.fsi b/src/Compiler/Service/ServiceParsedInputOps.fsi index 208ac9d4ca9..e94965bb2bd 100644 --- a/src/Compiler/Service/ServiceParsedInputOps.fsi +++ b/src/Compiler/Service/ServiceParsedInputOps.fsi @@ -71,7 +71,7 @@ type public CompletionContext = | Pattern of context: PatternContext /// Completing a method override (e.g. override this.ToStr|) - | MethodOverride of enclosingTypeNameRange: range * inheritTypeNameRange: range + | MethodOverride of enclosingTypeNameRange: range type public ModuleKind = { IsAutoOpen: bool diff --git a/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.debug.bsl b/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.debug.bsl index 639629290bc..c79b0d89366 100644 --- a/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.debug.bsl +++ b/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.debug.bsl @@ -2538,8 +2538,6 @@ FSharp.Compiler.EditorServices.CompletionContext+Inherit: System.Tuple`2[Microso FSharp.Compiler.EditorServices.CompletionContext+Inherit: System.Tuple`2[Microsoft.FSharp.Collections.FSharpList`1[System.String],Microsoft.FSharp.Core.FSharpOption`1[System.String]] path FSharp.Compiler.EditorServices.CompletionContext+MethodOverride: FSharp.Compiler.Text.Range enclosingTypeNameRange FSharp.Compiler.EditorServices.CompletionContext+MethodOverride: FSharp.Compiler.Text.Range get_enclosingTypeNameRange() -FSharp.Compiler.EditorServices.CompletionContext+MethodOverride: FSharp.Compiler.Text.Range get_inheritTypeNameRange() -FSharp.Compiler.EditorServices.CompletionContext+MethodOverride: FSharp.Compiler.Text.Range inheritTypeNameRange FSharp.Compiler.EditorServices.CompletionContext+OpenDeclaration: Boolean get_isOpenType() FSharp.Compiler.EditorServices.CompletionContext+OpenDeclaration: Boolean isOpenType FSharp.Compiler.EditorServices.CompletionContext+ParameterList: FSharp.Compiler.Text.Position Item1 @@ -2592,7 +2590,7 @@ FSharp.Compiler.EditorServices.CompletionContext: Boolean get_IsUnionCaseFieldsD FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext AttributeApplication FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext Invalid FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewInherit(FSharp.Compiler.EditorServices.InheritanceContext, System.Tuple`2[Microsoft.FSharp.Collections.FSharpList`1[System.String],Microsoft.FSharp.Core.FSharpOption`1[System.String]]) -FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewMethodOverride(FSharp.Compiler.Text.Range, FSharp.Compiler.Text.Range) +FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewMethodOverride(FSharp.Compiler.Text.Range) FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewOpenDeclaration(Boolean) FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewParameterList(FSharp.Compiler.Text.Position, System.Collections.Generic.HashSet`1[System.String]) FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewPattern(FSharp.Compiler.EditorServices.PatternContext) diff --git a/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.release.bsl b/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.release.bsl index 639629290bc..c79b0d89366 100644 --- a/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.release.bsl +++ b/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.release.bsl @@ -2538,8 +2538,6 @@ FSharp.Compiler.EditorServices.CompletionContext+Inherit: System.Tuple`2[Microso FSharp.Compiler.EditorServices.CompletionContext+Inherit: System.Tuple`2[Microsoft.FSharp.Collections.FSharpList`1[System.String],Microsoft.FSharp.Core.FSharpOption`1[System.String]] path FSharp.Compiler.EditorServices.CompletionContext+MethodOverride: FSharp.Compiler.Text.Range enclosingTypeNameRange FSharp.Compiler.EditorServices.CompletionContext+MethodOverride: FSharp.Compiler.Text.Range get_enclosingTypeNameRange() -FSharp.Compiler.EditorServices.CompletionContext+MethodOverride: FSharp.Compiler.Text.Range get_inheritTypeNameRange() -FSharp.Compiler.EditorServices.CompletionContext+MethodOverride: FSharp.Compiler.Text.Range inheritTypeNameRange FSharp.Compiler.EditorServices.CompletionContext+OpenDeclaration: Boolean get_isOpenType() FSharp.Compiler.EditorServices.CompletionContext+OpenDeclaration: Boolean isOpenType FSharp.Compiler.EditorServices.CompletionContext+ParameterList: FSharp.Compiler.Text.Position Item1 @@ -2592,7 +2590,7 @@ FSharp.Compiler.EditorServices.CompletionContext: Boolean get_IsUnionCaseFieldsD FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext AttributeApplication FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext Invalid FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewInherit(FSharp.Compiler.EditorServices.InheritanceContext, System.Tuple`2[Microsoft.FSharp.Collections.FSharpList`1[System.String],Microsoft.FSharp.Core.FSharpOption`1[System.String]]) -FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewMethodOverride(FSharp.Compiler.Text.Range, FSharp.Compiler.Text.Range) +FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewMethodOverride(FSharp.Compiler.Text.Range) FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewOpenDeclaration(Boolean) FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewParameterList(FSharp.Compiler.Text.Position, System.Collections.Generic.HashSet`1[System.String]) FSharp.Compiler.EditorServices.CompletionContext: FSharp.Compiler.EditorServices.CompletionContext NewPattern(FSharp.Compiler.EditorServices.PatternContext) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CompletionProviderTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CompletionProviderTests.fs index 01e0ff07f80..5c7f3b9fa83 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CompletionProviderTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CompletionProviderTests.fs @@ -1754,3 +1754,101 @@ for Some ((0, C System.Double. )) VerifyCompletionListExactly(fileContents, "| [ B G.", [ "b" ]) VerifyCompletionListExactly(fileContents, "| A.", [ "B"; "C" ]) VerifyCompletionList(fileContents, "for Some ((0, C System.Double.", [ "Epsilon"; "MaxValue" ], [ "Abs" ]) + + [] + let ``Completion list for override does not contain virtual method if there is a sealed override higher up in the hierarchy`` () = + let fileContents = + """ +[] +type A () = + inherit System.Dynamic.SetIndexBinder (null) + + override _.a + +[] +type B () = + inherit System.Dynamic.DynamicMetaObjectBinder () + + override x. +""" + + // SetIndexBinder inherits from DynamicMetaObjectBinder, but overrides and seals Bind and the ReturnType property + VerifyCompletionListExactly( + fileContents, + "override _.a", + [ + "BindDelegate" + "Equals" + "FallbackSetIndex" + "Finalize" + "GetHashCode" + "ToString" + ] + ) + + VerifyCompletionListExactly( + fileContents, + "override x.", + [ + "Bind" + "BindDelegate" + "Equals" + "Finalize" + "GetHashCode" + "get_ReturnType" + "ToString" + ] + ) + + [] + let ``Completion list for override does not contain virtual method if it is already overridden in the same type`` () = + let fileContents = + """ +type G<'a> () = + override _. + + override x.ToString () = "" + +[ unit + abstract member A1: string -> unit + abstract member A2: unit -> unit + + member NotVirtual () = () + +type B () = + inherit A () + + override A1 () = () + override x.b + +type C () = + inherit A () = + + override A1 () = () + override x.c + override A1 s = () +""" + + VerifyCompletionListExactly(fileContents, "override _.", [ "Equals"; "Finalize"; "GetHashCode" ]) + VerifyCompletionListExactly(fileContents, "override x.b", [ "A1"; "A2"; "Equals"; "Finalize"; "GetHashCode"; "ToString" ]) + VerifyCompletionListExactly(fileContents, "override x.c", [ "A2"; "Equals"; "Finalize"; "GetHashCode"; "ToString" ]) + + [] + let ``Completion list for override is empty when the caret is on the self identifier`` () = + let fileContents = + """ +type A () = + override a + +type B () = + override _ + +type C () = + override c.b () = () +""" + + VerifyNoCompletionList(fileContents, "override a") + VerifyNoCompletionList(fileContents, "override _") + VerifyNoCompletionList(fileContents, "override c")